[pulseaudio-discuss] [PATCH 1/5] core: Add infrastructure for synchronizing HW and SW volume changes

[Colin Guthrie]

'Twas brillig, and Tanu Kaskinen at 15/10/10 11:30 did gyre and gimble:
> On Fri, 2010-10-15 at 13:00 +0300, Jyri Sarha wrote:
>>>> @@ -1459,7 +1493,8 @@ void pa_sink_set_volume(
>>>>           * apply one to s->soft_volume */
>>>>
>>>>          pa_cvolume_reset(&s->soft_volume, s->sample_spec.channels);
>>>> -        s->set_volume(s);
>>>> +        if (!(s->flags & PA_SINK_SYNC_VOLUME))
>>>> +            s->set_volume(s);
>>>>
>>>>      } else
>>>>          /* If we have no function set_volume(), then the soft volume
>>>
>>> Hmm, should this "if" have an "else" that sets send_msg to TRUE
>>> (send_msg is used just below where the context ends) otherwise if
>>> send_msg is set to false, then the message will not be pushed to the
>>> asyncq and thus the set_volume() function will never be called in
>>> pa_sink_process_msg()[2]
>>>
>>
>> It looks like you are right. I am only wondering how the code
>> still seems to work. If I understand correctly the same problem
>> is there also without sync-volume if sink updates soft volume
>> within set_volume(). The updated soft volume would not be
>> propagated to IO-thread.
> 
> No, soft volume syncing is not the problem, the problem is that in sync
> volume mode set_volume is not called at all when send_msg is FALSE. In
> all cases where send_msg is FALSE, the soft volume syncing is handled
> differently - for example, when moving a stream,
> pa_sink_input_finish_move() first calls pa_sink_set_volume() with
> send_msg as FALSE, and after that it sends PA_SINK_MESSAGE_FINISH_MOVE.
> The soft volume syncing is done by the PA_SINK_MESSAGE_FINISH_MOVE
> handler.

I've committed this patch now, but thinking about this in a bit more
depth, is simply setting send_msg = true the right thing to do or should
there be more structure in place to deal with this flag?

AFAICT it's fine the way it is, but as you've got a better idea of
what's going on here than me, I figured I'd ask :D

Col



-- 

Colin Guthrie
gmane(at)colin.guthr.ie
http://colin.guthr.ie/

Day Job:
  Tribalogic Limited [http://www.tribalogic.net/]
Open Source:
  Mageia Contributor [http://www.mageia.org/]
  PulseAudio Hacker [http://www.pulseaudio.org/]
  Trac Hacker [http://trac.edgewall.org/]