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

[Michael Niedermayer]

On Sun, Mar 08, 2009 at 12:21:32PM +0200, Kostya wrote:
> On Sat, Mar 07, 2009 at 08:41:04PM +0100, Michael Niedermayer wrote:
> > 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.
> > > > 
> [...]
> > > 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?
>  
> It turned out that I don't need it but ELBG does.

as random.* is no longer part of the public API/ABI you cant use it from
outside libavutil no matter what excuse.


[...]

> +static int encode_frame(AVCodecContext *avctx, uint8_t *buf, int buf_size, void *data)

buf should be const


> +{
> +    Msvideo1EncContext * const c = avctx->priv_data;
> +    AVFrame *pict = data;
> +    AVFrame * const p = &c->pic;
> +    const uint16_t *src;
> +    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

this way of misusing global_quality is rejected, please refer to my
previous reviews for details


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

I have never wished to cater to the crowd; for what I know they do not
approve, and what they approve I do not know. -- Epicurus
-------------- 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/20090308/2a6aac1a/attachment.pgp>