Rockbox

Tasklist

FS#9114 - Pause Between Tracks

Attached to Project: Rockbox
Opened by Linus Nielsen Feltzing (linusnielsen) - Tuesday, 24 June 2008, 10:22 GMT
Task Type Patches
Category Applications
Status New
Assigned To No-one
Operating System All players
Severity Low
Priority Normal
Reported Version Daily build (which?)
Due in Version Undecided
Due Date Undecided
Percent Complete 0%
Votes 0
Private No

Details

This patch adds a new playback option, Pause Between Tracks. It pauses the playback automatically after each track, and the user must press Play to play the next.

This is useful for e.g dance classes, sound effects etc.

There is a noticeable lag in the simulator, where it starts the playback of the next song before pausing, but who cares? It seems to work fine on the target anyway.

Try it!
This task depends upon

Comment by Linus Nielsen Feltzing (linusnielsen) - Tuesday, 24 June 2008, 10:22 GMT
...and here's the patch.
Comment by Marianne Arnold (pixelma) - Tuesday, 24 June 2008, 11:03 GMT
Maybe I'm misunderstanding something (but don't think because compiling for my OndioFM fails "In file included from playback.c:42: codecs.h:222: warning:..." etc.) but it looks like you enabled compiling playback.c on hwcodec (in apps/SOURCES).

Edit: It only gives the warning, failing had another reason).
Comment by Linus Nielsen Feltzing (linusnielsen) - Tuesday, 24 June 2008, 11:21 GMT
Yes, it is supposed to compile playback.c, because I didn't want to duplicate the event handling code.

I'll fix the warnings, though.
Comment by Linus Nielsen Feltzing (linusnielsen) - Tuesday, 24 June 2008, 11:25 GMT
Here is an updated version, without warnings.
Comment by Steve Bavin (pondlife) - Tuesday, 24 June 2008, 16:04 GMT
Maybe the event handling code could be moved to another module?

I'd personally/selfishly prefer to keep playback.c and HWCODEC apart, because I have no HWCODEC target to test with.
Comment by Linus Nielsen Feltzing (linusnielsen) - Tuesday, 24 June 2008, 20:21 GMT
Maybe. Care to modify the patch accordingly?
Comment by Steve Bavin (pondlife) - Wednesday, 25 June 2008, 08:52 GMT
Of course, but probably not for a week or 2 ;-)

Real Life is such an intrusive thing.
Comment by Linus Nielsen Feltzing (linusnielsen) - Tuesday, 05 August 2008, 09:10 GMT
The question is where to move it, since the code really belongs in the playback module. I could of course duplicate the code...
Comment by Rosso Maltese (asettico) - Monday, 03 August 2009, 11:13 GMT
Resync'd against r22137.
Comment by Bryan Childs (GodEater) - Tuesday, 15 December 2009, 08:57 GMT
Resync'd against 24003.
Comment by Bryan Childs (GodEater) - Tuesday, 15 December 2009, 09:25 GMT
Added a check to make sure the pause doesn't happen if the DAP has party mode set.
Comment by Rosso Maltese (asettico) - Tuesday, 15 December 2009, 10:15 GMT
Ops, I did a re-sync last Sunday, but I forgot to upload it, sorry.

I not completely agree with the last check: also during a party I would like to play a track and pause at the end.
I think the 2 settings could survive independently. ;-)
Comment by Bryan Childs (GodEater) - Tuesday, 15 December 2009, 10:28 GMT
But if the track pauses whilst you're in party mode, there's no way to get the player to advance to the next track without leaving party mode. Why would you want this behaviour?
Comment by Rosso Maltese (asettico) - Tuesday, 15 December 2009, 14:00 GMT
Uhmmm... interesting question. :-) Which behaviour should be stronger?
If party mode wins, then pause between track should be disabled; this means that it should disappear from the related menu (I didn't remember of any menu entry showed as "disabled").
If pause between tracks wins, then it should enable the only key(s) to allow the desired behaviour (just play, I suppose).
But who decide which will be stronger? The developer, a poll, or YACS (yet another configuration setting)? ;-)
Comment by Bryan Childs (GodEater) - Tuesday, 15 December 2009, 14:04 GMT
Well, myself, Llorean and LinusN all think party mode should take precedence over this setting. We don't have to have the menu item appear as disabled though, so long as we document this behaviour and it is understood.
Comment by Rosso Maltese (asettico) - Tuesday, 15 December 2009, 14:21 GMT
And what about a three value setting instead of o boolean?
Something like: off, on but in party mode, always on.
Comment by Bryan Childs (GodEater) - Tuesday, 15 December 2009, 14:42 GMT
Party mode is designed to give you uninterrupted music at a party. It's supposed to stop people fiddling with the controls so that the music doesn't stop when someone's looking at the player and pressing buttons inadvertently. You must go to some weird parties if you want the music to stop after every song and have a very experienced Rockbox user return to the player to get it to move to the next track.

In my opinion Party mode should take precedence over *everything* which interferes with music playback being continuous.
Comment by Frank Gevaerts (fg) - Tuesday, 15 December 2009, 14:44 GMT
I think this should be kept simple, with party mode overriding everything.

I can see a use for a mode where any track that starts playing should not be interrupted, which could coexist (and be useful) with pause between tracks, but I don't think that's the same as party mode.
Comment by Steve Bavin (pondlife) - Tuesday, 15 December 2009, 15:04 GMT
Getting way off-topic for this patch, but this is a decent example of why I'd like party mode to be replaced with (or redefined as) keylock mode...
Comment by Bryan Childs (GodEater) - Tuesday, 15 December 2009, 15:14 GMT
@pondlife, please bring that discussion to IRC ;)
Comment by Bryan Childs (GodEater) - Wednesday, 16 December 2009, 12:38 GMT
Resync'd against 24024, and fixed compilation so it would work on HWCODEC.
Comment by Jonathan Gordon (jdgordon) - Thursday, 17 December 2009, 08:47 GMT
I'm currently against commiting that patch. playback.c really should not be compiled for hwcodec and the added functions there don't really deserve to be in playback.c anyway. That fil is crowded enough as it is, and they dont really have anything to do with the playback engine.

so the needed changes:
1) get the callback/event stuff out of playback.c, either into a new file or I tihnk playlist.c makes more sense (its a playlist mode just as much a playback mode... or even in the menus file and fix the setting callback to do the initialisation)
2) change audio_pause() to audio_pause(bool block) which calls queue_post() instead of queue_send() so both hwcodec and swcodec can use the same codepath in the event callback (I have no idea why audio_pause() currently blocks...
Comment by Steve Bavin (pondlife) - Thursday, 17 December 2009, 09:02 GMT
Ow, I hadn't noticed that.

I don't think playback.c should need changing for this feature - assuming there's already an end-of-track event you can use (i.e. aftert the PCM output is done, might need to check how crossfade reacts too).

This is more a UI-level feature in terms of the code structure, I'd think. Maybe it could go in playlist.c, though that's a bit of a mess (or was last time I looked).

Sorry - not wishing to be a monstrous killjoy, just wishing to keep the code structured right.. I'm happy to have a look at this over Christmas week if needed (and I can get near a PC).
Comment by Mike Schmitt (Falco98) - Tuesday, 12 January 2010, 08:38 GMT
Linus, this patch seems to share a lot of functionality with what would be needed to implement a tool whereby someone can long-select a track name in the 'current playlist' and select a "stop after this song" option from the menu there. That is to say, both require a system to be in place where the player plans to stop at the end of its currently-playing track (at some point), even though it's not at the end of the playlist yet. Would you be willing to make such an addition here, or at least keep such a possibility in mind while finalizing the design of the functionality of this one, so the processes introduced here could be used to help make the feature I'm talking about?
Comment by Rosso Maltese (asettico) - Friday, 12 November 2010, 14:38 GMT
Synced against r28563.
It works, but it has a bug: if pause between tracks is enabled, inserting something in the playlist when a track is playing, it pauses current track, even if the end is not reached.
Comment by Mossroy (mossroy) - Sunday, 09 June 2013, 13:39 GMT
Thanks Rosso for this patch!
I managed to use it on Rockbox 3.7 : it compiled and seems to work like a charm.

It's a feature I was missing : I'm really happy to have it now :-)

That's something I love about free software : I have bought this Archos Jukebox Recorder v1 more than 10 years ago and I still can upgrade it, and modify its software to fit my needs.
It Rockbox did not exist, I would still be stuck with this basic and buggy firmware that was originally on the device.
I don't like to trash an item that works and can still be usefull. After all these years, I still can use my device, and rockbox trully expanded its software capabilities

Thanks to all the people who worked on this project
Comment by Richard O'Beirne (robeirne) - Tuesday, 18 March 2014, 23:02 GMT
I too would find this feature extremely useful when listening to audio books while going to sleep. However, I cannot get this patch to load on my old iPod nano 1G. When I enter "patch < 9114........" I always get the message "can't find file to patch at input line 5". Looking at the code of the patch the file structure does not seem to be the same as on my ipod. Am I doing something wrong?

Loading...