[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