[ogg-dev] Fwd: Merging jorbis upstream and the cortado jorbis fork back into one

Gregory Maxwell gmaxwell at gmail.com
Thu Nov 4 09:23:17 PDT 2010


Hans de Goede <hdegoede at redhat.com> wrote:
> I've attached a patch with these 2 bugfixes to this mail. Note that this
> patch has the original cortado git headers for reference, but the actual
> patches / diffs are not directly from cortado's git. They have been
> rebased to apply to jorbis-0.0.17

On further review of the patch I believe it may have highlighted an
additional logic error in jorbis.

The libvorbis code contains only one unique ilog2 function:

static int ilog2(unsigned int v){
  int ret=0;
  if(v)--v;
  while(v){
    ret++;
    v>>=1;
  }
  return(ret);
}

The same code is duplicated in several places.

This was incorrectly ported to java, due to java requirements about
conditionals, as:

private static int ilog2(int v){
  int ret=0;
  while(v>1){
    ret++;
    v>>>=1;
  }
  return(ret);
}

In Mapping0.java I corrected this to:

private static int ilog2(int v){
  int ret=0;
  if (v>0)v--;
  while(v>0){
    ret++;
    v>>>=1;
  }
  return(ret);
}


But I didn't realize that there were other copies of this function in
DspState.java, Floor1.java, and Info.java  (matching the duplication
that occurs in libvorbis).  I suspect that these need all be using the
'fixed' function.

Most (all?) of this time this function is used its used to find the
minimum bits required to code some range, so the upward rounding is
required. (especially the floor1 instance).

I've added looking into this to my todo, but it may take me a while to
get around to it.


More information about the ogg-dev mailing list