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

Aurelien Jacobs aurel
Fri Dec 28 20:19:08 CET 2007


On Fri, 28 Dec 2007 12:10:48 -0500
"Ronald S. Bultje" <rsbultje at gmail.com> wrote:

> Hi,
> 
> On Dec 28, 2007 10:59 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
> 
> > I dont understand why you need a goto or while() ?
> >
> > a simple:
> > if(protocol_mask & BLAH_UDP){
> >    try udp
> >    if(succeess)
> >        return 0
> > }
> > if(protocol_mask & BLAH_TCP){
> >    try tcp
> >    if(succeess)
> >        return 0;
> > }
> 
> 
> Most code between udp, udp-multicast and tcp is shared, so this'd lead to
> duplication between try udp and try tcp. The for is already there, I can
> just re-use that.
> 
> I can also drop rtsp_default_mask, as Michael suggested, I actually wanted
> to do that when I thought about it later, but I was already off by then. New
> patch doing that attached, interested in comments on this approach.
> 
> [...]
> 
> Index: ffmpeg/libavformat/rtsp.h
> ===================================================================
> --- ffmpeg.orig/libavformat/rtsp.h	2007-12-27 17:39:59.000000000 -0500
> +++ ffmpeg/libavformat/rtsp.h	2007-12-28 11:57:57.000000000 -0500
> @@ -29,6 +29,7 @@
>      RTSP_PROTOCOL_RTP_UDP = 0,
>      RTSP_PROTOCOL_RTP_TCP = 1,
>      RTSP_PROTOCOL_RTP_UDP_MULTICAST = 2,
> +    RTSP_PROTOCOL_RTP_LAST = 3
>  };

You shouldn't set an explict value to RTP_LAST to make adding
new protocol easier.
Moreover, I guess you don't want it to be part of API/ABI. This
would require bumping major version when adding a new protocol.
So you should probably add a comment such as:
/**< This is not part of public API and shouldn't be used outside of ffmpeg */

> Index: ffmpeg/libavformat/rtsp.c
> ===================================================================
> --- ffmpeg.orig/libavformat/rtsp.c	2007-12-28 12:08:05.000000000 -0500
> +++ ffmpeg/libavformat/rtsp.c	2007-12-28 12:09:21.000000000 -0500
> @@ -78,8 +78,6 @@
>  /* XXX: currently, the only way to change the protocols consists in
>     changing this variable */
>  
> -int rtsp_default_protocols = (1 << RTSP_PROTOCOL_RTP_UDP);

Strictly speeking, this breaks API/ABI so this requires
major version bump.

Aurel




More information about the ffmpeg-devel mailing list