[ogg-dev] liboggplay: RGBA overlay video, rendering with libtiger

[ogg.k.ogg.k at googlemail.com]

> Should these be wrapped in HAVE_KATE? Same with the definition of the
> functions on oggplay.c? If I'm not building with Kate support it would
> prevent dead code from being around.

They could, but would change the API, and the implementation just
returns a "not implemented" error currently if HAVE_KATE is undefined,
(and a couple checks for validity) so it's not much dead code at all.
If the 'moving target API' isn't a problem I can change it so.

> In glut_player.c, If realloc fails then it returns NULL and this will
> overwrite the existing value of 'buffer' with null. That 'buffer' will
> then never be free'd and result in a memory leak. Although I suspect

Good point. Common lazy programer idiom. Will fix.

> In oggplay.c there are checks like the following that do 'track >=
> me->num_tracks' and at least one that does 'track > me->num_tracks'. I
> think it should be '>' not '>=' in all cases, what do you think? Code

I think it should be >= all the time. I'll check those out.

> Check for NULL return from malloc needed here (the sample problem
> occurs in existing code with 'record' but a bug has been raised about
> that):

Yes, adapted from existing, will change.

> NULL check needed here:
>
> +      record = (OggPlayOverlayRecord*)calloc (1, size);
> +      record->header.samples_in_record = 1;

OK

Thanks for the comments!