[FFmpeg-devel] [PATCH] ARMovie/RPL demuxer rev4

[Michael Niedermayer]

On Fri, Mar 28, 2008 at 05:30:13PM -0700, Eli Friedman wrote:
> On Fri, Mar 28, 2008 at 4:38 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> > On Fri, Mar 28, 2008 at 04:13:02PM -0700, Eli Friedman wrote:
> >  > Per subject, another revision of the ARMovie/RPL demuxer.  Addresses
> >  > review comments.
> >
> >  Not all, you are still duplicating the
> >  read_line();
> >  read_int();
> >  all over the place instead of reading ints directly from ByteIOContext.
> 
> Mmm... I've been keeping it that way because the spec allows for more
> complicated lines like specifying the audio bits per samples as "8
> linear unsigned" vs. "8 signed", or the width as "120 width [4:3]" to
> specify an aspect ratio.  None of the RPL samples actually seem to use
> this, though.

hmm, ok but its still code duplication, what about a
read_line_and_int(){
    read_line()
    read_int()
}


> 
> >  > +        if (offset >= 0xFFFFFFFFUL || video_size >= 0xFFFFFFFFUL ||
> >  > +            audio_size >= 0xFFFFFFFFUL)
> >  > +            error |= -1;
> >
> >  What exactly is this check good for?
> 
> Overflow, so it doesn't silently accept 10000000000000 as an offset.

What is the problem if it does accept 10000000000000 as an offset?
And why is 0xFFFFFFFEUL different?

[...] 

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I have often repented speaking, but never of holding my tongue.
-- Xenocrates
-------------- 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/20080329/cad2ebb1/attachment.pgp>