[FFmpeg-devel] [PATCH] RTSP muxer, round 2

Martin Storsjö martin
Wed Jan 13 08:48:39 CET 2010


On Wed, 13 Jan 2010, Luca Barbato wrote:

> On 01/11/2010 11:32 PM, Martin Storsj? wrote:
> > Hi,
> > 
> > Here's the second round of the RTSP muxer. Now it uses chained muxers, 
> > so no changes to the RTP muxer was needed, and the number of patches is 
> > much smaller.
> 
> Looks nice indeed

Thanks!

> > I've been testing and discussing this with Luca A. a bit lately, and he's 
> > generally quite ok with it.
> 
> Good =)
> 
> > This still uses the is_output flag that Ronald opposed, though. The ones 
> > in rtsp_open could be avoided by splitting that into two separate 
> > functions, sharing the common code (general setup, tcp connection opening 
> > and the make_setup_request loop) by splitting it out to separate 
> > functions. The is_output flag usage in make_setup_request could perhaps be 
> > avoided by making the mode value as a parameter to the function.
> > 
> > That leaves setting up and cleaning up the transport_priv member. Since 
> > everything in rtsp_parse_transport (that sets the transport and 
> > lower_transport parameters) is equal for both RTSP/RTP output and input, I 
> > don't feel that a e.g. RTP_OUT and RTP_IN value for transport (that was 
> > suggested) would be suitable. So therefore I still feel that the current 
> > is_output flag is the best solution to this issue.
> 
> why you think is not suitable exactly?

If we would split the transport parameter into e.g. RTP_IN and RTP_OUT, we 
would need to know whether we should set RTP_IN or RTP_OUT in 
rtsp_parse_transport, since the transport lines of rtsp look the same 
regardless of if it's output or input, so we'd still need to have the 
is_output flag there. So I don't see that as an improvment compared to the 
current approach.

> Something that I picked up while reading one time the patchset:
> 
> In 0009 you use
> 
> +        sdp = av_mallocz(8192);
> 
> It could be a problem with streams with large extradata (e.g
> vorbis/theora), not a blocker but it will have to be taken care later
> (like when we'll got an rtp muxer that need more than 8k)

Good point; we'll have to remember that when RTP muxers for these formats 
appear. :-)

FYI, the sdp buffer in ffmpeg.c is only 2048 bytes large, so we'd hit the 
same problem there, too.

> In 0010:
> 
> +    CODEC_ID_PCM_MULAW,
> 
> Why that?

I copied the default codec parameters from the rtp muxer, which had these 
both. Any other suggestions for defaults? CODEC_ID_NONE for both audio and 
video probably also is an option, or e.g. mpeg4 and aac, which work well 
with the players I've been testing with.

> Beside that and the is_output question the whole set looks fine.

Great, thanks!

// Martin



More information about the ffmpeg-devel mailing list