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

[Marton Balint <cus@passwd.hu>]

On Fri, 25 Aug 2023, Steven Liu wrote:

> Steven Liu <lq@chinaffmpeg.org> 于2023年8月25日周五 18:05写道：
>>
>> Add option named rtmp_enhanced_codec,
>> it would support hvc1,av01,vp09 now,
>> the fourcc is using Array of strings.
>>
>> Signed-off-by: Steven Liu <lq@chinaffmpeg.org>
>> ---
>>  doc/protocols.texi      |  6 ++++++
>>  libavformat/rtmpproto.c | 37 +++++++++++++++++++++++++++++++++++++
>>  2 files changed, 43 insertions(+)
>>
>> diff --git a/doc/protocols.texi b/doc/protocols.texi
>> index b3fad55591..bd2b25e502 100644
>> --- a/doc/protocols.texi
>> +++ b/doc/protocols.texi
>> @@ -896,6 +896,12 @@ be named, by prefixing the type with 'N' and specifying the name before
>>  the value (i.e. @code{NB:myFlag:1}). This option may be used multiple
>>  times to construct arbitrary AMF sequences.
>>
>> +@item rtmp_enhanced_codecs
>> +Specify the list of codecs the client advertises to support in an
>> +enhanced RTMP stream.

>> This option should set a string like @code{hvc1,av01,vp09}
>> +for multiple codecs, or @code{hvc1} for only one codec,
>> +set codec fourcc into fourCcLive property into Connect Command Message,

This option should be set to a comma separated list of fourcc values, like 
@code{hvc1,av01,vp09} for multiple codecs or @code{hvc1} for only one 
codec. The specified list will be presented in the "fourCcLive" property 
of the Connect Command Message.


>> +
>>  @item rtmp_flashver
>>  Version of the Flash plugin used to run the SWF player. The default
>>  is LNX 9,0,124,2. (When publishing, the default is FMLE/3.0 (compatible;
>> diff --git a/libavformat/rtmpproto.c b/libavformat/rtmpproto.c
>> index f0ef223f05..10e0aed539 100644
>> --- a/libavformat/rtmpproto.c
>> +++ b/libavformat/rtmpproto.c
>> @@ -127,6 +127,7 @@ typedef struct RTMPContext {
>>      int           nb_streamid;                ///< The next stream id to return on createStream calls
>>      double        duration;                   ///< Duration of the stream in seconds as returned by the server (only valid if non-zero)
>>      int           tcp_nodelay;                ///< Use TCP_NODELAY to disable Nagle's algorithm if set to 1
>> +    char          *enhanced_codecs;           ///< codec list in enhanced rtmp
>>      char          username[50];
>>      char          password[50];
>>      char          auth_params[500];
>> @@ -336,6 +337,41 @@ static int gen_connect(URLContext *s, RTMPContext *rt)
>>      ff_amf_write_field_name(&p, "app");
>>      ff_amf_write_string2(&p, rt->app, rt->auth_params);
>>
>> +    if (rt->enhanced_codecs) {
>> +        uint32_t list_len = 0;
>> +        char *fourcc_data = rt->enhanced_codecs;
>> +        int fourcc_str_len = strlen(fourcc_data);
>> +
>> +        // check the string, fourcc + ',' + ...  + end fourcc correct length should be (4+1)*n+4
>> +        if ((fourcc_str_len + 1) % 5 != 0) {
>> +            av_log(s, AV_LOG_ERROR, "Malformed rtmp_enhanched_codecs, "
>> +                   "should be of the form hvc1[,av01][,vp09][,...]\n");
>> +            return AVERROR(EINVAL);
>> +        }
>> +
>> +        list_len = (fourcc_str_len + 1) / 5;
>> +        // write the fourCcList field name

Useless comment

>> +        ff_amf_write_field_name(&p, "fourCcList");
>> +
>> +        // write the fourcc array length

Useless comment

>> +        ff_amf_write_array_start(&p, list_len);
>> +
>> +        while(fourcc_data - rt->enhanced_codecs < fourcc_str_len) {
>> +            unsigned char fourcc[5];
>> +            if (!strncasecmp(fourcc_data, "hvc1", 4) ||
>> +                !strncasecmp(fourcc_data, "av01", 4) ||
>> +                !strncasecmp(fourcc_data, "vp09", 4)) {
>
> s/strncasecmp/av_strncasecmp/g fix local.

Why you want this to be case insensitive? The specs only contains the 
lowercase variants as examples. So I'd rather use simply strncmp.

>
>> +                    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.

>> +            }
>> +
>> +            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 
client advetises to support, and for the legacy counterpart of this, 
"audioCodecs" and "videoCodecs", we also advertise to support almost 
everything. So for consistency we should follow the same route, report to 
support every codec that is in the specs, unless this causes some 
compatibility issue I don't see.

Thanks,
Marton

>>      {"rtmp_live", "Specify that the media is a live stream.", OFFSET(live), AV_OPT_TYPE_INT, {.i64 = -2}, INT_MIN, INT_MAX, DEC, "rtmp_live"},
>>      {"any", "both", 0, AV_OPT_TYPE_CONST, {.i64 = -2}, 0, 0, DEC, "rtmp_live"},
>>      {"live", "live stream", 0, AV_OPT_TYPE_CONST, {.i64 = -1}, 0, 0, DEC, "rtmp_live"},
>> --
>> 2.40.0
>>
>> _______________________________________________
>> 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".
> _______________________________________________
> 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".
_______________________________________________
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".