[FFmpeg-devel] Realmedia patch

[Ronald S. Bultje]

Hi Luca,

On Sat, Aug 30, 2008 at 6:30 AM, Luca Abeni <lucabe72 at email.it> wrote:
> Ronald S. Bultje wrote:
> [...]
>> @@ -918,7 +920,9 @@
>>              if (transport[0] != '\0')
>>                  av_strlcat(transport, ",", sizeof(transport));
>>              snprintf(transport + strlen(transport), sizeof(transport) - strlen(transport) - 1,
>> -                     "RTP/AVP/UDP;unicast;client_port=%d-%d",
>> +                     "%s;unicast;client_port=%d-%d",
>
> Is "client_port=%d-%d" really needed here, or would "client_port=%d"
> be enough? (RDT does not seem to have an RTCP-like companion protocol)

UDP has one port per "m" stream (i.e. in the case of audio+video, they
stream over a separate UDP session with separate port). So I think it
is needed. (This is of course in the theoretical case that Realmedia
RDT supports UDP - you and I both have seen only TCP-supporting
servers...)

>
>
>> +                     rt->server_type == RTSP_SERVER_RDT ?
>> +                         "x-pn/tng/udp" : "RTP/AVP/UDP",
>
> I am not sure if the "rt->server_type == RTSP_SERVER_RDT ? ..."
> construct is a good idea here. It seems to assume that if
> server_type != RTSP_SERVER_RDT, then the transport is always
> RTP. I'd say that a more explicit "if()" or a "switch" can be
> a better idea. But if the maintainer is ok with this, I will
> not object.
> (this same comment applies to all the "... ? ... : ..."
> contructs introduced in this patch.

I did a if() case on the top, removing all the (admittedly ugly)
..==..?..:.. constructs in the buildup loop.

>> Index: ffmpeg-svn/libavformat/rdt.c
>> ===================================================================
>> --- /dev/null 1970-01-01 00:00:00.000000000 +0000
>> +++ ffmpeg-svn/libavformat/rdt.c      2008-08-29 21:25:13.000000000 -0400
>> @@ -0,0 +1,71 @@
>> +/*
>> + * RTP RDT (Realmedia) protocol.
>> + * Copyright (c) 2007 Ronald S. Bultje
> [...]
>> +
>> +/**
>> + * @file rtp_rm.c
>
> rdt.c
>
>
>> + * @brief RDT (Realmedia) protocol support
>> + * @author Ronald S. Bultje <rbultje at ronald.bitfreak.net>
>> + */
>> +
>> +#include "avformat.h"
>> +#include "libavutil/avstring.h"
>> +#include "rtp_internal.h"
>
> This should probably be rdt.h

All fixed.

New patch attached.

Ronald
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: rtsp-setup.patch
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080830/8fcf34f6/attachment.txt>