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

[Martin Storsjö]

On Thu, 7 Oct 2010, Aurelien Jacobs wrote:

> On Thu, Oct 07, 2010 at 01:01:16PM +0300, Martin Storsj? 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:
> > 
> > [...]
> > 
> > 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
> > +    }
> 
> Would be cleaner like this:
> 
>     if (CONFIG_RTSP_DEMUXER && s->iformat)
>         err = rtsp_setup_input_streams(s, reply);
>     else if (CONFIG_RTSP_MUXER)
>         err = rtsp_setup_output_streams(s, host);

Ah, good point, we rely on that on many other places, too. I'll change it 
to use that.

> > 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 looks like a good plan. I like it.
> 
> > Then there's of course the question of how to name all the files.
> 
> I have no strong feeling about the naming. Your proposition is not bad.
> Maybe you should find a better name for rtspcommon. Maybe a name which
> don't contain "rtsp" ?

I changed them locally to rtsp.c (containing stuff handling the RTSPState 
struct which all of them use) and rtspproto.c (which is all the code that 
talks to a RTSP server).

// Martin