Re: Sndio support in MPV

On Wed Nov 25, 2020 at 6:11 AM -03, Alexandre Ratchov wrote:
> On Tue, Nov 24, 2020 at 11:04:22PM -0300, Érico Nogueira wrote:
> > On Tue Nov 24, 2020 at 10:52 AM -03, Érico Nogueira wrote:
> > > Hi!
> > >
> > > I don't keep up with MPV [1] development, so I was quite surprised that
> > > the latest release dropped support for sndio [2]. I was looking into
> > > whether it would be possible to simply revert the commit, but it turns
> > > out they made quite a few changes after it, so additional fixes are
> > > needed. I already "fixed" it enough to at least build successfully, but
> > > it doesn't yet work, and I will admit that I don't really understand all
> > > that's going on in that codebase.
> > >
> > > - [1] https://github.com/mpv-player/mpv
> > > - [2]
> > > https://github.com/mpv-player/mpv/commit/71d218eae4b4d93ada34ff74906f71ad359c84bc
> > >
> > > So, in order to avoid repeated work, I was wondering if anyone already
> > > tried to go this route? I couldn't find any open PRs in their
> > > repository.
> > >
> > > My hope is that they would consider adding the backend back once someone
> > > has done the work of porting it over to their new architecture. If
> > > anyone is pessimisitic about that, let me know too, I guess :P
> > >
> > > Thanks,
> > > Érico
> > 
> > Quick update, I have a branch for it that works (at least on my machine :).
> > I don't know if it's the best design for their new AO API, but it's (I
> > think) a natural evolution from the previous version. Michael Forney has
> > suggested using their pull API instead of the current push (it uses
> > callbacks called by the audio library for feeding audio when requested
> > instead of feeding audio into it ever so often), but I believe that
> > would require a threaded implementation inside the ao_sndio.c file,
> > which might not be accepted upstream.
> > 
> > Has there been discussion into providing any such API inside libsndio?
>
> There were no discussions about this. Probably because push/pull are
> different APIs for the same thing, and one API is easy to implement
> using the other. On Unix-like systems, all call-backs are necessarily
> implemented using a thread (explicit or hidden) which will call
> write() and the call-back in a loop.

Indeed. Since none of the other backends use explicit threads, I figured
that'd be a harder sell, so to speak. Having the API built into sndio
would make it easier (and allow for a single source of truth, as well).

That said, I don't think it will be necessary for this use case.

>
> > >From what I can see, the ao_jack.c and ao_sdl.c backends use this model,
> > and it seems to result in pretty clean code.
>
> I'll check this one; note that on OpenBSD sdl uses sndio, so if sdl
> works well, it would be easy to make sndio work and look clean as
> well.
>
> > One other thing I found to be "missing" was a `sio_pause()` call; mpv's
> > AO wants to write() before calling start(), which I worked around by
> > doing my own buffering and flushing it when start() is called. If
> > there's some trick or obvious functionality I missed, please let me
> > know.
>
> In play-only mode, sio_start() doesn't start immediately. It waits
> until the buffer is full and starts automatically only once the buffer
> is ready.

It would be nice to clear this up in the man page, then. From
sio_open(3):

  Once enough data is queued to ensure that play buffers
  will not underrun, actual playback is started automatically.

It isn't immediately obvious that playback waits for a full buffer,
instead of an arbitrary point where "enough data" exists.

>
> > The branch is in [1], and since upstream has removed sndio support once
> > already, I'd like to guarantee that the code is reasonable and that it
> > works fine before making a PR their way. Therefore, I'd highly
> > appreciate testing and code review, if anyone's up to it.
> > 
> > - [1] https://github.com/ericonr/mpv/tree/sndio
> > 
>
> thank you very much, I'll check this

Thanks,
Érico