[FFmpeg-devel] [RFC] Separating the RTSP muxer/demuxer and SDP demuxer
Fri Oct 8 08:32:26 CEST 2010
On Thu, 7 Oct 2010, Josh Allmann wrote:
> On 7 October 2010 03:01, Martin Storsj? <martin at martin.st> wrote:
> > Hi,
> > Currently, the RTSP demuxer, muxer and the SDP demuxer are quite tightly
> > coupled, forcing the dependencies of the SDP demuxer to be built even if
> > only the RTSP muxer is desired, and vice versa.
> > A quick recap of what the three muxers/demuxers do and their actual
> > dependencies:
> > - The RTSP muxer speaks with a RTSP server, using common code for sending
> > ?and parsing RTSP requests. On setup, it creates chained RTP muxers for
> > ?each stream, adding a dependency on the RTP muxers.
> > - The RTSP demuxer speaks with a RTSP server, using common code for
> > ?sending and parsing RTSP requests. When the SDP description is received,
> > ?it parses it and creates RTP depacketizers for this, using the rtpdec*
> > ?code.
> > - The SDP demuxer doesn't speak with any RTSP server, but is given a SDP
> > ?file directly. It parses this and sets the connections up just like the
> > ?RTSP demuxer, though, and uses the rtpdec* code.
> > All of the three use the same RTSPState struct as priv_data.
Thanks for having a look at this!
> The biggest problem, IMO, is that the dependencies specify a
> topological ordering (rtp > sdp > rtsp), but the code does not respect
> that. Mostly this is because of common/shared code, but things are
> getting leaky (eg, we do some RTCP handling in the RTSP layer). This
> suggests a larger refactoring may be useful soon.
Yes, the common code calling out to specific code makes it hard to enforce
And while some refactorings may be needed, I'm no fan of "large
refactorings" as in tearing up lots of code and rewriting it in some other
way. I do prefer smaller steps of fixing one small thing at a time. This
split is of course hard to do in that way :-)
> > 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
> Somehow I am not keen on the naming, since it is not being used by
> RTP. rtspsdpcommon.c is the best I can think of, but it's wordy.
That's a bit wordy yes, I still think I prefer the current naming
> 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.
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.
> > So, what do you think? Is splitting it this way worthwhile, or does it
> > just separate the code too much, making it harder to follow when the
> > control flow jumps around between all of the files?
> I think this split is a necessary, if woolly, step forward.
Is the actual split into separate files also necessary, or just the fixing
of conditional compilation? I.e., if the same effect is achieved by just
adding #ifdefs to the current rtsp.c, would that be better or worse than
splitting it into many small files?
More information about the ffmpeg-devel