(no subject)

[Monty]

>   In bitvise.c there must be buffer cleared after malloc:

You found an initialization bug, but not as big as you think (I've fixed it in
CVS).

The buffer was in fact not initialized properly. However, one only has to zero
the first byte.  If you trace the code, you'll notice that the end byte after
the base case is never uninitialized.

> void _oggpack_writeinit(oggpack_buffer *b){
>   memset(b,0,sizeof(oggpack_buffer));
>   b->ptr=b->buffer=malloc(BUFFER_INCREMENT);
> +  memset(b->ptr,0,BUFFER_INCREMENT);

This should just be b->buffer[0]='\0'; as is done in _oggpack_reset();

>   b->storage=BUFFER_INCREMENT;
> }
> 
> void _oggpack_write(oggpack_buffer *b,unsigned long value,int bits){
>   if(b->endbyte+4>=b->storage){
>     b->buffer=realloc(b->buffer,b->storage+BUFFER_INCREMENT);
> +    memset(b->buffer+b->storage,0,BUFFER_INCREMENT);

Not needed.

> ////////////////////////////////////////////////////
> // { because of this
>   b->ptr[0]|=value<<b->endbit;  
> // }
> ////////////////////////////////////////////////////

If you look further, you'll see that every byte past the first *is*
initialized.  It does not need to be zeroed:

  if(bits>=8){
    b->ptr[1]=value>>(8-b->endbit);  
    if(bits>=16){
      b->ptr[2]=value>>(16-b->endbit);  
      if(bits>=24){
        b->ptr[3]=value>>(24-b->endbit);  
        if(bits>=32){
          if(b->endbit)
            b->ptr[4]=value>>(32-b->endbit);
          else
            b->ptr[4]=0;

The only initialization needed is the very first byte after the initial malloc.

> 2.)
> undefined under win32 so define them:
> 
> #define M_PI (3.14159265359)

#ifndef M_PI
#define M_PI (3.14159265359)
#endif

:-)  I'll get that in too.  Despite the ifdef, it's not that ugly.

> #define rint(x) (floor((x)+0.5)) // is this correct?

No; rint() returns a double, so that doesn't produce an interger-valued double.
What you were trying to do is not exactly right either, but it probably doesn't
matter in our use of it. Nothing in Vorbis is sensitive to rounding quirks. 

There's really no rint() in MSVC?  

> 3.)
> thousands of tons things like that had become
> 
> double out[winsize/2];
> to
> double *out=(double *) _alloca((winsize/2)*sizeof(double));

Yeah, knew about these, sorry (it's a GCC-ism).  Alot of those are in
temporary code, so I hadn't been stressing.  BTW, is there a reason you use
_alloca() instead of alloca()?  I would expect MSVC++ to have the standard
alloca() call.  My preference is to write neutral code that compiles everywhere
without dealing in minor platform-specific modifications everywhere.  I'm
violently allergic to #ifdef.

> 4.)
> I found different behaviour between gcc and msvc when dividing very small doubles (like 2.13e-312)
> 
> 
> void _vs_residue_quantize(double *data,double *curve,
> 				 vorbis_info *vi,int n){
> 
>   /* The following is temporary, hardwired bullshit */
>   int i;
> 
>   for(i=0;i<n;i++){
> 
>     int val=rint(data[i]/curve[i]);
> 
> if curve[i] is too small (zero?), under gcc val==-2147483648 (-INF?), under msvc val==0

This one is weird...

First, there shouldn't be any values that small.  Maybe this is something I
missed in handling silence? (It doesn't matter if amp==0, but I should have
short-circuited encoding in that case and not gotten that far).

The values you're reporting above are longs, and the storage is double.
Could you doublecheck?

What are the values in amp[] at that point (these are also doubles)?
if the amp[] isn't zero, could you make a quick dump of the data values into a
text file (as doubles) and mail them along to me?

Thanks,

Monty

--- >8 ----
List archives:  http://www.xiph.org/archives/
Ogg project homepage: http://www.xiph.org/ogg/