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

[Reimar Döffinger]

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

>
>> That is assuming that the sio_stop/sio_start sequence doesn't have
>> any strange side-effects.
>>
>
>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.