diff mbox series

[FFmpeg-devel,2/5] avcodec/aac_ac3_parser: don't fill stream info in the sync function

Message ID 20221022210226.2200-2-jamrial@gmail.com
State Accepted
Commit b5abf6fdfc0d1bd8b009f8877589f1bc7eec3e9c
Headers show
Series [FFmpeg-devel,1/5,v2] avcodec/ac3dec: split off code discarding garbage at the beginning of a packet | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 fail Make fate failed
andriy/make_x86 success Make finished
andriy/make_fate_x86 fail Make fate failed

Commit Message

James Almer Oct. 22, 2022, 9:02 p.m. UTC
Have it only find frame boundaries. The stream props will then be filled once
we have an assembled frame.

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavcodec/aac_ac3_parser.c | 63 +++++++++++++++++++++++++++++--------
 libavcodec/aac_ac3_parser.h | 12 +------
 libavcodec/aac_parser.c     |  7 +----
 libavcodec/ac3_parser.c     | 16 +---------
 4 files changed, 53 insertions(+), 45 deletions(-)

Comments

Michael Niedermayer Oct. 23, 2022, 3:35 p.m. UTC | #1
On Sat, Oct 22, 2022 at 06:02:23PM -0300, James Almer wrote:
> Have it only find frame boundaries. The stream props will then be filled once
> we have an assembled frame.
> 
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  libavcodec/aac_ac3_parser.c | 63 +++++++++++++++++++++++++++++--------
>  libavcodec/aac_ac3_parser.h | 12 +------
>  libavcodec/aac_parser.c     |  7 +----
>  libavcodec/ac3_parser.c     | 16 +---------
>  4 files changed, 53 insertions(+), 45 deletions(-)

i probably forgot something but this breaks:

--- ./tests/ref/fate/adtstoasc_ticket3715	2022-10-22 10:37:36.046518037 +0200
+++ tests/data/fate/adtstoasc_ticket3715	2022-10-23 17:32:17.096459090 +0200
@@ -1,4 +1,4 @@
-29ef0632a8eb5c336bf45a1d5076626e *tests/data/fate/adtstoasc_ticket3715.mov
+4110be924e21846d0e174fac679b062e *tests/data/fate/adtstoasc_ticket3715.mov
 33324 tests/data/fate/adtstoasc_ticket3715.mov
 #extradata 0:        2, 0x00340022
 #tb 0: 1/44100
Test adtstoasc_ticket3715 failed. Look at tests/data/fate/adtstoasc_ticket3715.err for details.
tests/Makefile:306: recipe for target 'fate-adtstoasc_ticket3715' failed
make: *** [fate-adtstoasc_ticket3715] Error 1

[...]
Andreas Rheinhardt Oct. 23, 2022, 5:34 p.m. UTC | #2
Michael Niedermayer:
> On Sat, Oct 22, 2022 at 06:02:23PM -0300, James Almer wrote:
>> Have it only find frame boundaries. The stream props will then be filled once
>> we have an assembled frame.
>>
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>>  libavcodec/aac_ac3_parser.c | 63 +++++++++++++++++++++++++++++--------
>>  libavcodec/aac_ac3_parser.h | 12 +------
>>  libavcodec/aac_parser.c     |  7 +----
>>  libavcodec/ac3_parser.c     | 16 +---------
>>  4 files changed, 53 insertions(+), 45 deletions(-)
> 
> i probably forgot something but this breaks:
> 
> --- ./tests/ref/fate/adtstoasc_ticket3715	2022-10-22 10:37:36.046518037 +0200
> +++ tests/data/fate/adtstoasc_ticket3715	2022-10-23 17:32:17.096459090 +0200
> @@ -1,4 +1,4 @@
> -29ef0632a8eb5c336bf45a1d5076626e *tests/data/fate/adtstoasc_ticket3715.mov
> +4110be924e21846d0e174fac679b062e *tests/data/fate/adtstoasc_ticket3715.mov
>  33324 tests/data/fate/adtstoasc_ticket3715.mov
>  #extradata 0:        2, 0x00340022
>  #tb 0: 1/44100
> Test adtstoasc_ticket3715 failed. Look at tests/data/fate/adtstoasc_ticket3715.err for details.
> tests/Makefile:306: recipe for target 'fate-adtstoasc_ticket3715' failed
> make: *** [fate-adtstoasc_ticket3715] Error 1
> 
> [...]
> 

You are not alone: Same thing happened with patchwork.

- Andreas
James Almer Oct. 23, 2022, 10:59 p.m. UTC | #3
On 10/23/2022 2:34 PM, Andreas Rheinhardt wrote:
> Michael Niedermayer:
>> On Sat, Oct 22, 2022 at 06:02:23PM -0300, James Almer wrote:
>>> Have it only find frame boundaries. The stream props will then be filled once
>>> we have an assembled frame.
>>>
>>> Signed-off-by: James Almer <jamrial@gmail.com>
>>> ---
>>>   libavcodec/aac_ac3_parser.c | 63 +++++++++++++++++++++++++++++--------
>>>   libavcodec/aac_ac3_parser.h | 12 +------
>>>   libavcodec/aac_parser.c     |  7 +----
>>>   libavcodec/ac3_parser.c     | 16 +---------
>>>   4 files changed, 53 insertions(+), 45 deletions(-)
>>
>> i probably forgot something but this breaks:
>>
>> --- ./tests/ref/fate/adtstoasc_ticket3715	2022-10-22 10:37:36.046518037 +0200
>> +++ tests/data/fate/adtstoasc_ticket3715	2022-10-23 17:32:17.096459090 +0200
>> @@ -1,4 +1,4 @@
>> -29ef0632a8eb5c336bf45a1d5076626e *tests/data/fate/adtstoasc_ticket3715.mov
>> +4110be924e21846d0e174fac679b062e *tests/data/fate/adtstoasc_ticket3715.mov
>>   33324 tests/data/fate/adtstoasc_ticket3715.mov
>>   #extradata 0:        2, 0x00340022
>>   #tb 0: 1/44100
>> Test adtstoasc_ticket3715 failed. Look at tests/data/fate/adtstoasc_ticket3715.err for details.
>> tests/Makefile:306: recipe for target 'fate-adtstoasc_ticket3715' failed
>> make: *** [fate-adtstoasc_ticket3715] Error 1
>>
>> [...]
>>
> 
> You are not alone: Same thing happened with patchwork.

Yes, i noticed and fixed it last night, but didn't send the patch for it 
since it was late. Will do it in a bit.

> 
> - Andreas
> 
> _______________________________________________
> 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/libavcodec/aac_ac3_parser.c b/libavcodec/aac_ac3_parser.c
index b14b1e31f9..6df064e28d 100644
--- a/libavcodec/aac_ac3_parser.c
+++ b/libavcodec/aac_ac3_parser.c
@@ -26,6 +26,8 @@ 
 #include "libavutil/common.h"
 #include "parser.h"
 #include "aac_ac3_parser.h"
+#include "ac3_parser_internal.h"
+#include "adts_header.h"
 
 int ff_aac_ac3_parse(AVCodecParserContext *s1,
                      AVCodecContext *avctx,
@@ -48,7 +50,7 @@  get_next:
             len=0;
             for(i=s->remaining_size; i<buf_size; i++){
                 s->state = (s->state<<8) + buf[i];
-                if((len=s->sync(s->state, s, &s->need_next_header, &new_frame_start)))
+                if((len=s->sync(s->state, &s->need_next_header, &new_frame_start)))
                     break;
             }
             if(len<=0){
@@ -79,42 +81,77 @@  get_next:
     *poutbuf = buf;
     *poutbuf_size = buf_size;
 
-    /* update codec info */
-    if(s->codec_id)
-        avctx->codec_id = s->codec_id;
-
     if (got_frame) {
+        int bit_rate;
+
         /* Due to backwards compatible HE-AAC the sample rate, channel count,
            and total number of samples found in an AAC ADTS header are not
            reliable. Bit rate is still accurate because the total frame
            duration in seconds is still correct (as is the number of bits in
            the frame). */
         if (avctx->codec_id != AV_CODEC_ID_AAC) {
-            avctx->sample_rate = s->sample_rate;
+            AC3HeaderInfo hdr, *phrd = &hdr;
+            int offset = ff_ac3_find_syncword(buf, buf_size);
+
+            if (offset < 0)
+                return i;
+
+            buf += offset;
+            buf_size -= offset;
+            while (buf_size > 0) {
+                int ret = avpriv_ac3_parse_header(&phrd, buf, buf_size);
+
+                if (ret < 0 || hdr.frame_size > buf_size)
+                    return i;
+
+                if (buf_size > hdr.frame_size) {
+                    buf += hdr.frame_size;
+                    buf_size -= hdr.frame_size;
+                    continue;
+                }
+                break;
+            }
+
+            avctx->sample_rate = hdr.sample_rate;
+
+            if (hdr.bitstream_id > 10)
+                avctx->codec_id = AV_CODEC_ID_EAC3;
+
             if (!CONFIG_EAC3_DECODER || avctx->codec_id != AV_CODEC_ID_EAC3) {
                 av_channel_layout_uninit(&avctx->ch_layout);
-                if (s->channel_layout) {
-                    av_channel_layout_from_mask(&avctx->ch_layout, s->channel_layout);
+                if (hdr.channel_layout) {
+                    av_channel_layout_from_mask(&avctx->ch_layout, hdr.channel_layout);
                 } else {
                     avctx->ch_layout.order       = AV_CHANNEL_ORDER_UNSPEC;
-                    avctx->ch_layout.nb_channels = s->channels;
+                    avctx->ch_layout.nb_channels = hdr.channels;
                 }
 #if FF_API_OLD_CHANNEL_LAYOUT
 FF_DISABLE_DEPRECATION_WARNINGS
                 avctx->channels = avctx->ch_layout.nb_channels;
-                avctx->channel_layout = s->channel_layout;
+                avctx->channel_layout = hdr.channel_layout;
 FF_ENABLE_DEPRECATION_WARNINGS
 #endif
             }
-            s1->duration = s->samples;
-            avctx->audio_service_type = s->service_type;
+            s1->duration = hdr.num_blocks * 256;
+            avctx->audio_service_type = hdr.bitstream_mode;
+            if (hdr.bitstream_mode == 0x7 && hdr.channels > 1)
+                avctx->audio_service_type = AV_AUDIO_SERVICE_TYPE_KARAOKE;
+            bit_rate = hdr.bit_rate;
+        } else {
+            AACADTSHeaderInfo hdr, *phrd = &hdr;
+            int ret = avpriv_adts_header_parse(&phrd, buf, buf_size);
+
+            if (ret < 0)
+                return i;
+
+            bit_rate = hdr.bit_rate;
         }
 
         /* Calculate the average bit rate */
         s->frame_number++;
         if (!CONFIG_EAC3_DECODER || avctx->codec_id != AV_CODEC_ID_EAC3) {
             avctx->bit_rate +=
-                (s->bit_rate - avctx->bit_rate) / s->frame_number;
+                (bit_rate - avctx->bit_rate) / s->frame_number;
         }
     }
 
diff --git a/libavcodec/aac_ac3_parser.h b/libavcodec/aac_ac3_parser.h
index 8b93cbf84f..560bba54f5 100644
--- a/libavcodec/aac_ac3_parser.h
+++ b/libavcodec/aac_ac3_parser.h
@@ -39,24 +39,14 @@  typedef enum {
 
 typedef struct AACAC3ParseContext {
     ParseContext pc;
-    int frame_size;
     int header_size;
-    int (*sync)(uint64_t state, struct AACAC3ParseContext *hdr_info,
-            int *need_next_header, int *new_frame_start);
-
-    int channels;
-    int sample_rate;
-    int bit_rate;
-    int samples;
-    uint64_t channel_layout;
-    int service_type;
+    int (*sync)(uint64_t state, int *need_next_header, int *new_frame_start);
 
     int remaining_size;
     uint64_t state;
 
     int need_next_header;
     int frame_number;
-    enum AVCodecID codec_id;
 } AACAC3ParseContext;
 
 int ff_aac_ac3_parse(AVCodecParserContext *s1,
diff --git a/libavcodec/aac_parser.c b/libavcodec/aac_parser.c
index f3baf7cde3..f295dfccdd 100644
--- a/libavcodec/aac_parser.c
+++ b/libavcodec/aac_parser.c
@@ -27,8 +27,7 @@ 
 #include "get_bits.h"
 #include "mpeg4audio.h"
 
-static int aac_sync(uint64_t state, AACAC3ParseContext *hdr_info,
-        int *need_next_header, int *new_frame_start)
+static int aac_sync(uint64_t state, int *need_next_header, int *new_frame_start)
 {
     GetBitContext bits;
     AACADTSHeaderInfo hdr;
@@ -46,10 +45,6 @@  static int aac_sync(uint64_t state, AACAC3ParseContext *hdr_info,
         return 0;
     *need_next_header = 0;
     *new_frame_start  = 1;
-    hdr_info->sample_rate = hdr.sample_rate;
-    hdr_info->channels    = ff_mpeg4audio_channels[hdr.chan_config];
-    hdr_info->samples     = hdr.samples;
-    hdr_info->bit_rate    = hdr.bit_rate;
     return size;
 }
 
diff --git a/libavcodec/ac3_parser.c b/libavcodec/ac3_parser.c
index 425e1b4742..8885e1c72e 100644
--- a/libavcodec/ac3_parser.c
+++ b/libavcodec/ac3_parser.c
@@ -215,8 +215,7 @@  int av_ac3_parse_header(const uint8_t *buf, size_t size,
     return 0;
 }
 
-static int ac3_sync(uint64_t state, AACAC3ParseContext *hdr_info,
-        int *need_next_header, int *new_frame_start)
+static int ac3_sync(uint64_t state, int *need_next_header, int *new_frame_start)
 {
     int err;
     union {
@@ -238,19 +237,6 @@  static int ac3_sync(uint64_t state, AACAC3ParseContext *hdr_info,
     if(err < 0)
         return 0;
 
-    hdr_info->sample_rate = hdr.sample_rate;
-    hdr_info->bit_rate = hdr.bit_rate;
-    hdr_info->channels = hdr.channels;
-    hdr_info->channel_layout = hdr.channel_layout;
-    hdr_info->samples = hdr.num_blocks * 256;
-    hdr_info->service_type = hdr.bitstream_mode;
-    if (hdr.bitstream_mode == 0x7 && hdr.channels > 1)
-        hdr_info->service_type = AV_AUDIO_SERVICE_TYPE_KARAOKE;
-    if(hdr.bitstream_id>10)
-        hdr_info->codec_id = AV_CODEC_ID_EAC3;
-    else if (hdr_info->codec_id == AV_CODEC_ID_NONE)
-        hdr_info->codec_id = AV_CODEC_ID_AC3;
-
     *new_frame_start  = (hdr.frame_type != EAC3_FRAME_TYPE_DEPENDENT);
     *need_next_header = *new_frame_start || (hdr.frame_type != EAC3_FRAME_TYPE_AC3_CONVERT);
     return hdr.frame_size;