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

Luca Abeni lucabe72
Mon Jan 5 14:11:01 CET 2009


Hi Ronald,

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.


> RTSP-MS has several m= lines (one for each
> bitrate/codec/etc. choice), and therefore the same rtpmap: is repeated
> for each m= line in the SDP.  This causes the dynamic RTP parser for
> _all_ preceeding m= lines to be re-initialized (see
> sdp_parse_rtpmap()) every time we encounter a rtpmap: line in the SDP.

Are you saying that all those different "m=" lines have the same payload
type? If yes, then I think this is not supported by the current SDP
demuxer...  (I do not know if this is a limitation of the SDP demuxer,
or if some standard states that different "m=" lines should have different
payload types).


> Is there a reason for this or is it OK to change this so rtpmap: lines
> only affect the last m= line's RTPSStream? (Two patches of course, one
> for the removal of the for() loop and the st= line change, and one for
> reindenting the rest).
> 
>         } 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);
>             }

Not sure about this... The change looks innocent enough, but I am not
sure if it breaks something.
Anyway, if you remove the for() loop you implicitly assume that the
"current stream" is the latest one in the streams array
(s->streams[s->nb_streams - 1]); so, I think you can remove the
"if (rtsp_st->sdp_payload_type == payload_type)" check.



				Luca




More information about the ffmpeg-devel mailing list