[FFmpeg-devel] [PATCH] RoQ decoder 4:4:4 update
Michael Niedermayer
michaelni
Mon Jun 4 23:42:25 CEST 2007
Hi
On Mon, Jun 04, 2007 at 02:51:39PM -0400, Eric Lasota wrote:
> Diego Biurrun wrote:
> >Please attach the changes as a single diff, it's far easier to review
> >that way.
> >
> >Diego
>
> Done. <cid:part1.04060503.05030209 at icculus.org>
[...]
> -void ff_apply_vector_2x2(RoqContext *ri, int x, int y, roq_cell *cell)
> +void ff_unpack_roq_cell(roq_cell *cell, roq_mb2 u)
> {
> - unsigned char *yptr;
> + int i;
>
> - yptr = ri->current_frame->data[0] + (y * ri->y_stride) + x;
> - *yptr++ = cell->y[0];
> - *yptr++ = cell->y[1];
> - yptr += (ri->y_stride - 2);
> - *yptr++ = cell->y[2];
> - *yptr++ = cell->y[3];
> - ri->current_frame->data[1][(y/2) * (ri->c_stride) + x/2] = cell->u;
> - ri->current_frame->data[2][(y/2) * (ri->c_stride) + x/2] = cell->v;
> + for(i=0;i<4;i++) {
> + u[0][i] = cell->y[i];
> + u[1][i] = cell->u;
> + u[2][i] = cell->v;
> + }
> }
what is the advantage of pre unpacking cells to 4:4:4 instead of doing
it during rendering?
[...]
> - row_inc = ri->y_stride - 4;
> - c_row_inc = (ri->c_stride) - 2;
> - *yptr++ = y0 = cell->y[0]; *uptr++ = u = cell->u; *vptr++ = v = cell->v;
> - *yptr++ = y0;
> - *yptr++ = y1 = cell->y[1]; *uptr++ = u; *vptr++ = v;
> - *yptr++ = y1;
> + for(cp=0;cp<3;cp++)
> + for(n=0;n<4;n++)
> + {
> + u[cp][offsets[n] ] = base[n][cp][0];
> + u[cp][offsets[n]+1] = base[n][cp][1];
> + u[cp][offsets[n]+4] = base[n][cp][2];
> + u[cp][offsets[n]+5] = base[n][cp][3];
> + }
> +}
{} placement is inconsistant with how its done in the rest of the file
[...]
> + for(x=0;x<sz;x++)
> + out[x] = in[x];
memcpy()
[...]
> -void ff_apply_motion_8x8(RoqContext *ri, int x, int y,
> - int deltax, int deltay)
> +void ff_apply_motion(RoqContext *ri, int x, int y, int deltax,
> + int deltay, int sz)
> {
> - int mx, my, i, j, hw;
> - unsigned char *pa, *pb;
> + int mx, my, cp;
>
> mx = x + deltax;
> my = y + deltay;
>
> /* check MV against frame boundaries */
> - if ((mx < 0) || (mx > ri->avctx->width - 8) ||
> - (my < 0) || (my > ri->avctx->height - 8)) {
> + if ((mx < 0) || (mx > ri->avctx->width - sz) ||
> + (my < 0) || (my > ri->avctx->height - sz)) {
changing 8x8/4x4 to nxn should be in a seperate patch, this is not related
to a 420 -> 444 change
[...]
> +typedef unsigned char roq_mb2[3][4];
> +typedef unsigned char roq_mb4[3][16];
> +typedef unsigned char roq_mb8[3][64];
these do nothing but obfuscate the code
> +
> +typedef struct
> +{
> + int dx, dy;
> +} motion_vect;
unused
[...]
> -
> + int width, height;
these are unused too, please dont submit patches with random meaningless
changes
[...]
> -void ff_apply_vector_2x2(RoqContext *ri, int x, int y, roq_cell *cell);
> -void ff_apply_vector_4x4(RoqContext *ri, int x, int y, roq_cell *cell);
> +void ff_unpack_roq_cell(roq_cell *cell, roq_mb2 u);
> +void ff_unpack_roq_qcell(roq_mb2 *cb2, int *cell_idxs, roq_mb4 u);
> +void ff_enlarge_roq_mb4(roq_mb4 base, roq_mb8 u);
>
> -void ff_apply_motion_4x4(RoqContext *ri, int x, int y, int deltax, int deltay);
> -
> -void ff_apply_motion_8x8(RoqContext *ri, int x, int y, int deltax, int deltay);
> +void ff_apply_motion(RoqContext *ri, int x, int y, int deltax, int deltay, int sz);
> +void ff_apply_vector_2x2(RoqContext *ri, int x, int y, roq_mb2 ucb);
> +void ff_apply_vector_4x4(RoqContext *ri, int x, int y, roq_mb4 ucb);
> +void ff_apply_vector_8x8(RoqContext *ri, int x, int y, roq_mb8 ucb);
reordering of functions
[...]
> @@ -63,16 +62,20 @@
> if((nv2 = chunk_arg & 0xff) == 0 && nv1 * 6 < chunk_size)
> nv2 = 256;
> for(i = 0; i < nv1; i++) {
> - ri->cells[i].y[0] = *buf++;
> - ri->cells[i].y[1] = *buf++;
> - ri->cells[i].y[2] = *buf++;
> - ri->cells[i].y[3] = *buf++;
> - ri->cells[i].u = *buf++;
> - ri->cells[i].v = *buf++;
> + cell.y[0] = *buf++;
> + cell.y[1] = *buf++;
> + cell.y[2] = *buf++;
> + cell.y[3] = *buf++;
> + cell.u = *buf++;
> + cell.v = *buf++;
> + ff_unpack_roq_cell(&cell, ri->unpacked_cb2[i]);
ff_unpack_roq_cell(buf, ri->unpacked_cb2[i]);
> }
> - for(i = 0; i < nv2; i++)
> + for(i = 0; i < nv2; i++) {
> for(j = 0; j < 4; j++)
> - ri->qcells[i].idx[j] = *buf++;
> + qcell.idx[j] = *buf++;
> + ff_unpack_roq_qcell(ri->unpacked_cb2, qcell.idx, ri->unpacked_cb4[i]);
ff_unpack_roq_qcell(, buf,...
[...]
> case RoQ_ID_SLD:
> - qcell = ri->qcells + buf[bpos++];
> - ff_apply_vector_4x4(ri, xp, yp, ri->cells + qcell->idx[0]);
> - ff_apply_vector_4x4(ri, xp+4, yp, ri->cells + qcell->idx[1]);
> - ff_apply_vector_4x4(ri, xp, yp+4, ri->cells + qcell->idx[2]);
> - ff_apply_vector_4x4(ri, xp+4, yp+4, ri->cells + qcell->idx[3]);
> + ff_apply_vector_8x8(ri, xp, yp, ri->unpacked_cb4_enlarged[buf[bpos++]]);
changing 4 4x4 to 8x8 is not related to a 420->444 change and must thus
be in a seperate patch
[...]
> case RoQ_ID_SLD:
> - qcell = ri->qcells + buf[bpos++];
> - ff_apply_vector_2x2(ri, x, y, ri->cells + qcell->idx[0]);
> - ff_apply_vector_2x2(ri, x+2, y, ri->cells + qcell->idx[1]);
> - ff_apply_vector_2x2(ri, x, y+2, ri->cells + qcell->idx[2]);
> - ff_apply_vector_2x2(ri, x+2, y+2, ri->cells + qcell->idx[3]);
> + ff_apply_vector_4x4(ri, x, y, ri->unpacked_cb4[buf[bpos++]]);
changing 4 2x2 to 4x4 is not related to a 420->444 either
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
In a rich man's house there is no place to spit but his face.
-- Diogenes of Sinope
-------------- 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/20070604/ee639d08/attachment.pgp>
More information about the ffmpeg-devel
mailing list