[Ffmpeg-devel] [PATCH] flash screen video encoder

[Michael Niedermayer]

Hi

On Sun, Jan 21, 2007 at 05:39:32PM +0100, Benjamin Larsson wrote:
> Hi, thanks for the review.
> 
> The plan is to get a working encoder commited and then later add 2 pass
> and optimizations.
> 
> 

[...]
> >
> >[...]
> >  
> >
> >>+                ptr[0] = 0;
> >>+                ptr[1] = 0;
> >>+                buf_pos +=2;
> >>    
> >>
> >
> >please use bytestream.h
> >  
> >
> 
> Do you mean bytestream_put_be16(ptr,0) ?

yes exactly


> 
> >
> >  
> >
> >>+            }
> >>+        }
> >>+    }
> >>+
> >>+    if (pred_blocks)
> >>+        *I_frame = 0;
> >>+    else
> >>+        *I_frame = 1;
> >>    
> >>
> >
> >actually you dont need to keep track of pred_blocks, a simple check for the
> >buf_size vs. the number of blocks*2 should do i think
> >  
> >
> 
> I prefer it this way as I could extend the I frame decision then. Think
> of a frame that all but one block changes. Then it might be of advantage
> to encode it as an I frame. The format only allow block changes at I
> frames.

ok

[...]



[...]
> +static int flashsv_encode_init(AVCodecContext *avctx)
> +{
> +    FlashSVContext *s = (FlashSVContext *)avctx->priv_data;
> +
> +    s->avctx = avctx;
> +
> +    if ((avctx->width > 4095) || (avctx->height > 4095)) {
> +        av_log(avctx, AV_LOG_ERROR, "Input dimensions to large, input must be max 4096x4096 !\n");
> +        return -1;
> +    }
> +
> +    if (avcodec_check_dimensions(avctx, avctx->width, avctx->height) < 0) {
> +        return -1;
> +    }
> +
> +    s->first_frame = 1;
> +
> +    // Needed if zlib unused or init aborted before deflateInit
> +    memset(&(s->zstream), 0, sizeof(z_stream));
> +/*
> +    s->zstream.zalloc = NULL; //av_malloc;
> +    s->zstream.zfree = NULL; //av_free;
> +    s->zstream.opaque = NULL;
> +    zret = deflateInit(&(s->zstream), 9);
> +    if (zret != Z_OK) {
> +        av_log(avctx, AV_LOG_ERROR, "Inflate init error: %d\n", zret);
> +        return -1;
> +    }
> +*/
> +
> +    s->image_width = avctx->width;
> +    s->image_height = avctx->height;
> +
> +    if ((s->tmpblock = av_mallocz(3*256*256)) == NULL) {
> +        av_log(avctx, AV_LOG_ERROR, "Can't allocate compression buffer.\n");
> +        return -1;
> +    }
> +
> +    if (!s->encbuffer)
> +        s->encbuffer = av_mallocz(s->image_width*s->image_height*3);

how can the buffer be already allocated? and if so how can we be sure its
old size is large enough?


> +
> +
> +    return 0;
> +}
> +
> +#ifdef CONFIG_ENCODERS

the 2 functions above are also encoder specific i think and it would be nice
if the encoder and decoder would be in seperate files, would avoid all the
CONFIG_EN/DECODERS

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The greatest way to live with honor in this world is to be what we pretend
to be. -- Socrates
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070121/1cb46a9c/attachment.pgp>