[Ffmpeg-devel] Re: Bethsoft VID demuxer and decoder

[Nicholas T]

Hi,

   Please answer my questions in the top part of the message - maybe
you didn't see it, or forgot about it after reading inline
comments...thanks.

On 4/4/07, Michael Niedermayer <michaelni at gmx.at> wrote:
> ive been thinking about something like:
nice, though I don't think it would quite be functional. I implemented
any simplifications though.

> remaining= width;
> while(rle = *buf++){
>     int len= rle&0x7F;
>     while(len > remaining){
>         if(rle < 0x80) bytestream_get_buffer(&buf, dst, remaining);
>         else if(intra) memset(dst, *buf, remaining);
you need to memcpy/memset up to the width, i.e. remaining - x, not
just remaining. say you're in the middle of a line, you would copy
into (linesize-width) instead of the correct pixels on the next line,
though dst would be correct because of the two following lines. This
is why there is the complication with current line, and xoffset.

>         len -= remaining;
>         dst += remaining + linesize - width;
dst += linesize - width? remaining = width, so this is the same as +=
linesize, which doesn't make sense.

>         if(dst == picture_end)
>             goto end;
I like my checks, they aren't in sub-loops, and don't require gotos,
despite slightly more arithmetic complexity.

>         remaining= width;
you never set remaining elsewhere.

>     }
>     if(rle < 0x80) bytestream_get_buffer(&buf, dst, len);
>     else if(intra) memset(dst, *buf++, len);
>     dst += len;
> }
> end:

> yes, differencs between timestamps are 16bit this doesnt make the timestamps
> 16bit though
err, what do you want me to set it to? I'm not understanding,
sorry...is the timestamp actually stored in an int16 now?

> the comment is useless, it says nothing which isnt obvious from the
> function name
at the university they make us write comments for every function, even
if the name is obvious, sorry for this junk.

> useless cast
missed 2, now fixed.

> breaks negative linesize
right

> superfluous ()
fixed

> *buf++ is more commonly used, iam fine with buf++[0] too if you prefer
I can go with the standard, no problem. on the other hand,
AV_RB32(&p->buf[0]) is pretty bad though...I accidentally copied it
from segafilm; in fact it's in 12 libavformat files.

> indention doesnt look correct
dunno what happened, kwrite formatting errors...?

> get_le16(pb) is probably a better idea
sure

> this is vidbuf_start[vidbuf_nbytes++] = block_type;
fixed, sorry I was revising (probably not a good idea to have a vidbuf
pointer and then change vidbuf_start) and didn't evaulate what I was
doing...

> see av_fast_realloc()
thanks, done.

> > +#undef BUFFER_INCREMENT
> useless
I change the name, but I guess it's not too common, so probably no undef needed.

Nicholas

-- 
http://ntung.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bethsoft.diff
Type: text/x-patch
Size: 19316 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070404/f68335ac/attachment.bin>