[FFmpeg-devel] rtsp.c: a=rtpmap: parsing implementation?

[Ronald S. Bultje]

Hi Luca,

On Mon, Jan 5, 2009 at 8:11 AM, Luca Abeni <lucabe72 at email.it> wrote:
> Ronald S. Bultje wrote:
>> rtsp.c:500 has this code
>>
>>         } else if (av_strstart(p, "rtpmap:", &p)) {
>>             /* NOTE: rtpmap is only supported AFTER the 'm=' tag */
>>             get_word(buf1, sizeof(buf1), &p);
>>             payload_type = atoi(buf1);
>>             for(i = 0; i < s->nb_streams;i++) {
>>                 st = s->streams[i];
>>                 rtsp_st = st->priv_data;
>>                 if (rtsp_st->sdp_payload_type == payload_type) {
>>                     sdp_parse_rtpmap(st->codec, rtsp_st, payload_type, p);
>>                 }
>>             }
>>
>> You'll see the a=rtpmap: line is applied to all RTSPStreams, not just
>> the current one.
>
> I did not look at the full code, but according to the code you pasted
> above the line is not applied to all the streams. It is only applied to
> the stream with the specified payload type (and the for() loop should
> just be used to search for the correct stream).
> And this looks consistent with the RFC.

Aha, so the RFC says this is OK... So here's a hypothetical SDP (or
see [1] for a real one) that would "open twice" with this:

m=audio 0 RTP/AVP 96
a=rtpmap:96 x-asf-pf/1000
m=audio 0 RTP/AVP 96
a=rtpmap:96 x-asf-pf/1000
[.. this can repeat several times ..]

[1] http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/2008-November/056144.html

I suppose according to the RFC, omitting the second rtpmap would be
fine, right? do you think we could come up with a SDP parsing
implementation that supports both without opening the RTP parser
(sdp_parse_rtpmap()) N times for the stream represented by m= line 0?
The patch below (left in the reply for convenience) probably isn't
right to support both, it basically switches from supporting
preferentially "your" SDP to supporting only "my" SDP, which is worse.
I'd rather have a patch that supports both. :-).

> Are you saying that all those different "m=" lines have the same payload
> type?

Yes. Again, see [1] above.

>>         } else if (av_strstart(p, "rtpmap:", &p)) {
>>             /* NOTE: rtpmap is only supported AFTER the 'm=' tag */
>>             get_word(buf1, sizeof(buf1), &p);
>>             payload_type = atoi(buf1);
>>             st = s->streams[s->nb_streams - 1];
>>             rtsp_st = st->priv_data;
>>             if (rtsp_st->sdp_payload_type == payload_type) {
>>                 sdp_parse_rtpmap(st->codec, rtsp_st, payload_type, p);
>>             }
[..]

Ronald