Re: [FFmpeg-devel] [PATCH v2 1/2] avcodec/aac_ac3_parser: remove dead code

[Nicolas Gaullier <nicolas.gaullier@cji.paris>]

On 7/7/25 15:40, James Almer wrote:
> On 7/7/2025 10:19 AM, Nicolas Gaullier wrote:
>> ff_adts_header_parse() gets the object_type from only 2 bits.
>> See also 696ea1c2236842572df88d573e24a39be3f19c98.
>>
>> Introduced in 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
>>           }
>
> This isn't dead code. It just moves the getbitcontext creation to the
> wrapper function ff_adts_header_parse_buf().
>
> Also, the buffer needs AV_INPUT_BUFFER_PADDING_SIZE of padding bytes, so
> maybe use a zeroed local buffer while at it and memcpy
> AV_AAC_ADTS_HEADER_SIZE bytes to it, then pass that.
Sorry, I just realized my commit message is misleading.
What I was meaning is that the "key_frame" setting was dead code, and 
the rest follows.

Indeed, the wrapping thing is not dead code, but once the dynamic 
key_frame setting removed, it seemed to me logical to revert to the 
original, simpler ff_adts_header_parse_buf() wrapper.

The padding is handled far upstream, it comes straight from 
av_parser_parse2() in the api. I doubled check, there is really nothing 
changed here, except the size of the GetBitContext but with no 
performance change (and anyway, the wrapper makes it smaller).

Maybe it would be clearer this way:

avcodec/aac_ac3_parser: simplify/remove unused USAC/ADTS code

ff_adts_header_parse() gets the object_type from only 2 bits.
See also 696ea1c2236842572df88d573e24a39be3f19c98.

Use ff_adts_header_parse_buf() back to simplify as GetBitContext is no 
longer needed.

Partially reverts 64bb91fd3b5a00a8849531c7e8dd207f2a626096.

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