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

[Luca Barbato]

Nicholas T wrote:

>>> +//#define printf please_use_av_log
>>> +//#define fprintf please_use_av_log
>> unacceptable
> No need to say so, it was obviously for debugging purposes.

you may use dprintf or av_log

> 
>>> +// TODO: reduce redundant code
>> yes
> Should I make a .h file for the constants?

could be nice

> 
>> trailing whitespace is forbidden in svn
> do you mean a newline with only spaces afterwards?

"     "
"this     "
and so on ^^

> 
>> constants be they #defined or from a enum should be all UPPERCASE
> done, but they're a little bit less legible
> VIDEOOFFSETDIFFERENCEFRAME_BLOCK vs VideoOffsetDifferenceFrame_Block

what about OFFSET_DIFF or something similar? VIDEO is reduntant, and if
the code appears just on frame processing code or block processing code
the other parts are reduntant too.


> 
>>> for(a = 0; a < 256; a++) { set_palette(a, a, a, a); }
>> this is wrong
> This was also sort of obviously debugging code. The palette is now set by
> the file.
> I didn't want to introduce new features at the risk of introducing new
> bugs,
> and having
> so many bugs I couldn't fix the first ones. This was actually rather
> trivial.

Mark those blocks with

#if 1 //XXX remove me later

#endif

if you are sending an RFC patch once you have a good patch remind to
remove them.

> sorta fixed, I'm ignoring the possibilities of overflow after a line wrap,
> b/c it's impossible with the typically supported frame widths

Beware on such assumption ^^

lu