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

On 2023-03-26 15:58 +0200, Reimar Döffinger wrote:
>
> > 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:
> >>
> >> 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.

I would like doing something like that after a release.

Would need some design and probably more or less changes in all aos
depending on the exact architecture chosen.


> 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.

I intent to apply all of the patches after a quick test session in one go.

Will do it probably on Friday.


Thank you all,
  Alexander