diff mbox

[FFmpeg-devel] lavc/amrwbdec: Do not ignore NO_DATA frames

Message ID CAB0OVGr4w6h4O_L5vBeBjma+2aZg+Zrk+wdStwbrjuZBeXuPbw@mail.gmail.com
State Accepted
Headers show

Commit Message

Carl Eugen Hoyos Jan. 30, 2019, 1:47 p.m. UTC 
2019-01-29 22:47 GMT+01:00, Carl Eugen Hoyos <ceffmpeg@gmail.com>:
> Hi!
>
> Attached patch fixes decoding NO_DATA amr-wb frames.

Now with patch.

Carl Eugen

Comments

Paul B Mahol Jan. 30, 2019, 7:33 p.m. UTC | #1 
On 1/30/19, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
> 2019-01-29 22:47 GMT+01:00, Carl Eugen Hoyos <ceffmpeg@gmail.com>:
>> Hi!
>>
>> Attached patch fixes decoding NO_DATA amr-wb frames.
>
> Now with patch.
>
> Carl Eugen
>

Are you sure that reference decoder gives complete silence for such frames?

Please use memset.
Carl Eugen Hoyos Jan. 30, 2019, 8:08 p.m. UTC | #2 
2019-01-30 20:33 GMT+01:00, Paul B Mahol <onemda@gmail.com>:
> On 1/30/19, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
>> 2019-01-29 22:47 GMT+01:00, Carl Eugen Hoyos <ceffmpeg@gmail.com>:

>>> Attached patch fixes decoding NO_DATA amr-wb frames.
>>
>> Now with patch.

> Are you sure that reference decoder gives complete silence
> for such frames?

Probably not, the specification suggests "random signal" and
"a muting technique" to "gradually decrease the output level".

As said, the issue here is that current FFmpeg shortens the output
making it unusable, my patch fixes this.

> Please use memset.

The C standard apparently that memset(, 0, ) is not
required to work for float, do we require this?

Carl Eugen

The calloc function allocates space for an array of nmemb
objects, each of whose size is size. The space is initialized
to all bits zero. Note that this need not be the same as the
representation of floating-point zero...
Carl Eugen Hoyos Feb. 9, 2019, 7:14 p.m. UTC | #3 
2019-01-30 14:47 GMT+01:00, Carl Eugen Hoyos <ceffmpeg@gmail.com>:
> 2019-01-29 22:47 GMT+01:00, Carl Eugen Hoyos <ceffmpeg@gmail.com>:
>> Hi!
>>
>> Attached patch fixes decoding NO_DATA amr-wb frames.
>
> Now with patch.

Ping.

This patch is supposed to fix the duration of files with NO_DATA
frames.

Carl Eugen
Paul B Mahol Feb. 9, 2019, 7:37 p.m. UTC | #4 
On 2/9/19, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
> 2019-01-30 14:47 GMT+01:00, Carl Eugen Hoyos <ceffmpeg@gmail.com>:
>> 2019-01-29 22:47 GMT+01:00, Carl Eugen Hoyos <ceffmpeg@gmail.com>:
>>> Hi!
>>>
>>> Attached patch fixes decoding NO_DATA amr-wb frames.
>>
>> Now with patch.
>
> Ping.
>
> This patch is supposed to fix the duration of files with NO_DATA
> frames.
>

I guess if its not broken that its ok.
Carl Eugen Hoyos April 5, 2020, midnight UTC | #5 
Am Sa., 9. Feb. 2019 um 20:43 Uhr schrieb Paul B Mahol <onemda@gmail.com>:
>
> On 2/9/19, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
> > 2019-01-30 14:47 GMT+01:00, Carl Eugen Hoyos <ceffmpeg@gmail.com>:
> >> 2019-01-29 22:47 GMT+01:00, Carl Eugen Hoyos <ceffmpeg@gmail.com>:
> >>> Hi!
> >>>
> >>> Attached patch fixes decoding NO_DATA amr-wb frames.
> >>
> >> Now with patch.
> >
> > Ping.
> >
> > This patch is supposed to fix the duration of files with NO_DATA
> > frames.
> >
>
> I guess if its not broken that its ok.

Patch applied.

Thank you, Carl Eugen
James Almer April 5, 2020, 2:45 p.m. UTC | #6 
On 1/30/2019 10:47 AM, Carl Eugen Hoyos wrote:
> 2019-01-29 22:47 GMT+01:00, Carl Eugen Hoyos <ceffmpeg@gmail.com>:
>> Hi!
>>
>> Attached patch fixes decoding NO_DATA amr-wb frames.
> 
> Now with patch.
> 
> Carl Eugen

> From 0a8c318c49ec358ad646ed601588154cf7d7da37 Mon Sep 17 00:00:00 2001
> From: Carl Eugen Hoyos <ceffmpeg@gmail.com>
> Date: Tue, 29 Jan 2019 22:46:37 +0100
> Subject: [PATCH] lavc/amrwbdec: Do not ignore NO_DATA frames.
> 
> Fixes the actual output duration of the sample in ticket #7113.
> ---
>  libavcodec/amrwbdata.h |    2 +-
>  libavcodec/amrwbdec.c  |    9 ++++++++-
>  2 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/libavcodec/amrwbdata.h b/libavcodec/amrwbdata.h
> index 8a8cbfd..95c0aaa 100644
> --- a/libavcodec/amrwbdata.h
> +++ b/libavcodec/amrwbdata.h
> @@ -1884,7 +1884,7 @@ static const float lpf_7_coef[31] = { // low pass, 7kHz cutoff
>  /** Core frame sizes in each mode */
>  static const uint16_t cf_sizes_wb[] = {
>      132, 177, 253, 285, 317, 365, 397, 461, 477,
> -    40 /// SID/comfort noise frame
> +    40, 0, 0, 0, 0, 0, 0
>  };
>  
>  #endif /* AVCODEC_AMRWBDATA_H */
> diff --git a/libavcodec/amrwbdec.c b/libavcodec/amrwbdec.c
> index 47fe7eb..b488a5d 100644
> --- a/libavcodec/amrwbdec.c
> +++ b/libavcodec/amrwbdec.c
> @@ -1119,12 +1119,19 @@ static int amrwb_decode_frame(AVCodecContext *avctx, void *data,
>      buf_out = (float *)frame->data[0];
>  
>      header_size      = decode_mime_header(ctx, buf);
> +    expected_fr_size = ((cf_sizes_wb[ctx->fr_cur_mode] + 7) >> 3) + 1;
> +
> +    if (ctx->fr_cur_mode == NO_DATA) {
> +        for (i = 0; i < frame->nb_samples; i++)
> +            buf_out[i] = 0.f;

You can use av_samples_set_silence() for this.

> +        *got_frame_ptr = 1;
> +        return expected_fr_size;
> +    }
>      if (ctx->fr_cur_mode > MODE_SID) {
>          av_log(avctx, AV_LOG_ERROR,
>                 "Invalid mode %d\n", ctx->fr_cur_mode);
>          return AVERROR_INVALIDDATA;
>      }
> -    expected_fr_size = ((cf_sizes_wb[ctx->fr_cur_mode] + 7) >> 3) + 1;
>  
>      if (buf_size < expected_fr_size) {
>          av_log(avctx, AV_LOG_ERROR,
> -- 
> 1.7.10.4
>
Carl Eugen Hoyos April 5, 2020, 3:07 p.m. UTC | #7 
Am So., 5. Apr. 2020 um 16:45 Uhr schrieb James Almer <jamrial@gmail.com>:
>
> On 1/30/2019 10:47 AM, Carl Eugen Hoyos wrote:
> > 2019-01-29 22:47 GMT+01:00, Carl Eugen Hoyos <ceffmpeg@gmail.com>:
> >> Hi!
> >>
> >> Attached patch fixes decoding NO_DATA amr-wb frames.
> >
> > Now with patch.
> >
> > Carl Eugen
>
> > From 0a8c318c49ec358ad646ed601588154cf7d7da37 Mon Sep 17 00:00:00 2001
> > From: Carl Eugen Hoyos <ceffmpeg@gmail.com>
> > Date: Tue, 29 Jan 2019 22:46:37 +0100
> > Subject: [PATCH] lavc/amrwbdec: Do not ignore NO_DATA frames.
> >
> > Fixes the actual output duration of the sample in ticket #7113.
> > ---
> >  libavcodec/amrwbdata.h |    2 +-
> >  libavcodec/amrwbdec.c  |    9 ++++++++-
> >  2 files changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/libavcodec/amrwbdata.h b/libavcodec/amrwbdata.h
> > index 8a8cbfd..95c0aaa 100644
> > --- a/libavcodec/amrwbdata.h
> > +++ b/libavcodec/amrwbdata.h
> > @@ -1884,7 +1884,7 @@ static const float lpf_7_coef[31] = { // low pass, 7kHz cutoff
> >  /** Core frame sizes in each mode */
> >  static const uint16_t cf_sizes_wb[] = {
> >      132, 177, 253, 285, 317, 365, 397, 461, 477,
> > -    40 /// SID/comfort noise frame
> > +    40, 0, 0, 0, 0, 0, 0
> >  };
> >
> >  #endif /* AVCODEC_AMRWBDATA_H */
> > diff --git a/libavcodec/amrwbdec.c b/libavcodec/amrwbdec.c
> > index 47fe7eb..b488a5d 100644
> > --- a/libavcodec/amrwbdec.c
> > +++ b/libavcodec/amrwbdec.c
> > @@ -1119,12 +1119,19 @@ static int amrwb_decode_frame(AVCodecContext *avctx, void *data,
> >      buf_out = (float *)frame->data[0];
> >
> >      header_size      = decode_mime_header(ctx, buf);
> > +    expected_fr_size = ((cf_sizes_wb[ctx->fr_cur_mode] + 7) >> 3) + 1;
> > +
> > +    if (ctx->fr_cur_mode == NO_DATA) {
> > +        for (i = 0; i < frame->nb_samples; i++)
> > +            buf_out[i] = 0.f;
>
> You can use av_samples_set_silence() for this.

(Is memset(0) valid for floats?)

Patch sent, thank you!

Carl Eugen
James Almer April 5, 2020, 3:10 p.m. UTC | #8 
On 4/5/2020 12:07 PM, Carl Eugen Hoyos wrote:
> Am So., 5. Apr. 2020 um 16:45 Uhr schrieb James Almer <jamrial@gmail.com>:
>>
>> On 1/30/2019 10:47 AM, Carl Eugen Hoyos wrote:
>>> 2019-01-29 22:47 GMT+01:00, Carl Eugen Hoyos <ceffmpeg@gmail.com>:
>>>> Hi!
>>>>
>>>> Attached patch fixes decoding NO_DATA amr-wb frames.
>>>
>>> Now with patch.
>>>
>>> Carl Eugen
>>
>>> From 0a8c318c49ec358ad646ed601588154cf7d7da37 Mon Sep 17 00:00:00 2001
>>> From: Carl Eugen Hoyos <ceffmpeg@gmail.com>
>>> Date: Tue, 29 Jan 2019 22:46:37 +0100
>>> Subject: [PATCH] lavc/amrwbdec: Do not ignore NO_DATA frames.
>>>
>>> Fixes the actual output duration of the sample in ticket #7113.
>>> ---
>>>  libavcodec/amrwbdata.h |    2 +-
>>>  libavcodec/amrwbdec.c  |    9 ++++++++-
>>>  2 files changed, 9 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/libavcodec/amrwbdata.h b/libavcodec/amrwbdata.h
>>> index 8a8cbfd..95c0aaa 100644
>>> --- a/libavcodec/amrwbdata.h
>>> +++ b/libavcodec/amrwbdata.h
>>> @@ -1884,7 +1884,7 @@ static const float lpf_7_coef[31] = { // low pass, 7kHz cutoff
>>>  /** Core frame sizes in each mode */
>>>  static const uint16_t cf_sizes_wb[] = {
>>>      132, 177, 253, 285, 317, 365, 397, 461, 477,
>>> -    40 /// SID/comfort noise frame
>>> +    40, 0, 0, 0, 0, 0, 0
>>>  };
>>>
>>>  #endif /* AVCODEC_AMRWBDATA_H */
>>> diff --git a/libavcodec/amrwbdec.c b/libavcodec/amrwbdec.c
>>> index 47fe7eb..b488a5d 100644
>>> --- a/libavcodec/amrwbdec.c
>>> +++ b/libavcodec/amrwbdec.c
>>> @@ -1119,12 +1119,19 @@ static int amrwb_decode_frame(AVCodecContext *avctx, void *data,
>>>      buf_out = (float *)frame->data[0];
>>>
>>>      header_size      = decode_mime_header(ctx, buf);
>>> +    expected_fr_size = ((cf_sizes_wb[ctx->fr_cur_mode] + 7) >> 3) + 1;
>>> +
>>> +    if (ctx->fr_cur_mode == NO_DATA) {
>>> +        for (i = 0; i < frame->nb_samples; i++)
>>> +            buf_out[i] = 0.f;
>>
>> You can use av_samples_set_silence() for this.
> 
> (Is memset(0) valid for floats?)

I would hope so, considering this function is used in half a dozen
filters, and the core encoding lavc code...

> 
> Patch sent, thank you!
> 
> Carl Eugen
> _______________________________________________
> 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".
>
Carl Eugen Hoyos April 5, 2020, 3:11 p.m. UTC | #9 
Am So., 5. Apr. 2020 um 17:10 Uhr schrieb James Almer <jamrial@gmail.com>:
>
> On 4/5/2020 12:07 PM, Carl Eugen Hoyos wrote:
> > Am So., 5. Apr. 2020 um 16:45 Uhr schrieb James Almer <jamrial@gmail.com>:
> >>
> >> On 1/30/2019 10:47 AM, Carl Eugen Hoyos wrote:
> >>> 2019-01-29 22:47 GMT+01:00, Carl Eugen Hoyos <ceffmpeg@gmail.com>:
> >>>> Hi!
> >>>>
> >>>> Attached patch fixes decoding NO_DATA amr-wb frames.
> >>>
> >>> Now with patch.
> >>>
> >>> Carl Eugen
> >>
> >>> From 0a8c318c49ec358ad646ed601588154cf7d7da37 Mon Sep 17 00:00:00 2001
> >>> From: Carl Eugen Hoyos <ceffmpeg@gmail.com>
> >>> Date: Tue, 29 Jan 2019 22:46:37 +0100
> >>> Subject: [PATCH] lavc/amrwbdec: Do not ignore NO_DATA frames.
> >>>
> >>> Fixes the actual output duration of the sample in ticket #7113.
> >>> ---
> >>>  libavcodec/amrwbdata.h |    2 +-
> >>>  libavcodec/amrwbdec.c  |    9 ++++++++-
> >>>  2 files changed, 9 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/libavcodec/amrwbdata.h b/libavcodec/amrwbdata.h
> >>> index 8a8cbfd..95c0aaa 100644
> >>> --- a/libavcodec/amrwbdata.h
> >>> +++ b/libavcodec/amrwbdata.h
> >>> @@ -1884,7 +1884,7 @@ static const float lpf_7_coef[31] = { // low pass, 7kHz cutoff
> >>>  /** Core frame sizes in each mode */
> >>>  static const uint16_t cf_sizes_wb[] = {
> >>>      132, 177, 253, 285, 317, 365, 397, 461, 477,
> >>> -    40 /// SID/comfort noise frame
> >>> +    40, 0, 0, 0, 0, 0, 0
> >>>  };
> >>>
> >>>  #endif /* AVCODEC_AMRWBDATA_H */
> >>> diff --git a/libavcodec/amrwbdec.c b/libavcodec/amrwbdec.c
> >>> index 47fe7eb..b488a5d 100644
> >>> --- a/libavcodec/amrwbdec.c
> >>> +++ b/libavcodec/amrwbdec.c
> >>> @@ -1119,12 +1119,19 @@ static int amrwb_decode_frame(AVCodecContext *avctx, void *data,
> >>>      buf_out = (float *)frame->data[0];
> >>>
> >>>      header_size      = decode_mime_header(ctx, buf);
> >>> +    expected_fr_size = ((cf_sizes_wb[ctx->fr_cur_mode] + 7) >> 3) + 1;
> >>> +
> >>> +    if (ctx->fr_cur_mode == NO_DATA) {
> >>> +        for (i = 0; i < frame->nb_samples; i++)
> >>> +            buf_out[i] = 0.f;
> >>
> >> You can use av_samples_set_silence() for this.
> >
> > (Is memset(0) valid for floats?)
>
> I would hope so, considering this function is used in half a dozen
> filters, and the core encoding lavc code...

=-)

Is the patch ok?

Carl Eugen
Carl Eugen Hoyos April 11, 2020, 12:26 p.m. UTC | #10 
Am Mi., 30. Jan. 2019 um 21:08 Uhr schrieb Carl Eugen Hoyos
<ceffmpeg@gmail.com>:
>
> 2019-01-30 20:33 GMT+01:00, Paul B Mahol <onemda@gmail.com>:
> > On 1/30/19, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
> >> 2019-01-29 22:47 GMT+01:00, Carl Eugen Hoyos <ceffmpeg@gmail.com>:
>
> >>> Attached patch fixes decoding NO_DATA amr-wb frames.
> >>
> >> Now with patch.
>
> > Are you sure that reference decoder gives complete silence
> > for such frames?
>
> Probably not, the specification suggests "random signal" and
> "a muting technique" to "gradually decrease the output level".

Added this information as a comment.

Carl Eugen
diff mbox

Patch

From 0a8c318c49ec358ad646ed601588154cf7d7da37 Mon Sep 17 00:00:00 2001
From: Carl Eugen Hoyos <ceffmpeg@gmail.com>
Date: Tue, 29 Jan 2019 22:46:37 +0100
Subject: [PATCH] lavc/amrwbdec: Do not ignore NO_DATA frames.

Fixes the actual output duration of the sample in ticket #7113.
---
 libavcodec/amrwbdata.h |    2 +-
 libavcodec/amrwbdec.c  |    9 ++++++++-
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/libavcodec/amrwbdata.h b/libavcodec/amrwbdata.h
index 8a8cbfd..95c0aaa 100644
--- a/libavcodec/amrwbdata.h
+++ b/libavcodec/amrwbdata.h
@@ -1884,7 +1884,7 @@  static const float lpf_7_coef[31] = { // low pass, 7kHz cutoff
 /** Core frame sizes in each mode */
 static const uint16_t cf_sizes_wb[] = {
     132, 177, 253, 285, 317, 365, 397, 461, 477,
-    40 /// SID/comfort noise frame
+    40, 0, 0, 0, 0, 0, 0
 };
 
 #endif /* AVCODEC_AMRWBDATA_H */
diff --git a/libavcodec/amrwbdec.c b/libavcodec/amrwbdec.c
index 47fe7eb..b488a5d 100644
--- a/libavcodec/amrwbdec.c
+++ b/libavcodec/amrwbdec.c
@@ -1119,12 +1119,19 @@  static int amrwb_decode_frame(AVCodecContext *avctx, void *data,
     buf_out = (float *)frame->data[0];
 
     header_size      = decode_mime_header(ctx, buf);
+    expected_fr_size = ((cf_sizes_wb[ctx->fr_cur_mode] + 7) >> 3) + 1;
+
+    if (ctx->fr_cur_mode == NO_DATA) {
+        for (i = 0; i < frame->nb_samples; i++)
+            buf_out[i] = 0.f;
+        *got_frame_ptr = 1;
+        return expected_fr_size;
+    }
     if (ctx->fr_cur_mode > MODE_SID) {
         av_log(avctx, AV_LOG_ERROR,
                "Invalid mode %d\n", ctx->fr_cur_mode);
         return AVERROR_INVALIDDATA;
     }
-    expected_fr_size = ((cf_sizes_wb[ctx->fr_cur_mode] + 7) >> 3) + 1;
 
     if (buf_size < expected_fr_size) {
         av_log(avctx, AV_LOG_ERROR,
-- 
1.7.10.4