[pulseaudio-discuss] Updated volume ramping feature - patch 1

[Lennart Poettering]

On Thu, 16.07.09 10:48, Zheng, Huan (huan.zheng at intel.com) wrote:

I am very sorry for the much too long delay in reviewing this! Busy
with prepping PA for 0.9.16 and F12. I am really sorry for that.

> I modified the volume ramping feature per your suggestions.
> 1, rewind envelop when rewinding sink-input. And keep the envelop around until i->sink->max_rewind is processed after the envelop is finished.
> 2, make all envelop manipulation inside the IO context, main thread just set the pa_envelope_def and then send out SET_ENVELOPE message to let IO thread create the envelop. And this will make it possible to create envelope in other places, not just inside set_volume_with_ramping set_mute_with_ramping.
> 3, future_volume, future_mute is introduced to avoid distortion before envelop takes effect, e.g. when unmute, if you change the thread_info.muted immediately, you will hear a short loud sound before ramping up takes effect, future_mute could avoid this issue.
> 4, do not apply sink's volume when setting ramp info. Since it does
> not help a lot to avoid calculation overflow.

Looks pretty good.

One comment on the first patch:

Regarding your change in linear_get_int(): you convert to float
here. This is problematic. The whole reason we implement the envelope
in both ints and floats is that we can be able to do enevlope
adjustmens without any FPU opcodes when looking at integer samples. I
am pretty sure there's a good reason why you added the "float" here,
but this probably should be solved differently.

(Also, as a side note: in normal calculations it seldomly makes sense
to use the float type since no real-life CPU implements a native type
for that. CPU's whith FP units tend to implement double as native type
and just convert float to and from it for each operation. Using 32bit
floats only makes sense in array, i.e. where you have bulk data to
process. So if you process 32bit float samples it makes sense to do
the operations in 32bit floats. But if you are just converting from
integer as temporary step it's probably a better idea to use doubles.)

Otherwise this patch looks very good to me, though we should probably
add comments explaining in envelope.h which function may be called
from which thread.

I'll comment on the other patches seperately.

Lennart

-- 
Lennart Poettering                        Red Hat, Inc.
lennart [at] poettering [dot] net
http://0pointer.net/lennart/           GnuPG 0x1A015CC4