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

[Aurelien Jacobs]

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);

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

Aurel