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

[Marton Balint <cus@passwd.hu>]

On Thu, 24 Aug 2023, Tristan Matthews wrote:

> Hi,
>
> On Thu, Aug 24, 2023 at 1:32 AM Steven Liu <lq@chinaffmpeg.org> wrote:
>>
>> 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 | 38 ++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 44 insertions(+)
>>
>> diff --git a/doc/protocols.texi b/doc/protocols.texi
>> index b3fad55591..f2930fb3a2 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_codec

This is a list, so make it -rtmp_enhanced_codecs

>> +Specify that the media is an enhanced rtmp live stream. This option should
>> +set a sting like @code{hvc1,av01,vp09} for multiple codecs, or @code{hvc1}
>
> I think this should be more like "Specify the codecs to use in an
> enhanced rtmp live stream", the wording here makes it sound more like
> a boolean flag.

Actually it is a *supported* list, not a to-be-used list. So maybe 
"Specify the list of codecs the client advertises to support in an 
enhanced RTMP stream" is more appropriate.

>
> Also nit: "set a string"
>
>> +for only one codec, set codec fourcc into fourCcLive property into
>> +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..f7ce04244f 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 list in enhanced rtmp

char *enhanced_codecs

>
> nit: "codec list"
>>      char          username[50];
>>      char          password[50];
>>      char          auth_params[500];
>> @@ -336,6 +337,42 @@ 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) {
>> +        uint32_t list_len = 0;
>> +        char *fourcc_data = rt->enhanced;
>> +        int fourcc_str_len = fourcc_data ? strlen(fourcc_data) : 0;

fourcc_data is always true.

>> +
>> +        // check the string, fourcc + ',' + ...  + end fourcc correct length should be (4+1)*n+4
>> +        if ((fourcc_str_len + 1) % 5 != 0)
>> +            return AVERROR(EINVAL);
>> +
>> +        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(&p, list_len);
>> +
>> +        while(fourcc_data) {

Still always true.

>> +            unsigned char fourcc[5];
>> +            switch (*(uint32_t *)fourcc_data) {

AV_RN32

>> +                case MKTAG('h', 'v', 'c', '1'):
>> +                case MKTAG('a', 'v', '0', '1'):
>> +                case MKTAG('v', 'p', '0', '9'):
>> +                    strncpy(fourcc, fourcc_data, 4);
>> +                    fourcc[4] = '\0';

av_strlcpy(fourcc, fourcc_data, sizeof(fourcc));

>> +                    ff_amf_write_string(&p, fourcc);
>> +                    break;
>> +                default:
>> +                    return AVERROR(EINVAL);
>> +            }
>> +
>> +            fourcc_data += (fourcc_str_len - (fourcc_data - rt->enhanced)) > 4 ? 5 : 4;

Why not simply fourcc_data += 5?

>> +            if (fourcc_data - rt->enhanced >= fourcc_str_len)
>> +                break;

Why not check this as the loop condition instead?

Regards,
Marton

>> +        }
>> +    }
>> +
>>      if (!rt->is_input) {
>>          ff_amf_write_field_name(&p, "type");
>>          ff_amf_write_string(&p, "nonprivate");
>> @@ -3104,6 +3141,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_codec", "Specify that the codec in enhanced rtmp live stream", OFFSET(enhanced), AV_OPT_TYPE_STRING, {.str = NULL }, 0, 0, ENC},
>
> I think this should be more like "Specify the codec(s) to use in an
> enhanced rtmp live stream" ?
>
>>      {"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".