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

[Steven Liu <lingjiujianke@gmail.com>]

Martin Storsjö <martin@martin.st> 于2023年8月25日周五 17:04写道：
>
> On Fri, 25 Aug 2023, Steven Liu 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 | 35 +++++++++++++++++++++++++++++++++++
> > 2 files changed, 41 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,
> > +
> > @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..ea25454362 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,39 @@ 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)
> > +            return AVERROR(EINVAL);
>
> If we trigger this error, we definitely should tell the user what was
> wrong. 'Malformed rtmp_enhanched_codecs, should be of the form
> "fourcc[,fourcc..]"' or something like that? (Would it be better to use
> "frcc" or just "hvc1" or something else there, to show that it needs to be
> exactly a four char string for each element?)
Agreed
>
> > +
> > +        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?

> as well. That doesn't fit quite as neatly into a switch, but would be a
> more unwieldy if statement though.
>
> > +                case MKTAG('a', 'v', '0', '1'):
> > +                case MKTAG('v', 'p', '0', '9'):
> > +                    av_strlcpy(fourcc, fourcc_data, sizeof(fourcc));
> > +                    ff_amf_write_string(&p, fourcc);
> > +                    break;
> > +                default:
>
> When this fails, the user definitely needs to know what was wrong, so we
> should print the string that didn't match the expectation. E.g.
> ("unsupported codec fourcc, %.*s", 4, fourcc_data) or something like that?
good suggestion.
>
> // Martin
>
> _______________________________________________
> 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".