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
_______________________________________________
pulseaudio-discuss mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Reply via email to