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

Josh Allmann joshua.allmann
Fri Oct 8 10:57:36 CEST 2010


Hi Martin,

On 7 October 2010 23:32, Martin Storsj? <martin at martin.st> wrote:
> Hi Josh,
>
> 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.

If others think this is a good idea, I'll volunteer to do the work.

>> > 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?
>

Conditional compilation mostly, but having the SDP stuff in its own
file is nicer, organizationally. I am not a fan of all the smaller
common-code files. Given a choice, I would favor ifdeffery -- it
avoids having to yank so much stuff out at once. Perhaps with ifdefs,
we can yank stuff out more incrementally, especially if we do refactor
to multi-stream RTP.

Josh



More information about the ffmpeg-devel mailing list