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

2023-08-25 Thread Martin Storsjö

On Fri, 25 Aug 2023, Steven Liu wrote:


Martin Storsjö  于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(, "fourCcList");
>> > +
>> > +// write the fourcc array length
>> > +ff_amf_write_array_start(, 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
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


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

2023-08-25 Thread Steven Liu
Martin Storsjö  于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(, "fourCcList");
> >> > +
> >> > +// write the fourcc array length
> >> > +ff_amf_write_array_start(, 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. 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.
>
> // 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-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".


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

2023-08-25 Thread Martin Storsjö

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(, "fourCcList");
> +
> +// write the fourcc array length
> +ff_amf_write_array_start(, 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.


// 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-requ...@ffmpeg.org with subject "unsubscribe".


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

2023-08-25 Thread Steven Liu
Martin Storsjö  于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 
> > ---
> > 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
> > doubleduration;   ///< 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(, "app");
> > ff_amf_write_string2(, 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(, "fourCcList");
> > +
> > +// write the fourcc array length
> > +ff_amf_write_array_start(, 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(, 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-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".


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

2023-08-25 Thread Martin Storsjö

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 
---
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
doubleduration;   ///< 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(, "app");
ff_amf_write_string2(, 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?)



+
+list_len = (fourcc_str_len + 1) / 5;
+// write the fourCcList field name
+ff_amf_write_field_name(, "fourCcList");
+
+// write the fourcc array length
+ff_amf_write_array_start(, 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 
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(, 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?


// 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-requ...@ffmpeg.org with subject "unsubscribe".


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

2023-08-24 Thread Steven Liu
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 
---
 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
 doubleduration;   ///< 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(, "app");
 ff_amf_write_string2(, 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);
+
+list_len = (fourcc_str_len + 1) / 5;
+// write the fourCcList field name
+ff_amf_write_field_name(, "fourCcList");
+
+// write the fourcc array length
+ff_amf_write_array_start(, 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'):
+case MKTAG('a', 'v', '0', '1'):
+case MKTAG('v', 'p', '0', '9'):
+av_strlcpy(fourcc, fourcc_data, sizeof(fourcc));
+ff_amf_write_string(, fourcc);
+break;
+default:
+return AVERROR(EINVAL);
+}
+
+fourcc_data += 5;
+}
+}
+
 if (!rt->is_input) {
 ff_amf_write_field_name(, "type");
 ff_amf_write_string(, "nonprivate");
@@ -3104,6 +3138,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},
 {"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-requ...@ffmpeg.org with subject "unsubscribe".