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

[Chris Double]

Hi ogg.k ogg.k, some drive by comments on the patch from a quick look.

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.
-----------------8<---------------------------
OggPlayErrorCode
oggplay_get_kate_category(OggPlay *me, int track, const char** category);

OggPlayErrorCode
oggplay_get_kate_language(OggPlay *me, int track, const char** language);

OggPlayErrorCode
oggplay_set_kate_tiger_rendering(OggPlay *me, int track, int use_tiger);

OggPlayErrorCode
oggplay_overlay_kate_track_on_video(OggPlay *me, int kate_track, int
video_track);
-----------------8<---------------------------

Typo in the comment (is should be if):

+  unsigned char   * rgba; /* may be NULL is no alpha */

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
it'd crash anyway due to buffer being NULL when float_to_short_array
is called:

+    buffer = realloc(buffer, size * sizeof (short) * channels);

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
following usually indexes into decode_data by the track number which
is an error if the track == num_tracks I think.

+  if (track < 0 || track >= me->num_tracks) {
+    return E_OGGPLAY_BAD_TRACK;

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):

+    orecord = (OggPlayOverlayRecord*)malloc (size);
+    oggplay_data_initialise_header((OggPlayDecode *)decode,
&(orecord->header));

NULL check needed here:

+      record = (OggPlayOverlayRecord*)calloc (1, size);
+      record->header.samples_in_record = 1;

Chris.