[FFmpeg-devel] a64 encoder

[Diego Biurrun]

On Thu, Jan 15, 2009 at 03:02:31PM +0100, Bitbreaker/METALVOTZE wrote:
> 
> Attached you'll find the patch for review, if you find it worth being 
> added to the repository, i'd be happy.

All your header files are missing multiple inclusion guards.
Some of your files have tabs and trailing whitespace.

> --- libavcodec/a64colormixes.h	(Revision 0)
> +++ libavcodec/a64colormixes.h	(Revision 0)
> @@ -0,0 +1,113 @@
> + * Copyright (c) 2009 Tobias Bindhammer.

pointless period, same below

> --- libavcodec/a64ecmcharset.h	(Revision 0)
> +++ libavcodec/a64ecmcharset.h	(Revision 0)
> @@ -0,0 +1,295 @@
> +
> +/**
> + * @file a64ecmcharset.h
> + * A64 Video Encoder - Symbol Tables for ECM Hybrid Mode
> + */
> +
> +static uint32_t ecm_symbols[][8]={

This will need #include <stdint.h> to pass 'make checkheaders'.

> --- libavcodec/a64multi.c	(Revision 0)
> +++ libavcodec/a64multi.c	(Revision 0)
> @@ -0,0 +1,419 @@
> +/*
> + * A64 Video Encoder Multicolor Modes

pointless capitalization

> +/**
> + * @file a64multi.c
> + * A64 Video Multicolor Modes
> + */

ditto

> +int a64multi_close_encoder(AVCodecContext *avctx) {

Please use K&R style function declarations.

> +    if(c->multicol_frame_counter>0) {
> +        //c->multicol_cs_lifetime=c->multicol_frame_counter;
> +    }

useless {}

> +		

trailing whitespace, also in other places

> +    if(multicol_charmaps!=NULL) av_free(multicol_charmaps);
> +    if(multicol_bitmap!=NULL) av_free(multicol_bitmap);
> +    if(multicol_charset!=NULL) av_free(multicol_charset);
> +    if(multicol_charset_stats!=NULL) av_free(multicol_charset_stats);
> +    if(multicol_resolve_map!=NULL) av_free(multicol_resolve_map);
> +    if(multicol_colram_map!=NULL) av_free(multicol_colram_map);
> +    if(meta_charset!=NULL) av_free(meta_charset);

I'd put spaces around the != and the statement on the next line.

> +    /* fill up charset with framedata until lifetime exeeded */
> +    if(c->multicol_frame_counter<c->multicol_cs_lifetime) {
> +    	if(c->multicol_5col==1) a64_bgr_to_meta(avctx, p, meta_charset+32000*c->multicol_frame_counter,c->multicol_dithersteps*4);

tabs

> +static void find_favourite(int charpos, int lastchar, uint32_t* diff, uint32_t* best) {

favorite, American English is preferred.

> --- libavcodec/a64multi.h	(Revision 0)
> +++ libavcodec/a64multi.h	(Revision 0)
> @@ -0,0 +1,82 @@
> +
> +/**
> + * @file a64multi.h
> + * A64 Video Encoder - Headers for Multicolor Modes
> + */
> +
> +static const uint32_t dpattern[8][4][4]= {

stdint.h

> --- libavcodec/a64ecmh.h	(Revision 0)
> +++ libavcodec/a64ecmh.h	(Revision 0)
> @@ -0,0 +1,3 @@
> +int a64ecmh_encode_frame(AVCodecContext *, unsigned char *, int, AVFrame *);
> +int a64ecmh_close_encoder(AVCodecContext *);
> +int a64ecmh_init_encoder(AVCodecContext *);

missing license header, etc...

Diego