[MPlayer-dev-eng] [PATCH] EOSD support for VDPAU (2nd)

[Reimar Döffinger]

On Sun, Feb 22, 2009 at 06:16:25PM +0100, Grigori G wrote:
> +static void eosd_init(void) {
> +    int i;
> +
> +    eosd_surface_count = EOSD_SURFACES_INITIAL;
> +    eosd_surfaces = calloc(eosd_surface_count, sizeof(*eosd_surfaces));
> +    eosd_targets = calloc(eosd_surface_count, sizeof(*eosd_targets));
> +    eosd_initialized = 1;
> +
> +    for (i=0; i<eosd_surface_count; i++)
> +        eosd_surfaces[i].surface = VDP_INVALID_HANDLE;
> +}

I think it's a one-line change to handle this case in generate_eosd...

> +        // None found, allocate a new surface
> +        if (!found) {
> +            for (j=0; j<eosd_surface_count; j++) {
> +                if (eosd_surfaces[j].in_use == 0) {
> +                    if (eosd_surfaces[j].surface != VDP_INVALID_HANDLE)
> +                        vdp_bitmap_surface_destroy(eosd_surfaces[j].surface);
> +                    found = 1;
> +                    vdp_st = vdp_bitmap_surface_create(vdp_device, VDP_RGBA_FORMAT_A8,
> +                        i->w, i->h, VDP_TRUE, &eosd_surfaces[j].surface);
> +                    CHECK_ST_WARNING("EOSD: error when creating surface")
> +                    eosd_surfaces[j].w = i->w;
> +                    eosd_surfaces[j].h = i->h;
> +                    eosd_surfaces[j].in_use = 1;
> +                    eosd_targets[eosd_render_count].surface = eosd_surfaces[j].surface;
> +                    break;
> +                }
> +            }
> +        }
> +        // Allocate new space for surface/target arrays
> +        if (!found) {
> +            int k;
> +            j = eosd_surface_count;
> +            eosd_surface_count *= 2;
> +            eosd_surfaces = realloc(eosd_surfaces, eosd_surface_count * sizeof(*eosd_surfaces));
> +            eosd_targets = realloc(eosd_targets, eosd_surface_count * sizeof(*eosd_targets));

Since realloc is specified to work fine for NULL, you can just add e.g.
if (!eosd_surface_count) eosd_surface_count = EOSD_SURFACES_INITIAL;
or whatever construct you want to use...

> +            vdp_st = vdp_bitmap_surface_create(vdp_device, VDP_RGBA_FORMAT_A8,
> +                i->w, i->h, VDP_TRUE, &eosd_surfaces[j].surface);
> +            CHECK_ST_WARNING("EOSD: error when creating surface")
> +            eosd_surfaces[j].w = i->w;
> +            eosd_surfaces[j].h = i->h;
> +            eosd_surfaces[j].in_use = 1;
> +            eosd_targets[eosd_render_count].surface = eosd_surfaces[j].surface;

This part is duplicated. Also duplicating makes things look more complex
that it is, this part of the code is _always_ executed when the first
loop does not find any candidates.
The structure should be
> search for a surface to reuse
> if none found
>     search for a location to reuse and free existing unsuitable surface
>     if still not found realloc
>     create a new surface in found location (does not matter if we had
>     to realloc or not).

I think the rest is okay.