[FFmpeg-devel] [PATCH] RoQ video encoder, take 2

[Michael Niedermayer]

Hi

On Sun, May 27, 2007 at 12:21:03PM +0200, Vitor wrote:
> Hi,
> 
> I've considerably changed my patch. Indeed, changing the codebook 
> generator code to the ELBG algorithm lead to better image quality and 
> comparable speed. I've tried to do a very general codebook generator, to 
> maybe reuse in future in another VQ encoder.

could you move this code into a elbg.c/elbg.h ? and submit as a seperate
patch
having it in a seperate patch should reduce the time (and review passes)
needed to get it into svn


[...]
> Comments welcome!

quick review below, i will review the splited patches much more completely


[...]

> +    for (i=0; i<size; i++)
> +        for (j=0; j<size; j++)
> +            diff += SQUARE(a[0][(i+y)*enc->y_stride+j+x] -
> +                           b[0][(i+bstartheight)*enc->y_stride+j+bstartwidth]);
> +
> +    for (i=0; i<size/2; i++)
> +        for (j=0; j<size/2; j++) {
> +            diff += 4*SQUARE(a[1][(i+y/2)*enc->c_stride+j+x/2] -
> +                             b[1][(i+bstartheight/2)*enc->c_stride+j+bstartwidth/2]);
> +            diff += 4*SQUARE(a[2][(i+y/2)*enc->c_stride+j+x/2] -
> +                             b[2][(i+bstartheight/2)*enc->c_stride+j+bstartwidth/2]);
> +        }

is the 4* for chroma improving quality?

and see DSPContext.sse


[...]
> +/**
> + * Returns SE between two 4x4 blocks
> + */
> +static inline int squared_diff_cluster4(roq_cell4 *a, roq_cell4 *b, int count)
> +{
> +    int diff=0;
> +
> +    while(count--)
> +        diff += squared_diff_cluster2(a++->block,b++->block, 4);
> +
> +    return diff;
> +}

isnt this is just
squared_diff_cluster2(a,b,4*count);


[...]
> +            vect.dx = 0;
> +            vect.dy = 0;
> +            num=0;
> +            offset = (i/blocksize)*enc->width/blocksize + j/blocksize - 1;
> +            if (offset < max && offset > 0) {
> +                EVAL_MOTION(this_motion[offset]);
> +                vect.dx = this_motion[offset].dx;
> +                vect.dy = this_motion[offset].dy;
> +                num++;
> +            }
> +
> +            offset = (i/blocksize - 1)*enc->width/blocksize + j/blocksize;
> +            if (offset < max && offset > 0) {
> +                EVAL_MOTION(this_motion[offset]);
> +                vect.dx += this_motion[offset].dx;
> +                vect.dy += this_motion[offset].dy;
> +                num++;
> +            }
> +
> +            offset = (i/blocksize - 1)*enc->width/blocksize + j/blocksize + 1;
> +            if (offset < max && offset > 0) {
> +                EVAL_MOTION(this_motion[offset]);
> +                vect.dx += this_motion[offset].dx;
> +                vect.dy += this_motion[offset].dy;
> +                num++;
> +            }
> +
> +            if (num != 0) {
> +                vect.dx = vect.dx/num;
> +                vect.dy = vect.dy/num;
> +
> +                EVAL_MOTION(vect);
> +            }

this can be simplified to something like:

off[0]= (i/blocksize)*enc->width/blocksize + j/blocksize - 1;
off[1]= off[0] - enc->width/blocksize + 1;
off[2]= off[1] + 1;
if(i){
    vect.dx= mid_pred(this_motion[off[0]].dx, this_motion[off[1]].dx, this_motion[off[2]].dx); 
    vect.dy= mid_pred(this_motion[off[0]].dy, this_motion[off[1]].dy, this_motion[off[2]].dy); 
    EVAL_MOTION(vect);
    for(k=0; k<3; k++)
        EVAL_MOTION(this_motion[off[k]]);
}else if(j)
    EVAL_MOTION(this_motion[off[0]]);

also the median not mean of the 3 vectors is normally used


[...]

> +int generate_codebooks4(RoqContext *enc, roq_tempdata_t *tempdata, roq_cell4 *input, int inputCount, uint32_t *resultCount, roq_cell4 **resultElements)

duplicate (with minor changes) of generate_codebooks2


[...]
> +    .pix_fmts = roq_pixelformats,

.pix_fmts= (enum PixelFormat[]){PIX_FMT_YUV420P, -1},


> +};

> Index: libavcodec/roqvideodec.c
> ===================================================================
> --- libavcodec/roqvideodec.c	(revision 0)
> +++ libavcodec/roqvideodec.c	(revision 0)

this should be diffed against roqvideo.c and the roqvideo/roqvideodec
split should be in a seperate patch


[...]
> Index: libavcodec/roqvideo.c
> ===================================================================
> --- libavcodec/roqvideo.c	(revision 8966)
> +++ libavcodec/roqvideo.c	(working copy)
> @@ -16,103 +16,50 @@
>   * You should have received a copy of the GNU Lesser General Public
>   * License along with FFmpeg; if not, write to the Free Software
>   * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> - *
>   */

cosmetic change

[...]


> -static int uiclip[1024], *uiclp;  /* clipping table */
> +#define avg2(a,b) ri->uiclp[(((int)(a)+(int)(b)+1)>>1)]
> +#define avg4(a,b,c,d) ri->uiclp[(((int)(a)+(int)(b)+(int)(c)+(int)(d)+2)>>2)]
> -#define avg2(a,b) uiclp[(((int)(a)+(int)(b)+1)>>1)]
> -#define avg4(a,b,c,d) uiclp[(((int)(a)+(int)(b)+(int)(c)+(int)(d)+2)>>2)]

why?


[...]
> -static void apply_vector_2x2(RoqContext *ri, int x, int y, roq_cell *cell)
> +void apply_vector_2x2(RoqContext *ri, int x, int y, roq_cell *cell)

non static functions need a ff_ prefix to prevent name clashes with other
libs
adding this ff_ prefix can be in a seperate patch if you prefer

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

Republics decline into democracies and democracies degenerate into
despotisms. -- Aristotle
-------------- 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/20070528/041bf33c/attachment.pgp>