[FFmpeg-devel] [RFC] Separating the RTSP muxer/demuxer and SDP demuxer

[Martin Storsjö]

On Fri, 8 Oct 2010, Josh Allmann wrote:

> On 7 October 2010 23:32, Martin Storsj? <martin at martin.st> wrote:
> > On Thu, 7 Oct 2010, Josh Allmann wrote:
> >
> >> On 7 October 2010 03:01, Martin Storsj? <martin at martin.st> wrote:
> >>
> >> > The attached patch splits the current code from rtsp.c and rtspenc.c into
> >> > the following files:
> >> >
> >> > [...]
> >> > rtprecv.c ? ?- functions for receiving RTP packets, shared by the RTSP and
> >> > ? ? ? ? ? ? ? SDP demuxers
> >>
> >> Also, I think rtsp_fetch_packet could be factored into the RTP layer
> >> but that is beyond the scope of this patch. That would also cleanly
> >> separate the SDP common code from this file.
> >
> 
> This is bikeshed territory, but it just feels weird having the SDP
> stuff in rtprecv.c. Maybe put that in the SDP demuxer? I don't think
> there will ever be a case where one disables SDP demuxing but enable
> RTSP. Or otherwise, ifdef it? Of course, that orphans
> rtsp_fetch_packet in the file... but as I said, I think we should put
> that in RTP.
> 
> > The problem is that everything in the RTP layer currently only works with
> > one single RTP stream at a time. The code in rtprecv.c takes care of
> > receiving multiple streams at once and reading from the correct once. So
> > in that sense, the code doesn't really fit anywhere in the current RTP
> > layer. That's the same for the RTCP handling in the RTSP code currently,
> > too - it doesn't belong in the code for handling one RTP stream since it
> > coordinates multiple of them.
> >
> >
> > Given that, that's perhaps what should be refactored out, a layer for
> > receiving and coordinating multiple RTP streams at once, which is
> > currently mixed into the RTSP and SDP demuxers. This is also why I think
> > the rtprecv name isn't really that bad.
> >
> 
> Yeah, I was thinking of an intermediate layer for multiple streams,
> too. That also follows the spirit of RFC 3550 more closely, which
> makes provisions for multiple RTP streams. For example, RTCP is
> supposed to act on all streams in the session.
> 
> rtsp_fetch_packet is the only code shared between SDP and RTSP. Even
> then, it has more to do with RTP packet fetching than anything
> intrinsically in RTSP (or SDP for that matter). Extricating
> rtsp_fetch_packet from both would make splitting the SDP stuff into
> its own file much more straightforward.

I think our disagreement stems from this misunderstanding. 
rtsp_fetch_packet isn't the only code shared between the two. The RTSP 
demuxer also uses the SDP parsing - you get the SDP description of the 
stream with the DESCRIBE RTSP method, and then parse it and set up the 
streams according to that.

(Otherwise, I wouldn't have put the code in this file unless it actually 
was used by both, or if I made a mistake. ;P)

Does this change your opinion on how it should be organized?

// Martin