[FFmpeg-devel] a64 encoder 5th round

[Michael Niedermayer]

On Fri, Jan 23, 2009 at 10:28:31AM +0100, Bitbreaker/METALVOTZE wrote:
> Cleaned up a few more things in the code, also added issues found by Aurel. 
> Anything else to fix?
[...]
> +/* own methods */
> +static void to_meta_with_crop(AVCodecContext *avctx, AVFrame *p, int *dest)
> +{
> +    int blockx, blocky, x, y;
> +    int luma = 0;

> +    int height = avctx->height;
> +    int width = avctx->width;

these could be set to FFMIN(width, C64XRES) ... i think


> +    uint8_t *src = p->data[0];
> +
> +    for (blocky = 0; blocky<height && blocky < C64YRES; blocky += 8) {
> +        for (blockx = 0; blockx<width && blockx < C64XRES; blockx += 8) {
> +            for (y = blocky; y < blocky+8 && y<C64YRES && y<height; y++) {
> +                for (x = blockx; x < blockx+8 && x<C64XRES && x<width; x += 2) {
> +                    /* build average over 2 pixels */
> +                    luma = (src[(x + 0 + y * p->linesize[0])] +
> +                            src[(x + 1 + y * p->linesize[0])]) / 2;
> +                    /* write blocks as linear data now so they are suitable for elbg */

> +                    dest[0] = luma; dest++;

*dest++ = ...


[...]
> +    const uint8_t vals[] = { 3, 2, 1, 0, 3 };

static const


> +    int luma;
> +    int pix;
> +    int lowdiff;
> +    int highdiff;
> +    int dsteps = c->mc_dithersteps;
> +    int maxindex = c->mc_use_5col + 3;
> +    int maxsteps = c->mc_dithersteps * maxindex + 1;
> +    int *best_cb = c->mc_best_cb;
> +
> +    /* now reduce colors first */
> +    for (x = 0; x < 256 * 32; x++) {
> +        luma = best_cb[x];
> +        pix = luma * maxsteps / 255;
> +        if (pix > maxsteps)
> +            pix = maxsteps;
> +        best_cb[x] = pix;
> +    }
> +
> +    /* and render charset */
> +    for (charpos = 0; charpos < 256; charpos++) {
> +        lowdiff = 0;
> +        highdiff = 0;
> +        for (y = 0; y < 8; y++) {
> +            row1 = 0;
> +            for (x = 0; x < 4; x++) {
> +                pix = best_cb[y * 4 + x];
> +                dither = pix % dsteps;
> +                index1 = pix / dsteps;
> +                if (index1 == maxindex)
> +                    index2 = maxindex;
> +                else
> +                    index2 = index1 + 1;
> +
> +                if (pix > 3 * dsteps)
> +                    highdiff += pix - 3 * dsteps;
> +                if (pix < dsteps)
> +                    lowdiff += dsteps - pix;
> +

> +                row1 <<= 2;
> +                if (prep_dither_patterns[dither][y & 3][x & 3])
> +                    row1 |= vals[index2];
> +                else
> +                    row1 |= vals[index1];
> +            }
> +            charset[y] = row1;

could be simplified slightly with put_bits()

besides the way you do dithering is uncommon ...
i assume you have a reason why you dont use ordered dither, or one of the
error diffusion dithers? last should be vastly better except maybe some
flickering between frames and the need to consider already set patterns
of neighborin blocks ...


[...]
> +static int a64multi_init_encoder(AVCodecContext *avctx)

av_cold

[...]
> Index: libavformat/a64.c
> ===================================================================
> --- libavformat/a64.c	(Revision 0)
> +++ libavformat/a64.c	(Revision 0)

you could split the muxer into its own patch

[...]
> +#include "libavcodec/avcodec.h"
> +#include "libavcodec/a64enc.h"
> +#include "libavcodec/bitstream.h"
> +#include "libavcodec/bytestream.h"
> +#include "avformat.h"

are all these needed?


> +
> +static int a64_write_header(struct AVFormatContext *s)
> +{
> +    AVCodecContext *avctx=s->streams[0]->codec;
> +    A64Context *c = avctx->priv_data;

A muxer cannot access the private contxt of the encoder


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If a bugfix only changes things apparently unrelated to the bug with no
further explanation, that is a good sign that the bugfix is wrong.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090123/3524735b/attachment.pgp>