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

[Marton Balint <cus@passwd.hu>]

On Sat, 26 Aug 2023, Steven Liu wrote:

> Marton Balint <cus@passwd.hu> 于2023年8月26日周六 15:52写道：
> Hi Marton,

[...]

>> >
>> >> +                    av_strlcpy(fourcc, fourcc_data, sizeof(fourcc));
>> >> +                    ff_amf_write_string(&p, fourcc);
>> >> +            } else {
>> >> +                    av_log(s, AV_LOG_ERROR, "Unsupported codec fourcc, %.*s\n", 4, fourcc_data);
>> >> +                    return AVERROR_PATCHWELCOME;
>>
>> As far as I saw the specs does not provide an explicit list of fourcc
>> that can be used, so we should not limit it either, it makes the code more
>> future-proof. Therefore I suggest we accept unknown fourccs, but warn the
>> user about them.
> I set unsupported codec fourcc, and return a patchwelcome is because
> there just add hevc,av1,vp9 codec supported in flvenc and flvdec,
> dose not add the others in the flv right, so if user want use another
> codecs, it should give a error and patch welcome.

Ok, but if you only want to support a strict list, then maybe 
AV_OPT_TYPE_FLAGS type would be a better option type? Fine with me either 
way.

>>
>> >> +            }
>> >> +
>> >> +            fourcc_data += 5;
>> >> +        }
>> >> +    }
>> >> +
>> >>      if (!rt->is_input) {
>> >>          ff_amf_write_field_name(&p, "type");
>> >>          ff_amf_write_string(&p, "nonprivate");
>> >> @@ -3104,6 +3140,7 @@ static const AVOption rtmp_options[] = {
>> >>      {"rtmp_conn", "Append arbitrary AMF data to the Connect message", OFFSET(conn), AV_OPT_TYPE_STRING, {.str = NULL }, 0, 0, DEC|ENC},
>> >>      {"rtmp_flashver", "Version of the Flash plugin used to run the SWF player.", OFFSET(flashver), AV_OPT_TYPE_STRING, {.str = NULL }, 0, 0, DEC|ENC},
>> >>      {"rtmp_flush_interval", "Number of packets flushed in the same request (RTMPT only).", OFFSET(flush_interval), AV_OPT_TYPE_INT, {.i64 = 10}, 0, INT_MAX, ENC},
>> >> +    {"rtmp_enhanced_codecs", "Specify the codec(s) to use in an enhanced rtmp live stream", OFFSET(enhanced_codecs), AV_OPT_TYPE_STRING, {.str = NULL }, 0, 0, ENC},
>>
>> Maybe make the default "av01,vp09,hevc"? This is a list of codecs the
> I think make it null should better, because not should every body's
> ffmpeg has been support hevc/av1/vp9 codec,

And not everybody is using libavcodec for decoding hevc/av1/vp9, so which 
decoders are compiled in does not really matter. Do we really want the 
default to *not support* any recent codec?

> so I set it null default, and the other reason is not sure all rtmp
> server has beed support enhanced rtmp, so make it null default
> when user not use codec hevc/av1/vp9.

I'd expect that legacy rtmp servers will just ignore "fourCcList" 
property as an unknown/unsupported property, but I am not sure what would 
actually happen... Maybe some tests should be made with the most common 
implementations. If it actually causes issues, then I am fine with 
the NULL default.

Regards,
Marton
_______________________________________________
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".