ogg dev - Aug 2008 - liboggplay and overlay video

> I think adding an RGB/RGBA frame type to liboggplay should *also*
> happen. yuv2rgb is tedious at best and hard to get right at worst, so
I'd added an RGBA type, but if this is going to also carry video,
it might be better to merge this with the existing video type, and
have a 'type' enum, or just several pointers, only one set being
non NULL (makes client code simpler). Would it be annoying to
users if the exposed struct was being changed (since I hear that
liboggplay is being used by Firefox) ? At the moment, changes
are limited to additions to the API.

Thanks

On Wed, Aug 20, 2008 at 2:27 AM, ogg.k.ogg.k at googlemail.com
<ogg.k.ogg.k at googlemail.com> wrote:
> I'd added an RGBA type, but if this is going to also carry video,
> it might be better to merge this with the existing video type, and
> have a 'type' enum,
Right now, OggPlayVideoData is just a struct with three arbitrarily
named pointers; it doesn't even know its own dimensions. I think
having several collections of arbitrary pointer sets which may be null
depending on a type field might be confusing. You could do something
fancy with unions, but since RGB/RGBA is generally chunked and YUV is
generally planar, having two separate types makes more sense. There's
also the 'array of plane pointers' approach, but I find
frame->data[2]
hard to read.

Having the alpha channel be a separate plane and optionally NULL in
the YUV and RGB types would work though.

typedef struct {
  unsigned char *y;
  unsigned char *u;
  unsigned char *v;
  unsigned char *mask; /* may be NULL */
} OggPlayVideoYUVData;

typedef struct {
  unsigned char *data; /* may be RGB or RGBA */
  unsigned char *mask; /* may be NULL if RGBA */
} OggPlayVideoRGBData;

That sort of thing.

 -r

> fancy with unions, but since RGB/RGBA is generally chunked and YUV is
> generally planar, having two separate types makes more sense. There's
OK
> typedef struct {
>   unsigned char *data; /* may be RGB or RGBA */
>   unsigned char *mask; /* may be NULL if RGBA */
> } OggPlayVideoRGBData;
That's what I have at the moment - two types and something like
the above, except that :

typedef struct {
  unsigned char *rgb; /* may be NULL if alpha */
  unsigned char *rgba; /* may be NULL if no alpha */
} OggPlayOverlayData;

(I think it's probably faster to keep RGBA in one plane, so memcpy
works if needed, etc, though no hard numbers to back it up).

If I go this way, I think the question of whether the API is supposed
to be not changed is moot, so it's all good.

Cheers