On Fri, May 24, 2019 at 21:51:35 +0900, Tetsuya Isaki wrote: > At Thu, 23 May 2019 17:08:42 +0300, > Valery Ushakov wrote: > > > +#if defined(__GNUC__) > > > +/* gcc defines '>>' as ASR. */ > > > +#define AUDIO_ASR(value, shift) ((value) >> (shift)) > > > +#else > > > +#define AUDIO_ASR(value, shift) ((value) / (1 << (shift))) > > > +#endif > > > > This feels inverted. The mathematically correct operation required > > here is symmetric division (that rounds towards zero). Arithmetic > > shift is flooring division (that rounds towards minus infinity). > > Both are not correct but are acceptable. > > # I also used to feel that division is the correct operation. > # But I don't think so now. > > For example, let's consider to convert 16bits to 15bits. > In case of round-to-zero, > 32767, 32766 => 16383 > : > 3, 2 => 1 > 1, 0 => 0 <---- *1 > -1 => 0 <---- *1 > -2, -3 => -1 > : > -32766, -32767 => -16383 > -32768 => -16384 <---- *2 > > In case of round-to-minus-inf, > 32767, 32766 => 16383 > : > 3, 2 => 1 > 1, 0 => 0 > -1, -2 => -1 > : > -32767, -32768 => -16384 > > As above, in round-to-zero, there are three inputs that output 0 > (*1) and there are only one input that output -16384 (*2). > In contrast, in round-to-minus-inf, all outputs correspond to two > input values. > > Which is "correct"?
Speaking in the abstract: The operation that happens here is rescaling, as far as I understand. Yes, the two-complement ranges are lopsided with an extra value on the negative side, but that is far less important, I think, than commuting with negation. If you negate all samples, you have basically the same audio https://manual.audacityteam.org/man/invert.html ("invert" is a bit unfortuante as a name b/c of the confusion with the bitwise operation), so it's nice to have: -scaleN(sample) = scaleN(-sample) > The correct operation is not exist whenever rounding to integer > occurs. > > And in audio area, we need to understand that both rounding > are not correct but are acceptable. I think you are confusing correctness and precision here. > Furthermore, we have to consider that this operation is performed > in the bottom of loop in realtime mixing. If both rounding are > acceptable, I'd like to use faster one. Of course, unless it is > "undefined". > > > So it feels confusing and wrong to *name* the operation the > > opposite of what it really is. > > What I want to do here is arithmetic right shift operation and 2nd > argument in this macro indicates shift count. So I named it ASR. My point is exactly that you are confusing implementation detail that uses right shift as a good enough implementation (which, I reiterate, I don't object to here) and what is the meaning of the operation that you are implementing/approximating (see my first paragraph above). -uwe
