[FFmpeg-devel,3/3] lavd/openal: return EAGAIN if no samples are available

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 libavdevice/openal-dec.c | 3 +++
 1 file changed, 3 insertions(+)

On Fri, 30 Sep 2016 11:29:07 +0200
Marton Balint <cus@passwd.hu> wrote:

> Signed-off-by: Marton Balint <cus@passwd.hu>
> ---
>  libavdevice/openal-dec.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/libavdevice/openal-dec.c b/libavdevice/openal-dec.c
> index 0647952..8773944 100644
> --- a/libavdevice/openal-dec.c
> +++ b/libavdevice/openal-dec.c
> @@ -191,6 +191,9 @@ static int read_packet(AVFormatContext* ctx, AVPacket *pkt)
>      alcGetIntegerv(ad->device, ALC_CAPTURE_SAMPLES, (ALCsizei) sizeof(ALCint), &nb_samples);
>      if (error = al_get_error(ad->device, &error_msg)) goto fail;
>  
> +    if (nb_samples == 0)
> +        return AVERROR(EAGAIN);
> +
>      /* Create a packet of appropriate size */
>      if ((error = av_new_packet(pkt, nb_samples*ad->sample_step)) < 0)
>          goto fail;

Kind of questionable. The libavformat machine is a state machine, so it
should either return a packet, EOF, some sort of timeout error, of
block until new data is available. It does not really have a concept of
having the API user wait for input (without blocking).

But I suspect this is a deeper issue with libavdevice and even some
network protocols in libavformat (I think ffmpeg.c and ffplay.c have
hacks for those), so go ahead.

Le nonidi 9 vendémiaire, an CCXXV, Marton Balint a écrit :
> Signed-off-by: Marton Balint <cus@passwd.hu>
> ---
>  libavdevice/openal-dec.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/libavdevice/openal-dec.c b/libavdevice/openal-dec.c
> index 0647952..8773944 100644
> --- a/libavdevice/openal-dec.c
> +++ b/libavdevice/openal-dec.c
> @@ -191,6 +191,9 @@ static int read_packet(AVFormatContext* ctx, AVPacket *pkt)
>      alcGetIntegerv(ad->device, ALC_CAPTURE_SAMPLES, (ALCsizei) sizeof(ALCint), &nb_samples);
>      if (error = al_get_error(ad->device, &error_msg)) goto fail;
>  
> +    if (nb_samples == 0)
> +        return AVERROR(EAGAIN);
> +

EAGAIN is only acceptable in non-blocking mode. You may be able to use REDO
(I do not remember the exact name), but beware you are not creating a
busy-wait loop.

Regards,

On Fri, 30 Sep 2016, Nicolas George wrote:

> Le nonidi 9 vendémiaire, an CCXXV, Marton Balint a écrit :
>> Signed-off-by: Marton Balint <cus@passwd.hu>
>> ---
>>  libavdevice/openal-dec.c | 3 +++
>>  1 file changed, 3 insertions(+)
>> 
>> diff --git a/libavdevice/openal-dec.c b/libavdevice/openal-dec.c
>> index 0647952..8773944 100644
>> --- a/libavdevice/openal-dec.c
>> +++ b/libavdevice/openal-dec.c
>> @@ -191,6 +191,9 @@ static int read_packet(AVFormatContext* ctx, AVPacket *pkt)
>>      alcGetIntegerv(ad->device, ALC_CAPTURE_SAMPLES, (ALCsizei) sizeof(ALCint), &nb_samples);
>>      if (error = al_get_error(ad->device, &error_msg)) goto fail;
>> 
>> +    if (nb_samples == 0)
>> +        return AVERROR(EAGAIN);
>> +
>
> EAGAIN is only acceptable in non-blocking mode. You may be able to use REDO
> (I do not remember the exact name), but beware you are not creating a
> busy-wait loop.
>

Alsa dev and v4l2 seem to work this way regardless if non blocking mode is 
set or not, and ffmpeg.c also waits if it encounters AVERROR(EAGAIN) in 
av_read_frame. So AVERROR(EAGAIN) return value seems to be consistent with 
existing code, which assumes that if EAGAIN is returned from 
av_read_frame, wait a bit.

If what you wrote is the way to move forward, then at least send a 
documentation patch, to establish this av_read_frame return 
value behaviour.

Regards,
Marton

Le nonidi 9 vendémiaire, an CCXXV, Marton Balint a écrit :
> Alsa dev and v4l2 seem to work this way regardless if non blocking mode is
> set or not,

After a quick glance, no, they do not, at least provided the kernel and
system libraries keep the same promise (which, AFAIK, they do).

>	      and ffmpeg.c also waits if it encounters AVERROR(EAGAIN) in
> av_read_frame.

It should not have to, but for a long time the demuxers were opened in
non-blocking mode, I suppose the code was not updated when it was changed.

>		 So AVERROR(EAGAIN) return value seems to be consistent with
> existing code, which assumes that if EAGAIN is returned from av_read_frame,
> wait a bit.
> 
> If what you wrote is the way to move forward, then at least send a
> documentation patch, to establish this av_read_frame return value behaviour.

I will consider doing that when time permit. But the behaviour is directly
copied from the Unix behaviour.

Regards,

On Fri, 30 Sep 2016, Nicolas George wrote:

> Le nonidi 9 vendémiaire, an CCXXV, Marton Balint a écrit :
>> Alsa dev and v4l2 seem to work this way regardless if non blocking mode is
>> set or not,
>
> After a quick glance, no, they do not, at least provided the kernel and
> system libraries keep the same promise (which, AFAIK, they do).
>

Hmm, apparently I missed the flag, you are right, sorry.

>> 	      and ffmpeg.c also waits if it encounters AVERROR(EAGAIN) in
>> av_read_frame.
>
> It should not have to, but for a long time the demuxers were opened in
> non-blocking mode, I suppose the code was not updated when it was changed.
>
>> 		 So AVERROR(EAGAIN) return value seems to be consistent with
>> existing code, which assumes that if EAGAIN is returned from av_read_frame,
>> wait a bit.
>>
>> If what you wrote is the way to move forward, then at least send a
>> documentation patch, to establish this av_read_frame return value behaviour.
>
> I will consider doing that when time permit. But the behaviour is directly
> copied from the Unix behaviour.
>

Thanks, I will rework the patch to work as the other devices.

Regards,
Marton