search

MusicPlayerDaemon / MPD

Music Player Daemon
https://www.musicpd.org/
GNU General Public License v2.0
2.2k stars 350 forks source link

Unpause and immediately pause skips a second #1136

Open fwsmit opened 3 years ago

fwsmit commented 3 years ago

Bug report

Describe the bug

Why I toggle mpd playback twice in a short time, a second of the song is skipped. This can be most easily reproduced when running the following commands when mpd is paused:`

mpc play && mpc pause

Expected Behavior

No / very little time is skipped

Actual Behavior

1 second is skipped

Version

Music Player Daemon 0.22.6 (0.22.6)
Copyright 2003-2007 Warren Dukes <warren.dukes@gmail.com>
Copyright 2008-2018 Max Kellermann <max.kellermann@gmail.com>
This is free software; see the source for copying conditions.  There is NO
warranty; not even MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

Database plugins:
 simple proxy upnp

Storage plugins:
 local udisks nfs curl

Neighbor plugins:
 upnp udisks

Decoders plugins:
 [mad] mp3 mp2
 [mpg123] mp3
 [vorbis] ogg oga
 [oggflac] ogg oga
 [flac] flac
 [opus] opus ogg oga
 [sndfile] wav aiff aif au snd paf iff svx sf voc w64 pvf xi htk caf sd2
 [audiofile] wav au aiff aif
 [dsdiff] dff
 [dsf] dsf
 [hybrid_dsd] m4a
 [faad] aac
 [mpcdec] mpc
 [wavpack] wv
 [modplug] 669 amf ams dbm dfm dsm far it med mdl mod mtm mt2 okt s3m stm ult umx xm
 [mikmod] amf dsm far gdm imf it med mod mtm s3m stm stx ult uni xm
 [sidplay] sid mus str prg P00
 [wildmidi] mid
 [fluidsynth] mid
 [ffmpeg] 16sv 3g2 3gp 4xm 8svx aa3 aac ac3 adx afc aif aifc aiff al alaw amr anim apc ape asf atrac au aud avi avm2 avs bap bfi c93 cak cin cmv cpk daud dct divx dts dv dvd dxa eac3 film flac flc fli fll flx flv g726 gsm gxf iss m1v m2v m2t m2ts m4a m4b m4v mad mj2 mjpeg mjpg mka mkv mlp mm mmf mov mp+ mp1 mp2 mp3 mp4 mpc mpeg mpg mpga mpp mpu mve mvi mxf nc nsv nut nuv oga ogm ogv ogx oma ogg omg opus psp pva qcp qt r3d ra ram rl2 rm rmvb roq rpl rvc shn smk snd sol son spx str swf tak tgi tgq tgv thp ts tsp tta xa xvid uv uv2 vb vid vob voc vp6 vmd wav webm wma wmv wsaud wsvga wv wve rtp:// rtsp:// rtsps://
 [gme] ay gbs gym hes kss nsf nsfe sap spc vgm vgz
 [pcm]

Filters:
 libsamplerate soxr

Tag plugins:
 id3tag

Output plugins:
 shout null fifo pipe alsa ao oss openal solaris pulse jack httpd recorder

Encoder plugins:
 null vorbis opus lame twolame wave flac

Archive plugins:
 [bz2] bz2
 [zzip] zip
 [iso] iso

Input plugins:
 file io_uring archive alsa tidal qobuz curl ffmpeg nfs mms cdio_paranoia

Playlist plugins:
 extm3u m3u pls xspf asx rss soundcloud flac cue embcue

Protocols:
 file:// alsa:// cdda:// ftp:// ftps:// gopher:// hls+http:// hls+https:// http:// https:// mms:// mmsh:// mmst:// mmsu:// nfs:// qobuz:// rtmp:// rtmpe:// rtmps:// rtmpt:// rtmpte:// rtmpts:// rtp:// rtsp:// rtsps:// scp:// sftp:// smb:// srtp:// tidal://

Other features:
 avahi dbus udisks epoll icu inotify ipv6 systemd tcp un

Log

Command output

❯ mpc play && mpc pause
Muse - Uprising
[playing] #22/23   0:02/4:12 (0%)
volume: n/a   repeat: off   random: off   single: off   consume: off
Muse - Uprising
[paused]  #22/23   0:02/4:12 (0%)
volume: n/a   repeat: off   random: off   single: off   consume: off
❯ mpc play && mpc pause
Muse - Uprising
[playing] #22/23   0:03/4:12 (1%)
volume: n/a   repeat: off   random: off   single: off   consume: off
Muse - Uprising
[paused]  #22/23   0:03/4:12 (1%)
volume: n/a   repeat: off   random: off   single: off   consume: off
❯ mpc play && mpc pause
Muse - Uprising
[playing] #22/23   0:04/4:12 (1%)
volume: n/a   repeat: off   random: off   single: off   consume: off
Muse - Uprising
[paused]  #22/23   0:04/4:12 (1%)
volume: n/a   repeat: off   random: off   single: off   consume: off

mpd output

config_file: loading file /home/friso/.config/mpd/mpd.conf
Mar 30 14:37 : exception: bind to '0.0.0.0:6600' failed (continuing anyway, because binding to '[::]:6600' succeeded): Failed to bind socket: Address already in use
Mar 30 14:37 : libsamplerate: libsamplerate converter 'Fastest Sinc Interpolator'
Mar 30 14:37 : vorbis: Xiph.Org libVorbis 1.3.7
Mar 30 14:37 : opus: libopus 1.3.1
Mar 30 14:37 : sndfile: libsndfile-1.0.31
Mar 30 14:37 : hybrid_dsd: The Hybrid DSD decoder is disabled because it was not explicitly enabled
Mar 30 14:37 : exception: Decoder plugin 'wildmidi' is unavailable: configuration file does not exist: /etc/timidity/timidity.cfg
Mar 30 14:37 : simple_db: reading DB
Mar 30 14:37 : output: No 'audio_output' defined in config file
Mar 30 14:37 : output: Attempt to detect audio output device
Mar 30 14:37 : output: Attempting to detect a alsa audio device
Mar 30 14:37 : output: Successfully detected a alsa audio device
Failed to initialize io_uring: io_uring_queue_init() failed: Cannot allocate memory
Mar 30 14:37 : exception: Input plugin 'tidal' is not configured: No Tidal application token configured
Mar 30 14:37 : exception: Input plugin 'qobuz' is not configured: No Qobuz app_id configured
Mar 30 14:37 : curl: version 7.75.0
Mar 30 14:37 : curl: with OpenSSL/1.1.1k
naglis commented 3 years ago

I am experiencing the same. I've played with available configuration options and found that reducing the buffer_time inside audio_output config block alleviates the problem for me. I am using ALSA output, so reducing it from the default 500000 μs to e.g. 25000 makes the skipped time almost imperceptible:

audio_output {
    type "alsa"
    name "ALSA output"
    buffer_time "25000"
}

However, the documentation states:

Don’t change unless you know what you’re doing.

and since I don't really know what I am doing I am not sure if it is a good solution. According to this blog post, reducing it increases MPD's CPU usage and that seems to be true. Not sure what other potentially negative effects changing it might have.

ncfavier commented 3 years ago

I have also noticed this issue, using both the pulse and pipewire outputs.

Sciencentistguy commented 2 years ago

I am also experiencing this. Is there a known solution?

fwsmit commented 2 years ago

Not really. There is a workaround mentioned above, but I guess that buffer is there for a reason, so I don't want to mess with it. I've tried looking if I can fix it in mpd, but haven't found where to begin

Sciencentistguy commented 2 years ago

Out of interest, what audio hardware do you have? And how is it connected? And which audio daemon do you use (pulse, pipewire, etc)

fwsmit commented 2 years ago

I've had this issue with both pulseuadio and pipewire. I'm using the internal intel audio of my laptop: Audio device [0403]: Intel Corporation Cannon Lake PCH cAVS [8086:a348] (rev 10) (prog-if 80) Subsystem: Hewlett-Packard Company Device [103c:8427], but it's also reproducible with a headset connected to bluetooth or on other hardware.

My full audio info can be found here.

ncfavier commented 2 years ago

I think the problem is that audio outputs don't have an interface for unpausing, and the unpausing logic actually re-opens the outputs, thus discarding buffers.

I think we need a virtual AudioOutput::Unpause that returns false, an overridden PulseOutput::Unpause that just does the opposite of PulseOutput::Pause (similarly for other outputs that support pausing), and then the unpausing logic should do something like if (!output.Unpause()) output.Open() for all outputs. I can try implementing this but I'm not too sure about how to handle the asynchronous stuff. @MaxKellermann thoughts?

fwsmit commented 2 years ago

I didn't think of this approach. I'm not familiar with the code, but from the linked code, I don't really see if it should work. I'm willing to test the code though, if you have a proof of concept.

incertia commented 2 years ago

I vaguely remember discussing this a long time ago in 2016 on the old mantis system (https://bugs.musicpd.org/view.php?id=4528 dead link now unfortunately) and it is ultimately what made me switch away from the mpd ecosystem. I am resurrecting what I can extract from my email archives here.

A NOTE has been added to this issue. 
====================================================================== 
https://bugs.musicpd.org/view.php?id=4528 
====================================================================== 
Reported By:                incertia
Assigned To:                
====================================================================== 
Project:                    MPD
Issue ID:                   4528
Category:                   MPD
Reproducibility:            always
Severity:                   minor
Priority:                   normal
Status:                     new
====================================================================== 
Date Submitted:             2016-05-16 19:32 UTC
Last Modified:              2016-05-17 20:11 UTC
====================================================================== 
Summary:                    pause command skips minor amounts of audio
Description: 
when pausing/unpausing mpd, mpd will pause to what seems like the nearest 0.25
second rather stopping at the current time slice.

Steps to Reproduce: 
pause/unpause repeatedly in rapid fashion

demo: https://my.mixtape.moe/cgqrlv.mp4
====================================================================== 

---------------------------------------------------------------------- 
 (0009939) cirrus (administrator) - 2016-05-17 15:31
 https://bugs.musicpd.org/view.php?id=4528#c9939 
---------------------------------------------------------------------- 
This is an unfortunate side effect of MPD clearing the ring buffer on pause. 

---------------------------------------------------------------------- 
 (0009941) cirrus (administrator) - 2016-05-17 18:29
 https://bugs.musicpd.org/view.php?id=4528#c9941 
---------------------------------------------------------------------- 
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) - 2016-05-17 19:14
 https://bugs.musicpd.org/view.php?id=4528#c9943 
---------------------------------------------------------------------- 
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) - 2016-05-17 19:11
 https://bugs.musicpd.org/view.php?id=4528#c9945 
---------------------------------------------------------------------- 
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) - 2016-05-17 19:17
 https://bugs.musicpd.org/view.php?id=4528#c9946 
---------------------------------------------------------------------- 
I think this problem also applies to "clicks while rewinding tracks in DSD DoP
mode " 

---------------------------------------------------------------------- 
 (0009947) cirrus (administrator) - 2016-05-17 19:18
 https://bugs.musicpd.org/view.php?id=4528#c9947 
---------------------------------------------------------------------- 
No, it doesn't. 

---------------------------------------------------------------------- 
 (0009948) incertia (reporter) - 2016-05-17 19:25
 https://bugs.musicpd.org/view.php?id=4528#c9948 
---------------------------------------------------------------------- 
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) - 2016-05-17 20:08
 https://bugs.musicpd.org/view.php?id=4528#c9950 
---------------------------------------------------------------------- 
Continuing with https://bugs.musicpd.org/view.php?id=4528#c9948, we actually
don't need to cork the stream in PulseOutput::Pause(). The stream ends up with
no data due to PulseOutput::Cancel(), so
<pre>
    if (!pa_stream_is_corked(stream) && !StreamPause(true, error)) {
        pa_threaded_mainloop_unlock(mainloop);
        LogError(error);
        return false;
    }
</pre>
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
<pre>
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;
}
</pre>
and still achieve the desired result. However, removing the mutex unlock/lock
sequence causes clients to timeout for some unknown reason. 

---------------------------------------------------------------------- 
 (0009951) cirrus (administrator) - 2016-05-17 20:11
 https://bugs.musicpd.org/view.php?id=4528#c9951 
---------------------------------------------------------------------- 
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. 

Issue History 
Date Modified    Username       Field                    Change               
====================================================================== 
2016-05-16 19:32 incertia       New Issue                                    
2016-05-16 19:33 incertia       Tag Attached: mpd                            
2016-05-16 19:33 incertia       Tag Attached: commands                       
2016-05-17 15:31 cirrus         Note Added: 0009939                          
2016-05-17 17:52 incertia       Note Added: 0009940                          
2016-05-17 18:03 incertia       Note Edited: 0009940                         
2016-05-17 18:28 incertia       Note Deleted: 0009940                        
2016-05-17 18:29 cirrus         Note Added: 0009941                          
2016-05-17 19:07 incertia       Note Added: 0009943                          
2016-05-17 19:11 incertia       Note Edited: 0009943                         
2016-05-17 19:11 cirrus         Note Added: 0009945                          
2016-05-17 19:14 incertia       Note Edited: 0009943                         
2016-05-17 19:14 incertia       Note Edited: 0009943                         
2016-05-17 19:17 VAMP           Note Added: 0009946                          
2016-05-17 19:18 cirrus         Note Added: 0009947                          
2016-05-17 19:25 incertia       Note Added: 0009948                          
2016-05-17 20:08 incertia       Note Added: 0009950                          
2016-05-17 20:11 cirrus         Note Added: 0009951                          
======================================================================

In particular, I still have no idea how to fix this issue and it also seems like it is a very low priority issue (despite offering not very good UX) so I wish anyone else wanting to take up the torch the best of luck.

iambeingtracked commented 1 year ago

2023 And that's still a problem. Will it ever be fixed?

MaxKellermann commented 1 year ago

If it's important enough that it's worth your time, go ahead and fix it. If it's not important enough for your time, it's not important enough.

iambeingtracked commented 1 year ago

But is that so hard to fix? I mean, if there is a problem it should be fixed. I wanted to know whether there's any progress on this

MaxKellermann commented 1 year ago

If you believe it should be fixed, go ahead and fix it. Put your time where your mouth is. If you're not willing to take the time to help fix it, please stop spamming here.

NexAdn commented 1 year ago

But is that so hard to fix? I mean, if there is a problem it should be fixed. I wanted to know whether there's any progress on this

Looking at the list of open issues, it's not very small. And I assume the maintainers, like most open source developers, work on this project in their free time. They have to prioritize stuff and this is a relatively minor problem when looking at the problem description. If this issue is important to you, you should really consider fixing it yourself. Developers are only humans with limited capacity to work on free and open source projects...

iambeingtracked commented 1 year ago

And I assume the maintainers, like most open source developers, work on this project in their free time.

I know. I just wanted to ask. If there's none, then okay. You know better what to do with the project than me