[FFmpeg-devel] [PATCH v3 0/2] fix HE-AAC profile probing and simplify

[FFmpeg-devel] [PATCH v3 0/2] fix HE-AAC profile probing and simplify

[Nicolas Gaullier]

v3: complete overhaul of the commit message of the first patch
Code is unchanged. Second patch is unchanged ("should be ok" by James).

Nicolas Gaullier (2):
  avcodec/aac_ac3_parser: simplify
  avcodec/aac_ac3_parser: do not override the profile set by the decoder

 libavcodec/aac_ac3_parser.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

-- 
2.47.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] [PATCH v3 1/2] avcodec/aac_ac3_parser: simplify

[Nicolas Gaullier]

Remove unused USAC/ADTS code: ff_adts_header_parse() parse the
object_type from a 2 bits field.
See also 696ea1c2236842572df88d573e24a39be3f19c98.

Use ff_adts_header_parse_buf() wrapper to simplify as GetBitContext is
no longer needed (it was introduced for USAC).

Partially reverts 64bb91fd3b5a00a8849531c7e8dd207f2a626096.

Signed-off-by: Nicolas Gaullier <nicolas.gaullier@cji.paris>
---
 libavcodec/aac_ac3_parser.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/libavcodec/aac_ac3_parser.c b/libavcodec/aac_ac3_parser.c
index e10ce13a3b..b49ce0277c 100644
--- a/libavcodec/aac_ac3_parser.c
+++ b/libavcodec/aac_ac3_parser.c
@@ -147,15 +147,14 @@ get_next:
         } else {
 #if CONFIG_AAC_PARSER
             AACADTSHeaderInfo hdr;
-            GetBitContext gb;
 
-            init_get_bits8(&gb, buf, buf_size);
             if (buf_size < AV_AAC_ADTS_HEADER_SIZE ||
-                ff_adts_header_parse(&gb, &hdr) < 0)
+                ff_adts_header_parse_buf(buf, &hdr) < 0)
                 return i;
 
             avctx->profile = hdr.object_type - 1;
-            s1->key_frame = (avctx->profile == AV_PROFILE_AAC_USAC) ? get_bits1(&gb) : 1;
+            /* ADTS does not support USAC */
+            s1->key_frame = 1;
             bit_rate = hdr.bit_rate;
 #endif
         }
-- 
2.47.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".

Re: [FFmpeg-devel] [PATCH v3 1/2] avcodec/aac_ac3_parser: simplify

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 1848 bytes --]

On Fri, Jul 11, 2025 at 11:54:29AM +0200, Nicolas Gaullier wrote:
> Remove unused USAC/ADTS code: ff_adts_header_parse() parse the
> object_type from a 2 bits field.
> See also 696ea1c2236842572df88d573e24a39be3f19c98.
> 
> Use ff_adts_header_parse_buf() wrapper to simplify as GetBitContext is
> no longer needed (it was introduced for USAC).
> 
> Partially reverts 64bb91fd3b5a00a8849531c7e8dd207f2a626096.
> 
> Signed-off-by: Nicolas Gaullier <nicolas.gaullier@cji.paris>
> ---
>  libavcodec/aac_ac3_parser.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/libavcodec/aac_ac3_parser.c b/libavcodec/aac_ac3_parser.c
> index e10ce13a3b..b49ce0277c 100644
> --- a/libavcodec/aac_ac3_parser.c
> +++ b/libavcodec/aac_ac3_parser.c
> @@ -147,15 +147,14 @@ get_next:
>          } else {
>  #if CONFIG_AAC_PARSER
>              AACADTSHeaderInfo hdr;
> -            GetBitContext gb;
>  
> -            init_get_bits8(&gb, buf, buf_size);
>              if (buf_size < AV_AAC_ADTS_HEADER_SIZE ||
> -                ff_adts_header_parse(&gb, &hdr) < 0)
> +                ff_adts_header_parse_buf(buf, &hdr) < 0)
>                  return i;
>  
>              avctx->profile = hdr.object_type - 1;
> -            s1->key_frame = (avctx->profile == AV_PROFILE_AAC_USAC) ? get_bits1(&gb) : 1;
> +            /* ADTS does not support USAC */
> +            s1->key_frame = 1;
>              bit_rate = hdr.bit_rate;

Are these 2 changes related ?
if not they are maybe better in 2 seperate patches

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Concerning the gods, I have no means of knowing whether they exist or not
or of what sort they may be, because of the obscurity of the subject, and
the brevity of human life -- Protagoras

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v3 1/2] avcodec/aac_ac3_parser: simplify

[Nicolas Gaullier]

On 7/12/25 12:52, Michael Niedermayer wrote:
> On Fri, Jul 11, 2025 at 11:54:29AM +0200, Nicolas Gaullier wrote:
>> Remove unused USAC/ADTS code: ff_adts_header_parse() parse the
>> object_type from a 2 bits field.
>> See also 696ea1c2236842572df88d573e24a39be3f19c98.
>>
>> Use ff_adts_header_parse_buf() wrapper to simplify as GetBitContext is
>> no longer needed (it was introduced for USAC).
>>
>> Partially reverts 64bb91fd3b5a00a8849531c7e8dd207f2a626096.
>>
>> Signed-off-by: Nicolas Gaullier <nicolas.gaullier@cji.paris>
>> ---
>>   libavcodec/aac_ac3_parser.c | 7 +++----
>>   1 file changed, 3 insertions(+), 4 deletions(-)
>>
>> diff --git a/libavcodec/aac_ac3_parser.c b/libavcodec/aac_ac3_parser.c
>> index e10ce13a3b..b49ce0277c 100644
>> --- a/libavcodec/aac_ac3_parser.c
>> +++ b/libavcodec/aac_ac3_parser.c
>> @@ -147,15 +147,14 @@ get_next:
>>           } else {
>>   #if CONFIG_AAC_PARSER
>>               AACADTSHeaderInfo hdr;
>> -            GetBitContext gb;
>>
>> -            init_get_bits8(&gb, buf, buf_size);
>>               if (buf_size < AV_AAC_ADTS_HEADER_SIZE ||
>> -                ff_adts_header_parse(&gb, &hdr) < 0)
>> +                ff_adts_header_parse_buf(buf, &hdr) < 0)
>>                   return i;
>>
>>               avctx->profile = hdr.object_type - 1;
>> -            s1->key_frame = (avctx->profile == AV_PROFILE_AAC_USAC) ? get_bits1(&gb) : 1;
>> +            /* ADTS does not support USAC */
>> +            s1->key_frame = 1;
>>               bit_rate = hdr.bit_rate;
> Are these 2 changes related ?
> if not they are maybe better in 2 seperate patches

They are related in that they both are partial revert of the same commit,
and they both apply to the same code section (so order matter).
I split it into two patches as I think it makes it clearer.

Nicolas

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v3 1/2] avcodec/aac_ac3_parser: simplify

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 2324 bytes --]

On Tue, Jul 15, 2025 at 09:38:53AM +0200, Nicolas Gaullier wrote:
> On 7/12/25 12:52, Michael Niedermayer wrote:
> > On Fri, Jul 11, 2025 at 11:54:29AM +0200, Nicolas Gaullier wrote:
> > > Remove unused USAC/ADTS code: ff_adts_header_parse() parse the
> > > object_type from a 2 bits field.
> > > See also 696ea1c2236842572df88d573e24a39be3f19c98.
> > > 
> > > Use ff_adts_header_parse_buf() wrapper to simplify as GetBitContext is
> > > no longer needed (it was introduced for USAC).
> > > 
> > > Partially reverts 64bb91fd3b5a00a8849531c7e8dd207f2a626096.
> > > 
> > > Signed-off-by: Nicolas Gaullier <nicolas.gaullier@cji.paris>
> > > ---
> > >   libavcodec/aac_ac3_parser.c | 7 +++----
> > >   1 file changed, 3 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/libavcodec/aac_ac3_parser.c b/libavcodec/aac_ac3_parser.c
> > > index e10ce13a3b..b49ce0277c 100644
> > > --- a/libavcodec/aac_ac3_parser.c
> > > +++ b/libavcodec/aac_ac3_parser.c
> > > @@ -147,15 +147,14 @@ get_next:
> > >           } else {
> > >   #if CONFIG_AAC_PARSER
> > >               AACADTSHeaderInfo hdr;
> > > -            GetBitContext gb;
> > > 
> > > -            init_get_bits8(&gb, buf, buf_size);
> > >               if (buf_size < AV_AAC_ADTS_HEADER_SIZE ||
> > > -                ff_adts_header_parse(&gb, &hdr) < 0)
> > > +                ff_adts_header_parse_buf(buf, &hdr) < 0)
> > >                   return i;
> > > 
> > >               avctx->profile = hdr.object_type - 1;
> > > -            s1->key_frame = (avctx->profile == AV_PROFILE_AAC_USAC) ? get_bits1(&gb) : 1;
> > > +            /* ADTS does not support USAC */
> > > +            s1->key_frame = 1;
> > >               bit_rate = hdr.bit_rate;
> > Are these 2 changes related ?
> > if not they are maybe better in 2 seperate patches
> 
> They are related in that they both are partial revert of the same commit,
> and they both apply to the same code section (so order matter).
> I split it into two patches as I think it makes it clearer.

I think it should be 3 patches not 2

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Dictatorship naturally arises out of democracy, and the most aggravated
form of tyranny and slavery out of the most extreme liberty. -- Plato

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v3 1/2] avcodec/aac_ac3_parser: simplify

[Nicolas Gaullier]

On 7/11/25 11:54, Nicolas Gaullier wrote:
> Remove unused USAC/ADTS code: ff_adts_header_parse() parse the
> object_type from a 2 bits field.
> See also 696ea1c2236842572df88d573e24a39be3f19c98.
>
> Use ff_adts_header_parse_buf() wrapper to simplify as GetBitContext is
> no longer needed (it was introduced for USAC).
>
> Partially reverts 64bb91fd3b5a00a8849531c7e8dd207f2a626096.
>
> Signed-off-by: Nicolas Gaullier <nicolas.gaullier@cji.paris>
> ---
>   libavcodec/aac_ac3_parser.c | 7 +++----
>   1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/libavcodec/aac_ac3_parser.c b/libavcodec/aac_ac3_parser.c
> index e10ce13a3b..b49ce0277c 100644
> --- a/libavcodec/aac_ac3_parser.c
> +++ b/libavcodec/aac_ac3_parser.c
> @@ -147,15 +147,14 @@ get_next:
>           } else {
>   #if CONFIG_AAC_PARSER
>               AACADTSHeaderInfo hdr;
> -            GetBitContext gb;
>   
> -            init_get_bits8(&gb, buf, buf_size);
>               if (buf_size < AV_AAC_ADTS_HEADER_SIZE ||
> -                ff_adts_header_parse(&gb, &hdr) < 0)
> +                ff_adts_header_parse_buf(buf, &hdr) < 0)
>                   return i;
>   
>               avctx->profile = hdr.object_type - 1;
> -            s1->key_frame = (avctx->profile == AV_PROFILE_AAC_USAC) ? get_bits1(&gb) : 1;
> +            /* ADTS does not support USAC */
> +            s1->key_frame = 1;
>               bit_rate = hdr.bit_rate;
>   #endif
>           }

Ping ?

If you don't like this patch for whatever reason, I can just drop it and 
send the second patch (which is more important - fix ticket) 
independently, no big deal.

I still think at least the key_frame setting/USAC thing should be 
removed because it makes think USAC is possible which is not at all.

Nicolas

_______________________________________________
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] [PATCH v3 2/2] avcodec/aac_ac3_parser: do not override the profile set by the decoder

[Nicolas Gaullier]

Parsing the ADTS header is not enough to detect HE-AAC v1/v2.

Regression since 64bb91fd3b5a00a8849531c7e8dd207f2a626096.

Fixes #11600

Signed-off-by: Nicolas Gaullier <nicolas.gaullier@cji.paris>
---
 libavcodec/aac_ac3_parser.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/libavcodec/aac_ac3_parser.c b/libavcodec/aac_ac3_parser.c
index b49ce0277c..51c801214c 100644
--- a/libavcodec/aac_ac3_parser.c
+++ b/libavcodec/aac_ac3_parser.c
@@ -152,7 +152,8 @@ get_next:
                 ff_adts_header_parse_buf(buf, &hdr) < 0)
                 return i;
 
-            avctx->profile = hdr.object_type - 1;
+            if (avctx->profile == AV_PROFILE_UNKNOWN)
+                avctx->profile = hdr.object_type - 1;
             /* ADTS does not support USAC */
             s1->key_frame = 1;
             bit_rate = hdr.bit_rate;
-- 
2.47.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".