diff mbox series

[FFmpeg-devel,2/2] aacdec: padding skip improvements

Message ID Nh4nBk8--3-9@lynne.ee
State New
Headers show
Series [FFmpeg-devel,1/2] lavc/internal: add skip_samples2 field | expand

Checks

Context Check Description
yinshiyou/configure_loongarch64 warning Failed to apply patch
andriy/configure_x86 warning Failed to apply patch

Commit Message

Lynne Oct. 19, 2023, 2:38 a.m. UTC 
Change from the previous version: by default, the current behaviour
of the decoder is preserved. Samples to skip are zero by default.
We can improve this later.

For some reason, this was never set, which meant all **raw** AAC in ADTS
streams, except faac, had extra samples at the start.

Despite this being a standard MDCT-based codec with a frame size of 1024,
hence a delay of 1024 samples at the start, all major encoders, excluding
faac and ffmpeg, use different amounts of padding.

Patch attached.

Comments

Anton Khirnov Oct. 19, 2023, 8:40 a.m. UTC | #1 
Quoting Lynne (2023-10-19 04:38:51)
> @@ -3471,6 +3478,9 @@ static const AVOption options[] = {
>        { "coded",    "order in which the channels are coded in the bitstream",
>          0, AV_OPT_TYPE_CONST, { .i64 = CHANNEL_ORDER_CODED }, .flags = AACDEC_FLAGS, "channel_order" },
>  
> +    { "padding", "Override the padding at the start of a stream.\n",
> +        offsetof(AACContext, override_padding), AV_OPT_TYPE_INT, { .i64 = 1024 }, 1024, 8192, AACDEC_FLAGS },

Why should this be a decoder option when the caller can already control
it by inserting AV_PKT_DATA_SKIP_SAMPLES?
Lynne Oct. 19, 2023, 12:48 p.m. UTC | #2 
Oct 19, 2023, 10:40 by anton@khirnov.net:

> Quoting Lynne (2023-10-19 04:38:51)
>
>> @@ -3471,6 +3478,9 @@ static const AVOption options[] = {
>>  { "coded",    "order in which the channels are coded in the bitstream",
>>  0, AV_OPT_TYPE_CONST, { .i64 = CHANNEL_ORDER_CODED }, .flags = AACDEC_FLAGS, "channel_order" },
>>  
>> +    { "padding", "Override the padding at the start of a stream.\n",
>> +        offsetof(AACContext, override_padding), AV_OPT_TYPE_INT, { .i64 = 1024 }, 1024, 8192, AACDEC_FLAGS },
>>
>
> Why should this be a decoder option when the caller can already control
> it by inserting AV_PKT_DATA_SKIP_SAMPLES?
>

CLI, sadly, mainly.
Anton Khirnov Oct. 19, 2023, 3:48 p.m. UTC | #3 
Quoting Lynne (2023-10-19 14:48:06)
> Oct 19, 2023, 10:40 by anton@khirnov.net:
> 
> > Quoting Lynne (2023-10-19 04:38:51)
> >
> >> @@ -3471,6 +3478,9 @@ static const AVOption options[] = {
> >>  { "coded",    "order in which the channels are coded in the bitstream",
> >>  0, AV_OPT_TYPE_CONST, { .i64 = CHANNEL_ORDER_CODED }, .flags = AACDEC_FLAGS, "channel_order" },
> >>  
> >> +    { "padding", "Override the padding at the start of a stream.\n",
> >> +        offsetof(AACContext, override_padding), AV_OPT_TYPE_INT, { .i64 = 1024 }, 1024, 8192, AACDEC_FLAGS },
> >>
> >
> > Why should this be a decoder option when the caller can already control
> > it by inserting AV_PKT_DATA_SKIP_SAMPLES?
> >
> 
> CLI, sadly, mainly.

So add a CLI option to override this side data along the lines of
display_rotation.
diff mbox series

Patch

From 691fdb2952707fffd6975ca7a555e198236e9168 Mon Sep 17 00:00:00 2001
From: Lynne <dev@lynne.ee>
Date: Tue, 3 Oct 2023 05:57:50 +0200
Subject: [PATCH] aacdec: padding skip improvements

For some reason, this was never set, which meant all **raw** AAC in ADTS
streams, except faac, had extra samples at the start.

Despite this being a standard MDCT-based codec with a frame size of 1024,
hence a delay of 1024 samples at the start, all major encoders, excluding
faac and FFmpeg, use 2048 samples of padding.
---
 libavcodec/aac.h             |  1 +
 libavcodec/aacdec_template.c | 20 +++++++++++++++-----
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/libavcodec/aac.h b/libavcodec/aac.h
index 285d3b7482..79bbb3cce5 100644
--- a/libavcodec/aac.h
+++ b/libavcodec/aac.h
@@ -298,6 +298,7 @@  struct AACContext {
     AVCodecContext *avctx;
     AVFrame *frame;
 
+    int override_padding;
     int is_saved;                 ///< Set if elements have stored overlap from previous frame.
     DynamicRangeControl che_drc;
 
diff --git a/libavcodec/aacdec_template.c b/libavcodec/aacdec_template.c
index 954399f86b..a5b46b09e3 100644
--- a/libavcodec/aacdec_template.c
+++ b/libavcodec/aacdec_template.c
@@ -1273,6 +1273,9 @@  static av_cold int aac_decode_init(AVCodecContext *avctx)
     if (ret < 0)
         return ret;
 
+    /* Usually overridden by side data */
+    avctx->internal->skip_samples = ac->override_padding;
+
     return 0;
 }
 
@@ -2417,14 +2420,16 @@  static int decode_dynamic_range(DynamicRangeControl *che_drc,
     return n;
 }
 
-static int decode_fill(AACContext *ac, GetBitContext *gb, int len) {
-    uint8_t buf[256];
-    int i, major, minor;
+static int decode_fill(AACContext *ac, GetBitContext *gb, int len)
+{
+    uint8_t buf[256] = { 0 };
+    int i, major, minor, micro;
 
     if (len < 13+7*8)
         goto unknown;
 
-    get_bits(gb, 13); len -= 13;
+    get_bits(gb, 13);
+    len -= 13;
 
     for(i=0; i+1<sizeof(buf) && len>=8; i++, len-=8)
         buf[i] = get_bits(gb, 8);
@@ -2434,7 +2439,9 @@  static int decode_fill(AACContext *ac, GetBitContext *gb, int len) {
         av_log(ac->avctx, AV_LOG_DEBUG, "FILL:%s\n", buf);
 
     if (sscanf(buf, "libfaac %d.%d", &major, &minor) == 2){
-        ac->avctx->internal->skip_samples = 1024;
+        ac->avctx->internal->skip_samples -= 1024;
+    } else if ((sscanf(buf, "avc %d.%d.%d", &major, &minor, &micro) == 3)) {
+        ac->avctx->internal->skip_samples -= 1024;
     }
 
 unknown:
@@ -3471,6 +3478,9 @@  static const AVOption options[] = {
       { "coded",    "order in which the channels are coded in the bitstream",
         0, AV_OPT_TYPE_CONST, { .i64 = CHANNEL_ORDER_CODED }, .flags = AACDEC_FLAGS, "channel_order" },
 
+    { "padding", "Override the padding at the start of a stream.\n",
+        offsetof(AACContext, override_padding), AV_OPT_TYPE_INT, { .i64 = 1024 }, 1024, 8192, AACDEC_FLAGS },
+
     {NULL},
 };
 
-- 
2.42.0