[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