[FFmpeg-devel] Indeo3 replacement, take 3

[Vitor Sessak]

Maxim wrote:
> Vitor Sessak schrieb:
>> Maxim wrote:
>> [...]
>> It is probably missing something like
>>
>>>     if (ctx->frame.data[0])
>>>         avctx->release_buffer(avctx, &ctx->frame);
>> in free_frame_buffers()
> 
> Thanks! Just added that release_buffer and the error message disappered!
> 
>>> - compiling with AltiVec enabled on my PPC G5 produces wrong checksums
>>> on delta frames. A brief check has shown that "copy_cell" function using
>>> DSP util's dsp.put_no_rnd_pixels_tab[0][0]" and
>>> dsp.put_no_rnd_pixels_tab[1][0] causes that problem. It just works fine
>>> with AltiVec disabled. Could someone skilled in the art help me with it?
>> I have a guess:
>>
>> Your code does:
>>
>>>     luma_pitch   = FFALIGN(luma_width,   8);
>>>     chroma_pitch = FFALIGN(chroma_width, 8);
>> [...]
>>
>>>         ctx->planes[p].pitch  = (!p ? luma_pitch  : chroma_pitch);
>>>         ctx->planes[p].width  = (!p ? luma_width  : chroma_width);
>>>         ctx->planes[p].height = (!p ? luma_height : chroma_height);
>> [...]
>>
>>>     /* copy using 16xH blocks */
>>>     for (i = cell->width >> 2; i > 0; src += 16, dst += 16, i--)
>>>         ctx->dsp.put_no_rnd_pixels_tab[0][0](dst, src, plane->pitch, h);
>> And ppc/dsputils_altivec.c says
>>
>>> /* next one assumes that ((line_size % 16) == 0) */
>>> void put_pixels16_altivec(uint8_t *block, const uint8_t *pixels, int
>>> line_size, int h)
>>> {
>> [...]
>>
>>>     c->put_pixels_tab[0][0] = put_pixels16_altivec;
>> So this should be the culprit. Just align to 16 instead of 8.
>>
> 
> Just tried it but it works still wrong on delta frames! I'll try to
> examine that later...

I've found another problem:

ppc/dsputils_altivec.c:

> /* next one assumes that ((line_size % 8) == 0) */
> void avg_pixels8_altivec(uint8_t * block, const uint8_t * pixels, int line_size, int h)
> {
> POWERPC_PERF_DECLARE(altivec_avg_pixels8_num, 1);
>     register vector unsigned char pixelsv1, pixelsv2, pixelsv, blockv;
>     int i;
> 
> POWERPC_PERF_START_COUNT(altivec_avg_pixels8_num, 1);
> 
>    for (i = 0; i < h; i++) {
>        /* block is 8 bytes-aligned, so we're either in the
                      ^^^^^^^^^^^^^^^
>           left block (16 bytes-aligned) or in the right block (not) */

Your patch:

> typedef struct Plane {
>     uint8_t         *buffers[2];
>     uint8_t         *pixels[2]; ///< pointer to the actual pixel data of the buffers above
>     uint32_t        width;
>     uint32_t        height;
>     uint32_t        pitch;
> } Plane;

[...]

> /**
>  *  Copy pixels of the cell(x + mv_x, y + mv_y) from the previous frame into
>  *  the cell(x, y) in the current frame.
>  */
> static void copy_cell(Indeo3DecodeContext *ctx, Plane *plane, Cell *cell)
> {
>     int     h, i, mv_x, mv_y, offset;
>     uint8_t *src, *dst;
> 
>     /* setup output and reference pointers */
>     offset  = (cell->ypos << 2) * plane->pitch + (cell->xpos << 2);
>     dst     = plane->pixels[ctx->buf_sel] + offset;
>     mv_y    = cell->mv_ptr[0];
>     mv_x    = cell->mv_ptr[1];
>     offset += mv_y * plane->pitch + mv_x;
>     src     = plane->pixels[ctx->buf_sel ^ 1] + offset;
> 
>     h = cell->height << 2;
> 
>     /* copy using 16xH blocks */
>     for (i = cell->width >> 2; i > 0; src += 16, dst += 16, i--)
>         ctx->dsp.put_no_rnd_pixels_tab[0][0](dst, src, plane->pitch, h);
>     /* copy using 8xH blocks */
>     if (cell->width & 2) {
>         ctx->dsp.put_no_rnd_pixels_tab[1][0](dst, src, plane->pitch, h);
>         src += 8;
>         dst += 8;
>     }

Note that since plane->pixels[] is not aligned, dst is not aligned 
neither. So I'd suggest something on the lines of

> typedef struct Plane {
>     uint8_t         *buffers[2];
>     DECLARE_ALIGNED_16(uint8_t, *pixels[2]); ///< pointer to the actual pixel data of the buffers above
>     uint32_t        width;
>     uint32_t        height;
>     uint32_t        pitch;
> } Plane;

-Vitor