[FFmpeg-devel] Realmedia patch

[Ronald S. Bultje]

Hi Luca,

On Mon, Aug 25, 2008 at 3:23 AM, Luca Abeni <lucabe72 at email.it> wrote:
> 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).
[..]
> @@ -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).

So, I'm sort of in doubt here. I can go two ways for this sort of stuff.

1: simply add the enum
2: add the enum plus a set of vfuncs (similar to the
RTPDynamicProtocol function pointers) in the style of
modify_setup_request(), modify_options_request(), parse_rtsp_reply()
etc. In that case, I could move all RDT-specific tags in the RTSP
session over to rdt.c. However, it'll probably require a lot more
code, and in the end the whole design may be wrong once I start
looking at MS RTSP.

Which one would you prefer? I suppose if I go for (1), which is
easier, I could always implement (2) once I go into MS RTSP and other
proprietary modifications. Of course, if you have something else in
mind that's fine also, please let me know so I have ideas on what
you're looking for before I start coding useless noise.

Thanks for review,
Ronald