[FFmpeg-devel] Realmedia patch

[Luca Abeni]

Hi Ronald,

Ronald S. Bultje wrote:
[...]
> All separate patches have been committed already, basically what I
> have left is two patches, my rtsp-realmedia.patch and a
> rtsp-streamselection.patch.

Ok, thanks. I think the first patch looks less controversial than the 
other one ;-)
So, let's start from it...

> Index: ffmpeg/libavformat/rtsp.c
> ===================================================================
> --- ffmpeg.orig/libavformat/rtsp.c	2008-07-03 08:46:52.000000000 -0400
> +++ ffmpeg/libavformat/rtsp.c	2008-07-03 08:48:34.000000000 -0400
> @@ -27,6 +27,7 @@
>  #include "network.h"
>  #include "rtsp.h"
>  
> +#include "md5.h"
>  #include "rtp_internal.h"
>  
>  //#define DEBUG
> @@ -523,6 +524,18 @@
>              rtsp_parse_range_npt(p, &start, &end);
>              s->start_time= start;
>              s->duration= (end==AV_NOPTS_VALUE)?AV_NOPTS_VALUE:end-start; // AV_NOPTS_VALUE means live broadcast (and can't seek)
> +        } else if (av_strstart(p, "AvgBitRate:integer;", &p)) {
> +            s->streams[s->nb_streams - 1]->codec->bit_rate = atoi(p);
> +        } else if (av_strstart(p, "RMFF 1.0 Flags:", &p) ||
> +                   av_strstart(p, "OpaqueData:", &p) ||
> +                   av_strstart(p, "ASMRuleBook:", &p)) {

I think that we should only use standard tags here (and not the "RMFF", 
"OpaqueData", "ASMRuleBook", ... keywords used by real). Can you just 
call parse_sdp_a_line() in the final "else" case (without checking for 
any particular tag)? Then, you can check for the real tags in the 
rdt_parse_sdp_line() function.

[...]
> +    /* request options supported by the server */
> +    for (is_real_datatype = 0;; is_real_datatype++) {
> +        snprintf(cmd, sizeof(cmd),
> +                 "OPTIONS %s RTSP/1.0\r\n", s->filename);
> +        if (is_real_datatype)
> +            av_strlcat(cmd,
> +                       "User-Agent: RealMedia Player Version 6.0.9.1235 (linux-2.0-libc6-i386-gcc2.95\r\n"
> +                       "ClientChallenge: 9e26d33f2984236010ef6253fb1887f7\r\n"
> +                       "PlayerStarttime: [28/03/2003:22:50:23 00:00]\r\n"
> +                       "CompanyID: KnKV4M4I/B2FjJ1TToLycw==\r\n"
> +                       "GUID: 00000000-0000-0000-0000-000000000000\r\n"
> +                       "RegionData: 0\r\n"
> +                       "ClientID: Linux_2.4_6.0.9.1235_play32_RN01_EN_586\r\n",
> +                       sizeof(cmd));

Is this really needed? I mean: claiming to be RealMedia Player, sending 
a random Starttime, etc...
Are all these tags needed? (what happens if you remove ClientID, 
User-Agent, etc...?)

(anyway, I think that the usage of the "OPTIONS" command to identify the 
server can be committed independently from the rest of the patch)

[...]
> +    if (is_real_datatype)
> +        av_strlcat(cmd,
> +                   "Bandwidth: 250000\r\n"
> +                   "GUID: 00000000-0000-0000-0000-000000000000\r\n"
> +                   "RegionData: 0\r\n"
> +                   "ClientID: Linux_2.4_6.0.9.1235_play32_RN01_EN_586\r\n"
> +                   "SupportsMaximumASMBandwidth: 1\r\n"
> +                   "Language: en-US\r\n"
> +                   "Require: com.real.retain-entity-for-setup\r\n",
> +                   sizeof(cmd));

Again, it would be better to avoid sending fake information.


> Index: ffmpeg/libavformat/Makefile.am
> ===================================================================
> --- ffmpeg.orig/libavformat/Makefile.am	2008-07-03 08:48:30.000000000 -0400
> +++ ffmpeg/libavformat/Makefile.am	2008-07-03 08:48:34.000000000 -0400
> @@ -53,6 +53,7 @@
>  	rtp_aac.c \
>  	rtp_h264.c \
>  	rtp_mpv.c \
> +	rtp_rm.c \

This looks like a leftover from a different patch



I also have another question: it seems to me that in this patch 
(rtsp-realmedia.patch) you are not explicitly creating new AVStreams 
with av_new_stream. So, only one AVStream is created, when the "m=" SDP 
line is parsed. But an RM stream can contain more than one stream (for 
example, an audio stream and a video stream), right? Where are the 
additional AVStreams created? I am wondering if it would simpler to use 
an approach similar to the one used for parsing TS in RTP (not creating 
and AVStream in rtsp.c, but leave the streams creation to the RM demuxer)?


				Luca