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

Michael Niedermayer michaelni
Sat Mar 7 01:40:49 CET 2009


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?


> +    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

also you are missing ratecontrol and 2pass ratecontrol


[...]
> +            // 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


[...]
> +            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


[...]
> +    bytestream_put_byte(&dst, 0);
> +    bytestream_put_byte(&dst, 0);

put_le16(0)


[...]
> 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

[...]
-- 
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.
-------------- 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/443bf04e/attachment.pgp>



More information about the ffmpeg-devel mailing list