[02/14] aac_adtstoasc_bsf: convert to the new API

Message ID 7218f706-6be8-1bef-4766-6e04b445b70b@gmail.com
State New
Headers show

Commit Message

James Almer Nov. 25, 2016, 2:02 a.m.
On 3/19/2016 1:02 PM, Anton Khirnov wrote:
> Quoting Luca Barbato (2016-03-07 09:10:14)
>> On 07/03/16 08:59, Luca Barbato wrote:
>>> On 04/03/16 09:15, Anton Khirnov wrote:
>>>> ---
>>>>  libavcodec/aac_adtstoasc_bsf.c | 95 ++++++++++++++++++++++++++----------------
>>>>  libavcodec/allcodecs.c         |  1 -
>>>>  libavcodec/bitstream_filters.c |  5 +++
>>>>  3 files changed, 65 insertions(+), 36 deletions(-)
>>>>
>>>
>>> Possibly Ok.
>>>
>>
>> Reading the others, why the par_out->extradata is not set?
> 
> Because it's set as side data in the first packet. Actually it should be
> actively unset from the output parameters.

This is not what should be done. If the stream has extradata during .init()
then it means it's ASC and not ADTS or even LATM. Deleting it unconditionally
breaks passthrough use cases.

The .init function should validate the extradata and let av_bsf_init() pass
it down the filter chain. See the attached patch.

Comments

Anton Khirnov Nov. 27, 2016, 1:27 p.m. | #1
Quoting James Almer (2016-11-25 03:02:30)
> On 3/19/2016 1:02 PM, Anton Khirnov wrote:
> > Quoting Luca Barbato (2016-03-07 09:10:14)
> >> On 07/03/16 08:59, Luca Barbato wrote:
> >>> On 04/03/16 09:15, Anton Khirnov wrote:
> >>>> ---
> >>>>  libavcodec/aac_adtstoasc_bsf.c | 95 ++++++++++++++++++++++++++----------------
> >>>>  libavcodec/allcodecs.c         |  1 -
> >>>>  libavcodec/bitstream_filters.c |  5 +++
> >>>>  3 files changed, 65 insertions(+), 36 deletions(-)
> >>>>
> >>>
> >>> Possibly Ok.
> >>>
> >>
> >> Reading the others, why the par_out->extradata is not set?
> > 
> > Because it's set as side data in the first packet. Actually it should be
> > actively unset from the output parameters.
> 
> This is not what should be done. If the stream has extradata during .init()
> then it means it's ASC and not ADTS or even LATM. Deleting it unconditionally
> breaks passthrough use cases.
> 
> The .init function should validate the extradata and let av_bsf_init() pass
> it down the filter chain. See the attached patch.

Looks sensible.

Queuing, thanks.

Patch

From 5f07937db0a12b6176bfc0414c530716cfb382e7 Mon Sep 17 00:00:00 2001
From: James Almer <jamrial@gmail.com>
Date: Thu, 24 Nov 2016 21:10:47 -0300
Subject: [PATCH] aac_adtstoasc_bsf: validate and forward extradata if the
 stream is already ASC

Fixes AAC AudioSpecificConfig passthrough.

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavcodec/aac_adtstoasc_bsf.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/libavcodec/aac_adtstoasc_bsf.c b/libavcodec/aac_adtstoasc_bsf.c
index 9168e2b..08d60eb 100644
--- a/libavcodec/aac_adtstoasc_bsf.c
+++ b/libavcodec/aac_adtstoasc_bsf.c
@@ -135,8 +135,16 @@  fail:
 
 static int aac_adtstoasc_init(AVBSFContext *ctx)
 {
-    av_freep(&ctx->par_out->extradata);
-    ctx->par_out->extradata_size = 0;
+    /* Validate the extradata if the stream is already MPEG-4 AudioSpecificConfig */
+    if (ctx->par_in->extradata) {
+        MPEG4AudioConfig mp4ac;
+        int ret = avpriv_mpeg4audio_get_config(&mp4ac, ctx->par_in->extradata,
+                                               ctx->par_in->extradata_size * 8, 1);
+        if (ret < 0) {
+            av_log(ctx, AV_LOG_ERROR, "Error parsing AudioSpecificConfig extradata!\n");
+            return ret;
+        }
+    }
 
     return 0;
 }
-- 
2.10.2