[FFmpeg-devel] [PATCH] encoder for adobe's flash ScreenVideo2 codec

[Vitor Sessak]

Joshua Warner wrote:
> On Tue, Jul 21, 2009 at 12:13 PM, Daniel Verkamp<daniel at drv.nu> wrote:
>> On Tue, Jul 21, 2009 at 10:57 AM, Joshua Warner<joshuawarner32 at gmail.com> wrote:
>>> Hi all,
>>>
>>> I've developed an encoder for Adobe's Flash ScreenVideo2 format, which is
>>> stored in flv files.  My encoder currently only supports a large subset of
>>> the format.  The only player that supports this codec (so far) is Adobe
>>> Flash Player itself, but ScreenVideo2 makes dramatic improvement in file
>>> size over ScreenVideo (currently in ffmpeg as flashsv) - and should be very
>>> useful for uploading screencasts, etc.  Most options (block size, etc) now
>>> just fall back on defaults because I couldn't find a general algorithm that
>>> produced consistantly better results than these.  All the code is in place
>>> to be able to change these parameters dynamically, so future improvements
>>> there should be easy.  The patch is attached.
>>>
>>> The codec is listed as flashsv2 in ffmpeg.
>>>
>>> Joshua Warner
>> Minor nitpicks:
>>  - "palette" is misspelled as "pallet" throughout
>>  - trailing whitespace on 2 lines
>>  - plenty of warnings from tools/patcheck
> 
> I ran tools/patcheck (which I wasn't aware of before), and most of the
> output was either false positives or would reduce readability of the
> code. 

It does gives a few false positives. All the "x==0 / x!=0 can be 
simplified to !x / x" is a matter of preferred style in FFmpeg (the 
readability depends on how used you are to it). The "missing const" are 
mostly correct.

> I'm not sure what to do about the comments that it complains
> are not doxygen compliant - they are things that don't have any
> applicability outside flashsv2enc.c... should I appease patcheck?

We prefer to add doxy comments for things that would improve the doxygen 
documentation of the file. Typically struct members and comments before 
functions describing what it does.

-Vitor