[pulseaudio-discuss] [PATCH 1/4] change bool save_sink to char *preferred_sink

[Tanu Kaskinen]

On Sat, 2018-12-15 at 16:47 +0800, Hui Wang wrote:
> On 2018/12/12 下午9:39, Tanu Kaskinen wrote:
> > Thanks for working on this! Sorry for slow review, I hope I'll be much
> > quicker to comment on subsequent iterations.
> > 
> > On Mon, 2018-11-05 at 09:47 +0800, Hui Wang wrote:
> > > And don't move the stream in the module-stream-restore anymore,
> > > And the preferred_sink is only set when user is calling the
> > > move_to() and the module-stream-restore maintains the saving and
> > > deleting of preferred_sink.
> > > 
> > > If the target of move_to() is default_sink, preferred_sink will be
> > > cleared and the entry->device will be cleared too from database.
> > Can you split this so that the first patch only changes save_sink to
> > preferred_sink, without any changes in behaviour? That is, put the
> > "don't move the stream in the module-stream-restore" and "if the target
> > of move_to() is default_sink" logic into separate patches.
> > 
> > Also replace tabs with spaces.
> OK, got it, will addressed all comments.
> > > Signed-off-by: Hui Wang <hui.wang at canonical.com>
> [...]
> > >           }
> > >       }
> > > @@ -2176,9 +2190,10 @@ static int extension_cb(pa_native_protocol *p, pa_module *m, pa_native_connectio
> > >   
> > >                   entry->muted = muted;
> > >                   entry->muted_valid = true;
> > > -
> > > -                entry->device = pa_xstrdup(device);
> > > -                entry->device_valid = device && !!entry->device[0];
> > > +		if (device && !pa_streq(device, m->core->default_sink->name) && !pa_streq(device, m->core->default_source->name)) {
> > > +		    entry->device = pa_xstrdup(device);
> > > +		    entry->device_valid = device && !!entry->device[0];
> > > +		}
> > What's the goal here? The client tries to change an entry in the
> > stream-restore database, why should that change be ignored if the
> > current default sink happens to be the same as the new device? Maybe
> > you intended to set entry->device to NULL in this case. But I don't
> > think that's necessary either - if the client wants to unset the
> > device, it can just give NULL as the device name. I don't think you
> > need to change anything here.
> > 
> > By the way, m->core->default_sink can be NULL, so that would have to be
> > checked if this code was kept.
> 
> Actually I didn't change this part first, but I remember the stream bond 
> did not work as expected, after changed as above, it worked as expected.
> 
> Supposing sink0 is hdmi, and is playing a music over sink0, sink1 is 
> speaker,  after I unplugged the hdmi cable, the music is switched to 
> speaker,  but music->preferred_sink is still sink0-hdmi, after I plug 
> the hdmi cable again, the music should be switched back to sink0-hdmi.  
> I remember after I unplugged the cable, a user-space app call 
> extension_cb to set the music->preferred_sink to be sink1-speaker 
> (default_sink), then the music can't be switched back to hdmi anymore.
> 
> I will test it again, if it is really not needed, I will drop these code.

I would guess that it's gnome-control-center that sets the device to
speakers when you select speakers as the output. gnome-control-center
updates the routing in all stream-restore entries. Once these patches
are merged (and once a new release is made), gnome-control-center can
be fixed to not mess with the stream-restore database any more. I
believe the current gnome-control-center behaviour is a workaround for
the fact that PulseAudio hasn't so far automatically moved streams when
the default sink changes.

-- 
Tanu

https://www.patreon.com/tanuk
https://liberapay.com/tanuk