[MPlayer-dev-eng] [PATCH] Bug fix in af_add/del/switch/clr

[Clément Bœsch]

On Wed, Jan 19, 2011 at 02:29:03PM +0900, Jehan Pagès wrote:
> Hi,
> 

Hi,

> I have been recently noticed by a user of a bug with audio filter
> loading-on-the-run commands. Some audio frames were lost, which was
> unnoticed when used punctually, but when loading/reloading/clearing a
> lot filters in some time span, more frames lost became noticeable,
> hence speed was accelerating. And consequently some videos were not
> resyncing well, so images and audios got desynchronized.
> 
> Though it seems unlikely to load filters every few milliseconds to
> notice this, the user who reported the bug used an audio equalizer
> filter reloading. I guess such an equalizer could be very often
> reloaded by a mplayer GUI while a user is trying to find the right
> values to apply to each frequency band (I guess this person was indeed
> developping some GUI to mplayer). And he wants to do this without
> music speeding or video desyncing. And anyway that's a bug, so it is
> good to fix. :-)
> After test, the problem occurred with any audio filter anyway.
> 
> -------------------
> To reproduce:
> 
> $ mkinfo /tmp/fifompdev
    ^^^^^^
     mkfifo I suppose?

> $ mplayer  -slave -input file=/tmp/fifompdev <your_media_file>
> $ while true ; do echo af_switch equalizer=0:0:0:0:0:0:0:0:1:1
> >/tmp/fifompdev; sleep 0.1; done
> 
> What happens and should not: in an audio file, the music will
> accelerate. In a video file, audio and video may desync.

I don't really have a desync but much more sound artifacts.

> -------------------
> 
> Basically to fix, I removed calls to reinit_audio_chain(); after such
> a dynamic load/unload of a filter. This was causing the issue and this
> was an unecessary call actually as any sound reinitialization was
> already dealed with in the functions which actually loaded/unloaded
> audio filters.
> 

While it may be actually uneeded, I don't understand why it is harmful.

> Attached is the small patch file.
> You can try the bug reproduction before and after, and confirms it has
> been fixed (I confirmed it on my machine and the user who reported me
> the bug also did for him).

Yes, I can confirm it fixes the issue.

Btw, I'm not qualified to make a proper review, I can only state the fact
that well, it works. So whenever a maintainer gives his ok, I'll commit
it. Until then, I can't do anything.

Regards,

-- 
Clément B.