[FFmpeg-devel] [PATCH] Add an RTSP muxer

[Luca Abeni]

Hi Martin,

I finally have some time for having a better look at your patches, and 
try them... I have no DSS installation, so I cannot test the RTSP muxer 
right now (I am installing a DSS in a virtual machine, so I hope to be 
able to test the muxer in a short time).

In the meanwhile, here are some untested comments on the code (I just 
quote the modified code, not the exact patch):
1) You do something like
+                if (rt->is_output) {
+                     AVFormatContext *rtpctx = rtsp_st->transport_priv;
+                     /* Don't call av_write_trailer if av_write_header
+                      * hasn't been called yet. Use ->priv_data to check
+                      * for this. */
+                     if (rtpctx->priv_data)
+                         av_write_trailer(rtpctx);
Maybe this "if" can be avoided? (what happens if we try to call 
av_write_trailer() anyway? I think it will just fail, no? Otherwise, can 
we ensure that transport_priv is not set if av_write_header has not been 
called?)

2) I think the "rtp_pb" field of RTSPStream is unneeded, right?

3) Instead of doing
+            if (!av_new_stream(rtpctx, 0)) {
+                err = AVERROR(ENOMEM);
+                goto fail;
+            }
and
+            /* Remove the local codec, link to the original codec
+             * context instead, to give the rtp muxer access to
+             * codec parameters. */
+            av_free(rtpctx->streams[0]->codec);
+            rtpctx->streams[0]->codec = st->codec;
Cannot you simply do
	rtpctx->streams[0] = st;
	rtpctx->nb_streams = 1;

4) In rtsp_write_packet(), fd_max looks useless

5) In rtsp_write_packet(), before invoking av_write_frame() you call 
av_rescale_q() to convert the timebase. I think this can be avoided by 
setting the timebase of the RTSP's AVStream equal to the timebase of the 
RTP muxer's AVStream. Basically, after invoking av_write_header() for 
the RTP muxer you can properly set the timebase for the corresponding 
RTSP AVStream.

Also, I am wondering if the initialisation of the RTP Muxer's 
AVFormatContext can be done in rtsp_open_transport_ctx() (instead of 
initialising it earlier and only calling av_write_header() in 
rtsp_open_transport_ctx()). Basically, in this way the SDP would be 
created based on the RTSP AVFormatContext (as your initial patch did), 
and the RTP muxers would be initialised only later.
I am not sure if this approach would simplify the code or not...


			Thanks,
				Luca