[FFmpeg-soc] [soc]: r369 - jpeg2000/j2kenc.c

[Kamil Nowosad]

Hi

On Fri, Jul 13, 2007 at 09:45:22PM +0200, Michael Niedermayer wrote:
> > > >  static double getwmsedec(int nmsedec, int bandpos, int lev, int bpno)
> > > >  {
> > > > -   static const double dwt_norms[4][10] = {
> > > > -    {1.000, 1.500, 2.750, 5.375, 10.68, 21.34, 42.67, 85.33, 170.7, 341.3},
> > > > -    {1.038, 1.592, 2.919, 5.703, 11.33, 22.64, 45.25, 90.48, 180.9},
> > > > -    {1.038, 1.592, 2.919, 5.703, 11.33, 22.64, 45.25, 90.48, 180.9},
> > > > -    {.7186, .9218, 1.586, 3.043, 6.019, 12.01, 24.00, 47.97, 95.93}};
> > > > +   static const int dwt_norms[4][10] = { // multiplied by 10000
> > > > +    {10000, 15000, 27500, 53750, 106800, 213400, 426700, 853300, 1707000, 3413000},
> > > > +    {10380, 15920, 29190, 57030, 113300, 226400, 452500, 904800, 1809000},
> > > > +    {10380, 15920, 29190, 57030, 113300, 226400, 452500, 904800, 1809000},
> > > > +    { 7186,  9218, 15860, 30430,  60190, 120100, 240000, 479700,  959300}};
> > > >  
> > > > -    double t = dwt_norms[bandpos][lev] * (1 << bpno);
> > > > -    return t * t * nmsedec / 8192.0;
> > > > +    int t = (dwt_norms[bandpos][lev]) * (1 << bpno);
> > > > +    return (double) t * (double) t * nmsedec;
> > > >  }
> > > 
> > > why is the returned values not int64_t or int?
> > > same question for the code which uses getwmsedec() ?
> > 
> > The values returned by this function can reach 2^81 (nmsedec - 2^23,
> > dwt_norms - 2^22, 1<<bpno - 2^7), and they are then summed up for whole
> > code block [about 20 times], and for whole tile (up to 256 code blocks).
> > But in some cases the values can be small. So dividing them by a
> > constant would make, in my opinion, too big precision loss.
> > Maybe there exists a smart solution, but I don't see it.
> 
> i really would like to see the float&double useage decreased, if not
> regression tests will be impossible as we will have different results on
> different compilers and hw
> also the code would be very slow on fpu-less systems
> that said i understand that the 9/7 wavelet is better (and faster on fpu+cpu)
> implemented with
> floats and so iam not objecting to have only a float implementation of that
> also i dont mind if the other wavelets are done with floats but i think
> in general float code should be avoided where possible
> 
> and in this specific case the value represents distortion, a 8bit 4096x4096
> image has a maximum squared distortion which fits in 40bit when a single
> pixel +-1 change can still be represented so i do think 64bit is plenty
> and theres no need or sense in floating point
> adding the maximum of variables which are not able to reach this maximum
> at the same times is not really usefull ...

Ok, so would something like that be good:

#define WMSEDEC_SHIFT 13

and in getwmsedec()

int64_t t = (dwt_norms[bandpos][lev]) * (1 << bpno);
return (t * t >> WMSEDEC_SHIFT) * nmsedec;
?

> also you could add the same dwt_norms cases first and then at the end
> add do the dwt_norms scaling and adding this would be more accurate
> with double as well as int

But dwt_norms are multiplied by 1<<bpno which changes in consecutive
coding passes, and the exact distortion values are needed, because they
are stored for each coding pass and minimum and maximum rate /
distortion slope on each coding pass (tile->minrd, maxrd) is computed.
Maybe i've understood you wrong, but i don't see where i could make such
changes.

-- 
Best regards,
Kamil Nowosad