[MPlayer-dev-eng] [PATCH] libass support for VDPAU

[Reimar Döffinger]

On Fri, Feb 06, 2009 at 04:16:39PM -0500, Eric Appleman wrote:
> Diego Biurrun wrote:
>> On Mon, Feb 02, 2009 at 01:17:05PM -0500, Eric Appleman wrote:
>>   
>>> If ass support for vdpau doesn't make the cut, I am going to cry.
>>
>> Better start working on it right away then.  The release date is as
>> hard a deadline as you can imagine.
>   
> I don't think Grigori posted this patch to the list yet.

I don't know how much point there is in reviewing this while vdpau is
not yet in, but how is the speed of this?
Having many hundreds of tiny textures cause serious performance issues
with -vo gl.

> +#define EOSD_MAX_SURFACES 1000

There should not be a fixed limit.

> +        renderColor.alpha = 1.0 - (float)((c >> 0) & 0xff) / 255;
> +        renderColor.blue = (float)((c >> 8) & 0xff) / 255;
> +        renderColor.green = (float)((c >> 16) & 0xff) / 255;
> +        renderColor.red = (float)((c >> 24) & 0xff) / 255;

You can avoid the cast by just using 255.0 instead of 255.
Also this should be aligned nicely.

> +    
> +    if (eosd_render_count > 0) {
> +        draw_eosd();
> +    }

Move the check into draw_eosd, this simplifies updating the OSD from
multiple places if it is ever needed.

> +// Round up to the next power of two
> +static uint32_t round_pot(uint32_t x) {
> +    x--;
> +    x |= x >> 1;
> +    x |= x >> 2;
> +    x |= x >> 4;
> +    x |= x >> 8;
> +    x |= x >> 16;
> +    x++;
> +    
> +    return x;
> +}

Why does everyone want to do this via bit-fiddling?
It's more and more complex code, speed does not matter
there's a good chance it isn't any faster anyway.

> +    // Nothing changed, no need to redraw
> +    if (imgs->changed == 0)
> +        return;
> +    eosd_render_count = 0;
> +    // There's nothing to render!
> +    if (!img)
> +        return;

You should avoid the putbits for the
imgs->changed == 1 case

> +            ax = round_pot(i->w);
> +            ay = round_pot(i->h);

Why? I expected the hardware all supports non-power-of-two since a long
time.
If you really _have_ to use power-of-two, be very careful that there are
no issues due to the not-initialized parts of the surface.
That's all for a first quick review.

Greetings,
Reimar Döffinger