[ogg-dev] libogg2 issue in revision 10730

[Marco Rossini]

hi all

I found that in the revision 10730 of the libogg2 library it is
impossible to do bitpacking. this is due to the implementation of the
(at least) two functions oggpack_writeinit() and oggpack_readinit().
they both take an (oggpack_buffer *) as an argument and immediately
erase all it's contents:

void oggpack_readinit(oggpack_buffer *b,ogg_reference *r){
  memset(b,0,sizeof(*b));

  b->tail=b->head=r;
  b->count=0;
  b->headptr=b->head->buffer->data+b->head->begin;
  b->headend=b->head->length;
  _span(b);
}

A program using the libogg2 library cannot know the size of an
oggpack_buffer, so you cannot define such a variable (only the pointer).
This is because the structure definition is hidden in ogginternal.h,
which is not included. Therefore when trying to do some bitpacking by
writing

oggpack_buffer * bs;
oggpack_readinit(bs, op->packet);

you immediately get a segmentation fault which really is undesirable.

However, I could be totally wrong as I might have overlooked something.

My suggestion is this: Just do a malloc() on the condition that b is
NULL:

void oggpack_readinit(oggpack_buffer *b,ogg_reference *r){
  if (!b)
    b = _ogg_malloc(sizeof(oggpack_buffer));
  memset(b,0,sizeof(*b));

  b->tail=b->head=r;
  b->count=0;
  b->headptr=b->head->buffer->data+b->head->begin;
  b->headend=b->head->length;
  _span(b);
}

That way you have to initialize your b before calling the function
though (like oggpack_buffer * b = NULL;), but you wouldn't have to
reveal the structure itself (enabling the definition of such a struct).

[I see, I can't make a short speech...]

While we're at it: I found that the function

extern int     ogg_page_getbuffer(ogg_page *og, unsigned char **buffer);

is declared in ogg.h but not defined. (I am glad however to find such a
function; earlier it did not exist. This should be possible for packets
too for reading and writing because it's faster if you don't write
single bits.)

Yet another thing: The only two structures whose definition is known to
the program are ogg_packet and ogg_page. There is a simmilar problem
with those like the one above: If uninitialized, you get segmentation
faults. You always have to memset(&og, 0, sizeof(ogg_page)) them before
using them. I think it would be alot more convenient to have some init
functions that do just that.

	marco