Re: [PATCH] Fix a sndio crash when exiting in muted state

> On 25 Mar 2023, at 16:22, Reimar Döffinger <[email protected]> wrote:
> 
>> On 24 Mar 2023, at 21:38, Alexander Strasser <[email protected]> wrote:
>> 
>> Hi Brad,
>> 
>> On 2023-03-22 22:58 +0100, Reimar Döffinger wrote:
>>>> Index: libao2/ao_sndio.c
>>>> ===================================================================
>>>> --- libao2/ao_sndio.c (revision 38412)
>>>> +++ libao2/ao_sndio.c (working copy)
>>>> @@ -159,6 +159,7 @@
>>>>   pfds = NULL;
>>>>   sio_close(hdl);
>>>>   hdl = NULL;
>>>> +    havevol = 0;
>>>>   return 0;
>>>> }
>>>> 
>>>> @@ -172,6 +173,7 @@
>>>>   hdl = NULL;
>>>>   free(pfds);
>>>>   pfds = NULL;
>>>> +    havevol = 0;
>>>> }
>>> 
>>> Well, it's probably a good idea to do that, but a aoctl happening after uninit (if I understand right) is the far worse issue an probably needs to be fixed by itself.
>> 
>> 
>> Could you test if the attached patch also fixes the issues you observed?
>> 
>> Only lightly tested but it would help if you could confirm if it fixes
>> the problem.
> 
> I don't know, this seems the far more obvious and straight-forward fix? IMO this looks just like an oversight really.
> 
> --- mplayer.c (revision 38410)
> +++ mplayer.c (working copy)
> @@ -672,6 +672,7 @@
>         if (mpctx->audio_out)
>             mpctx->audio_out->uninit(mpctx->eof ? 0 : 1);
>         mpctx->audio_out = NULL;
> +        mpctx->mixer.audio_out = NULL;
>     }
>   #ifdef CONFIG_GUI

After some discussion, probably all our patches make sense, but a full solution likely needs a bit more.
Essentially, I think the aos need to have a flag whether their mixer (volume etc) is a global, persistent one.
If it is, we should not NULL the mixer audio out here, but instead require the ao SETVOLUME remains valid/allowed after uninit.
To do it really properly, there would be also some additional logic to handle it fully correctly when different files are played with different -ao options.
Each of the patches on their own fixes that specific crash, and mine is probably safest there, but without Alexander's causes a regression (audio remains muted after MPlayer exits).
So if no objections, I think it would be best for someone (Alex?) to apply them all in one go.

Best regards,
Reimar
_______________________________________________
MPlayer-dev-eng mailing list
[email protected]
https://lists.mplayerhq.hu/mailman/listinfo/mplayer-dev-eng