> On Jun 30, 2025, at 11:08 AM, Nicolas Gaullier <nicolas.gaullier@cji.paris>
> wrote:
>
> On 6/28/25 03:54, Baptiste Coudurier wrote:
>>> On Jun 27, 2025, at 5:01 AM, Nicolas Gaullier <nicolas.gaullier@cji.paris>
>>> wrote:
>>>
>>> On 6/26/25 19:30, Baptiste Coudurier wrote:
>>>> Hi Nicolas
>>>>> On Jun 26, 2025, at 4:06 AM, Nicolas Gaullier
>>>>> <nicolas.gaullier@cji.paris> wrote:
>>>>>
>>>>> On 6/17/25 13:41, Nicolas Gaullier wrote:
>>>>>> Reverts 64bb91fd3b5a00a8849531c7e8dd207f2a626096 except the
>>>>>> key_frame setting for aac.
>>>>>> See also 696ea1c2236842572df88d573e24a39be3f19c98.
>>>>>> Don't force the profile as it may override the correct value
>>>>>> set by the decoder (HE/HEv2).
>>>>>>
>>>>>> Fixes #11600
>>>>>>
>>>>>> Signed-off-by: Nicolas Gaullier <nicolas.gaullier@cji.paris>
>>>>>> ---
>>>>>> libavcodec/aac_ac3_parser.c | 8 +++-----
>>>>>> 1 file changed, 3 insertions(+), 5 deletions(-)
>>>>>>
>>>>>> diff --git a/libavcodec/aac_ac3_parser.c b/libavcodec/aac_ac3_parser.c
>>>>>> index e10ce13a3b..1dcc2e735f 100644
>>>>>> --- a/libavcodec/aac_ac3_parser.c
>>>>>> +++ b/libavcodec/aac_ac3_parser.c
>>>>>> @@ -147,15 +147,13 @@ 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 ?
>>>>>
>>>>> https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=14810
>>>>>
>>>> How hard is it to detect HEAACv2 in the parser ? Maybe only set it if
>>>> profile is unknown?
>>>> It’s usually nice when parser can set profile.
>>>>
>>>> —
>>>> Baptiste
>>> Yes, you're right, I had the same feeling when reverting this...
>>>
>>> It does not seem so straightforward, because aac seems very modular while
>>> not using signaling, but it must be doable someway.
>>>
>>> I will dig into this and see if I can come up with something satisfactory...
>>>
>>> I think it would be an additional patch, and this one should remain the
>>> basic revert which, mostly, simply removes dead code.
>>>
>>> Maybe it would be desirable to follow this patch immediately with a new
>>> patch that could restore the "profile setting" thing correctly, but that
>>> would add some undetermined delay, so I would say it is best to apply this
>>> patch on its own.
>> Yeah, does adding “if avctx->profile == UNKNOWN” fix the ticket?
>
> Not exactly, I mean it would indeed fix the ticket but that would not make
> the parser succeed in setting the correct profile - even if using an FFMAX to
> be able to "upgrade" the profile gradually.
>
> I just checked how it works currently with the decoder, just by debugging a
> simple HEAACv2 raw sample of my own. There are three steps in three different
> code sections, here it is:
>
> 1: aacdec.c#2412 : ac->avctx->profile = ac->oc[1].m4ac.object_type - 1; =>
> set AAC LC (similar as current parser code)
>
> 2: aacdec.c#1946 : ac->avctx->profile = AV_PROFILE_AAC_HE;
>
> 3: aacsbr_templace.c#978: ac->avctx->profile = AV_PROFILE_AAC_HE_V2;
>
> [NB: in aacdec.c#1941, the corresponding line "ac->avctx->profile =
> AV_PROFILE_AAC_HE_V2;" is not reach.]
>
> Well, it seems far from easy. I think it would require a very good knowledge
> of the code to have the parser correctly set the profile. And at the end, on
> my side, I don't think I will have time for this in the next weeks.
>
>
> So, I think we really have to simply remove this line which sets the profile
> in the parser: there is nothing good with it.
>
It was added so IMHO we can assume it was wanted or needed. I think it’s safer
to check for unknown, it fixes the ticket and does not make it worse than it
currently is.
—
Baptiste
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
