[FFmpeg-devel] [PATCH] RTSP alternate protocol 2-3/3

[Michael Niedermayer]

On Tue, Mar 18, 2008 at 08:53:10AM -0400, Ronald S. Bultje wrote:
> Hi,
> 
> On Sun, Mar 2, 2008 at 10:07 AM, Ronald S. Bultje <rsbultje at gmail.com>
> wrote:
> 
> > On Sun, Mar 2, 2008 at 6:31 AM, Michael Niedermayer <michaelni at gmx.at>
> > wrote:
> >
> > > > > > > Also i suspect this code will leak various things.
> > > > > >
> > > > > > make_setup_request() doesn't allocate resources, be that memory or
> > > FDs.
> > > > > If
> > > > > > there's something specific that I'm missing, please be more
> > > specific.
> > > > > :-).
> > > > >
> > > > > There are 2 url_open() it does not seem its guranteed for them to be
> > > > > closed
> > > > > but then i dont know the code ...
> > > >
> > > >
> > > > That's closed in rtsp_close_stream(), which is called in open() when
> > > > make_setup_request() fails (in the fail: at the bottom).
> > >
> > > make_setup_request() fail looks like this:
> > > fail:
> > >    return err;
> > > }
> >
> >
> > I meant the function calling make_setup_request(), that's
> > rtsp_read_header().
> >
> >
> > > The calling code does not free resources allocated in
> > > make_setup_request() it
> > > also would be wrong to free it there anyway, it just calls
> > > make_setup_request()
> > > again overwriting whatever is already opened.
> >
> >
> > OK, I see what you mean now. I'll free resources in make_setup_request()
> > itself where relevant.
> 
> 
> I added a url_close() to make_setup_request(). New patch attached. I also
> fixed the indenting where appropriate (indent-only changes still in a
> separate patch).
[...]
> @@ -1001,6 +1005,10 @@
>      return 0;
>  
>  fail:
> +    if (i < rt->nb_rtsp_streams && rtsp_st->rtp_handle) {

how can i >= rt->nb_rtsp_stream be here ?


> +        url_close(rtsp_st->rtp_handle);
> +        rtsp_st->rtp_handle = NULL;

also if i understand the code correctly there can be several open which need
closing in case of a failure.

[...]


> Index: ffmpeg/libavformat/rtsp.h
> ===================================================================
> --- ffmpeg.orig/libavformat/rtsp.h	2008-02-11 08:42:13.000000000 -0500
> +++ ffmpeg/libavformat/rtsp.h	2008-02-14 08:52:54.000000000 -0500
> @@ -29,6 +29,10 @@
>      RTSP_PROTOCOL_RTP_UDP = 0,
>      RTSP_PROTOCOL_RTP_TCP = 1,
>      RTSP_PROTOCOL_RTP_UDP_MULTICAST = 2,
> +    /**
> +     * This is not part of public API and shouldn't be used outside of ffmpeg.
> +     */
> +    RTSP_PROTOCOL_RTP_LAST
>  };

this hunk is ok, feel free to commit it

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I wish the Xiph folks would stop pretending they've got something they
do not.  Somehow I fear this will remain a wish. -- M?ns Rullg?rd
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080319/2d0bb588/attachment.pgp>