[Ffmpeg-devel] [RFC] [PATCH] FLC/FLX/DTA encoder

[Måns Rullgård]

Alex Beregszaszi <alex at fsn.hu> writes:

> Hi,
>
>> > Known:
>> > * current patch has hard tabs and maybe identation issues (but any
>> > cosmetic recommendations are welcomed)
>> 
>> Why don't you do something about that?  You know what the rules are.
>
> I'm removing hard tabs. Btw, there are extreme long lines in
> encode_delta8(). That is a questionable part.

Lines should preferably be less than 80 characters.  There are of
course cases where there is no sane way to split a long line.

>> > Question:
>> > * should it be splitted into flicenc.c ?
>> 
>> Yes.  How different are FLC, FLX and DTA?  Perhaps even they should be
>> separated.  If they share enough code (I haven't looked closely) to
>> warrant being in the same file, they should still have #ifdefs to
>> allow individual selection.
>
> Actually most of the chunks are the same, only some extra chunks apply
> to each newer incarnation. I wouldnt hack around with ifdefs. Actually I
> dont think more than 0.5% of ffmpeg users will need FLC/FLX/DTA
> encoders, thus they could disable the whole thing.

Is it really that difficult to add #ifdef CONFIG_FLX_ENCODER or
whatever around the bits that are only used by it?  As a general rule,
codecs should be individually selectable.  Yours are no exception.

>> The decoder can be renamed to flicdec.c separately.
>
> You mean svn move?

Yes, together with the corresponding Makefile change.

-- 
M?ns Rullg?rd
mans at mansr.com