Hi Tanu, Tanu Kaskinen wrote: > On Wed, 2013-09-18 at 23:09 -0500, Hajime Fujita wrote: >> Hi Tanu, >> >> Thank you so much for your advice. >> >> Tanu Kaskinen wrote: >>> On Sun, 2013-09-15 at 00:24 -0500, Hajime Fujita wrote: >>>> For those who are interested in raop2 experiments, >>>> >>>> As Matthias pointed out [1], current volume calculation is incorrect. >>>> https://bugs.freedesktop.org/show_bug.cgi?id=42804#c43 >>>> >>>> I have uploaded a patch which calculates volume in more reasonable way. >>>> https://github.com/hfujita/pulseaudio-raop2/commit/69f3ce883dfb93eb7b24f98b9664e14071672475 >>>> >>>> First I have to confess that this is the first time for me to deal with >>>> the unit 'dB', so maybe I'm doing totally stupid. I'd be very happy if I >>>> could have any suggestions from who is more experienced in this field. >>>> >>>> I started looking at pa_raop_client_set_volume() in >>>> src/modules/raop/raop_client.c. It was using pa_sw_volume_to_dB to >>>> calculate dB from a linear scale. >>> >>> Correction: pa_volume_t doesn't use a linear scale, it uses a cubic >>> scale (see pa_sw_volume_from_linear() and pa_sw_volume_to_linear()). >> >> I see. I should have noticed some comments about cubic scale. >> http://cgit.freedesktop.org/pulseaudio/pulseaudio/tree/src/pulse/volume.c#n251 >> >>> >>>> Then I learned from code reading that pa_sw_volume_to_dB essentially >>>> calculates the following: >>>> 20 * log10((v/MAXVOLUME)^3) = 60 * log10(v/MAXVOLUME) >>>> This does not fit into the volume scale which RAOP is expecting, where >>>> -30.0 db is a kind of minimum [2]. >>>> >>>> [2]: http://git.zx2c4.com/Airtunes2/about/#id29 >>>> >>>> Also I looked at packet dumps from iPad and guessed how it calculated dB >>>> from a linear scale. It looked like iPad was using the following formula. >>>> db = 10 * log10(volume/MAXVOLUME/30) >>>> So for the time being I decided to follow this way. >>> >>> The way to convert pa_volume_t to dB is to use pa_sw_volume_to_dB(). >>> Doing anything else means that the result is not dB, it's something >>> different. >>> >>> The way to handle this is to ensure that the volume that is given to >>> pa_raop_client_set_volume() is never outside the supported range. In >>> module-raop-sink.c there's sink_set_volume_cb(), which calls >>> pa_raop_client_set_volume(). In sink_set_volume_cb() there's this line: >>> >>> pa_cvolume_set(&hw, s->sample_spec.channels, v); >>> >>> Before that line you should check what dB value v corresponds to, and if >>> it's outside the supported range, then you should modify v to fall into >>> the supported range. Any modification that you make to v will be >>> compensated with software volume, so the modifications shouldn't affect >>> the resulting volume at all, assuming that the RAOP device actually >>> attenuates the signal by the dB amount that you send to it. >>> >> >> According to your advice, I inserted an adjustment to v before >> pa_cvolume_set. Here's the patch. >> https://github.com/hfujita/pulseaudio-raop2/commit/c6337c0fef7710ee20b42ed3fb87ec12910ce35a >> >> Adjustment goes like this (pa_raop_client_adjust_volume()): >> adj(v) = v * (1 - minv/maxv) + minv; >> where pa_sw_volume_to_dB(minv) = -30.0, >> and pa_sw_volume_to_dB(maxv) = 0.0 >> >> The idea is that when the original v is 0, adj(0) = minv. And when the >> original v is maxv, adj(maxv) = maxv; >> >> Does this make sense? > > The idea was to simply adjust v to -30 dB if the original v is less than > that. There's no need to do any complicated calculations. Do you mean something like v = max(minv, v); /* where minv corresponds to -30.0dB */ ? If I do that, resulted volume will be almost muted in the range of 0% <= v <= 31%, then suddenly goes up beyond 31%. (31% is where dB becomes -30.0) I was afraid that this was not an intuitive behavior to users. >> As you noted, this also affects the software volume, and in my >> environment, the actual volume (volume heard from the speaker) may be >> way too small. > > What do you mean by way too small? The maximum volume will be the same > as before, but the minimum volume will be quieter than before. With the > original code you complained that you need to keep the volume at rather > low levels, so extending the scale towards low volumes should help with > this. Well, this could be just my personal feeling. Since it was a bit loud before (however it was almost the same volume level as iPad), this time I felt it was way too small. I wonder if it makes sense to 1) adjust v as adj(v) = v * (1 - minv/maxv) + minv; and 2) use the original v for pa_cvolume_set(), then 3) use adj(v) for setting the actual RAOP device volume. Under this condition, the calculated software volume always becomes 100%, as in the current (mainline) raop module. Thanks, Hajime
