2017-03-28 06:13 CEST

View Issue Details Jump to Notes ]
IDProjectCategoryView StatusLast Update
0004528MPDMPDpublic2016-05-17 22:11
Reporterincertia 
Assigned To 
PrioritynormalSeverityminorReproducibilityalways
StatusnewResolutionopen 
PlatformOSlinuxOS Version4.5.4
Product Version0.19 
Target VersionFixed in Version 
Summary0004528: pause command skips minor amounts of audio
Descriptionwhen pausing/unpausing mpd, mpd will pause to what seems like the nearest 0.25 second rather stopping at the current time slice.
Steps To Reproducepause/unpause repeatedly in rapid fashion

demo: https://my.mixtape.moe/cgqrlv.mp4
Tagscommands, mpd
Attached Files

-Relationships
+Relationships

-Notes

~0009939

cirrus (administrator)

This is an unfortunate side effect of MPD clearing the ring buffer on pause.

~0009941

cirrus (administrator)

Audio playback isn't that simple. You can't just stop sending data. If you do, your computer will keep on playing for a few seconds, until all buffers have run empty. Users however expect MPD to pause playback instantly after pressing "pause", which involves flushing all those buffers.

~0009943

incertia (reporter)

Last edited: 2016-05-17 21:14

View 4 revisions

Can't we just save which time slice we currently are on when we initiate a pause command and start reloading the audio buffer from that slice on an unpause operation?

e.g. for PulseAudio, we should be able to use pa_stream_cork(true/false) to have pulse halt audio immediately and retain the information in the buffer (unless my understanding from https://freedesktop.org/software/pulseaudio/doxygen/stream_8h.html#a14e698233ac2d246646651955ab0ec7b is incorrect).

~0009945

cirrus (administrator)

Of course you can do that. But I wouldn't, because this means massive amounts of overhead, for a lousy result. There's a better way to do it (keep samples in the buffer until it's really been played completely by all outputs, not just put in their ring buffers), but it's complicated to implement. Would take one quite a few hours to implement, and this isn't the most urgent request.

~0009946

VAMP (reporter)

I think this problem also applies to "clicks while rewinding tracks in DSD DoP mode "

~0009947

cirrus (administrator)

No, it doesn't.

~0009948

incertia (reporter)

Ok I just tried what I mentioned with pa_stream_cork and while it does offer better pausing, it produces unwanted static for a short period of time. I'll see if I can hack together a solution and upload a patch during my free time.

~0009950

incertia (reporter)

Continuing with 0004528:0009948, we actually don't need to cork the stream in PulseOutput::Pause(). The stream ends up with no data due to PulseOutput::Cancel(), so
    if (!pa_stream_is_corked(stream) && !StreamPause(true, error)) {
        pa_threaded_mainloop_unlock(mainloop);
        LogError(error);
        return false;
    }

ends up not really doing anything (behavior is the same when this bit is commented out). In fact, with pulse, AudioOutput::Pause() can behave like
inline void
AudioOutput::Pause()
{
    mutex.unlock();
    ao_plugin_cancel(this);
    mutex.lock();

    pause = true;
    CommandFinished();

    do {
        if (!WaitForDelay())
            break;

        mutex.unlock();
        // bool success = ao_plugin_pause(this);
        mutex.lock();

        // if (!success) {
        // 	Close(false);
        // 	break;
        // }
    } while (command == Command::NONE);

    pause = false;
}

and still achieve the desired result. However, removing the mutex unlock/lock sequence causes clients to timeout for some unknown reason.

~0009951

cirrus (administrator)

That's because you just created a busy loop with a mutex held. When you leave the unlock/lock in, it only occurs to be responsive randomly. This change is bad.
+Notes

-Issue History
Date Modified Username Field Change
2016-05-16 21:32 incertia New Issue
2016-05-16 21:33 incertia Tag Attached: mpd
2016-05-16 21:33 incertia Tag Attached: commands
2016-05-17 17:31 cirrus Note Added: 0009939
2016-05-17 20:29 cirrus Note Added: 0009941
2016-05-17 21:07 incertia Note Added: 0009943
2016-05-17 21:11 incertia Note Edited: 0009943 View Revisions
2016-05-17 21:11 cirrus Note Added: 0009945
2016-05-17 21:14 incertia Note Edited: 0009943 View Revisions
2016-05-17 21:14 incertia Note Edited: 0009943 View Revisions
2016-05-17 21:17 VAMP Note Added: 0009946
2016-05-17 21:18 cirrus Note Added: 0009947
2016-05-17 21:25 incertia Note Added: 0009948
2016-05-17 22:08 incertia Note Added: 0009950
2016-05-17 22:11 cirrus Note Added: 0009951
+Issue History