Panning #98

Merged
merged 4 commits into from Feb 14, 2017

Conversation

Projects
None yet
2 participants
Contributor

rakslice commented Jan 2, 2017

Per-app pan sliders, re: #97

I hacked this together quickly to get it working for my own needs; feel free to ignore or rip it apart. Notable things that are missing from this:

  • precalculating the coefficients for the pan settings to make the plugin faster, or any optimization at all really
  • L and R tick marks on the pan sliders, or some other visual indication that shows what the pan sliders are for -- maybe people would prefer rotating knobs for this?
  • tests for this stuff
BGMApp/BGMApp/BGMAppVolumes.mm
+- (id)initWithCoder:(NSCoder *)coder {
+ self = [super initWithCoder: coder];
+
+ [NSSlider setCellClass:[BGMAVM_PanSliderCell class]];
@rakslice

rakslice Jan 2, 2017 edited

Contributor

whoops, this [NSSlider setCellClass] call is spurious test code changing the global default slider cell class that shouldn't have been left in

-bool BGM_ClientMap::SetClientsRelativeVolume(pid_t inAppPID, Float32 inRelativeVolume)
+template <typename T>
+std::vector<BGM_Client*> * _Nullable GetClientsFromMap(std::map<T, std::vector<BGM_Client*>> & map, T key) {
+ // TODO assert mShadowMapsMutex is locked?
@rakslice

rakslice Jan 2, 2017

Contributor

I guess this function could be used with any map, so it doesn't even know which mutex to expect to be locked

bool SetClientsRelativeVolume(pid_t inAppPID, Float32 inRelativeVolume);
- // Returns true if a client with bundle ID inAppBundleID was found and its relative volume changed.
+ // Returns true if a client for bundle ID inAppBundleID was found and its relative volume changed.
bool SetClientsRelativeVolume(CACFString inAppBundleID, Float32 inRelativeVolume);
@rakslice

rakslice Jan 2, 2017

Contributor

I probably thought I was editing my own comment wording here, no nitpicking intended :)

@@ -271,7 +293,7 @@ - (void) setUpWithApp:(NSRunningApplication*)app context:(BGMAppVolumes*)ctx {
- (void) snap {
// Snap to the 50% point
- float midPoint = static_cast<float>((self.maxValue - self.minValue) / 2);
+ float midPoint = static_cast<float>((self.maxValue + self.minValue) / 2);
@rakslice

rakslice Jan 2, 2017 edited

Contributor

minValue is zero for the volume slider anyway, but I'll pay it forward for the next person who copy-pastes the code ;)

+
+ return self;
+}
+
@rakslice

rakslice Jan 3, 2017 edited

Contributor

If someone knows a better way to set a custom subclassed NSSlider coming in from Interface Builder to use a custom NSSliderCell subclass, I'm all for it; it took me hours of hitting my head against the wall of ambiguous documentation and forum posts to come up with this.

kyleneideck merged commit acf3976 into kyleneideck:master Feb 14, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Owner

kyleneideck commented Feb 14, 2017

@rakslice I didn't find any bugs or anything, so I went ahead and merged this. Thanks!

As far as I can remember, the only significant change I made was to move the sliders into a dropdown section:

Screenshot

I'm not really sure how people usually use pan/balance, so let me know if you find that gets in the way too much. Or if you disagree with anything else I changed, for that matter.

I've added L and R labels, and a centre tick mark since I thought it looked better with the L and R labels and differentiated the pan and volume sliders a bit. With the centre tick mark, snapping is handled automatically so I ended up removing the snap function you added.

I've also been adding people to the copyright notices of files they change, so let me know if you'd prefer not to be listed in those.

precalculating the coefficients for the pan settings to make the plugin faster, or any optimization at all really

For me, your code for applying the pan position in BGM_Device::ApplyClientRelativeVolume ran about as fast at the code for applying the app volume -- a bit under 0.5 us each. I didn't look into combining the two, either (or using the Accelerate framework to go beyond SSE2) but 1 us seems pretty reasonable to me.

Contributor

rakslice commented Feb 15, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment