[FFmpeg-devel] [PATCH] rtsp - alternate protocol

[Michael Niedermayer]

On Sat, Jan 05, 2008 at 09:21:52AM -0500, Ronald S. Bultje wrote:
> Hi,
> 
> On Jan 4, 2008 7:14 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> 
> > On Sat, Dec 29, 2007 at 12:41:09PM -0500, Ronald S. Bultje wrote:
> > [...]
> > > Index: ffmpeg/libavformat/rtsp.c
> > > ===================================================================
> > > --- ffmpeg.orig/libavformat/rtsp.c    2007-12-28 13:30:50.000000000-0500
> > > +++ ffmpeg/libavformat/rtsp.c 2007-12-29 12:19:33.000000000 -0500
> > > @@ -842,6 +842,18 @@
> > >      av_free(rt->rtsp_streams);
> > >  }
> > >
> > > +static int
> > > +unset_lowest_protocol(int mask)
> > > +{
> > > +    if (mask & RTSP_PROTOCOL_RTP_UDP)
> > > +        return mask & ~RTSP_PROTOCOL_RTP_UDP;
> > > +    else if (mask & RTSP_PROTOCOL_RTP_TCP)
> > > +        return mask & ~RTSP_PROTOCOL_RTP_TCP;
> > > +    else if (mask & RTSP_PROTOCOL_RTP_TCP)
> > > +        return mask & ~RTSP_PROTOCOL_RTP_UDP_MULTICAST;
> > > +    return 0;
> > > +}
> > > +
> > >  static int rtsp_read_header(AVFormatContext *s,
> > >                              AVFormatParameters *ap)
> > >  {
> > > @@ -883,7 +895,7 @@
> > >      }
> > >
> > >      if (!protocol_mask)
> > > -        protocol_mask = rtsp_default_protocols;
> > > +        protocol_mask = (1<<RTSP_PROTOCOL_RTP_LAST) - 1;
> > >
> > >      /* open the tcp connexion */
> > >      snprintf(tcpname, sizeof(tcpname), "tcp://%s:%d", host, port);
> > > @@ -978,6 +990,12 @@
> > >                   "Transport: %s\r\n",
> > >                   rtsp_st->control_url, transport);
> > >          rtsp_send_cmd(s, cmd, reply, NULL);
> > > +        if (reply->status_code == 461 /* Unsupported protocol */ && i
> > == 0 &&
> > > +            (protocol_mask = unset_lowest_protocol(protocol_mask)) !=
> > 0) {
> > > +            j = RTSP_RTP_PORT_MIN;
> > > +            i--;
> > > +            continue;
> > > +        } else
> >
> > the code still breaks if i reorder the ifs() for TCP and UDP
> > also the way you implement this is ugly
> > you just list a bunch of variables, reset them and run a continue
> > this will just break if any new variable is added
> > you must re execute the original code which sets the variables NOT
> > duplicate it
> >
> > and iam almost certain this code leaks everything from allocated memory to
> > various opened streams
> > the loop looks like it is for opening multiple streams and NOT
> > for iterating over protocols, a continue should only be executed to
> > go to the next stream NEVER to go back to a previous stream after
> > hacking 2 variables and leaving all allocated resources for the streams
> > to leak
> 
> 
> So what's the preferred way to fix this, move code into a separate function
> and add a fail: handler in it to free memory, return an error and re-try
> with another protocol? If you dislike the continue (which I understand and
> agree with), and other dislike the goto (which I would normally prefer),
> then that's the last thing I can think of to do this...

Dont ask me how to do it, i dont know the code though the suggestion of a
seperate function sounds good ... 

maybe something like:

balh(context, protocol){
    if(protocol == TCP){
    }else if(protocol == UDP){
    }

    common code

    return 0;
    fail:
    cleanup
}

while(protocol_mask){
    int protocol= protocol_mask & ~(protocol_mask-1);
    if(balh(context, protocol) >= 0)
        break;
    protocol_mask -= protocol;
}

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Republics decline into democracies and democracies degenerate into
despotisms. -- Aristotle
-------------- 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/20080105/ae67423e/attachment.pgp>