[FFmpeg-devel] [PATCH] Apple Video Encoder (rpza)

[Diego Biurrun]

On Sun, Jan 23, 2011 at 06:06:17PM +0100, Vitor Sessak wrote:
>
> Git-friendly patch attached so patchwork will catch it up.
>
> --- a/libavcodec/Makefile
> +++ b/libavcodec/Makefile
> @@ -136,6 +136,9 @@ endif
>  ifneq ($(CONFIG_SVQ1_DECODER)$(CONFIG_SVQ1_ENCODER),)
>      OBJS+= svq1.o
>  endif
> +ifneq ($(CONFIG_RPZA_ENCODER),)
> +    OBJS+= rpzaenc.o
> +endif

Wow, this is old - it does not apply of course.

> --- a/libavcodec/allcodecs.c
> +++ b/libavcodec/allcodecs.c
> @@ -189,6 +189,9 @@ void avcodec_register_all(void)
>  #ifdef CONFIG_LIBGSM
>      register_avcodec(&libgsm_encoder);
>  #endif //CONFIG_LIBGSM
> +#ifdef CONFIG_RPZA_ENCODER
> +    register_avcodec(&rpza_encoder);
> +#endif //CONFIG_RPZA_ENCODER
>  #endif /* CONFIG_ENCODERS */

same

> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -1943,6 +1943,7 @@ extern AVCodec sonic_encoder;
>  extern AVCodec sonic_ls_encoder;
>  extern AVCodec svq1_encoder;
>  extern AVCodec x264_encoder;
> +extern AVCodec rpza_encoder;

alphabetical order

> --- /dev/null
> +++ b/libavcodec/rpzaenc.c
> @@ -0,0 +1,1169 @@
> +/*
> + * Quicktime RPZA Video Encoder.

s/.//

> + * Copyright (C) 2005 the ffmpeg project
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
> + *
> + */

not the standard license header

> +/**
> + * @file rpzaenc.c

Drop the filename.

> +#ifdef CONFIG_ENCODERS

This looks pointless, the file is only ever compiled if this is set.

> +#if 0
> +static void print_rgb24_color(uint8_t *color)
> +{
> +    printf("r{%02d} g{%02d} b{%02d}", color[0], color[1], color[2]);
> +}
> +
> +static void print_block(uint8_t *block, BlockInfo *bi)
> +{
> +    int x, y;
> +
> +    for (y = 0; y < bi->block_height; y++) {
> +        for (x = 0; x < bi->block_width; x++) {
> +            printf("%d: ", x);
> +            print_rgb24_color(&block[x * PIXELSTRIDE]);
> +            printf("\n");
> +        }
> +        printf("\n");
> +        block += bi->rowstride;
> +    }
> +}
> +#endif

I think such debug code should be removed from the final version.

> +static void get_max_component_diff(BlockInfo *bi, uint8_t *block_ptr, 
> +        uint8_t *min, uint8_t *max, channel_offset *chan)
> +/*
> + * Find the channel that has the largest difference between minimum and maximum
> + * color values. Put the minimum value in min, maximum in max and the channel
> + * in chan.
> + */ 
> +{

The comment placement looks weird, same below.

> +AVCodec rpza_encoder = {
> +    "rpza",
> +    CODEC_TYPE_VIDEO,
> +    CODEC_ID_RPZA,
> +    sizeof(RpzaContext),
> +    rpza_encode_init,
> +    rpza_encode_frame,
> +    rpza_encode_end,
> +    .pix_fmts= (enum PixelFormat[]){PIX_FMT_RGB24, -1},
> +};

long_name is missing


This is also missing changelog and docs update, version bump.

Diego