[FFmpeg-devel] A64 encoder

[Diego Biurrun]

On Tue, Jul 06, 2010 at 03:07:56PM +0200, Tobias Bindhammer wrote:
> [...]

The patch is missing changelog updates as well as
minor version bumps in libavcodec/libavformat.

> --- libavcodec/a64multienc.c	(Revision 0)
> +++ libavcodec/a64multienc.c	(Revision 0)
> @@ -0,0 +1,251 @@
> +
> +/* own stuff */
> +static void to_meta_with_crop(AVCodecContext *avctx, AVFrame *p, int *dest)

cryptic comment

> +    for (blocky = 0; blocky<height; blocky += 8) {
> +        for (blockx = 0; blockx<C64XRES; blockx += 8) {
> +            for (y = blocky; y < blocky+8 && y<height; y++) {
> +                for (x = blockx; x < blockx+8 && x<C64XRES; x += 2) {
> +                    if(x<width) {

spaces around operators please

> +                        /* build average over 2 pixels */
> +                        luma = (src[(x + 0 + y * p->linesize[0])] +
> +                                src[(x + 1 + y * p->linesize[0])]) / 2;
> +                        //farben < 1 als 1-val auf low_diff aufadieren je block
> +                        //farben > 4 als val-4 aufaddieren auf highdiff
> +                        //wenn highdiff<lowdiff: hohe werte limitieren auf max: 4
> +                        //sonst kleine werte limitieren auf 1

Ahem - remove or translate German comments..

> +static void render_charset(AVCodecContext *avctx, uint8_t *charset, uint8_t *colrammap)

Break long lines where easily possible please.

> +/* encoder stuff */
> +static av_cold int a64multi_close_encoder(AVCodecContext *avctx)

We're in the encoder supposedly, so why the comment?

> +    av_log(avctx, AV_LOG_INFO, "charset lifetime set to %d frame(s)\n", c->mc_lifetime);

long line, see above, more below

> --- libavformat/a64.c	(Revision 0)
> +++ libavformat/a64.c	(Revision 0)
> @@ -0,0 +1,72 @@
> +
> +    switch (avctx->codec->id) {
> +    case CODEC_ID_A64_MULTI:
> +        header[2]=0x00;
> +        header[3]=4;
> +        header[4]=2;
> +        break;
> +    case CODEC_ID_A64_MULTI5:
> +        header[2]=0x01;
> +        header[3]=4;
> +        header[4]=3;
> +        break;

spaces around the = would make this more readable

> +AVOutputFormat a64_muxer = {
> +    .name = "a64",
> +    .long_name = NULL_IF_CONFIG_SMALL("a64 - video for commodore 64"),

Commodore

Diego