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

[Ronald S. Bultje]

Hi,

On Mon, Jan 4, 2010 at 10:11 AM, Martin Storsj? <martin at martin.st> wrote:
[..]

0001 - inet_ntop()
I'd prefer to drop all usage of it and use getaddrinfo() with the
proper flags instead.

0002 - resolve address
Adds usage of a deprecated function, I'd say I'm against this.

Yes, that sucks, we need to convert rtsp.c to IPv6 (and its API), and
badly so. But we need to do it some time.

0003-5 is for Luca (A).
0006-0007 appear to touch upon a deeper issue, I'd just add the
AVFormatContext (that's not ic, right?) to the struct instead. This is
a bit ugly. You're removing ic in 0014, which I think is a bad idea.
I'd drop these two and convert ic into being the AVFormatContext that
you take pb from and use as a log context. (That's how the RTP/RTSP
demuxer parts work.)
0008-0009, this is the AVFC that I'm talking about one line above, if
you add the whole AVFC then they aren't needed.

0010 - yes
0011 - recipee for disaster if you ask me... (See 0006-0009/0014 comment.)

0017 - hmm? I wonder if some flag in AVFormatContext doesn't already
specify this...
0018/0019 - bad idea, I don't want rtsp.c to become a "if muxer ...
else ...", if this is it then the common code should be split and left
in rtsp.c and the rest in rtspdec/enc.c.

I'm wondering if you can't use the transport property for this.
RDT_IN, RTP_IN or RTP_OUT or so. Then patch 0019 makes more sense. I
left the rest of the patches because they're doomed to get similar
resonses from me. My basic question: is this the right way to add a
RTSP muxer (i.e. using common code and then do if (output) { do one
thing } else { ... })? I'd say no.

Ronald