Rockbox

…and here’s the patch.

   pause_between_tracks.patch (6 KiB)

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).

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.

Here is an updated version, without warnings.

   pause_between_tracks-2.patch (6.5 KiB)

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.

Maybe. Care to modify the patch accordingly?

Of course, but probably not for a week or 2

Real Life is such an intrusive thing.

The question is where to move it, since the code really belongs in the playback module. I could of course duplicate the code…

Resync’d against r22137.

   apps-9114pause_between_tracks... (6.3 KiB)

Resync’d against 24003.

   pause_between_tracks_24003.di... (6.4 KiB)

Added a check to make sure the pause doesn’t happen if the DAP has party mode set.

   pause_between_tracks_24003_2.... (6.4 KiB)

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.

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?

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)?

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.

And what about a three value setting instead of o boolean?
Something like: off, on but in party mode, always on.

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.

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.

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…

@pondlife, please bring that discussion to IRC ;)

Resync’d against 24024, and fixed compilation so it would work on HWCODEC.

   pause_between_tracks_latest.d... (6.6 KiB)

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…

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).

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?

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.

   9114-apps-Pause_between_track... (6.2 KiB)

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

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?