[FFmpeg-devel] [PATCH] avformat/hlsenc: fix CODECS Attribute hard code in hevc EXT-X-STREAM-INF

[Jack Lau]

> On Mar 3, 2025, at 09:08, Steven Liu <lingjiujianke at gmail.com> wrote:
> 
> Jack Lau via ffmpeg-devel <ffmpeg-devel at ffmpeg.org <mailto:ffmpeg-devel at ffmpeg.org>> 于2025年3月2日周日 21:31写道：
>> 
>> fix ticket: 10786
>> parse the SPS from extradata and get profile_compatibility, tier, constraints which was been hard code before.
>> 
>> HEVC CODECS Attribute reference to: ISO/IEC14496-15
>> Signed-off-by: Jack Lau <jacklau1222 at qq.com>
>> ---
>> libavformat/hlsenc.c | 41 ++++++++++++++++++++++++++++++++++++++---
>> 1 file changed, 38 insertions(+), 3 deletions(-)
>> 
>> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
>> index 6148685f40..849130196f 100644
>> --- a/libavformat/hlsenc.c
>> +++ b/libavformat/hlsenc.c
>> @@ -379,7 +379,10 @@ static void write_codec_attr(AVStream *st, VariantStream *vs)
>>     } else if (st->codecpar->codec_id == AV_CODEC_ID_HEVC) {
>>         uint8_t *data = st->codecpar->extradata;
>>         int profile = AV_PROFILE_UNKNOWN;
>> +        uint32_t profile_compatibility = AV_PROFILE_UNKNOWN;
>> +        char tier = 0;
>>         int level = AV_LEVEL_UNKNOWN;
>> +        char constraints[8] = "";
>> 
>>         if (st->codecpar->profile != AV_PROFILE_UNKNOWN)
>>             profile = st->codecpar->profile;
>> @@ -393,6 +396,8 @@ static void write_codec_attr(AVStream *st, VariantStream *vs)
>>                 uint8_t *rbsp_buf;
>>                 int remain_size = 0;
>>                 int rbsp_size = 0;
>> +                uint32_t profile_compatibility_flags = 0;
>> +                uint8_t high_nibble = 0;
>>                 /* skip start code + nalu header */
>>                 data += 6;
>>                 /* process by reference General NAL unit syntax */
>> @@ -406,8 +411,35 @@ static void write_codec_attr(AVStream *st, VariantStream *vs)
>>                 }
>>                 /* skip sps_video_parameter_set_id   u(4),
>>                  *      sps_max_sub_layers_minus1    u(3),
>> -                 *  and sps_temporal_id_nesting_flag u(1) */
>> +                 *  and sps_temporal_id_nesting_flag u(1)
>> +                 *
>> +                 * TIER represents the general_tier_flag, with 'L' indicating the flag is 0,
>> +                 * and 'H' indicating the flag is 1
>> +                 */
>> +                tier = (int)(rbsp_buf[1] & 0x20) == 0 ? 'L' : 'H';
> Since 'tier' is a char, the cast to (int) should be unnecessary.
Got it! remove (int) already in the latest patch.
>>                 profile = rbsp_buf[1] & 0x1f;
>> +                /* PROFILE_COMPATIBILITY is general_profile_compatibility_flags, but in reverse bit order,
>> +                 * in a hexadecimal representation (leading zeroes may be omitted).
>> +                 */
>> +                profile_compatibility_flags = AV_RB32(rbsp_buf + 2);
>> +                /* revise these bits to get the profile compatibility value */
>> +                {
> remove this braces
> 
>> +                   uint32_t x = profile_compatibility_flags;
>> +                    x = ((x & 0x55555555U) << 1) | ((x >> 1) & 0x55555555U);
>> +                    x = ((x & 0x33333333U) << 2) | ((x >> 2) & 0x33333333U);
>> +                    x = ((x & 0x0F0F0F0FU) << 4) | ((x >> 4) & 0x0F0F0F0FU);
>> +                    x = ((x & 0x00FF00FFU) << 8) | ((x >> 8) & 0x00FF00FFU);
>> +                    profile_compatibility = (x << 16) | (x >> 16);
>> +                }
> ditoo
Thank you for reminding me of this problem. In the latest patch, I not only removed braces, but also removed the temporary variable x. Direct bit operations on profile_compatibility_flags will save more resources.
>> +                /* skip 8 + 8 + 32
>> +                 * CONSTRAINTS is a hexadecimal representation of the general_constraint_indicator_flags.
>> +                 * each byte is separated by a '.', and trailing zero bytes may be omitted.
>> +                 * drop the trailing zero bytes refer to ISO/IEC14496-15.
>> +                 */
>> +                high_nibble = rbsp_buf[7] >> 4;
>> +                snprintf(constraints, sizeof(constraints),
>> +                         high_nibble ? "%02x.%x" : "%02x",
>> +                         rbsp_buf[6], high_nibble);
>>                 /* skip 8 + 8 + 32 + 4 + 43 + 1 bit */
>>                 level = rbsp_buf[12];
>>                 av_freep(&rbsp_buf);
>> @@ -417,8 +449,11 @@ static void write_codec_attr(AVStream *st, VariantStream *vs)
>>         }
>>         if (st->codecpar->codec_tag == MKTAG('h','v','c','1') &&
>>             profile != AV_PROFILE_UNKNOWN &&
>> -            level != AV_LEVEL_UNKNOWN) {
>> -            snprintf(attr, sizeof(attr), "%s.%d.4.L%d.B01", av_fourcc2str(st->codecpar->codec_tag), profile, level);
>> +            profile_compatibility != AV_PROFILE_UNKNOWN &&
>> +            tier != 0 &&
>> +            level != AV_LEVEL_UNKNOWN &&
>> +            constraints[0] != '\0') {
>> +            snprintf(attr, sizeof(attr), "%s.%d.%x.%c%d.%s", av_fourcc2str(st->codecpar->codec_tag), profile, profile_compatibility, tier, level, constraints);
>>         } else
>>             goto fail;
>>     } else if (st->codecpar->codec_id == AV_CODEC_ID_MP2) {
>> --
>> 2.47.1
>> 
>> _______________________________________________
> Otherwise, it looks okay to me. You need to ask the submitter of the
> ticket ID to check whether this patch can fix the issue.
Got it, I already ask the reporter, waiting for messages~
> 
> 
> Thanks
> Steven
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org <mailto:ffmpeg-devel at ffmpeg.org>
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org <mailto:ffmpeg-devel-request at ffmpeg.org> with subject "unsubscribe”.


Thanks for your detailed review

Regards
Jack