GNOME Bugzilla – Bug 581922
Need to show LFE slider where supported
Last modified: 2009-05-20 11:47:27 UTC
On top of Value, Balance and Fade sliders we unfortunately also need a seperate LFE (aka Subwoofer) slider. Neither Balance nor Fade influence the LFE, hence right now all channels can be configured by one way or the other, except for LFE.
That would be a simple extension of what we did for the fade slider, without the architectural changes, so it should be pretty straight forward.
Except that it's not a balance, but a volume, so we'd need to change gvc-channel-bar instead.
Created attachment 134566 [details] [review] gvc-lfe-1.patch Add an LFE slider when the sink supports it.
Created attachment 134581 [details] [review] gvc-lfe-2.patch With cleanups, and mouse scroll fixed up.
I think most of the feedback issues are fixed with these patches: http://0pointer.de/public/gvc/ (only the first patch does the LFE stuff (and is just an updated version of your patch), the other ones fix more feedback issues. I am a lazy person, so I haven't tested if I seperated the patches properly, that's why I am not opening three more bugs for the other three patches. It's late, I need to go to bed.) I am note sure if really all feedback issues are entirely fixed now. I got a bit lost in all that signal emission jungle between GvcMixerStream and GvcChannelMap. But at least they don't have any ill effects anymore AFAICS. Now, after this has been implemented I am not actually sure anymore that exposing the absolute LFE channel 1:1 in the UI is really a good idea. It might make more sense to expose it relative to the overall volume. I.e. if the slider is left of a specific marker the LFE will be lower in volume then the main speakers. If it is right of that marker it is higher in volume then the main speakers. Implementing such a relative LFE slider is a bit more complex though and I'd probably need to add more functions to the PA API to make this easy. The main missing calls would be similar to pa_cvolume_max()/pa_cvolume_scale() but which ignore the LFE. Or something like that. I need to think about that a bit more. For now I think the "absolute LFE" stuff we have with these patches is good enough. Moving to "relative LFE" we can do later. One last thought: it might make sense to move the 'operation running' logic from the sink/source/sink input mixer objects, and move them to GvcMixerStream, since they all three implement mostly the same logic in this respect.
As mentioned on IRC, feel free to commit those. I'm not too happy about patch 2, as it exposes PA datatypes in the headers, and I wanted to keep that to a minimum. But given that it's only used internally, that'll have to do. I'll modify the _is_running code to be shared when you've committed all this.
Commited. Thanks.
One fix down. commit 44591b9a4dc3935f8f9bf02c16b19717b505fd3a Author: Bastien Nocera <hadess@hadess.net> Date: Fri May 15 17:18:32 2009 +0100 Fix crash on startup restoring the event-role stream The event-role stream is the only GvcMixerStream class that isn't attached to a sink or source per se, so, because of the recent volume changes by Lennart, we need to attach a fake channel map to it, so volume can be propagated. That leaves the problem of the application stream volumes jumping to 100% before being restored to their correct values, and the is_running merge. Patch for the latter below.
Seemed to work fine, so Committed. commit a6038b152036cb3e1eca21a6b4bd749a15019fc9 Author: Bastien Nocera <hadess@hadess.net> Date: Fri May 15 17:24:41 2009 +0100 Merge the is_running logic It was the same in all the GvcMixerStream classes with real backing streams, so merge all that logic into gvc-mixer-stream.c
Jumping volume fixed! commit 226b1bc336921870b17077582f92f607f9d5de24 Author: Bastien Nocera <hadess@hadess.net> Date: Wed May 20 12:44:36 2009 +0100 Don't set the volume to 0 when create a stream The volume shouldn't be set to the default (0) when creating the stream, as we will be getting the actual volume from the channel map passed as a required constructor property. Fixes the volume jumping around on startup.