[FFmpeg-devel] [PATCH] MS Video 1 encoder

Michael Niedermayer michaelni
Sat Mar 7 20:41:04 CET 2009


On Sat, Mar 07, 2009 at 09:32:49AM +0200, Kostya wrote:
> On Sat, Mar 07, 2009 at 01:40:49AM +0100, Michael Niedermayer wrote:
> > On Fri, Mar 06, 2009 at 08:57:22PM +0200, Kostya wrote:
> > > $subj
> > > 
> > > It works more or less fine to me (except the codec itself being utter crap).
> > > You can even set quality with -qscale flag to get different grades of bad
> > > quality.
> > > 
> > > This codec may serve two goals: having video codec supported on Windows by
> > > default and (in the future) making regtests run even slower causing timeouts
> > > on FATE.
> > 
> > [...]
> > 
> > > +    *p = *pict;
> > > +    if(!c->prev)
> > > +        c->prev = av_malloc(pstride * (avctx->height + 3));
> > > +    prevptr = c->prev + pstride * (((avctx->height + 3)&~3) - 1);
> > > +    src = (uint16_t*)(p->data[0] + p->linesize[0]*(((avctx->height + 3)&~3) - 1));
> > 
> > could p->linesize[0] be negative?
>  
> unlikely until we have flip system
>  
> > > +    if(c->keyint >= avctx->keyint_min)
> > > +        keyframe = 1;
> > > +
> > > +    p->quality = avctx->global_quality;
> > > +    lambda = avctx->global_quality / FF_QUALITY_SCALE;
> > > +    lambda >>= 1; //range 0-16 should be enough here
> > 
> > i dont think your mapping of global_quality to lambda and how you use
> > it is in line with its definition
> 
> renamed
> 
> > also you are missing ratecontrol and 2pass ratecontrol
> 
> they are pretty useless here
> 
> > [...]
> > > +            // try to find optimal value to fill whole 4x4 block
> > > +            score = 0;
> > > +            ff_init_elbg(c->block, 3, 16, c->avg, 1, 1, c->output, &c->rnd);
> > > +            ff_do_elbg  (c->block, 3, 16, c->avg, 1, 1, c->output, &c->rnd);
> > > +            if(c->avg[0] == 1) // red component = 1 will be written as skip code
> > > +                c->avg[0] = 0;
> > 
> > using elbg to find the average is not pretty
> 
> indeed
> 
> > [...]
> > > +            for(j = 0; j < 4; j++){
> > > +                for(i = 0; i < 4; i++){
> > > +                    for(k = 0; k < 3; k++){
> > > +                        int t = c->codebook[c->output[i+j*4]*3 + k] - c->block[i*3+k+j*4*3];
> > 
> > i+j*4 is a common subexprssion of these an can be factored out in the
> > sense of loosing a loop
> > this likely applies to several othetr loops too and it likely also
> > would be simpler if "prev" would be stored as blocks instead of lines
> 
> factored it out when it occurs more than once
> 
> > [...]
> > > +    bytestream_put_byte(&dst, 0);
> > > +    bytestream_put_byte(&dst, 0);
> > 
> > put_le16(0)
> 
> changed
>  
> > [...]
> > > Index: libavcodec/elbg.c
> > > ===================================================================
> > > --- libavcodec/elbg.c	(revision 17606)
> > > +++ libavcodec/elbg.c	(working copy)
> > 
> > > @@ -105,7 +105,7 @@
> > >  {
> > >      int i=0;
> > >      /* Using linear search, do binary if it ever turns to be speed critical */
> > > -    int r = av_random(elbg->rand_state)%(elbg->utility_inc[elbg->numCB-1]-1) + 1;
> > > +    int r = elbg->utility_inc[elbg->numCB-1] == 1 ? 1 : av_random(elbg->rand_state)%(elbg->utility_inc[elbg->numCB-1]-1) + 1;
> > >      while (elbg->utility_inc[i] < r)
> > >          i++;
> > >  
> > 
> > this would benefit from a \n
> 
> changed
> 
> > [...]
> > -- 
> > Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> > 
> > Many things microsoft did are stupid, but not doing something just because
> > microsoft did it is even more stupid. If everything ms did were stupid they
> > would be bankrupt already.
> 
> How appropriate

[...]
> Index: libavcodec/msvideo1enc.c
> ===================================================================
> --- libavcodec/msvideo1enc.c	(revision 0)
> +++ libavcodec/msvideo1enc.c	(revision 0)
[...]
> +#include "avcodec.h"
> +#include "bytestream.h"

> +#include "libavutil/random.h"

Do you really need the MT? isnt a LFG good enough?


> +#include "elbg.h"
> +
> +/**
> + * Encoder context
> + */
> +typedef struct Msvideo1EncContext {
> +    AVCodecContext *avctx;
> +    AVFrame pic;
> +    AVRandomState rnd;
> +    uint8_t *prev;
> +
> +    int block[16*3];
> +    int block2[16*3];
> +    int codebook[8*3];
> +    int codebook2[8*3];
> +    int output[16*3];
> +    int output2[16*3];
> +    int avg[3];
> +    int keyint;
> +} Msvideo1EncContext;
> +
> +enum MSV1Mode{
> +    MODE_SKIP = 0,
> +    MODE_FILL,
> +    MODE_2COL,
> +    MODE_8COL,
> +};
> +
> +#define SKIP_PREFIX 0x8400
> +#define SKIPS_MAX 0x03FF
> +#define MKRGB555(in, off) ((in[off] << 10) | (in[off + 1] << 5) | (in[off + 2]))
> +
> +static const uint8_t remap[16] = { 0, 1, 4, 5, 2, 3, 6, 7, 8, 9, 12, 13, 10, 11, 14, 15 };
> +
> +static int encode_frame(AVCodecContext *avctx, uint8_t *buf, int buf_size, void *data)
> +{
> +    Msvideo1EncContext * const c = avctx->priv_data;
> +    AVFrame *pict = data;
> +    AVFrame * const p = &c->pic;

> +    uint16_t *src;

const


> +    uint8_t *prevptr;
> +    uint8_t *dst = buf;
> +    int keyframe = 0;
> +    int no_skips = 1;
> +    int i, j, k, x, y;
> +    int skips = 0;
> +    int qshift;
> +
> +    *p = *pict;
> +    if(!c->prev)
> +        c->prev = av_malloc(((avctx->width + 3) & ~3) * ((avctx->height + 3) & ~3) * 3);
> +    prevptr = c->prev;
> +    src = (uint16_t*)(p->data[0] + p->linesize[0]*(((avctx->height + 3)&~3) - 1));
> +    if(c->keyint >= avctx->keyint_min)
> +        keyframe = 1;
> +
> +    p->quality = avctx->global_quality;
> +    qshift = avctx->global_quality / FF_QUALITY_SCALE;
> +    qshift >>= 1; //range 0-16 should be enough here

its
SSE + bits*quality^2*SomeConstant IIRC

the log and shift stuff is because of the way H264 defines their QP
you are not writing a h264 codec and even if you did, you could freely
choose how to define your internal scales.
log/exp is mildly annoying ...

besides global_quality is not defined over H264 QP 


> +
> +    for(y = 0; y < avctx->height; y += 4){
> +        for(x = 0; x < avctx->width; x += 4){
> +            int bestmode = MODE_SKIP;
> +            int bestscore = INT_MAX;
> +            int flags = 0;
> +            int score;
> +
> +            for(j = 0; j < 4; j++){
> +                for(i = 0; i < 4; i++){
> +                    uint16_t val = src[x + i - j*p->linesize[0]/2];
> +                    for(k = 0; k < 3; k++){
> +                        c->block[(i + j*4)*3 + k] = (val >> (10-k*5)) & 0x1F;
> +                        c->block2[remap[i + j*4]*3 + k] = (val >> (10-k*5)) & 0x1F;
> +                    }
> +                }
> +            }
> +            if(!keyframe){
> +                bestscore = 0;
> +                for(i = 0; i < 4*4*3; i++){
> +                    int t = prevptr[i] - c->block[i];
> +                    bestscore += t*t;
> +                }
> +                bestscore >>= qshift;
> +                if(!skips)
> +                    bestscore += 2;
> +            }
> +            // try to find optimal value to fill whole 4x4 block
> +            score = 0;
> +            memset(c->avg, 0, sizeof(c->avg));

> +            for(j = 0; j < 4; j++)
> +                for(i = 0; i < 4; i++)
> +                    for(k = 0; k < 3; k++)
> +                        c->avg[k] += c->block[(i+j*4)*3+k];

same issue with the loop


> +            for(i = 0; i < 3; i++)
> +                c->avg[i] >>= 4;

you are missing rounding here


> +            if(c->avg[0] == 1) // red component = 1 will be written as skip code
> +                c->avg[0] = 0;
> +            for(j = 0; j < 4; j++){
> +                for(i = 0; i < 4; i++){
> +                    int ij = (i + j*4) * 3;

for(ij=0 ij<16 ...

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

Into a blind darkness they enter who follow after the Ignorance,
they as if into a greater darkness enter who devote themselves
to the Knowledge alone. -- Isha Upanishad
-------------- 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/20090307/19222a20/attachment.pgp>



More information about the ffmpeg-devel mailing list