[FFmpeg-devel] [PATCH] Realmedia / RTSP (RDT)

[Ronald S. Bultje]

Hi Michael,

2008/1/6 Michael Niedermayer <michaelni at gmx.at>:

Sorry for the delay. :). Let's get going again. Let me know if you
want a new thread.

> On Tue, Jan 01, 2008 at 03:34:44PM -0500, Ronald S. Bultje wrote:
>> @@ -908,11 +921,39 @@
>>      rt->rtsp_hd = rtsp_hd;
>>      rt->seq = 0;
>>
>> +    /* request options supported by the server */
>> +    snprintf(cmd, sizeof(cmd),
>> +             "OPTIONS %s RTSP/1.0\r\n"
>> +             "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",
>> +             s->filename);
>> +    rtsp_send_cmd(s, cmd, reply, NULL);
>> +    if (reply->status_code != RTSP_STATUS_OK) {
>> +        err = AVERROR_INVALIDDATA;
>> +        goto fail;
>> +    }
>
> Does this uncoditionally send this stuff for normal RTSP?

Yes, it was unconditional.

> I really think we
> should attempt a clean RTSP/RTP connection first and only if that fails try
> realcrap.

OK, I added a loop so it first tries regular OPTIONS and then
something Real-specific. All other code is under if
(is_real_datatype).

>> +    strcpy(challenge, reply->challenge);
>> +    is_real_datatype = !!challenge[0];
>> +
>>      /* describe the stream */
>>      snprintf(cmd, sizeof(cmd),
>>               "DESCRIBE %s RTSP/1.0\r\n"
>> +             "%s"
>>               "Accept: application/sdp\r\n",
>> -             s->filename);
>> +             s->filename,
>> +             !is_real_datatype ? "" :
>> +             "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");
>
> no, use
> if(is_real_datatype)
>    av_strlcat()

Fixed.

>> +        while (snum-- > 0) get_be16(pb);
>
> url_fskip()
> same for the other such cases

Fixed.

>> +    if (rm->audio_pkt_cnt == 0) {
>> +        int pos;
>> +
>> +        url_open_buf (&pb, buf, len, URL_RDONLY);
>> +        flags = (flags & PKT_FLAG_KEY) ? 2 : 0;
>> +        rdt->rmctx->pb = pb;
>> +        res = ff_rm_parse_packet (rdt->rmctx, s->st, len, pkt,
>> +                                  &seq, &flags, timestamp);
>> +        pos = url_ftell(pb);
>> +        url_close_buf (pb);
>> +        if (res < 0)
>> +            return res;
>> +        if (rm->audio_pkt_cnt > 0 &&
>> +            s->st->codec->codec_id == CODEC_ID_AAC) {
>
>> +            memcpy (rdt->buffer, buf + pos, len - pos);
>
> are you sure that this cant overflow the buffer?

Yes. The input comes from rtp_parse_packet() and rtsp_parse_packet(),
and the max size of the input buffer (buf/len) there is the same as
rdt->buffer.

>> +            url_open_buf (&pb, rdt->buffer, len - pos, URL_RDONLY);
>> +            rdt->rmctx->pb = pb;
>> +        }
>> +    } else {
>> +        ff_rm_retrieve_cache (rdt->rmctx, s->st, pkt);
>> +        if (rm->audio_pkt_cnt == 0 &&
>> +            s->st->codec->codec_id == CODEC_ID_AAC)
>> +            url_close_buf (pb);
>> +    }
>> +
>
>> +    return (rm->audio_pkt_cnt > 0) ? 1 : 0;
>
> the ? 1 : 0 is redundant

Fixed.

>> +static void
>> +rdt_parse_b64buf (unsigned char **target, unsigned int *target_len, const char *p)
>> +{
>> +    int len = strlen(p);
>> +    if (*p == '\"') {
>> +        p++;
>> +        len -= 2; /* skip embracing " at start/end */
>> +    }
>> +    *target_len = len * 3 / 4;
>> +    *target = av_mallocz(*target_len + FF_INPUT_BUFFER_PADDING_SIZE);
>> +    av_base64_decode(*target, p, *target_len);
>> +}
>
> instead of returning void returning target seems more logic

Changed.

>> +static void
>> +rdt_free_extradata (void *d)
>> +{
>> +    rdt_data *rdt = d;
>> +
>> +    // FIXME: close rm ctx
>> +    if (rdt->rmctx) av_free (rdt->rmctx->priv_data);
>
> yes, please close it

OK, so I tried it slightly differently, hope this method's OK.

New patch attached. I still can only play AAC, haven't really debugged
other formats yet, maybe I should start doing that...

Ronald
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: rtsp-realmedia.patch
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080628/4b0cd31d/attachment.txt>