[FFmpeg-devel] [PATCH] Split raw.c

[Panagiotis Issaris]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi Aurelien,

Aurelien Jacobs wrote:
> On Mon, 21 May 2007 17:35:21 +0200
> Panagiotis Issaris <takis.issaris at uhasselt.be> wrote:
> 
>> Hi,
>>
>> The attached patch splits raw.c into several parts. The pixel format
>> related functions are split into a separate rawpixfmt.[ch] pair and the
>> encoder and decoder are put into separate files. Three arrays and a
>> function had to be made global and were prefixed with ff_.
>>
>> rawpixfmt.c is compiled unconditionally, rawdec.c en rawenc.c are only
>> compiled when needed.
>>
>>  Makefile    |    4 -
>>  raw.h       |  200
>> +----------------------------------------------------------
>>  rawdec.c    |  127 +++----------------------------------
>>  rawenc.c    |  178 ----------------------------------------------------
>>  rawpixfmt.c |  148 ++-----------------------------------------
>>  rawpixfmt.h |  202
>> ++----------------------------------------------------------
>>  6 files changed, 39 insertions(+), 820 deletions(-)
>>
>> Index: libavcodec/rawdec.c
>> ===================================================================
>> --- libavcodec/rawdec.c	(revision 9092)
>> +++ libavcodec/rawdec.c	(working copy)
>> @@ -20,102 +20,27 @@
>>   */
>>  
>>  /**
>> - * @file raw.c
>> - * Raw Video Codec
>> + * @file rawdec.c
>> + * Raw Video decoder
>>   */
>>  
>> -#include "avcodec.h"
>> +#include "raw.h"
>> +#include "rawpixfmt.h"
>>  
>> -typedef struct RawVideoContext {
>> -    unsigned char * buffer;  /* block of memory for holding one frame */
>> -    int             length;  /* number of bytes in buffer */
>> -    AVFrame pic;             ///< AVCodecContext.coded_frame
>> -} RawVideoContext;
>> +extern const PixelFormatTag ff_pixelFormatTags[];
> 
> This extern should be in rawpixfmt.h

Fixed this, although I cannot see why this would be useful as
ff_pixelFormatTags is only used in rawdec.c. Ofcourse, if others start
using it, this would be cleaner.

>> +extern const PixelFormatTag ff_pixelFormatBpsAVI[];
>> +extern const PixelFormatTag ff_pixelFormatBpsMOV[];
> 
> Don't need to be extern. They are only used by the decoder...

You're right... but...


>> -static int findPixelFormat(const PixelFormatTag *tags, unsigned int fourcc)
>> -{
> 
> AFAICT findPixelFormat is only used by the decoder, so it should stay as a
> static in rawdec.c

And right again, _but_ in some way it defeats one of the purposes of
this patch. The reason I had put them in rawpixfmt.c was that they
appeared to handle pixel format related stuff, which was why I wanted to
group that stuff together in a separate file. I wanted to keep the pure
decoding, encoding and other things in separate files.

But as it is preferred, and both ways allow raw.c to be split, I've
addressed both remarks in the attached patch anyway :)

>> @@ -223,4 +118,4 @@
>>      raw_close_decoder,
>>      raw_decode,
>>  };
>> -#endif // CONFIG_RAWVIDEO_DECODER
>> +
> 
> Cosmetics (adding useless empty line ;-)

Fixed :)


> Also as a general remark, the content of rawpixfmt.c might also be moved directly
> in utils.c (but that might as well be ugly, and anyway, I'm fine with rawpixfmt.c).

 raw.h       |  200
+----------------------------------------------------------
 rawdec.c    |   91 +--------------------------
 rawenc.c    |  178 ----------------------------------------------------
 rawpixfmt.c |  171 +-------------------------------------------------
 rawpixfmt.h |  202
++----------------------------------------------------------
 5 files changed, 27 insertions(+), 815 deletions(-)


With friendly regards,
Takis
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFGVDa99kOxLuzz4CkRAnUkAJ4g5aLa8OLViNp9j/ZGy0iX5O9Y+ACfZEFD
HsiwSccObq/3R4iE+rfiGzI=
=UPkx
-----END PGP SIGNATURE-----
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pi-20070523T144103-ffmpeg-raw_split.diff
Type: text/x-patch
Size: 29163 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070523/ddebe41e/attachment.bin>