rtmp: Gracefully ignore FCPublish and _checkbw errors

Message ID 1342778748-14914-1-git-send-email-samuel.pitoiset@gmail.com
State Superseded
Headers show

Commit Message

Samuel Pitoiset July 20, 2012, 10:05 a.m.
Those messages are adobe specific historical artefacs that some rtmp
servers do not support. It is safer to ignore their failure. This fixes
publishing streams to crtmpserver and it fixes bugzilla bug #309.
---
 libavformat/rtmpproto.c | 33 +++++++++++++++++++++++++++++++--
 1 file changed, 31 insertions(+), 2 deletions(-)

Comments

Luca Barbato July 20, 2012, 10:37 a.m. | #1
On 7/20/12 12:05 PM, Samuel Pitoiset wrote:
> Those messages are adobe specific historical artefacs that some rtmp
> servers do not support. It is safer to ignore their failure. This fixes
> publishing streams to crtmpserver and it fixes bugzilla bug #309.
> ---
>   libavformat/rtmpproto.c | 33 +++++++++++++++++++++++++++++++--
>   1 file changed, 31 insertions(+), 2 deletions(-)
>
> diff --git a/libavformat/rtmpproto.c b/libavformat/rtmpproto.c
> index 5c40eb5..973bc3b 100644
> --- a/libavformat/rtmpproto.c
> +++ b/libavformat/rtmpproto.c
> @@ -877,8 +877,25 @@ static int rtmp_parse_result(URLContext *s, RTMPContext *rt, RTMPPacket *pkt)
>               uint8_t tmpstr[256];
>
>               if (!ff_amf_get_field_value(pkt->data + 9, data_end,
> -                                        "description", tmpstr, sizeof(tmpstr)))
> -                av_log(s, AV_LOG_ERROR, "Server error: %s\n",tmpstr);
> +                                        "description", tmpstr, sizeof(tmpstr))) {
> +                if (!rt->is_input) {
> +                    uint8_t codestr[256];
> +
> +                    t = ff_amf_get_field_value(pkt->data, data_end,
> +                                               "code", codestr, sizeof(codestr));
> +                    if (!t && !strcmp(codestr, "NetConnection.Call.Failed")) {
> +                        if (!strcmp(tmpstr, "Specified stream not found in call to releaseStream")
> +                            || !strcmp(tmpstr, "call to function _checkbw failed")) {
> +                            /* Hack for crtmpserver, these messages are adobe
> +                             * specific historical artefacs that some rtmp
> +                             * servers do not support. It is safer to ignore
> +                             * their failure. */
> +                            break;
> +                        }
> +                    }
> +                }
> +                av_log(s, AV_LOG_ERROR, "Server error: %s\n", tmpstr);
> +            }

That code is too specific, you should ignore the failure for those two 
methods doesn't matter the return value (and/or optionally disable them)

Compare librtmp way to track results from methods.

>               return -1;
>           } else if (!memcmp(pkt->data, "\002\000\007_result", 10)) {
>               switch (rt->state) {
> @@ -957,6 +974,18 @@ static int rtmp_parse_result(URLContext *s, RTMPContext *rt, RTMPPacket *pkt)
>           } else if (!memcmp(pkt->data, "\002\000\010onBWDone", 11)) {
>               if ((ret = gen_check_bw(s, rt)) < 0)
>                   return ret;
> +        } else if (!memcmp(pkt->data, "\002\000\013onFCPublish", 11)) {
> +            uint8_t tmpstr[256];

onFCPublish?

> +            /* Hack for crtmpserver, it sends an invoke packet in order to
> +             * inform the client that the stream can be published. */
> +            t = ff_amf_get_field_value(pkt->data, data_end,
> +                                       "code", tmpstr, sizeof(tmpstr));
> +            if (!t && !strcmp(tmpstr, "NetStream.Publish.Start"))
> +                rt->state = STATE_CONNECTING;

Matching that string should be always correct and not an hack

lu
Diego Biurrun July 20, 2012, 10:42 a.m. | #2
On Fri, Jul 20, 2012 at 12:05:48PM +0200, Samuel Pitoiset wrote:
> Those messages are adobe specific historical artefacs that some rtmp
These messages are Adobe-specific historical artifacts that some RTMP

> servers do not support. It is safer to ignore their failure. This fixes
> publishing streams to crtmpserver and it fixes bugzilla bug #309.

Bugzilla

> --- a/libavformat/rtmpproto.c
> +++ b/libavformat/rtmpproto.c
> @@ -877,8 +877,25 @@ static int rtmp_parse_result(URLContext *s, RTMPContext *rt, RTMPPacket *pkt)
> +                            || !strcmp(tmpstr, "call to function _checkbw failed")) {
> +                            /* Hack for crtmpserver, these messages are adobe
> +                             * specific historical artefacs that some rtmp
> +                             * servers do not support. It is safer to ignore
> +                             * their failure. */
> +                            break;

see above

Diego
Samuel Pitoiset July 20, 2012, 2:11 p.m. | #3
On Fri, Jul 20, 2012 at 12:37 PM, Luca Barbato <lu_zero@gentoo.org> wrote:
> On 7/20/12 12:05 PM, Samuel Pitoiset wrote:
>>
>> Those messages are adobe specific historical artefacs that some rtmp
>> servers do not support. It is safer to ignore their failure. This fixes
>> publishing streams to crtmpserver and it fixes bugzilla bug #309.
>> ---
>>   libavformat/rtmpproto.c | 33 +++++++++++++++++++++++++++++++--
>>   1 file changed, 31 insertions(+), 2 deletions(-)
>>
>> diff --git a/libavformat/rtmpproto.c b/libavformat/rtmpproto.c
>> index 5c40eb5..973bc3b 100644
>> --- a/libavformat/rtmpproto.c
>> +++ b/libavformat/rtmpproto.c
>> @@ -877,8 +877,25 @@ static int rtmp_parse_result(URLContext *s,
>> RTMPContext *rt, RTMPPacket *pkt)
>>               uint8_t tmpstr[256];
>>
>>               if (!ff_amf_get_field_value(pkt->data + 9, data_end,
>> -                                        "description", tmpstr,
>> sizeof(tmpstr)))
>> -                av_log(s, AV_LOG_ERROR, "Server error: %s\n",tmpstr);
>> +                                        "description", tmpstr,
>> sizeof(tmpstr))) {
>> +                if (!rt->is_input) {
>> +                    uint8_t codestr[256];
>> +
>> +                    t = ff_amf_get_field_value(pkt->data, data_end,
>> +                                               "code", codestr,
>> sizeof(codestr));
>> +                    if (!t && !strcmp(codestr,
>> "NetConnection.Call.Failed")) {
>> +                        if (!strcmp(tmpstr, "Specified stream not found
>> in call to releaseStream")
>> +                            || !strcmp(tmpstr, "call to function _checkbw
>> failed")) {
>> +                            /* Hack for crtmpserver, these messages are
>> adobe
>> +                             * specific historical artefacs that some
>> rtmp
>> +                             * servers do not support. It is safer to
>> ignore
>> +                             * their failure. */
>> +                            break;
>> +                        }
>> +                    }
>> +                }
>> +                av_log(s, AV_LOG_ERROR, "Server error: %s\n", tmpstr);
>> +            }
>
>
> That code is too specific, you should ignore the failure for those two
> methods doesn't matter the return value (and/or optionally disable them)

Okay, I'll improve that part.
>
> Compare librtmp way to track results from methods.

Yes, librtmp only show an error message.

>
>>               return -1;
>>           } else if (!memcmp(pkt->data, "\002\000\007_result", 10)) {
>>               switch (rt->state) {
>> @@ -957,6 +974,18 @@ static int rtmp_parse_result(URLContext *s,
>> RTMPContext *rt, RTMPPacket *pkt)
>>           } else if (!memcmp(pkt->data, "\002\000\010onBWDone", 11)) {
>>               if ((ret = gen_check_bw(s, rt)) < 0)
>>                   return ret;
>> +        } else if (!memcmp(pkt->data, "\002\000\013onFCPublish", 11)) {
>> +            uint8_t tmpstr[256];
>
>
> onFCPublish?

The invoke onFCPublish sent by the server contains the channel if of
the stream, so I have to handle it.

>
>
>> +            /* Hack for crtmpserver, it sends an invoke packet in order
>> to
>> +             * inform the client that the stream can be published. */
>> +            t = ff_amf_get_field_value(pkt->data, data_end,
>> +                                       "code", tmpstr, sizeof(tmpstr));
>> +            if (!t && !strcmp(tmpstr, "NetStream.Publish.Start"))
>> +                rt->state = STATE_CONNECTING;
>
>
> Matching that string should be always correct and not an hack

Okay.

Patch

diff --git a/libavformat/rtmpproto.c b/libavformat/rtmpproto.c
index 5c40eb5..973bc3b 100644
--- a/libavformat/rtmpproto.c
+++ b/libavformat/rtmpproto.c
@@ -877,8 +877,25 @@  static int rtmp_parse_result(URLContext *s, RTMPContext *rt, RTMPPacket *pkt)
             uint8_t tmpstr[256];
 
             if (!ff_amf_get_field_value(pkt->data + 9, data_end,
-                                        "description", tmpstr, sizeof(tmpstr)))
-                av_log(s, AV_LOG_ERROR, "Server error: %s\n",tmpstr);
+                                        "description", tmpstr, sizeof(tmpstr))) {
+                if (!rt->is_input) {
+                    uint8_t codestr[256];
+
+                    t = ff_amf_get_field_value(pkt->data, data_end,
+                                               "code", codestr, sizeof(codestr));
+                    if (!t && !strcmp(codestr, "NetConnection.Call.Failed")) {
+                        if (!strcmp(tmpstr, "Specified stream not found in call to releaseStream")
+                            || !strcmp(tmpstr, "call to function _checkbw failed")) {
+                            /* Hack for crtmpserver, these messages are adobe
+                             * specific historical artefacs that some rtmp
+                             * servers do not support. It is safer to ignore
+                             * their failure. */
+                            break;
+                        }
+                    }
+                }
+                av_log(s, AV_LOG_ERROR, "Server error: %s\n", tmpstr);
+            }
             return -1;
         } else if (!memcmp(pkt->data, "\002\000\007_result", 10)) {
             switch (rt->state) {
@@ -957,6 +974,18 @@  static int rtmp_parse_result(URLContext *s, RTMPContext *rt, RTMPPacket *pkt)
         } else if (!memcmp(pkt->data, "\002\000\010onBWDone", 11)) {
             if ((ret = gen_check_bw(s, rt)) < 0)
                 return ret;
+        } else if (!memcmp(pkt->data, "\002\000\013onFCPublish", 11)) {
+            uint8_t tmpstr[256];
+
+            if (rt->state != STATE_FCPUBLISH)
+                 break;
+
+            /* Hack for crtmpserver, it sends an invoke packet in order to
+             * inform the client that the stream can be published. */
+            t = ff_amf_get_field_value(pkt->data, data_end,
+                                       "code", tmpstr, sizeof(tmpstr));
+            if (!t && !strcmp(tmpstr, "NetStream.Publish.Start"))
+                rt->state = STATE_CONNECTING;
         }
         break;
     case RTMP_PT_VIDEO: