Sorry, a mistake in the macro below:
On 22.06.2017 14:50, Seppo Ingalsuo wrote:
On 22.06.2017 12:02, Takashi Iwai wrote:
On Thu, 22 Jun 2017 10:47:54 +0200, Seppo Ingalsuo wrote:
On 22.06.2017 11:28, Takashi Iwai wrote:
On Thu, 22 Jun 2017 10:04:27 +0200, Seppo Ingalsuo wrote:
On 22.06.2017 04:31, Keyon Jie wrote:
On 2017年06月22日 01:12, Seppo Ingalsuo wrote: > The mono tone level can be controlled per channel. A similar > channel map > feature as in the volume component was added into volume command of > tone. > > The default tone frequency of 997 Hz was updated to a proper Q16.16 > value. > > Moved tone function pointer set to tone_new() since it is not > modified > later. > > Signed-off-by: Seppo Ingalsuo seppo.ingalsuo@linux.intel.com > --- > src/audio/tone.c | 56 > ++++++++++++++++++++++++++++++++++++++++++-------------- > 1 file changed, 42 insertions(+), 14 deletions(-) > > diff --git a/src/audio/tone.c b/src/audio/tone.c > index f45d8be..d1c7468 100644 > --- a/src/audio/tone.c > +++ b/src/audio/tone.c > @@ -56,7 +56,8 @@ > #define TONE_NUM_FS 13 /* Table size for > 8-192 > kHz range */ > #define TONE_AMPLITUDE_DEFAULT 2147484 /* -60 dB from full > scale */ > -#define TONE_FREQUENCY_DEFAULT 16334848 /* 997 in Q18.14 */ > +#define TONE_FREQUENCY_DEFAULT 65339392 /* 997 Hz in Q16.16 */ > +#define TONE_VOLUME_DEFAULT 65536 /* 1.0 in Q16.16 */ will they be more readable in hexadecimal?
Unfortunately the C preprocessor doesn't process float to fixed math so I can't use any nice macro to create these. In hex it would look like this and in my opinion it's not any better. The Q notation is only sometimes by chance such that integer and decimal bits are not mixed into same hex digit. For frequency Qx.y notation the used x is sufficient to get to ultrasound frequencies and y to make precise enough sweeps.
#define TONE_AMPLITUDE_DEFAULT 0x20C49C /* -60 dB from full scale */ #define TONE_FREQUENCY_DEFAULT 0xF940000 /* 997 Hz in Q18.14 */ #define TONE_FREQUENCY_DEFAULT 0x3E50000 /* 997 Hz in Q16.16 */ #define TONE_VOLUME_DEFAULT 0x10000 /* 1.0 in Q16.16 */
To convert these with any calculator to floats when debugging code (these are fractional numbers) there's extra effort needed with hex so I definitely prefer decimal. Hex is good for bit masks but these are not such.
It's a fixed-point, so you can introduce a simple macro like
#define TONE_FREQ_SHIFT 16 #define TONE_FREQ(x, y) ((x) << TONE_FREQ_SHIFT | (y))
then TONE_FREQUENCY_DEFAULT will be TONE_FREQ(997, 0), and so on. If you need to change the base shift, just change the TONE_FREQ_SHIFT and the rest remains as is.
Yes, that works for integers and it is readable but the needed frequencies are often decimal numbers in Hz. E.g. C5 note is 523.251 Hz or if a sweep is started from 4th third octave band the frequency would be 31.5 Hz.
Yeah, the decimal point is tricky.
So I myself do with Octave that I use as pocket calculator
printf('%d\n',round(523.251*2^16))
34291778
printf('%d\n',round(31.5*2^16))
2064384
printf('%d\n',round(10^(-60/20)*2^31))
2147484
I thought C compiler (not preprocessor) should optimize the conversion and handle as an integer constant.
#define TONE_FREQ(f) (int)((f) * (2 << 16)) /* f = float */
and int x = TONE_FREQ(523.251);
should be converted to
int x = 34291778;Thanks, that works. At least with the compiler that I use (http://alsa-project.org/main/index.php/Firmware) the optimizing does the conversion to fixed without increase of the firmware image size. I also checked from disassembly that this part of the code remains identical. I did a minor fix and added rounding into the macro since compiler doesn't round the cast to int:
#define TONE_FREQ(f) (int)((f) * (1 << 16)) /* f = float */
#define TONE_FREQ(f) (((int)((f) * (1 << 17)) + 1) >> 1) /* f = float */
Sorry, the rounded version overflows and depending on the way 20000 vs. 20000.0 as the parameter the compiler may even pass it silently. However this way the rounded Q16.16 conversion seems to work:
#define TONE_FREQ(f) (int)(((int64_t)((f) * 131072.0 + 1.0)) >> 1) /* f = float */
If there's no concern from other people who know better the compilers I'm OK to make this change to increase readability and help avoiding mistakes with frequencies.
Seppo
Takashi