diff mbox series

[FFmpeg-devel,v2,2/5] avdevice/alsa_dec: make sure we have enough data in non-blocking mode

Message ID 20210301222049.15456-1-cus@passwd.hu
State Accepted
Commit 1a90cf4410a3464c8d749242e23629776f310ee0
Headers show
Series Untitled series #3437
Related show

Commit Message

Marton Balint March 1, 2021, 10:20 p.m. UTC
Otherwise we might return 1-2 samples per packet if av_read_frame() call rate is
only sligthly less than the stream sample rate.

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 libavdevice/alsa.c     |  5 +++++
 libavdevice/alsa.h     |  1 +
 libavdevice/alsa_dec.c | 22 ++++++++++++----------
 3 files changed, 18 insertions(+), 10 deletions(-)

Comments

Marton Balint March 6, 2021, 5:25 p.m. UTC | #1
On Mon, 1 Mar 2021, Marton Balint wrote:

> Otherwise we might return 1-2 samples per packet if av_read_frame() call rate is
> only sligthly less than the stream sample rate.

Ping for this and the rest of the series. Note that the approach in 
this patch makes 1/5 unneeded because constant frame size is now indeed 
guaranteed.

Regards,
Marton

>
> Signed-off-by: Marton Balint <cus@passwd.hu>
> ---
> libavdevice/alsa.c     |  5 +++++
> libavdevice/alsa.h     |  1 +
> libavdevice/alsa_dec.c | 22 ++++++++++++----------
> 3 files changed, 18 insertions(+), 10 deletions(-)
>
> diff --git a/libavdevice/alsa.c b/libavdevice/alsa.c
> index 117b2ea144..ee282fac16 100644
> --- a/libavdevice/alsa.c
> +++ b/libavdevice/alsa.c
> @@ -286,6 +286,10 @@ av_cold int ff_alsa_open(AVFormatContext *ctx, snd_pcm_stream_t mode,
>         }
>     }
> 
> +    s->pkt = av_packet_alloc();
> +    if (!s->pkt)
> +        goto fail1;
> +
>     s->h = h;
>     return 0;
> 
> @@ -308,6 +312,7 @@ av_cold int ff_alsa_close(AVFormatContext *s1)
>     if (CONFIG_ALSA_INDEV)
>         ff_timefilter_destroy(s->timefilter);
>     snd_pcm_close(s->h);
> +    av_packet_free(&s->pkt);
>     return 0;
> }
> 
> diff --git a/libavdevice/alsa.h b/libavdevice/alsa.h
> index 1ed8c82199..07783c983a 100644
> --- a/libavdevice/alsa.h
> +++ b/libavdevice/alsa.h
> @@ -58,6 +58,7 @@ typedef struct AlsaData {
>     void *reorder_buf;
>     int reorder_buf_size; ///< in frames
>     int64_t timestamp; ///< current timestamp, without latency applied.
> +    AVPacket *pkt;
> } AlsaData;
> 
> /**
> diff --git a/libavdevice/alsa_dec.c b/libavdevice/alsa_dec.c
> index 6d568737b3..88bf32d25f 100644
> --- a/libavdevice/alsa_dec.c
> +++ b/libavdevice/alsa_dec.c
> @@ -104,34 +104,36 @@ static int audio_read_packet(AVFormatContext *s1, AVPacket *pkt)
>     int64_t dts;
>     snd_pcm_sframes_t delay = 0;
> 
> -    if (av_new_packet(pkt, s->period_size * s->frame_size) < 0) {
> -        return AVERROR(EIO);
> +    if (!s->pkt->data) {
> +        int ret = av_new_packet(s->pkt, s->period_size * s->frame_size);
> +        if (ret < 0)
> +            return ret;
> +        s->pkt->size = 0;
>     }
> 
> -    while ((res = snd_pcm_readi(s->h, pkt->data, s->period_size)) < 0) {
> +    do {
> +        while ((res = snd_pcm_readi(s->h, s->pkt->data + s->pkt->size, s->period_size - s->pkt->size / s->frame_size)) < 0) {
>         if (res == -EAGAIN) {
> -            av_packet_unref(pkt);
> -
>             return AVERROR(EAGAIN);
>         }
> +        s->pkt->size = 0;
>         if (ff_alsa_xrun_recover(s1, res) < 0) {
>             av_log(s1, AV_LOG_ERROR, "ALSA read error: %s\n",
>                    snd_strerror(res));
> -            av_packet_unref(pkt);
> -
>             return AVERROR(EIO);
>         }
>         ff_timefilter_reset(s->timefilter);
> -    }
> +        }
> +        s->pkt->size += res * s->frame_size;
> +    } while (s->pkt->size < s->period_size * s->frame_size);
> 
> +    av_packet_move_ref(pkt, s->pkt);
>     dts = av_gettime();
>     snd_pcm_delay(s->h, &delay);
>     dts -= av_rescale(delay + res, 1000000, s->sample_rate);
>     pkt->pts = ff_timefilter_update(s->timefilter, dts, s->last_period);
>     s->last_period = res;
> 
> -    pkt->size = res * s->frame_size;
> -
>     return 0;
> }
> 
> -- 
> 2.26.2
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Marton Balint March 10, 2021, 7:25 p.m. UTC | #2
On Sat, 6 Mar 2021, Marton Balint wrote:

>
>
> On Mon, 1 Mar 2021, Marton Balint wrote:
>
>> Otherwise we might return 1-2 samples per packet if av_read_frame() call 
> rate is
>> only sligthly less than the stream sample rate.
>
> Ping for this and the rest of the series. Note that the approach in 
> this patch makes 1/5 unneeded because constant frame size is now indeed 
> guaranteed.

Will apply the series soon.

Regards,
Marton

>>
>> Signed-off-by: Marton Balint <cus@passwd.hu>
>> ---
>> libavdevice/alsa.c     |  5 +++++
>> libavdevice/alsa.h     |  1 +
>> libavdevice/alsa_dec.c | 22 ++++++++++++----------
>> 3 files changed, 18 insertions(+), 10 deletions(-)
>>
>> diff --git a/libavdevice/alsa.c b/libavdevice/alsa.c
>> index 117b2ea144..ee282fac16 100644
>> --- a/libavdevice/alsa.c
>> +++ b/libavdevice/alsa.c
>> @@ -286,6 +286,10 @@ av_cold int ff_alsa_open(AVFormatContext *ctx, 
> snd_pcm_stream_t mode,
>>         }
>>     }
>> 
>> +    s->pkt = av_packet_alloc();
>> +    if (!s->pkt)
>> +        goto fail1;
>> +
>>     s->h = h;
>>     return 0;
>> 
>> @@ -308,6 +312,7 @@ av_cold int ff_alsa_close(AVFormatContext *s1)
>>     if (CONFIG_ALSA_INDEV)
>>         ff_timefilter_destroy(s->timefilter);
>>     snd_pcm_close(s->h);
>> +    av_packet_free(&s->pkt);
>>     return 0;
>> }
>> 
>> diff --git a/libavdevice/alsa.h b/libavdevice/alsa.h
>> index 1ed8c82199..07783c983a 100644
>> --- a/libavdevice/alsa.h
>> +++ b/libavdevice/alsa.h
>> @@ -58,6 +58,7 @@ typedef struct AlsaData {
>>     void *reorder_buf;
>>     int reorder_buf_size; ///< in frames
>>     int64_t timestamp; ///< current timestamp, without latency applied.
>> +    AVPacket *pkt;
>> } AlsaData;
>> 
>> /**
>> diff --git a/libavdevice/alsa_dec.c b/libavdevice/alsa_dec.c
>> index 6d568737b3..88bf32d25f 100644
>> --- a/libavdevice/alsa_dec.c
>> +++ b/libavdevice/alsa_dec.c
>> @@ -104,34 +104,36 @@ static int audio_read_packet(AVFormatContext *s1, 
> AVPacket *pkt)
>>     int64_t dts;
>>     snd_pcm_sframes_t delay = 0;
>> 
>> -    if (av_new_packet(pkt, s->period_size * s->frame_size) < 0) {
>> -        return AVERROR(EIO);
>> +    if (!s->pkt->data) {
>> +        int ret = av_new_packet(s->pkt, s->period_size * s->frame_size);
>> +        if (ret < 0)
>> +            return ret;
>> +        s->pkt->size = 0;
>>     }
>> 
>> -    while ((res = snd_pcm_readi(s->h, pkt->data, s->period_size)) < 0) {
>> +    do {
>> +        while ((res = snd_pcm_readi(s->h, s->pkt->data + s->pkt->size, 
> s->period_size - s->pkt->size / s->frame_size)) < 0) {
>>         if (res == -EAGAIN) {
>> -            av_packet_unref(pkt);
>> -
>>             return AVERROR(EAGAIN);
>>         }
>> +        s->pkt->size = 0;
>>         if (ff_alsa_xrun_recover(s1, res) < 0) {
>>             av_log(s1, AV_LOG_ERROR, "ALSA read error: %s\n",
>>                    snd_strerror(res));
>> -            av_packet_unref(pkt);
>> -
>>             return AVERROR(EIO);
>>         }
>>         ff_timefilter_reset(s->timefilter);
>> -    }
>> +        }
>> +        s->pkt->size += res * s->frame_size;
>> +    } while (s->pkt->size < s->period_size * s->frame_size);
>> 
>> +    av_packet_move_ref(pkt, s->pkt);
>>     dts = av_gettime();
>>     snd_pcm_delay(s->h, &delay);
>>     dts -= av_rescale(delay + res, 1000000, s->sample_rate);
>>     pkt->pts = ff_timefilter_update(s->timefilter, dts, s->last_period);
>>     s->last_period = res;
>> 
>> -    pkt->size = res * s->frame_size;
>> -
>>     return 0;
>> }
>> 
>> -- 
>> 2.26.2
>>
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Marton Balint March 12, 2021, 11:15 p.m. UTC | #3
On Wed, 10 Mar 2021, Marton Balint wrote:

>
>
> On Sat, 6 Mar 2021, Marton Balint wrote:
>
>>
>>
>> On Mon, 1 Mar 2021, Marton Balint wrote:
>>
>>> Otherwise we might return 1-2 samples per packet if av_read_frame() call 
>> rate is
>>> only sligthly less than the stream sample rate.
>>
>> Ping for this and the rest of the series. Note that the approach in 
>> this patch makes 1/5 unneeded because constant frame size is now indeed 
>> guaranteed.
>
> Will apply the series soon.

Applied.

Regards,
Marton

>
> Regards,
> Marton
>
>>>
>>> Signed-off-by: Marton Balint <cus@passwd.hu>
>>> ---
>>> libavdevice/alsa.c     |  5 +++++
>>> libavdevice/alsa.h     |  1 +
>>> libavdevice/alsa_dec.c | 22 ++++++++++++----------
>>> 3 files changed, 18 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/libavdevice/alsa.c b/libavdevice/alsa.c
>>> index 117b2ea144..ee282fac16 100644
>>> --- a/libavdevice/alsa.c
>>> +++ b/libavdevice/alsa.c
>>> @@ -286,6 +286,10 @@ av_cold int ff_alsa_open(AVFormatContext *ctx, 
>> snd_pcm_stream_t mode,
>>>         }
>>>     }
>>> 
>>> +    s->pkt = av_packet_alloc();
>>> +    if (!s->pkt)
>>> +        goto fail1;
>>> +
>>>     s->h = h;
>>>     return 0;
>>> 
>>> @@ -308,6 +312,7 @@ av_cold int ff_alsa_close(AVFormatContext *s1)
>>>     if (CONFIG_ALSA_INDEV)
>>>         ff_timefilter_destroy(s->timefilter);
>>>     snd_pcm_close(s->h);
>>> +    av_packet_free(&s->pkt);
>>>     return 0;
>>> }
>>> 
>>> diff --git a/libavdevice/alsa.h b/libavdevice/alsa.h
>>> index 1ed8c82199..07783c983a 100644
>>> --- a/libavdevice/alsa.h
>>> +++ b/libavdevice/alsa.h
>>> @@ -58,6 +58,7 @@ typedef struct AlsaData {
>>>     void *reorder_buf;
>>>     int reorder_buf_size; ///< in frames
>>>     int64_t timestamp; ///< current timestamp, without latency applied.
>>> +    AVPacket *pkt;
>>> } AlsaData;
>>> 
>>> /**
>>> diff --git a/libavdevice/alsa_dec.c b/libavdevice/alsa_dec.c
>>> index 6d568737b3..88bf32d25f 100644
>>> --- a/libavdevice/alsa_dec.c
>>> +++ b/libavdevice/alsa_dec.c
>>> @@ -104,34 +104,36 @@ static int audio_read_packet(AVFormatContext *s1, 
>> AVPacket *pkt)
>>>     int64_t dts;
>>>     snd_pcm_sframes_t delay = 0;
>>> 
>>> -    if (av_new_packet(pkt, s->period_size * s->frame_size) < 0) {
>>> -        return AVERROR(EIO);
>>> +    if (!s->pkt->data) {
>>> +        int ret = av_new_packet(s->pkt, s->period_size * s->frame_size);
>>> +        if (ret < 0)
>>> +            return ret;
>>> +        s->pkt->size = 0;
>>>     }
>>> 
>>> -    while ((res = snd_pcm_readi(s->h, pkt->data, s->period_size)) < 0) {
>>> +    do {
>>> +        while ((res = snd_pcm_readi(s->h, s->pkt->data + s->pkt->size, 
>> s->period_size - s->pkt->size / s->frame_size)) < 0) {
>>>         if (res == -EAGAIN) {
>>> -            av_packet_unref(pkt);
>>> -
>>>             return AVERROR(EAGAIN);
>>>         }
>>> +        s->pkt->size = 0;
>>>         if (ff_alsa_xrun_recover(s1, res) < 0) {
>>>             av_log(s1, AV_LOG_ERROR, "ALSA read error: %s\n",
>>>                    snd_strerror(res));
>>> -            av_packet_unref(pkt);
>>> -
>>>             return AVERROR(EIO);
>>>         }
>>>         ff_timefilter_reset(s->timefilter);
>>> -    }
>>> +        }
>>> +        s->pkt->size += res * s->frame_size;
>>> +    } while (s->pkt->size < s->period_size * s->frame_size);
>>> 
>>> +    av_packet_move_ref(pkt, s->pkt);
>>>     dts = av_gettime();
>>>     snd_pcm_delay(s->h, &delay);
>>>     dts -= av_rescale(delay + res, 1000000, s->sample_rate);
>>>     pkt->pts = ff_timefilter_update(s->timefilter, dts, s->last_period);
>>>     s->last_period = res;
>>> 
>>> -    pkt->size = res * s->frame_size;
>>> -
>>>     return 0;
>>> }
>>> 
>>> -- 
>>> 2.26.2
>>>
>>> _______________________________________________
>>> ffmpeg-devel mailing list
>>> ffmpeg-devel@ffmpeg.org
>>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>
>>> To unsubscribe, visit link above, or email
>>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff mbox series

Patch

diff --git a/libavdevice/alsa.c b/libavdevice/alsa.c
index 117b2ea144..ee282fac16 100644
--- a/libavdevice/alsa.c
+++ b/libavdevice/alsa.c
@@ -286,6 +286,10 @@  av_cold int ff_alsa_open(AVFormatContext *ctx, snd_pcm_stream_t mode,
         }
     }
 
+    s->pkt = av_packet_alloc();
+    if (!s->pkt)
+        goto fail1;
+
     s->h = h;
     return 0;
 
@@ -308,6 +312,7 @@  av_cold int ff_alsa_close(AVFormatContext *s1)
     if (CONFIG_ALSA_INDEV)
         ff_timefilter_destroy(s->timefilter);
     snd_pcm_close(s->h);
+    av_packet_free(&s->pkt);
     return 0;
 }
 
diff --git a/libavdevice/alsa.h b/libavdevice/alsa.h
index 1ed8c82199..07783c983a 100644
--- a/libavdevice/alsa.h
+++ b/libavdevice/alsa.h
@@ -58,6 +58,7 @@  typedef struct AlsaData {
     void *reorder_buf;
     int reorder_buf_size; ///< in frames
     int64_t timestamp; ///< current timestamp, without latency applied.
+    AVPacket *pkt;
 } AlsaData;
 
 /**
diff --git a/libavdevice/alsa_dec.c b/libavdevice/alsa_dec.c
index 6d568737b3..88bf32d25f 100644
--- a/libavdevice/alsa_dec.c
+++ b/libavdevice/alsa_dec.c
@@ -104,34 +104,36 @@  static int audio_read_packet(AVFormatContext *s1, AVPacket *pkt)
     int64_t dts;
     snd_pcm_sframes_t delay = 0;
 
-    if (av_new_packet(pkt, s->period_size * s->frame_size) < 0) {
-        return AVERROR(EIO);
+    if (!s->pkt->data) {
+        int ret = av_new_packet(s->pkt, s->period_size * s->frame_size);
+        if (ret < 0)
+            return ret;
+        s->pkt->size = 0;
     }
 
-    while ((res = snd_pcm_readi(s->h, pkt->data, s->period_size)) < 0) {
+    do {
+        while ((res = snd_pcm_readi(s->h, s->pkt->data + s->pkt->size, s->period_size - s->pkt->size / s->frame_size)) < 0) {
         if (res == -EAGAIN) {
-            av_packet_unref(pkt);
-
             return AVERROR(EAGAIN);
         }
+        s->pkt->size = 0;
         if (ff_alsa_xrun_recover(s1, res) < 0) {
             av_log(s1, AV_LOG_ERROR, "ALSA read error: %s\n",
                    snd_strerror(res));
-            av_packet_unref(pkt);
-
             return AVERROR(EIO);
         }
         ff_timefilter_reset(s->timefilter);
-    }
+        }
+        s->pkt->size += res * s->frame_size;
+    } while (s->pkt->size < s->period_size * s->frame_size);
 
+    av_packet_move_ref(pkt, s->pkt);
     dts = av_gettime();
     snd_pcm_delay(s->h, &delay);
     dts -= av_rescale(delay + res, 1000000, s->sample_rate);
     pkt->pts = ff_timefilter_update(s->timefilter, dts, s->last_period);
     s->last_period = res;
 
-    pkt->size = res * s->frame_size;
-
     return 0;
 }