[FFmpeg-devel] Realmedia patch

[Luca Abeni]

Hi Ronald,

Ronald S. Bultje wrote:
[...]
>> (anyway, I think that the usage of the "OPTIONS" command to identify the
>> server can be committed independently from the rest of the patch)
> 
> Attached is a patch that implements just OPTIONS. I'll send a few more
> that implement RDT-compatible DESCRIBE and SETUP.

Index: ffmpeg-svn/libavformat/rtsp.c
===================================================================
--- ffmpeg-svn.orig/libavformat/rtsp.c	2008-08-23 14:26:05.000000000 -0400
+++ ffmpeg-svn/libavformat/rtsp.c	2008-08-24 12:41:47.000000000 -0400
@@ -710,6 +710,9 @@
          reply->seq = strtol(p, NULL, 10);
      } else if (av_stristart(p, "Range:", &p)) {
          rtsp_parse_range_npt(p, &reply->range_start, &reply->range_end);
+    } else if (av_stristart(p, "RealChallenge1:", &p)) {
+        skip_spaces(&p);
+        av_strlcpy(reply->challenge, p, sizeof(reply->challenge));

In an ideal world, I think that some kind of "generic attribute" mechanism
should be implemented, instead of adding real-specific fields (such as
"challenge") to RTSPHeader (think about someone who might want to add
support for M$ RTSP, or for some other proprietary RTSP variant).

But if the maintainer is ok with this, I do not object.


@@ -1035,7 +1038,8 @@
      int port, ret, err;
      RTSPHeader reply1, *reply = &reply1;
      unsigned char *content = NULL;
-    int protocol_mask = 0;
+    int protocol_mask = 0, is_real_datatype;

Again, here I'd like to have something more generic. Instead
of a "is_real_datatype" variable, we can have some kind of
"rtsp_flavour" enum (feel free to find a better name), which
can currently assume only the "STANDARD_RTSP" and "REAL_RTSP"
values (but someone can extend it in the future).


+    /* request options supported by the server */

I'd add another comment saying that this is done to identify
the server type.

+    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,
+                       "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",
+                       sizeof(cmd));

If we cannot understand how these values are generated and what they
mean, I think we need a comment saying that they are needed for
having valid responses from a real server, and saying where these
values come from (a quick google search seems to indicate that they
are from xine, but I do not know... Just write where you copied them
from).


+        rtsp_send_cmd(s, cmd, reply, NULL);
+        if (!is_real_datatype && reply->challenge[0]) {
+            continue;
+        } else if (reply->status_code != RTSP_STATUS_OK) {
+            err = AVERROR_INVALIDDATA;
+            goto fail;

I'd move this check before the previous one (first, check for
errors, then try to parse the data)



				Luca