[MPlayer-dev-eng] [PATCH] Add audio support for sndio API.

[Alexandre Ratchov]

On Thu, Dec 19, 2013 at 11:25:25AM +0100, Reimar Döffinger wrote:
> Alexandre Ratchov <alex at caoua.org> wrote:
> >On Wed, Dec 18, 2013 at 07:09:01PM +0100, Reimar Döffinger wrote:
> >> On Wed, Dec 18, 2013 at 10:02:20AM +0100, Alexandre Ratchov wrote:
> >> > +    int space;
> >> > +
> >> > +    /*
> >> > +     * prepare to start; then refill the buffer with the number of
> >bytes
> >> > +     * audio_pause() consumed (this will trigger start)
> >> > +     */
> >> > +    space = par.bufsz * par.pchan * par.bps - delay;
> >> > +    delay = 0;
> >> > +    if (!sio_start(hdl))
> >> > +        mp_msg(MSGT_AO, MSGL_ERR, "ao2: resume: couldn't
> >start\n");
> >> > +    mp_ao_resume_refill(&audio_out_sndio, space);
> >> 
> >> The delay = 0 doesn't look right.
> >> If there is still data in the buffers when we resume, that data will
> >> cause a delay, too, not just the data we refill.
> >> This looks to me like it should cause permanent A-V desync if pausing
> >> only for a very short time (well, until you pause long enough to
> >> completely empty the buffer at least).
> >
> >You're right; so the "delay" counter must be saved before we call
> >sio_stop(). See new diff, below.
> >
> >It works here, no A/V desync. Pausing drains the buffer (I hear
> >~250ms of audio after I hit space). Resuming, plays the same amount
> >of silence instead of the original sound. A/V sync seems robust. I
> >tried to amplify the effect by forcing a huge buffer (1.3s), and
> >couldn't trigger any desync.
> 
> After a pause-resume sequence the delay is still set to 0.
> Also you seem to have tested the opposite of what I said. I see an issue when pausing for only a few milliseconds. Setting the delay to 0 isn't wrong when you wait until the buffers have drained. But it is wrong if you don't.
> 
> >> It should also result in delay potentially becoming negative.
> >>
> >
> >This can't happen, it's warranted by sndio internals that the delay
> >is in the 0..(bufsz-1) range (except during execution of
> >sio_write(), when it can't be calculated)
> 
> Sorry, but this makes 0 sense to me. Delay is something you
> calculate yourself in e.g. movcb, you don't get it from sndio, so
> I don't understand how what sndio does helps you.

During playback, the delay is defined as:

delay = (samples_written - samples_played) / sample_rate

where samples_played is the sum of "deltas" reported by the
onmove() call-back.

It can't be negative (sndio pauses when the buffer is empty) and
can't exceed buffer size (sio_write() will block if there isn't
enough buffer space).

> Also, since reset does not drop the buffers I can't see how it
> should be correct to set delay to 0, any new data should be
> delayed by as much time as there is data the old buffers.
>

When we call sio_start() to resume, we consider buffers are drained
(thus empty) and no samples have been played yet. Thus the delay
counter (with above definition) is zero. At this stage, get_space()
returns the buffer size.

Then, we refill the buffer with the same amount of samples the
buffer used to contain right before we called sio_start(), thus the
delay counter reaches its pre-pause value. So at this stage,
get_space() and get_delay() return the same values as when pause()
was called. In other words the "state" is fully restored across a
pause()/resume() cycle.

> >
> >Delays caused by drained buffers are the only side effects.
> 
> I thought of side-effects like stopping to call the movecb
> callback, which would basically break the whole thing.

the onmove() call-back wont be called after sio_stop() returns. It
may be called during the sio_stop() call, hence the last fix.

-- Alexandre