[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