[FFmpeg-devel] [PATCH v2 2/2] avformat/rtmpproto: support enhanced rtmp

[Martin Storsjö]

On Fri, 25 Aug 2023, Steven Liu wrote:

> Martin Storsjö <martin at martin.st> 于2023年8月25日周五 17:38写道：
>>
>> On Fri, 25 Aug 2023, Steven Liu wrote:
>>
>> >> > +        list_len = (fourcc_str_len + 1) / 5;
>> >> > +        // write the fourCcList field name
>> >> > +        ff_amf_write_field_name(&p, "fourCcList");
>> >> > +
>> >> > +        // write the fourcc array length
>> >> > +        ff_amf_write_array_start(&p, list_len);
>> >> > +
>> >> > +        while(fourcc_data - rt->enhanced_codecs < fourcc_str_len) {
>> >> > +            unsigned char fourcc[5];
>> >> > +            switch (AV_RN32(fourcc_data)) {
>> >> > +                case MKTAG('h', 'v', 'c', '1'):
>> >>
>> >> This feels a bit convoluted to use AV_RN32 + MKTAG on data that is already
>> >> just a string - just using strncmp(fourcc_data, "hvc1", 4) would work just
>> > I want use strncmp at first, but i think strncmp will slower than
>> > switch uint32_t value, so i chose use uint32_t. or should i use
>> > strncmp?
>>
>> Yes, strncmp will be slower than switching on an uint32_t - but this isn't
>> really performance sensitive code. We do this once only, on startup, on a
>> handful of fourccs. So I believe code clarity is more important than
>> absolute performance here.
> Okay, i use strncasecmp next version patch.

I don't see a need for this to be case insensitive?

> But i must told you, I think MKTAG is clarity than use if+strncmp to me, 
> Because there looks like a fourcc codec list align code block.

If you prefer MKTAG for clarity, then I guess the current solution is fine 
here for me as well - it's not a hard opinion from me. Let's see if 
someone else wants to comment on it first maybe.

// Martin