[libav-devel] [PATCH 1/2] rtmp: check ff_rtmp_packet_create calls.

Martin Storsjö martin at martin.st
Tue May 22 22:45:51 CEST 2012


On Tue, 22 May 2012, Samuel Pitoiset wrote:

> ---
> libavformat/rtmppkt.c   |    6 +-
> libavformat/rtmpproto.c |  223 +++++++++++++++++++++++++++++++++++------------
> 2 files changed, 173 insertions(+), 56 deletions(-)
>
> diff --git a/libavformat/rtmppkt.c b/libavformat/rtmppkt.c
> index 8c455a0..00507d2 100644
> --- a/libavformat/rtmppkt.c
> +++ b/libavformat/rtmppkt.c
> @@ -79,6 +79,7 @@ int ff_rtmp_packet_read(URLContext *h, RTMPPacket *p,
>     uint32_t extra = 0;
>     enum RTMPPacketType type;
>     int size = 0;
> +    int ret;
>
>     if (ffurl_read(h, &hdr, 1) != 1)
>         return AVERROR(EIO);
> @@ -129,8 +130,9 @@ int ff_rtmp_packet_read(URLContext *h, RTMPPacket *p,
>     if (hdr != RTMP_PS_TWELVEBYTES)
>         timestamp += prev_pkt[channel_id].timestamp;
>
> -    if (ff_rtmp_packet_create(p, channel_id, type, timestamp, data_size))
> -        return -1;
> +    ret = ff_rtmp_packet_create(p, channel_id, type, timestamp, data_size);
> +    if (ret < 0)
> +        return ret;
>     p->extra = extra;

As Diego pointed out, this isn't what the commit message says. Include 
"pass the returned codes through" or something similar and it should be 
fine.

>     // save history
>     prev_pkt[channel_id].channel_id = channel_id;
> diff --git a/libavformat/rtmpproto.c b/libavformat/rtmpproto.c
> index 1b86c44..4c46a1d 100644
> --- a/libavformat/rtmpproto.c
> +++ b/libavformat/rtmpproto.c
> @@ -115,12 +115,17 @@ static const uint8_t rtmp_server_key[] = {
> /**
>  * Generate 'connect' call and send it to the server.
>  */
> -static void gen_connect(URLContext *s, RTMPContext *rt)
> +static int gen_connect(URLContext *s, RTMPContext *rt)
> {
>     RTMPPacket pkt;
>     uint8_t *p;
> +    int ret;
> +
> +    ret = ff_rtmp_packet_create(&pkt, RTMP_SYSTEM_CHANNEL, RTMP_PT_INVOKE,
> +                                0, 4096);
> +    if (ret < 0)
> +        return ret;
>
> -    ff_rtmp_packet_create(&pkt, RTMP_SYSTEM_CHANNEL, RTMP_PT_INVOKE, 0, 4096);
>     p = pkt.data;
>
>     ff_amf_write_string(&p, "connect");
> @@ -165,19 +170,24 @@ static void gen_connect(URLContext *s, RTMPContext *rt)
>
>     ff_rtmp_packet_write(rt->stream, &pkt, rt->chunk_size, rt->prev_pkt[1]);
>     ff_rtmp_packet_destroy(&pkt);
> +
> +    return 0;
> }
>
> /**
>  * Generate 'releaseStream' call and send it to the server. It should make
>  * the server release some channel for media streams.
>  */
> -static void gen_release_stream(URLContext *s, RTMPContext *rt)
> +static int gen_release_stream(URLContext *s, RTMPContext *rt)
> {
>     RTMPPacket pkt;
>     uint8_t *p;
> +    int ret;
>
> -    ff_rtmp_packet_create(&pkt, RTMP_SYSTEM_CHANNEL, RTMP_PT_INVOKE, 0,
> -                          29 + strlen(rt->playpath));
> +    ret = ff_rtmp_packet_create(&pkt, RTMP_SYSTEM_CHANNEL, RTMP_PT_INVOKE,
> +                                0, 29 + strlen(rt->playpath));
> +    if (ret < 0)
> +        return ret;
>
>     av_log(s, AV_LOG_DEBUG, "Releasing stream...\n");
>     p = pkt.data;
> @@ -188,19 +198,24 @@ static void gen_release_stream(URLContext *s, RTMPContext *rt)
>
>     ff_rtmp_packet_write(rt->stream, &pkt, rt->chunk_size, rt->prev_pkt[1]);
>     ff_rtmp_packet_destroy(&pkt);
> +
> +    return 0;
> }
>
> /**
>  * Generate 'FCPublish' call and send it to the server. It should make
>  * the server preapare for receiving media streams.
>  */
> -static void gen_fcpublish_stream(URLContext *s, RTMPContext *rt)
> +static int gen_fcpublish_stream(URLContext *s, RTMPContext *rt)
> {
>     RTMPPacket pkt;
>     uint8_t *p;
> +    int ret;
>
> -    ff_rtmp_packet_create(&pkt, RTMP_SYSTEM_CHANNEL, RTMP_PT_INVOKE, 0,
> -                          25 + strlen(rt->playpath));
> +    ret = ff_rtmp_packet_create(&pkt, RTMP_SYSTEM_CHANNEL, RTMP_PT_INVOKE,
> +                                0, 25 + strlen(rt->playpath));
> +    if (ret < 0)
> +        return ret;
>
>     av_log(s, AV_LOG_DEBUG, "FCPublish stream...\n");
>     p = pkt.data;
> @@ -211,19 +226,24 @@ static void gen_fcpublish_stream(URLContext *s, RTMPContext *rt)
>
>     ff_rtmp_packet_write(rt->stream, &pkt, rt->chunk_size, rt->prev_pkt[1]);
>     ff_rtmp_packet_destroy(&pkt);
> +
> +    return 0;
> }
>
> /**
>  * Generate 'FCUnpublish' call and send it to the server. It should make
>  * the server destroy stream.
>  */
> -static void gen_fcunpublish_stream(URLContext *s, RTMPContext *rt)
> +static int gen_fcunpublish_stream(URLContext *s, RTMPContext *rt)
> {
>     RTMPPacket pkt;
>     uint8_t *p;
> +    int ret;
>
> -    ff_rtmp_packet_create(&pkt, RTMP_SYSTEM_CHANNEL, RTMP_PT_INVOKE, 0,
> -                          27 + strlen(rt->playpath));
> +    ret = ff_rtmp_packet_create(&pkt, RTMP_SYSTEM_CHANNEL, RTMP_PT_INVOKE,
> +                                0, 27 + strlen(rt->playpath));
> +    if (ret < 0)
> +        return ret;
>
>     av_log(s, AV_LOG_DEBUG, "UnPublishing stream...\n");
>     p = pkt.data;
> @@ -234,19 +254,25 @@ static void gen_fcunpublish_stream(URLContext *s, RTMPContext *rt)
>
>     ff_rtmp_packet_write(rt->stream, &pkt, rt->chunk_size, rt->prev_pkt[1]);
>     ff_rtmp_packet_destroy(&pkt);
> +
> +    return 0;
> }
>
> /**
>  * Generate 'createStream' call and send it to the server. It should make
>  * the server allocate some channel for media streams.
>  */
> -static void gen_create_stream(URLContext *s, RTMPContext *rt)
> +static int gen_create_stream(URLContext *s, RTMPContext *rt)
> {
>     RTMPPacket pkt;
>     uint8_t *p;
> +    int ret;
>
>     av_log(s, AV_LOG_DEBUG, "Creating stream...\n");
> -    ff_rtmp_packet_create(&pkt, RTMP_SYSTEM_CHANNEL, RTMP_PT_INVOKE, 0, 25);
> +    ret = ff_rtmp_packet_create(&pkt, RTMP_SYSTEM_CHANNEL, RTMP_PT_INVOKE,
> +                                0, 25);
> +    if (ret < 0)
> +        return ret;
>
>     p = pkt.data;
>     ff_amf_write_string(&p, "createStream");
> @@ -256,6 +282,8 @@ static void gen_create_stream(URLContext *s, RTMPContext *rt)
>
>     ff_rtmp_packet_write(rt->stream, &pkt, rt->chunk_size, rt->prev_pkt[1]);
>     ff_rtmp_packet_destroy(&pkt);
> +
> +    return 0;
> }
>
>
> @@ -263,13 +291,17 @@ static void gen_create_stream(URLContext *s, RTMPContext *rt)
>  * Generate 'deleteStream' call and send it to the server. It should make
>  * the server remove some channel for media streams.
>  */
> -static void gen_delete_stream(URLContext *s, RTMPContext *rt)
> +static int gen_delete_stream(URLContext *s, RTMPContext *rt)
> {
>     RTMPPacket pkt;
>     uint8_t *p;
> +    int ret;
>
>     av_log(s, AV_LOG_DEBUG, "Deleting stream...\n");
> -    ff_rtmp_packet_create(&pkt, RTMP_SYSTEM_CHANNEL, RTMP_PT_INVOKE, 0, 34);
> +    ret = ff_rtmp_packet_create(&pkt, RTMP_SYSTEM_CHANNEL, RTMP_PT_INVOKE,
> +                                0, 34);
> +    if (ret < 0)
> +        return ret;
>
>     p = pkt.data;
>     ff_amf_write_string(&p, "deleteStream");
> @@ -279,20 +311,26 @@ static void gen_delete_stream(URLContext *s, RTMPContext *rt)
>
>     ff_rtmp_packet_write(rt->stream, &pkt, rt->chunk_size, rt->prev_pkt[1]);
>     ff_rtmp_packet_destroy(&pkt);
> +
> +    return 0;
> }
>
> /**
>  * Generate 'play' call and send it to the server, then ping the server
>  * to start actual playing.
>  */
> -static void gen_play(URLContext *s, RTMPContext *rt)
> +static int gen_play(URLContext *s, RTMPContext *rt)
> {
>     RTMPPacket pkt;
>     uint8_t *p;
> +    int ret;
>
>     av_log(s, AV_LOG_DEBUG, "Sending play command for '%s'\n", rt->playpath);
> -    ff_rtmp_packet_create(&pkt, RTMP_VIDEO_CHANNEL, RTMP_PT_INVOKE, 0,
> -                          29 + strlen(rt->playpath));
> +    ret = ff_rtmp_packet_create(&pkt, RTMP_VIDEO_CHANNEL, RTMP_PT_INVOKE,
> +                                0, 29 + strlen(rt->playpath));
> +    if (ret < 0)
> +        return ret;
> +
>     pkt.extra = rt->main_channel_id;
>
>     p = pkt.data;
> @@ -306,7 +344,10 @@ static void gen_play(URLContext *s, RTMPContext *rt)
>     ff_rtmp_packet_destroy(&pkt);
>
>     // set client buffer time disguised in ping packet
> -    ff_rtmp_packet_create(&pkt, RTMP_NETWORK_CHANNEL, RTMP_PT_PING, 1, 10);
> +    ret = ff_rtmp_packet_create(&pkt, RTMP_NETWORK_CHANNEL, RTMP_PT_PING,
> +                                1, 10);
> +    if (ret < 0)
> +        return ret;
>
>     p = pkt.data;
>     bytestream_put_be16(&p, 3);
> @@ -315,19 +356,25 @@ static void gen_play(URLContext *s, RTMPContext *rt)
>
>     ff_rtmp_packet_write(rt->stream, &pkt, rt->chunk_size, rt->prev_pkt[1]);
>     ff_rtmp_packet_destroy(&pkt);
> +
> +    return 0;
> }
>
> /**
>  * Generate 'publish' call and send it to the server.
>  */
> -static void gen_publish(URLContext *s, RTMPContext *rt)
> +static int gen_publish(URLContext *s, RTMPContext *rt)
> {
>     RTMPPacket pkt;
>     uint8_t *p;
> +    int ret;
>
>     av_log(s, AV_LOG_DEBUG, "Sending publish command for '%s'\n", rt->playpath);
> -    ff_rtmp_packet_create(&pkt, RTMP_SOURCE_CHANNEL, RTMP_PT_INVOKE, 0,
> -                          30 + strlen(rt->playpath));
> +    ret = ff_rtmp_packet_create(&pkt, RTMP_SOURCE_CHANNEL, RTMP_PT_INVOKE,
> +                                0, 30 + strlen(rt->playpath));
> +    if (ret < 0)
> +        return ret;
> +
>     pkt.extra = rt->main_channel_id;
>
>     p = pkt.data;
> @@ -339,48 +386,68 @@ static void gen_publish(URLContext *s, RTMPContext *rt)
>
>     ff_rtmp_packet_write(rt->stream, &pkt, rt->chunk_size, rt->prev_pkt[1]);
>     ff_rtmp_packet_destroy(&pkt);
> +
> +    return ret;
> }
>
> /**
>  * Generate ping reply and send it to the server.
>  */
> -static void gen_pong(URLContext *s, RTMPContext *rt, RTMPPacket *ppkt)
> +static int gen_pong(URLContext *s, RTMPContext *rt, RTMPPacket *ppkt)
> {
>     RTMPPacket pkt;
>     uint8_t *p;
> +    int ret;
> +
> +    ret = ff_rtmp_packet_create(&pkt, RTMP_NETWORK_CHANNEL, RTMP_PT_PING,
> +                                ppkt->timestamp + 1, 6);
> +    if (ret < 0)
> +        return ret;
>
> -    ff_rtmp_packet_create(&pkt, RTMP_NETWORK_CHANNEL, RTMP_PT_PING, ppkt->timestamp + 1, 6);
>     p = pkt.data;
>     bytestream_put_be16(&p, 7);
>     bytestream_put_be32(&p, AV_RB32(ppkt->data+2));
>     ff_rtmp_packet_write(rt->stream, &pkt, rt->chunk_size, rt->prev_pkt[1]);
>     ff_rtmp_packet_destroy(&pkt);
> +
> +    return 0;
> }
>
> /**
>  * Generate server bandwidth message and send it to the server.
>  */
> -static void gen_server_bw(URLContext *s, RTMPContext *rt)
> +static int gen_server_bw(URLContext *s, RTMPContext *rt)
> {
>     RTMPPacket pkt;
>     uint8_t *p;
> +    int ret;
> +
> +    ret = ff_rtmp_packet_create(&pkt, RTMP_NETWORK_CHANNEL, RTMP_PT_SERVER_BW,
> +                                0, 4);
> +    if (ret < 0)
> +        return ret;
>
> -    ff_rtmp_packet_create(&pkt, RTMP_NETWORK_CHANNEL, RTMP_PT_SERVER_BW, 0, 4);
>     p = pkt.data;
>     bytestream_put_be32(&p, 2500000);
>     ff_rtmp_packet_write(rt->stream, &pkt, rt->chunk_size, rt->prev_pkt[1]);
>     ff_rtmp_packet_destroy(&pkt);
> +
> +    return 0;
> }
>
> /**
>  * Generate check bandwidth message and send it to the server.
>  */
> -static void gen_check_bw(URLContext *s, RTMPContext *rt)
> +static int gen_check_bw(URLContext *s, RTMPContext *rt)
> {
>     RTMPPacket pkt;
>     uint8_t *p;
> +    int ret;
>
> -    ff_rtmp_packet_create(&pkt, RTMP_SYSTEM_CHANNEL, RTMP_PT_INVOKE, 0, 21);
> +    ret = ff_rtmp_packet_create(&pkt, RTMP_SYSTEM_CHANNEL, RTMP_PT_INVOKE,
> +                                0, 21);
> +    if (ret < 0)
> +        return ret;
>
>     p = pkt.data;
>     ff_amf_write_string(&p, "_checkbw");
> @@ -389,21 +456,30 @@ static void gen_check_bw(URLContext *s, RTMPContext *rt)
>
>     ff_rtmp_packet_write(rt->stream, &pkt, rt->chunk_size, rt->prev_pkt[1]);
>     ff_rtmp_packet_destroy(&pkt);
> +
> +    return ret;
> }
>
> /**
>  * Generate report on bytes read so far and send it to the server.
>  */
> -static void gen_bytes_read(URLContext *s, RTMPContext *rt, uint32_t ts)
> +static int gen_bytes_read(URLContext *s, RTMPContext *rt, uint32_t ts)
> {
>     RTMPPacket pkt;
>     uint8_t *p;
> +    int ret;
> +
> +    ret = ff_rtmp_packet_create(&pkt, RTMP_NETWORK_CHANNEL, RTMP_PT_BYTES_READ,
> +                                ts, 4);
> +    if (ret < 0)
> +        return ret;
>
> -    ff_rtmp_packet_create(&pkt, RTMP_NETWORK_CHANNEL, RTMP_PT_BYTES_READ, ts, 4);
>     p = pkt.data;
>     bytestream_put_be32(&p, rt->bytes_read);
>     ff_rtmp_packet_write(rt->stream, &pkt, rt->chunk_size, rt->prev_pkt[1]);
>     ff_rtmp_packet_destroy(&pkt);
> +
> +    return 0;
> }
>
> //TODO: Move HMAC code somewhere. Eventually.
> @@ -599,6 +675,7 @@ static int rtmp_parse_result(URLContext *s, RTMPContext *rt, RTMPPacket *pkt)
> {
>     int i, t;
>     const uint8_t *data_end = pkt->data + pkt->data_size;
> +    int ret;
>
> #ifdef DEBUG
>     ff_rtmp_packet_dump(s, pkt);
> @@ -622,8 +699,11 @@ static int rtmp_parse_result(URLContext *s, RTMPContext *rt, RTMPPacket *pkt)
>         break;
>     case RTMP_PT_PING:
>         t = AV_RB16(pkt->data);
> -        if (t == 6)
> -            gen_pong(s, rt, pkt);
> +        if (t == 6) {
> +            ret = gen_pong(s, rt, pkt);
> +            if (ret < 0)
> +                return ret;
> +        }
>         break;
>     case RTMP_PT_CLIENT_BW:
>         if (pkt->data_size < 4) {
> @@ -648,14 +728,22 @@ static int rtmp_parse_result(URLContext *s, RTMPContext *rt, RTMPPacket *pkt)
>             switch (rt->state) {
>             case STATE_HANDSHAKED:
>                 if (!rt->is_input) {
> -                    gen_release_stream(s, rt);
> -                    gen_fcpublish_stream(s, rt);
> +                    ret = gen_release_stream(s, rt);
> +                    if (ret < 0)
> +                        return ret;
> +                    ret = gen_fcpublish_stream(s, rt);
> +                    if (ret < 0)
> +                        return ret;

Since this bloats the code quite a bit, you can write a bit more compactly 
like this:

if ((ret = gen_release_stream(s, rt)) < 0)
     return ret;

That saves a few lines, if you prefer that. (I guess the same can be 
applied at most of the places in this file. In the gen_*() functions, it's 
not as bad since it's just one instance per function, but for places where 
this, when the reader rather would try to focus on the higher level 
aspects, shortening the code might make it more readable.)

>                     rt->state = STATE_RELEASING;
>                 } else {
> -                    gen_server_bw(s, rt);
> +                    ret = gen_server_bw(s, rt);
> +                    if (ret < 0)
> +                        return ret;
>                     rt->state = STATE_CONNECTING;
>                 }
> -                gen_create_stream(s, rt);
> +                ret = gen_create_stream(s, rt);
> +                if (ret < 0)
> +                    return ret;
>                 break;
>             case STATE_FCPUBLISH:
>                 rt->state = STATE_CONNECTING;
> @@ -679,9 +767,13 @@ static int rtmp_parse_result(URLContext *s, RTMPContext *rt, RTMPPacket *pkt)
>                     rt->main_channel_id = av_int2double(AV_RB64(pkt->data + 21));
>                 }
>                 if (rt->is_input) {
> -                    gen_play(s, rt);
> +                    ret = gen_play(s, rt);
> +                    if (ret < 0)
> +                        return ret;
>                 } else {
> -                    gen_publish(s, rt);
> +                    ret = gen_publish(s, rt);
> +                    if (ret < 0)
> +                        return ret;
>                 }
>                 rt->state = STATE_READY;
>                 break;
> @@ -711,7 +803,9 @@ static int rtmp_parse_result(URLContext *s, RTMPContext *rt, RTMPPacket *pkt)
>             if (!t && !strcmp(tmpstr, "NetStream.Play.UnpublishNotify")) rt->state = STATE_STOPPED;
>             if (!t && !strcmp(tmpstr, "NetStream.Publish.Start")) rt->state = STATE_PUBLISHING;
>         } else if (!memcmp(pkt->data, "\002\000\010onBWDone", 11)) {
> -            gen_check_bw(s, rt);
> +            ret = gen_check_bw(s, rt);
> +            if (ret < 0)
> +                return ret;
>         }
>         break;
>     }
> @@ -754,14 +848,16 @@ static int get_packet(URLContext *s, int for_header)
>         rt->bytes_read += ret;
>         if (rt->bytes_read > rt->last_bytes_read + rt->client_report_size) {
>             av_log(s, AV_LOG_DEBUG, "Sending bytes read report\n");
> -            gen_bytes_read(s, rt, rpkt.timestamp + 1);
> +            ret = gen_bytes_read(s, rt, rpkt.timestamp + 1);
> +            if (ret < 0)
> +                return ret;
>             rt->last_bytes_read = rt->bytes_read;
>         }
>
>         ret = rtmp_parse_result(s, rt, &rpkt);
>         if (ret < 0) {//serious error in current packet
>             ff_rtmp_packet_destroy(&rpkt);
> -            return -1;
> +            return ret;
>         }
>         if (rt->state == STATE_STOPPED) {
>             ff_rtmp_packet_destroy(&rpkt);
> @@ -825,16 +921,23 @@ static int get_packet(URLContext *s, int for_header)
> static int rtmp_close(URLContext *h)
> {
>     RTMPContext *rt = h->priv_data;
> +    int ret;
>
>     if (!rt->is_input) {
>         rt->flv_data = NULL;
>         if (rt->out_pkt.data_size)
>             ff_rtmp_packet_destroy(&rt->out_pkt);
> -        if (rt->state > STATE_FCPUBLISH)
> -            gen_fcunpublish_stream(h, rt);
> +        if (rt->state > STATE_FCPUBLISH) {
> +            ret = gen_fcunpublish_stream(h, rt);
> +            if (ret < 0)
> +                return ret;
> +        }
> +    }
> +    if (rt->state > STATE_HANDSHAKED) {
> +        ret = gen_delete_stream(h, rt);
> +        if (ret < 0)
> +            return ret;
>     }
> -    if (rt->state > STATE_HANDSHAKED)
> -        gen_delete_stream(h, rt);

This is not right. If an allocation fails in any of these functions, you'd 
just return from rtmp_close and leak the allocated data structures and the 
nested urlprotocol. So for this case, it's better to just ignore any 
failure in these and run the function to the end. If you want to, you 
might want to keep track of the return value in order to indicate that not 
everything worked out fine. But I'm not sure if there's any code anywhere 
that checks the return value from ffurl_close() (what would one do in that 
case anyway?). But make sure all the allocated memory is freed in any 
case.

>
>     av_freep(&rt->flv_data);
>     ffurl_close(rt->stream);
> @@ -871,12 +974,15 @@ static int rtmp_open(URLContext *s, const char *uri, int flags)
>     if (ffurl_open(&rt->stream, buf, AVIO_FLAG_READ_WRITE,
>                    &s->interrupt_callback, NULL) < 0) {
>         av_log(s , AV_LOG_ERROR, "Cannot open connection %s\n", buf);
> +        ret = AVERROR(EIO);
>         goto fail;
>     }

I'd rather use if ((ret = ffurl_open()) < 0) here, so that you pass the 
actual error along, instead of inventing a new one.

>
>     rt->state = STATE_START;
> -    if (rtmp_handshake(s, rt))
> +    if (rtmp_handshake(s, rt)) {
> +        ret = AVERROR(EIO);
>         goto fail;
> +    }

This might be ok. Or does rtmp_handshake return an error code we'd like to 
pass on?

>     rt->chunk_size = 128;
>     rt->state = STATE_HANDSHAKED;
> @@ -886,8 +992,8 @@ static int rtmp_open(URLContext *s, const char *uri, int flags)
>
>     rt->app = av_malloc(APP_MAX_LENGTH);
>     if (!rt->app) {
> -        rtmp_close(s);
> -        return AVERROR(ENOMEM);
> +        ret = AVERROR(ENOMEM);
> +        goto fail;
>     }

Hmm, except for unifying things, does this make any difference - I don't 
think so. But it's ok to keep it included (add a mention about unifying 
error handling in the commit message).

>     //extract "app" part from path
> @@ -922,8 +1028,8 @@ static int rtmp_open(URLContext *s, const char *uri, int flags)
>     if (!rt->playpath) {
>         rt->playpath = av_malloc(PLAYPATH_MAX_LENGTH);
>         if (!rt->playpath) {
> -            rtmp_close(s);
> -            return AVERROR(ENOMEM);
> +            ret = AVERROR(ENOMEM);
> +            goto fail;
>         }
>
>         if (!strchr(fname, ':') &&
> @@ -960,13 +1066,17 @@ static int rtmp_open(URLContext *s, const char *uri, int flags)
>
>     av_log(s, AV_LOG_DEBUG, "Proto = %s, path = %s, app = %s, fname = %s\n",
>            proto, path, rt->app, rt->playpath);
> -    gen_connect(s, rt);
> +    if (gen_connect(s, rt) < 0) {
> +        ret = AVERROR(ENOMEM);
> +        goto fail;
> +    }

Same here as above (and probably in a few other places as well), passing 
the actual returned value along is better.

>     do {
>         ret = get_packet(s, 1);
>     } while (ret == EAGAIN);
> -    if (ret < 0)
> +    if (ret < 0) {
>         goto fail;
> +    }

Unrelated cosmetic change

>     if (rt->is_input) {
>         // generate FLV header for demuxer
> @@ -987,7 +1097,7 @@ static int rtmp_open(URLContext *s, const char *uri, int flags)
>
> fail:
>     rtmp_close(s);
> -    return AVERROR(EIO);
> +    return ret;
> }
>
> static int rtmp_read(URLContext *s, uint8_t *buf, int size)
> @@ -1024,6 +1134,7 @@ static int rtmp_write(URLContext *s, const uint8_t *buf, int size)
>     int pktsize, pkttype;
>     uint32_t ts;
>     const uint8_t *buf_temp = buf;
> +    int ret;
>
>     do {
>         if (rt->skip_bytes) {
> @@ -1059,7 +1170,11 @@ static int rtmp_write(URLContext *s, const uint8_t *buf, int size)
>             }
>
>             //this can be a big packet, it's better to send it right here
> -            ff_rtmp_packet_create(&rt->out_pkt, RTMP_SOURCE_CHANNEL, pkttype, ts, pktsize);
> +            ret = ff_rtmp_packet_create(&rt->out_pkt, RTMP_SOURCE_CHANNEL,
> +                                        pkttype, ts, pktsize);
> +            if (ret < 0)
> +                return ret;
> +
>             rt->out_pkt.extra = rt->main_channel_id;
>             rt->flv_data = rt->out_pkt.data;
>
> -- 
> 1.7.10.2


// Martin


More information about the libav-devel mailing list