Add play and pause commands #342

Closed
wants to merge 1 commit into
from

Projects

None yet

3 participants

@simpkins
Contributor
simpkins commented Jan 9, 2013

Add commands that always play and always pause, in addition to the
current toggle pause command.

@simpkins simpkins Add play and pause commands
Add commands that always play and always pause, in addition to the
current toggle pause command.
ea61725
Owner

Sorry, your pull request got lost in my inbox somehow.

Note that pauseMutex does not “protect” any variables currently. It’s a
hack that blocks the player thread so we don’t have to check a variable
in a busy loop.

Regarding separate play/pause commands: I guess you’re trying to
implement some kind of remote control?

Contributor

Yes, I use this for remote scripts that want to unconditionally pause
the player, regardless of the current state. (In my case, pausing the
player when the screen locks.)

My diff updates the code so that pauseMutex does protect the doQuit and
doPause variables. The pauseCond condition variable is used to signal
when these variables change (so the player thread can wait on the
condition variable rather than checking them in a busy loop).

This removes the need for the pthread_mutex_trylock() hack in
BarUiDoSkipSong(). It also makes the use of the doQuit variable
well-defined. (In general, if one thread updates doQuit, there is no
guarantee that other threads will ever see the change unless there is
also some other synchronization primitive that provides a memory fence.
The x86 architecture provides fairly conservative memory ordering
guarantees, so other threads will always see the doPause update on x86
platforms, but this isn't true for other architectures.)

I would like to +1 this feature. I have also made a remote control app and there is no feedback from pianobar for if it is currently playing or paused and this would make sure that my play/pause button was always in sync.

Any reason this shouldn't/can't be added?

Owner

Sorry it took me so long to review this. Although I made a few
adjustments here and there (see branch “pause”) I really like the
threading fixes :) Have a look at it.

I rejected a similar patch (non-toggling play/pause) some time ago, but
there seems to be demand, so I’ll apply this soon [with the warning that
using pianobar on headless machines is a pain and you should use a
different application for that purpose ;)].

@PromyLOPh PromyLOPh added a commit that closed this pull request Feb 26, 2013
@simpkins @PromyLOPh simpkins + Add play and pause commands
Add commands that always play and always pause, in addition to the
current toggle pause command.

Closes #342.
847ca41
@PromyLOPh PromyLOPh closed this in 847ca41 Feb 26, 2013

This is great news. Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment