[FFmpeg-devel] [PATCH] RDT/Realmedia patches #2

Michael Niedermayer michaelni
Wed Nov 12 13:15:21 CET 2008


On Tue, Nov 11, 2008 at 04:00:42PM -0500, Ronald S. Bultje wrote:
> Hi Michael,
> 
> On Tue, Nov 11, 2008 at 2:56 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> > It doesnt matter which applies so much, what matters is that the variable
> > where and are completely undocumented and i have absolutely no idea what
> > they are without "reverse engeneering" it out of the code.
> 
> OK, this patch adds a comment (and nothing else) on the meaning of
> these variables where we parse them.
> 
> Ronald

> Index: ffmpeg-svn/libavformat/rdt.c
> ===================================================================
> --- ffmpeg-svn.orig/libavformat/rdt.c	2008-11-11 15:30:12.000000000 -0500
> +++ ffmpeg-svn/libavformat/rdt.c	2008-11-11 15:59:36.000000000 -0500
> @@ -184,6 +184,21 @@
>      }
>      if (len < 10)
>          return -1;
> +    /**
> +     * Layout of the header (in bits):

> +     * 1:  Length included flag
> +     * 1:  Need reliable flag

these sound like good variable names, maybe a little abbreviated like
len_included need_reliable. But its really too terse as comment explaining
things, also it may make sense to put the description on the multimedia
wiki ...


> +     * 5:  ID of a set of streams of one media type (e.g. audio of video)

Considering the surrounding comments i suspect it should be

ID of s set of streams of identical content, though possibly different
bitrate, codec, ...



> +     * 1:  Is reliable flag
> +     * 16: sequence number
> +     * 1:  back-to-back flag
> +     * 1:  slow data flag
> +     * 5:  ID of the stream within this particular set. A set can contain
> +     *     multiple streams of different codecs and/or bitrates

> +     * 1:  keyframe flag (unset if packet belongs to keyframe)

id call it non-keyframe flag if 1 indicates non keyframes


> +     * 32: timestamp

DTS? PTS? PCR/SCR?


> +     * 16: ?
> +     */

>      if (sn)  *sn  = (buf[0]>>1) & 0x1f;
>      if (seq) *seq = AV_RB16(buf+1);
>      if (ts)  *ts  = AV_RB32(buf+4);

these names are too short

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Many that live deserve death. And some that die deserve life. Can you give
it to them? Then do not be too eager to deal out death in judgement. For
even the very wise cannot see all ends. -- Gandalf
-------------- 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/20081112/b0504612/attachment.pgp>



More information about the ffmpeg-devel mailing list