[FFmpeg-devel] [PATCH] RTSP-MS 10/15: ASF header parsing

[Michael Niedermayer]

On Tue, Feb 03, 2009 at 08:05:34AM -0500, Ronald S. Bultje wrote:
> Hi,
> 
> On Tue, Feb 3, 2009 at 7:25 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
> > why is rm/rdt and asf using different things?
> 
> I have an item on my list to refactor RM/RDT to use the
> RTSPState->{rm,fmt?}_ctx also, I just didn't get to that yet. The
> per-stream approach works for RM/RDT but not for ASF because ASF has
> one global header in the SDP, whereas RM/RDT has one header per
> m=-line (per stream).
> 
> Before you ask me to refactor RM/RDT while I'm at it, can we just
> rename / recomment this one s/asf/fmt/ and do the refactoring of
> RM/RDT in a separate patch after ASF/RTP integration is complete?

to be honest i really think the rtp/rtsp code needs MUCH more cleanup
and documentation done before any new additions not less and later /
a promise to do 1 of 100 cleanups afterwards

i simply cannot review your patches as long as rtsp/rtp is in the state
it currently is.
as you seem to make no attempt to resolve this but rather try to convince
me to accept more new code, i will not appove _any_ further additions to
rtp/rtsp until every single field of every structure is documented and
every function is as well. And i mean clear and unambigous documentation
not things like:
Structure listing useful vars to parse RTP packet payload
// moved out of rtp.c, because the h264 decoder needs to know about this structure..
/* only used for MP2T payloads */
// TODO: move into statistics

hell pretty much all existing comments are useless
the comments should say what a variables is not what uses it, i can grep to
know what uses it, and not just where it should be moved to, that is usefull
to know yes but hell we still have no clue what the variable actually means

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Frequently ignored awnser#1 FFmpeg bugs should be sent to our bugtracker. User
questions about the command line tools should be sent to the ffmpeg-user ML.
And questions about how to use libav* should be sent to the libav-user ML.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090203/f99a4da4/attachment.pgp>