On 05/14/2013 11:46 AM, Tanu Kaskinen wrote:
> On Tue, 2013-05-14 at 10:31 +0200, David Henningsson wrote:
>> This bug [1] is still reported as present in 3.0, so I tried to track it
>> down, but I'm not sure what the best fix is. Here's where it crashes:
>>
>> module-stream-restore.c:
>> if (sink_input->save_volume &&
>> pa_sink_input_is_volume_readable(sink_input)) {
>> pa_assert(sink_input->volume_writable);
>>
>> According to this code, volume_writeable must be true if save_volume is
>> true. It apparently isn't always so.
>>
>> But what is the right solution?
>> 1) Ignore volume_writable, i e, just remove the assertion, or
>> 2) If volume_writable is false, skip saving the volume, but not mute
>> and route, or
>> 3) If volume_writable is false, skip saving the sink input completely
>>
>> Or, should one
>> 4) Change everywhere that sets save_volume to "true" to also check for
>> volume_writable and if so never set save_volume to true?
>>
>> I'm not sure what the thoughts are behind these two variables, so
>> looking for advice here.
>
> If volume_writable is false, it doesn't make sense to save the volume,
> because the volume should only be saved when the user sets the volume,
> but the user can't set the volume if it's not writable. So, I think 1 is
> not good. 2 is something that could be considered, but 3 is not. I would
> prefer 4 in some form, but it's bad if every place where save_volume is
> enabled needs to check volume_writable. I think
> pa_sink_input_set_save_volume() would make sense. The function would
> fail if saving the volume is not allowed. The callers could check if the
> function fails, but I don't think that would be necessary in most cases.
Save_volume was not set in very many places, so I believe the just sent
patch would suffice?
--
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic
