[08/12] rtpdec: Free depacketizers if the init function failed

Message ID 1424795718-79013-8-git-send-email-martin@martin.st
State Committed
Commit 078d43e23a7a3d64aafee8a58b380d3e139b3020
Headers show

Commit Message

Martin Storsjö Feb. 24, 2015, 4:35 p.m.
This is different from how it is handled in codecs/demuxers/muxers
though (where the close function isn't called if the open function
failed), but since the number of depacketizers that have an .init
function is quite limited, this is easy to change.

The main point is that if the init function failed, we shouldn't
try to use that depacketizer at all - this makes sure that the
parse function doesn't need to check for the things that were
initialized in the init function.
---
 libavformat/rdt.c           |  3 ---
 libavformat/rtpdec.h        |  2 +-
 libavformat/rtpdec_mpegts.c |  3 ---
 libavformat/rtsp.c          | 30 ++++++++++++++++++++++--------
 4 files changed, 23 insertions(+), 15 deletions(-)

Comments

Luca Barbato Feb. 24, 2015, 6:55 p.m. | #1
On 24/02/15 17:35, Martin Storsjö wrote:
> This is different from how it is handled in codecs/demuxers/muxers
> though (where the close function isn't called if the open function
> failed), but since the number of depacketizers that have an .init
> function is quite limited, this is easy to change.
> 
> The main point is that if the init function failed, we shouldn't
> try to use that depacketizer at all - this makes sure that the
> parse function doesn't need to check for the things that were
> initialized in the init function.
> ---
>  libavformat/rdt.c           |  3 ---
>  libavformat/rtpdec.h        |  2 +-
>  libavformat/rtpdec_mpegts.c |  3 ---
>  libavformat/rtsp.c          | 30 ++++++++++++++++++++++--------
>  4 files changed, 23 insertions(+), 15 deletions(-)
> 

Ok, we probably should make the same for the codec/demuxer/muxer/etc.

lu
Martin Storsjö Feb. 24, 2015, 9:02 p.m. | #2
On Tue, 24 Feb 2015, Luca Barbato wrote:

> On 24/02/15 17:35, Martin Storsjö wrote:
>> This is different from how it is handled in codecs/demuxers/muxers
>> though (where the close function isn't called if the open function
>> failed), but since the number of depacketizers that have an .init
>> function is quite limited, this is easy to change.
>>
>> The main point is that if the init function failed, we shouldn't
>> try to use that depacketizer at all - this makes sure that the
>> parse function doesn't need to check for the things that were
>> initialized in the init function.
>> ---
>>  libavformat/rdt.c           |  3 ---
>>  libavformat/rtpdec.h        |  2 +-
>>  libavformat/rtpdec_mpegts.c |  3 ---
>>  libavformat/rtsp.c          | 30 ++++++++++++++++++++++--------
>>  4 files changed, 23 insertions(+), 15 deletions(-)
>>
>
> Ok, we probably should make the same for the codec/demuxer/muxer/etc.

Yes, possibly, but that's much harder to do (here it's only a handful of 
depacketizers). The main point here wasn't so much that we call free or 
not, but just to actually check the return value of init at all, which we 
didn't do before.

// Martin

Patch

diff --git a/libavformat/rdt.c b/libavformat/rdt.c
index 2574496..96b9eeb 100644
--- a/libavformat/rdt.c
+++ b/libavformat/rdt.c
@@ -298,9 +298,6 @@  rdt_parse_packet (AVFormatContext *ctx, PayloadContext *rdt, AVStream *st,
     int seq = 1, res;
     AVIOContext pb;
 
-    if (!rdt->rmctx)
-        return AVERROR(EINVAL);
-
     if (rdt->audio_pkt_cnt == 0) {
         int pos;
 
diff --git a/libavformat/rtpdec.h b/libavformat/rtpdec.h
index 8992c38..6087b7e 100644
--- a/libavformat/rtpdec.h
+++ b/libavformat/rtpdec.h
@@ -129,7 +129,7 @@  struct RTPDynamicProtocolHandler {
                             PayloadContext *priv_data, const char *line);
     /** Free any data needed by the rtp parsing for this dynamic data.
       * Don't free the protocol_data pointer itself, that is freed by the
-      * caller. */
+      * caller. This is called even if the init method failed. */
     void (*free)(PayloadContext *protocol_data);
     /** Parse handler for this dynamic packet */
     DynamicPayloadPacketHandlerProc parse_packet;
diff --git a/libavformat/rtpdec_mpegts.c b/libavformat/rtpdec_mpegts.c
index 72e11a8..eb88873 100644
--- a/libavformat/rtpdec_mpegts.c
+++ b/libavformat/rtpdec_mpegts.c
@@ -60,9 +60,6 @@  static int mpegts_handle_packet(AVFormatContext *ctx, PayloadContext *data,
     // different ranges.
     *timestamp = RTP_NOTS_VALUE;
 
-    if (!data->ts)
-        return AVERROR(EINVAL);
-
     if (!buf) {
         if (data->read_buf_index >= data->read_buf_size)
             return AVERROR(EAGAIN);
diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
index 50cefb0..49a89df 100644
--- a/libavformat/rtsp.c
+++ b/libavformat/rtsp.c
@@ -193,6 +193,25 @@  static void init_rtp_handler(RTPDynamicProtocolHandler *handler,
     }
 }
 
+static void finalize_rtp_handler_init(AVFormatContext *s, RTSPStream *rtsp_st,
+                                      AVStream *st)
+{
+    if (rtsp_st->dynamic_handler && rtsp_st->dynamic_handler->init) {
+        int ret = rtsp_st->dynamic_handler->init(s, st ? st->index : -1,
+                                                 rtsp_st->dynamic_protocol_context);
+        if (ret < 0) {
+            if (rtsp_st->dynamic_protocol_context) {
+                if (rtsp_st->dynamic_handler->free)
+                    rtsp_st->dynamic_handler->free(
+                        rtsp_st->dynamic_protocol_context);
+                av_free(rtsp_st->dynamic_protocol_context);
+            }
+            rtsp_st->dynamic_protocol_context = NULL;
+            rtsp_st->dynamic_handler = NULL;
+        }
+    }
+}
+
 /* parse the rtpmap description: <codec_name>/<clock_rate>[/<other params>] */
 static int sdp_parse_rtpmap(AVFormatContext *s,
                             AVStream *st, RTSPStream *rtsp_st,
@@ -261,9 +280,7 @@  static int sdp_parse_rtpmap(AVFormatContext *s,
     default:
         break;
     }
-    if (rtsp_st->dynamic_handler && rtsp_st->dynamic_handler->init)
-        rtsp_st->dynamic_handler->init(s, st->index,
-                                       rtsp_st->dynamic_protocol_context);
+    finalize_rtp_handler_init(s, rtsp_st, st);
     return 0;
 }
 
@@ -444,8 +461,7 @@  static void sdp_parse_line(AVFormatContext *s, SDPParseState *s1,
                 handler = ff_rtp_handler_find_by_id(
                               rtsp_st->sdp_payload_type, AVMEDIA_TYPE_DATA);
                 init_rtp_handler(handler, rtsp_st, NULL);
-                if (handler && handler->init)
-                    handler->init(s, -1, rtsp_st->dynamic_protocol_context);
+                finalize_rtp_handler_init(s, rtsp_st, NULL);
             }
         } else if (rt->server_type == RTSP_SERVER_WMS &&
                    codec_type == AVMEDIA_TYPE_DATA) {
@@ -469,9 +485,7 @@  static void sdp_parse_line(AVFormatContext *s, SDPParseState *s1,
                 handler = ff_rtp_handler_find_by_id(
                               rtsp_st->sdp_payload_type, st->codec->codec_type);
                 init_rtp_handler(handler, rtsp_st, st);
-                if (handler && handler->init)
-                    handler->init(s, st->index,
-                                  rtsp_st->dynamic_protocol_context);
+                finalize_rtp_handler_init(s, rtsp_st, st);
             }
             if (rt->default_lang[0])
                 av_dict_set(&st->metadata, "language", rt->default_lang, 0);