[vlc-devel] [PATCH] aout: do not reset the instance volume to -1 on destroy

[Rémi Denis-Courmont]

Le torstaina 18. tammikuuta 2018, 10.22.11 EET Steve Lhomme a écrit :
> Le 17/01/2018 à 19:06, Rémi Denis-Courmont a écrit :
> > Le keskiviikkona 17. tammikuuta 2018, 18.53.04 EET Steve Lhomme a écrit :
> >> There might be other modules started that have already set their proper
> >> value.
> > 
> > First, that wouldn´t actually fix the problem - the reported value is
> > whatever
> I'm not sure if you're referring to my code or the sentence in the
> commit log. If it's for the code I can tell you it fixes that particular
> issue.

So what happens if the being-destroyed aout has updated the volume shortly 
before it got destroyed? You still end up with a garbage value at the input 
manager level. This patch only makes the problem less apparent, and thus much 
harder to debug going forward.

If there is only one output at time, this patch has no effect other than 
leaving a stale volume value in the input manager.

> > the last update of any aout was. It is not meaningful with more than one
> > aout.
> > 
> > And then, there cannot be more than one concurrent aout per input with the
> > default ES output parameters.
> 
> During playback yes. But when switching aout that's not what the code does.
> 
> In input/decoder.c there's a input_resource_GetAout() which creates the
> new aout (sets the volume) and then a
> input_resource_PutAout() (destroys the previous aout, resets the volume).

input_resource_PutAout() does not destroy the aout. It just shelves it for 
later reuse by input_resource_GetAout() (unless there are two aouts, but that 
is broken anyway, as noted above).

Also, as far as I am aware, and as far as I can read the code, it releases the 
audio output before it gets it back. But even if that´s not the case, I don't 
see how this patch can solve the problem.

You simply cannot expect a single value at input or input manager level to 
represent the state of more than one non-synchronized children.

-- 
雷米‧德尼-库尔蒙
https://www.remlab.net/