[FFmpeg-devel] a64 encoder
Vitor Sessak
vitor1001
Fri Jan 16 23:21:22 CET 2009
Bitbreaker/METALVOTZE wrote:
>> The patch is also very big, spliting it per encoder would definitly be a
>> good idea
>
> Second iteration of the diff with all suggested changes given so far.
> Petscii modes are now split off, what lets things shrink to a third in
> size :-)
Thanks, it is much easier to understand now.
>
> Index: libavcodec/a64ecmcharset.h
> ===================================================================
> --- libavcodec/a64ecmcharset.h (Revision 0)
> +++ libavcodec/a64ecmcharset.h (Revision 0)
> @@ -0,0 +1,297 @@
> +/*
> + * a64 video encoder - symbol tables for ECM hybrid mode
> + * Copyright (c) 2009 Tobias Bindhammer
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg 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.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg 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 FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +/**
> + * @file a64ecmcharset.h
> + * a64 video encoder - symbol tables for ECM hybrid mode
> + */
> +
> +#ifndef A64ECMCHARSET_INCLUDED
> +#define A64ECMCHARSET_INCLUDED
> +
> +#include <stdint.h>
> +
> +static uint8_t ecm_symbols[][8]={
static const uint8_t ecm_symbols[][8]={
[...]
> + }
> + if(pix<minpix) minpix=pix;
> + if(pix>maxpix) maxpix=pix;
> + }
> + charset[charpos*8+y]=row;
> + }
> + /* are we in 5col mode and need to adjust pixels? */
> + if(c->mc_use_5col && minpix<dsteps && maxpix>3*dsteps) {
> + if(lowerr>higherr) {
> + for(y=0;y<8;y++) {
> + for(x=0;x<4;x++) {
> + if(c->meta_charset[charpos*32+y*4+x]>3*dsteps) c->meta_charset[charpos*32+y*4+x]=3*dsteps;
> + }
> + }
> + }
Its a bit personal taste, but I'd find it more readable without the
braces here and in other places...
[...]
> +
> +/* encoder methods */
> +int av_a64multi_close_encoder(AVCodecContext *avctx)
the av_ prefix is reserved for functions defined in public libav*
headers. Its better to use ff_ here.
> +
> +int av_a64multi_init_encoder(AVCodecContext *avctx)
> +{
> + A64Context *c = avctx->priv_data;
> +
> + c->mc_lifetime=avctx->global_quality/=FF_QP2LAMBDA;
> + if(avctx->global_quality<1) avctx->global_quality=4;
> +
> + c->mc_frame_counter=0;
> + c->mc_dithersteps=8;
> + c->mc_use_5col=avctx->codec->id==CODEC_ID_A64_MULTI5;
> +
> + if(c->mc_charmaps==NULL) c->mc_charmaps=av_malloc (1000* c->mc_lifetime*sizeof(int));
> + if(c->mc_resolve_map==NULL) c->mc_resolve_map=av_malloc(1000* c->mc_lifetime*sizeof(int));
> + if(c->mc_block_stats==NULL) c->mc_block_stats=av_malloc(1000* c->mc_lifetime*sizeof(int));
> + if(c->meta_charset==NULL) c->meta_charset=av_malloc (32000*c->mc_lifetime*sizeof(uint8_t));
The argument to those ifs will always evaluate to true, init_encoder()
is only called once per context.
> + * @file a64enc.c
> + * a64 video encoder
> + */
> +
> +#include "a64enc.h"
> +#include "a64tables.h"
> +
> +static av_cold int a64_init_encoder(AVCodecContext *avctx)
> +{
> + A64Context *c=avctx->priv_data;
> + int a,b;
> + int col1,col2;
> +
> + for(b=0;b<16;b++) {
> + for(a=0;a<5;a++) {
> + c->colr[b][a] = a64_palette[b][0]*a/4;
> + c->colg[b][a] = a64_palette[b][1]*a/4;
> + c->colb[b][a] = a64_palette[b][2]*a/4;
> + }
> + for(a=0;a<16;a++) {
> + c->allowed_mix_map[a][b]=-1;
> + c->flicker_map[a][b]=abs(
> + (a64_palette[b][0]*(double)0.3+a64_palette[b][1]*(double)0.59+a64_palette[b][2]*(double)0.11) / 100 -
> + (a64_palette[a][0]*(double)0.3+a64_palette[a][1]*(double)0.59+a64_palette[a][2]*(double)0.11) / 100
> + );
> + }
> + }
> +
> + c->num_mixes=FF_ARRAY_ELEMS(a64_color_mixes);
> + av_log(avctx, AV_LOG_ERROR,"elems: %d\n",c->num_mixes);
> + for(a=0;a<c->num_mixes;a++) {
> + col1=a64_color_mixes[a][0];
> + col2=a64_color_mixes[a][1];
> + c->allowed_mix_map[col1][col2]=a;
> + c->allowed_mix_map[col2][col1]=a;
> + c->color_mixes_rgb[a][0] = (a64_palette[col1][0] + a64_palette[col2][0])/2;
> + c->color_mixes_rgb[a][1] = (a64_palette[col1][1] + a64_palette[col2][1])/2;
> + c->color_mixes_rgb[a][2] = (a64_palette[col1][2] + a64_palette[col2][2])/2;
> + }
> +
> + avcodec_get_frame_defaults(&c->picture);
> + avctx->coded_frame = &c->picture;
> + avctx->coded_frame->pict_type = FF_I_TYPE;
> + avctx->coded_frame->key_frame = 1;
> + if(!avctx->codec_tag) avctx->codec_tag = avcodec_pix_fmt_to_codec_tag(avctx->pix_fmt);
> +
> + switch(avctx->codec->id) {
> + case CODEC_ID_A64_MULTI:
> + case CODEC_ID_A64_MULTI5:
> + av_a64multi_init_encoder(avctx);
> + break;
> + }
> +
> + return 0;
> +}
> +
> +static int a64_close_encoder(AVCodecContext *avctx)
> +{
> + if(avctx->codec->id == CODEC_ID_A64_MULTI || avctx->codec->id == CODEC_ID_A64_MULTI5)
> + av_a64multi_close_encoder(avctx);
> + return 0;
> +}
> +
> +static int a64_encode_frame(AVCodecContext *avctx, unsigned char *buf, int buf_size, void *data)
> +{
> + A64Context *c = avctx->priv_data;
> + AVFrame *pict = data;
> + AVFrame * const p= (AVFrame*)&c->picture;
> +
> + *p = *pict;
> + p->pict_type= FF_I_TYPE;
> + p->key_frame= 1;
> +
> + switch(avctx->codec->id) {
> + case CODEC_ID_A64_MULTI:
> + case CODEC_ID_A64_MULTI5:
> + return av_a64multi_encode_frame(avctx,buf,buf_size,p);
> + break;
> + case CODEC_ID_A64_ECM_HYBRID:
> + return av_a64ecmh_encode_frame(avctx,buf,buf_size,p);
> + break;
> + }
> + return -1;
> +}
IMO it would be cleaner if you just remove this wrapper function, move
the AVCodec a64***={...} declaration to where the xxx_decode_frame()
function is and make a64_init_encoder() a non static function (renamed
to something like ff_a64_shared_init()).
-Vitor
More information about the ffmpeg-devel
mailing list