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

[Martin Storsjö]

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.

The rtpdec code has dependencies on some other demuxers, that are chained 
for decoding some formats. More precisely, the asf, rm, mpegts and mov 
demuxers are used.

This in practice means that the asf, rm, mpegts and mov demuxers are 
enabled even if I only wanted to use the RTSP muxer, not needing any of 
those demuxers.

I tried doing the split, and the attached diff isn't something that could 
be used as such, but it's an initial proof of concept.

Since the common code paths calls out to the use case specific code, 
splitting of the code is a bit trickier and needs function prototypes for 
the use case specific parts, and needs to enclose the callers within small 
#ifdefs within the functions, like this for example:

-    if (s->iformat)
+    if (s->iformat) {
+#if CONFIG_RTSP_DEMUXER
         err = rtsp_setup_input_streams(s, reply);
-    else
+#endif
+    } else {
+#if CONFIG_RTSP_MUXER
         err = rtsp_setup_output_streams(s, host);
+#endif
+    }

This is in the common RTSP connection code, while the setup_*_streams 
functions are in either the demuxer or in the muxer. We already have some 
similar code in rtsp_fetch_packet, like this one:

    switch(rt->lower_transport) {
    default:
#if CONFIG_RTSP_DEMUXER
    case RTSP_LOWER_TRANSPORT_TCP:
        len = tcp_read_packet(s, &rtsp_st, rt->recvbuf, RECVBUF_SIZE);
        break;
#endif
    case RTSP_LOWER_TRANSPORT_UDP:

The attached patch splits the current code from rtsp.c and rtspenc.c into
the following files:

rtsp.c       - handling of RTSP protocol things, shared by the RTSP muxer 
               and demuxer
rtspenc.c    - RTSP muxer only
rtspcommon.c - common code shared by all three components, such as opening 
               of the transport contexts, cleanup of transport contexts, 
               common parsing helpers such as get_word(), get_word_sep(), 
               get_word_until_chars()
rtprecv.c    - functions for receiving RTP packets, shared by the RTSP and 
               SDP demuxers
rtspdec.c    - RTSP demuxer only
sdpdec.c     - SDP demuxer only

(This is written on top of the patches from the SAP muxer, where common 
code for setting up a chained RTP muxer is split into rtpenc_chain.c.)

So, the files built for each of the three components would be:

RTSP muxer: rtsp.c, rtspcommon.c, rtspenc.c
RTSP demuxer: rtsp.c, rtspcommon.c, rtprecv.c, rtspdec.c
SDP demuxer: rtspcommon.c, rtprecv.c, sdpdec.o

And the dependencies pulling in the rest of the rtpdec* code would be in
rtprecv, not used by the RTSP muxer.

Then there's of course the question of how to name all the files. Perhaps 
it would be better to rename rtsp.c to rtspproto.c, and rtspcommon.c to 
rtsp.c.

Except for splitting the code into separate files, all non-static 
functions would need ff_ prefixes.

With all this in place, if I do e.g. --disable-everything 
--enable-demuxer=rtsp, I get these components enabled:
demuxers: asf, mov, mpegts, rm, rtsp
protocols: http, tcp, rtp, udp

for --disable-everything --enable-demuxer=sdp:
demuxers: asf, mov, mpegts, rm, sdp
protocols: rtp, udp

for --disable-everything --enable-muxer=rtsp:
muxers: rtsp, rtp
demuxers: none
protocols: http, tcp, rtp, udp


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?

// Martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: rtsp-split.patch
Type: text/x-diff
Size: 102104 bytes
Desc: 
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20101007/5f59a91b/attachment.patch>