[FFmpeg-devel] [PATCH] encoder for adobe's flash ScreenVideo2 codec

Joshua Warner joshuawarner32
Tue Jul 21 21:09:04 CEST 2009


On Tue, Jul 21, 2009 at 11:46 AM, Diego Biurrun <diego at biurrun.de> wrote:
>
> On Tue, Jul 21, 2009 at 08:57:15AM -0600, Joshua Warner wrote:
> >
> > I've developed an encoder for Adobe's Flash ScreenVideo2 format, which is
> > stored in flv files. ?My encoder currently only supports a large subset of
> > the format. ?The only player that supports this codec (so far) is Adobe
> > Flash Player itself,
>
> What about writing a decoder first?

I didn't write the decoder first because I never found full example
files to test it on (only fragments garnered by monitoring network
traffic during an Adobe Breeze recording).? I can test the encoder by
playing the resulting files in flash player.  Plus, as of now, I have
no need for a decoder.  I will get to it, eventually...
>
>
> Your patch is missing a documentation update.

What, exactly needs to be changed here?
>
>
> > --- libavcodec/Makefile ? ? ? (revision 19479)
> > +++ libavcodec/Makefile ? ? ? (working copy)
> > @@ -92,6 +92,7 @@
> > ?OBJS-$(CONFIG_FLASHSV_DECODER) ? ? ? ? += flashsv.o
> > ?OBJS-$(CONFIG_FLASHSV_ENCODER) ? ? ? ? += flashsvenc.o
> > +OBJS-$(CONFIG_FLASHSV2_ENCODER) ? ? ? ? += flashsv2enc.o
>
> alignment
>
> > --- libavcodec/flashsv2enc.c ?(revision 0)
> > +++ libavcodec/flashsv2enc.c ?(revision 0)
> > @@ -0,0 +1,962 @@
> > +
> > +#include <zlib.h>
>
> If this encoder needs zlib, then you should declare the respective
> dependency in configure.
>
> > +typedef struct Block {
> > +} Block;
> > +
> > +typedef struct Pallet {
> > +} Pallet;
>
> Ugh, capitalized names..
>
> > +static int generate_default_pallet(Pallet * pallet);
>
> Please reorder the functions so that this forward declaration is
> unnecessary.
>
> > + ? ? ? ? ? ?b->width = (col < s->cols - 1)
> > + ? ? ? ? ? ? ? ?|| (s->image_width % s->block_width ==
> > + ? ? ? ? ? ? ? ? ? ?0) ? s->block_width : s->image_width % s->block_width;
> > + ? ? ? ? ? ?b->height = (row < s->rows - 1)
> > + ? ? ? ? ? ? ? ?|| (s->image_height % s->block_height ==
> > + ? ? ? ? ? ? ? ? ? ?0) ? s->block_height : s->image_height %
> > + ? ? ? ? ? ? ? ?s->block_height;
>
> This sure is unreadable.
>
> > + ? ? ? ? ? ?b->row = row;
> > + ? ? ? ? ? ?b->col = col;
> > + ? ? ? ? ? ?b->enc = encbuf;
> > + ? ? ? ? ? ?b->data = databuf;
>
> vertical alignment
>
> > +static void reset_stats(FlashSV2Context * s)
> > +{
> > + ? ?s->diff_blocks = 0.1;
> > + ? ?s->tot_blocks = 1;
> > + ? ?s->diff_lines = 0.1;
> > + ? ?s->tot_lines = 1;
> > + ? ?s->raw_size = s->comp_size = s->uncomp_size = 10;
>
> ditto
>
> > + ? ?if (avcodec_check_dimensions(avctx, avctx->width, avctx->height) < 0) {
> > + ? ? ? ?return -1;
> > + ? ?}
>
> pointless {}
>
> > + ? ?s->image_width = avctx->width;
> > + ? ?s->image_height = avctx->height;
> > +
> > + ? ?s->block_width = (s->image_width / 12) & ~15;
> > + ? ?s->block_height = (s->image_height / 12) & ~15;
>
> vertical alignment
>
> > + ? ?s->rows = (s->image_height + s->block_height - 1) / s->block_height;
> > + ? ?s->cols = (s->image_width + s->block_width - 1) / s->block_width;
>
> vertical alignment
>
> > + ? ?s->frame_size = s->image_width * s->image_height * 3;
> > + ? ?s->blocks_size = s->rows * s->cols * sizeof(Block);
>
> vertical alignment
>
> > + ? ?s->encbuffer = av_mallocz(s->frame_size);
> > + ? ?s->keybuffer = av_mallocz(s->frame_size);
> > + ? ?s->databuffer = av_mallocz(s->frame_size * 6);
> > + ? ?s->current_frame = av_mallocz(s->frame_size);
> > + ? ?s->key_frame = av_mallocz(s->frame_size);
> > + ? ?s->frame_blocks = av_mallocz(s->blocks_size);
> > + ? ?s->key_blocks = av_mallocz(s->blocks_size);
> > + ? ?s->pallet.index = av_mallocz(1 << 15);
>
> vertical alignment
>
> There are more instances below, please prettyprint your code where it
> helps readability.
>
> > +static int encode_zlib(Block * b, uint8_t * buf, int *buf_size, int comp)
> > +{
> > + ? ?int res =
> > + ? ? ? ?compress2(buf, buf_size, b->sl_begin, b->sl_end - b->sl_begin,
> > + ? ? ? ? ? ? ? ? ?comp);
>
> weird linebreaks
>
> > +static int encode_bgr(Block * b, uint8_t * src, int stride)
> > +{
> > + ? ?int i;
> > + ? ?uint8_t *ptr = b->enc;
> > + ? ?for (i = 0; i < b->start; i++) {
> > + ? ? ? ?memcpy(ptr + i * b->width * 3, src + i * stride, b->width * 3);
> > + ? ?}
> > + ? ?b->sl_begin = ptr + i * b->width * 3;
> > + ? ?for (; i < b->start + b->len; i++) {
> > + ? ? ? ?memcpy(ptr + i * b->width * 3, src + i * stride, b->width * 3);
> > + ? ?}
> > + ? ?b->sl_end = ptr + i * b->width * 3;
> > + ? ?for (; i < b->height; i++) {
> > + ? ? ? ?memcpy(ptr + i * b->width * 3, src + i * stride, b->width * 3);
> > + ? ?}
>
> many pointless {}
>
> > +static inline unsigned int chroma_diff(unsigned int c1, unsigned int c2)
> > +{
> > + ? ?unsigned int t1 =
> > + ? ? ? ?(c1 & 0x000000ff) + ((c1 & 0x0000ff00) >> 8) +
> > + ? ? ? ?((c1 & 0x00ff0000) >> 16);
> > + ? ?unsigned int t2 =
> > + ? ? ? ?(c2 & 0x000000ff) + ((c2 & 0x0000ff00) >> 8) +
> > + ? ? ? ?((c2 & 0x00ff0000) >> 16);
>
> again, weird linebreaks
>
> > +static inline int encode_15_7_sl(Pallet * pallet, uint8_t * dest,
> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? uint8_t * src, int width, int dist)
> > +{
> > + ? ?int len = 0, x;
> > + ? ?for (x = 0; x < width; x++) {
> > + ? ? ? ?len += write_pixel_15_7(pallet, dest + len, src + 3 * x, dist);
> > + ? ?}
>
> pointless {}
>
> More below, please be consistent and drop them where unnecessary.
>
> > +static int encode_15_7(Pallet * pallet, Block * b, uint8_t * src,
> > + ? ? ? ? ? ? ? ? ? ? ? int stride, int dist)
> > +{
> > + ? ?int i, len;
> > + ? ?uint8_t *ptr = b->enc;
> > + ? ?for (i = 0; i < b->start; i++) {
> > + ? ? ? ?len =
> > + ? ? ? ? ? ?encode_15_7_sl(pallet, ptr, src + i * stride, b->width, dist);
>
> weird linebreak
>
> > + ? ?for (; i < b->start + b->len; i++) {
> > + ? ? ? ?len =
> > + ? ? ? ? ? ?encode_15_7_sl(pallet, ptr, src + i * stride, b->width, dist);
> > + ? ? ? ?ptr += len;
>
> again
>
> More below, please change them and earn extra good karma.
I'll fix the above issues and resubmit the patch some time tomorrow.
>
> > +#define FLASHSV2_DUMB

FLASHSV2_DUMB forces the encoder to use the build-in defaults instead
of the experimental "smart" parameter-choosing code, which (as
described in my original post) I haven't been able to get it to make
decisions that are consistently better than the defaults.? I wanted to
leave the "smart" code in place as a starting point for future work on
this tangent.? Should I do this differently?
>
>
> debug leftover?
>
> Diego
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at mplayerhq.hu
> https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-devel



More information about the ffmpeg-devel mailing list