[FFmpeg-devel] [PATCH] Mimic decoder

Michael Niedermayer michaelni
Wed Mar 12 02:13:31 CET 2008


On Tue, Mar 11, 2008 at 06:51:49PM -0300, Ramiro Polla wrote:
> Hello,
>
> I have taken libmimic [0] and used it to implement a native Mimic [1] 
> decoder for FFmpeg.
>
> The Mimic codec is used by MSN Messenger to transmit Webcam conversations 
> (not Video Conversations). It is also supported by aMSN and Mercury 
> Messenger. IIRC, those two programs allow to record the streamed data. 
> There's also MSN Webcam Recorder [2] that can record those streams.
>
> I have the whole history in an SVN server [3] that goes all the way back to 
> the first integration that had only minor changes from the original code 
> (like glib's ints to C99).
>
> There's one big issue that we discussed some on IRC and I decided to take 
> to the list. The original code does lots of deblocking as post-processing. 
> Some people on IRC said libavcodec should not do pure post-processing (it's 
> not in-loop), and that I should drop that code. But pretty much every user 
> that will use this expects the output to be the same as in MSN Messenger 
> (which does the deblocking). So, is there any chance the deblocking can be 
> accepted in the codec? If so, I'd need to clean it up some more.

no
Either the deblock is worse than what we have in which case ->/dev/null
or its better in which case please move it to libpostproc or libavfilter
But most likely MS copy and pasted it from the mpeg4 reference, which
would make it equal quality but very significantly slower to libpp.


[...]
>     int width;
>     int height;
>     enum PixelFormat pix_fmt;

Duplicate of the respective things in AVCodecContext


>     int f[2];

!

(which is a one letter awnser for no in case you didnt guess it)
that is please change this to qscale, qp or whatever ...


[...]
> /*
>  * vlc_decode_block
>  *
>  * De-serialize (reconstruct) a variable length coded 8x8 block.
>  */
> static int vlc_decode_block(MimicContext *ctx, DCTELEM *block, int num_coeffs)
> {
>     unsigned int pos;
> 
>     memset(block, 0, 64 * sizeof(DCTELEM));

clear_blocks() in dsp


> 
>     /* The DC-value is read in as is. */
>     block[0] = get_bits(&ctx->gb, 8);
> 
>     for(pos = 1; pos < num_coeffs; pos++) {
>         uint32_t vlc, num_bits;
>         int value;
> 
>         vlc = get_vlc2(&ctx->gb, ctx->vlc1.table, ctx->vlc1.bits, 4);
>         if(vlc == -1)
>             return 0;
>         if(vlc == 3) /* end-of-block code */
>             return 1;
> 
>         pos +=     magic_values[vlc][0];
>         num_bits = magic_values[vlc][1];

the 2 magic_values together fit in 7 bit
if you reshuffle the entries in huffcodes/huffbits then something like:
pos     += vlc&15;
num_bits = vlc>>4;
would do the same and you would not need the magic_values table. 


> 
>         value = get_bits(&ctx->gb, num_bits);
> 
>         block[ctx->scantable.permutated[pos]] = ctx->vlcdec_lookup[num_bits][value];

you are missing a check for pos being < 64 here

and

value= 1 - (1<<num_bits);
if(num_bits>1)
    value+= get_bits(num_bits-1);
if(get_bits1())
    value -= value;

block[ctx->scantable.permutated[pos]] = value;

Or if you think the table matters speedwise then make it static dont
duplicate it in every decoder instance.


>     }
> 
>     return 1;
> }
> 

> static void dequant_block(MimicContext *ctx, DCTELEM *block, int is_chrom)
> {
>     int i, f = ctx->f[is_chrom];
> 
>     /* FFmpeg's IDCT behaves somewhat different from the original code, so
>      * a factor of 4 was added to the input */
> 
>     /* De-quantize. */
>     block[0] <<= 3;
>     block[1] <<= 4;
>     block[8] <<= 4;
> 
>     for(i = 2; i < 64; i++) {
>         if(i == 8)
>             continue;
> 
>         /* TODO Use >> 10 instead of / 1001 */
>         block[i] = (block[i] * f) / 1001;
>     }
> }

Do this during block decode, it will be much faster as >90% of all coeffs
are 0


> 
> /*
>  * decode_main
>  *
>  * Main decoding loop.
>  */
> static int decode(MimicContext *ctx, int quality, int num_coeffs, int is_pframe)
> {
>     int y, x, i, chrom_ch, offset;
>     DECLARE_ALIGNED_16(DCTELEM, dct_block[64]);
>     uint8_t *src, *dst, *p;
>     uint8_t *base_src, *base_dst;
>     uint32_t bit;
>     int y_stride = ctx->picture.linesize[0];
>     int crcb_stride = ctx->picture.linesize[1];

>     int crcb_size = ctx->picture.linesize[1] * (ctx->height>>1);
> 
>     /* Clear Cr and Cb planes. */
>     memset(ctx->picture.data[1], 128, crcb_size);
>     memset(ctx->picture.data[2], 128, crcb_size);

remove this insanity please


> 
>     ctx->f[0] = av_clip(10000-quality, 1000, 10000)<<2;
>     ctx->f[1] = av_clip(10000-quality, 2000, 10000)<<2;
> 

>     /* Decode Y plane. */
>     for(y = ctx->num_vblocks_y - 1; y >= 0 ; y--) {

use luma instead of "y" its confusing when mixed wth x/y


>         offset = (y_stride << 3) * y;
>         src = ctx->prev_frame.data[0] + offset;
>         dst = ctx->picture.data[0]  + offset;
>         for(x = 0; x < ctx->num_hblocks_y; x++) {
>             /* Check for a change condition in the current block. */
>             bit = is_pframe ? get_bits1(&ctx->gb) : 0;
>             if(!bit) {
>                 /* Yes: Is the new content the same as it was in one of
>                  * the 15 last frames preceding the previous? */
>                 if(is_pframe)
>                     bit = get_bits1(&ctx->gb);
>                 if(!bit) {
>                     /* No: decode it. */
>                     if(!vlc_decode_block(ctx, dct_block, num_coeffs)) {
>                         /* Corruped frame, return. */
>                         return 0;
>                     }
>                     dequant_block(ctx, dct_block, 0);
>                     ctx->dsp.idct_put(dst+7*y_stride, -y_stride, dct_block);

Id change linesize to *-1 and make data[*] point to the last line,
that way all this would look like a normal top to bottom decoder.



>                 } else {
>                     uint32_t backref;
>                     /* Yes: read the backreference (4 bits) and copy. */
>                     backref = get_bits(&ctx->gb, 4);
>                     p = ctx->buf_ptrs[(ctx->ptr_index + backref) % 16].data[0];

&15


>                     p += offset + (x << 3);
>                     for(i = 7; i >= 0; i--) {
>                         int new_offset = y_stride * i;
>                         memcpy(dst + new_offset, p + new_offset, 8);
>                     }
>                 }
>             } else {

>                 /* No change no worries: just copy from the previous frame. */
>                 for(i = 7; i >= 0; i--) {
>                     int new_offset = y_stride * i;
>                     memcpy(dst + new_offset, src + new_offset, 8);
>                 }

see put_pixels_tab in dsp


>             }
>             src += 8;
>             dst += 8;
>         }
>     }
> 
>     /* Decode Cr and Cb planes. */
>     for(chrom_ch = 0; chrom_ch < 2; chrom_ch++) {
>         base_src = (chrom_ch ? ctx->prev_frame.data[1] : ctx->prev_frame.data[2]);
>         base_dst = (chrom_ch ? ctx->picture.data[1] : ctx->picture.data[2]);

base_src= ctx->prev_frame.data[2-chrom_ch];
base_dst= ctx->picture   .data[2-chrom_ch];

and merge the chroma and luma loops if possible


[...]
>     /*
>      * Make a copy of the current frame and store in
>      * the circular pointer list of 16 entries.
>      */
>     ctx->prev_frame = ctx->buf_ptrs[ctx->ptr_index];
> 
>     av_picture_copy((AVPicture*)&ctx->prev_frame, (AVPicture*)&ctx->picture,
>                     ctx->pix_fmt, ctx->width, ctx->height);

no, remove all copiing its unneeded.



[...]

>         /* TODO find out why dsp.bswap_buf crashes */

size in 32bit vs size in 8bit ? Iam just guessing of course ...


[...]
-- 
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/20080312/8b1b51aa/attachment.pgp>



More information about the ffmpeg-devel mailing list