Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Control for pitch adjust #534

Merged
merged 5 commits into from Jan 16, 2023
Merged

Control for pitch adjust #534

merged 5 commits into from Jan 16, 2023

Conversation

HEnquist
Copy link
Contributor

@HEnquist HEnquist commented Feb 7, 2022

This is a prototype that exposes a control for adjusting the sample rate of the Blackhole device by +- 1%. This functionality is provided by the standard Alsa loopback device on Linux, and is very useful for syncing the loopback to other things, that may or may not be another Alsa device.

For now it uses the balance control of the playback side for the control. The balance control is perhaps not the ideal choice., but I have not yet figured out how to (and if it's possible) to add a custom control.

Would you be willing to include something like this in Blackhole?

@CLAassistant
Copy link

CLAassistant commented Feb 7, 2022 •
edited

CLA assistant check
All committers have signed the CLA.

@HEnquist
Copy link
Contributor Author

Any input on this? Do I need to read and agree to the long CLA first?

@devinroth
Copy link
Member

devinroth commented Feb 26, 2022 via email

@HEnquist
Copy link
Contributor Author

Alright, understood. I just find these legal things so tedious, but I have read and signed it now.

@devinroth
Copy link
Member

This is interesting. Yes a custom control would be a better solution. I'm curious on how this would be implemented. Would you just monitor the clocks of BlackHole and another device and adjust BlackHole when it starts to drift?

My solution, was to synchronize BlackHole clock with that of a different audio device. That's still in development but there were a couple hiccups that need to be ironed out.

@HEnquist
Copy link
Contributor Author

HEnquist commented Mar 11, 2022 •
edited

I basically record from one device, do some processing on the audio data, push it to a buffer, and play it back on another device from that buffer. At the same time I monitor how many frames I have in the buffer. If it's lower or higher that the target value, I adjust the speed of the capture device to keep the level constant. When the capture device is an Alsa loopback (on Linux), I adjust the virtual clock of that device. When it's anything else, I instead insert an asynchronous resampler that I adjust. If Blackhole is also adjustable, then I can get rid of this resampler on MacOs too.

I'll continue and try to figure out how to add custom controls then. I think this adjust feature would coexist nicely with your other option of locking the clock to another device.

@HEnquist
Copy link
Contributor Author

BTW I'm struggling with finding relevant coreaudio documentation. Does anyone have any advice on where to find something? A book perhaps?

@devinroth
Copy link
Member

devinroth commented Mar 16, 2022 via email

@HEnquist
Copy link
Contributor Author

Uint32 should be just fine! I think an allowed range of +-1% is reasonable. Dividing that up into 2^32 steps already gives more precision than needed.
I think I have figured out how to add custom controls. Will try it as soon as I have a little time.

@HEnquist
Copy link
Contributor Author

It seems like custom controls are not shown in Audio Midi Setup. I think the rate control should be accessible from there, if not it needs some app to expose it to the user.
I have only managed to get it to show the stereo pan control. This works but it might be a bit confusing that the pitch is called "Balance". I have tried changing that label but without success. I just want a slider called pitch, do you have any ideas for how to achieve that?

@HEnquist
Copy link
Contributor Author

I'm guessing that Audio Midi Setup only checks for (and shows) a few different standard controls. It doesn't seem to be possible to customize those with names etc (or at least I haven't managed).
What do you prefer, using an existing control like "Balance" and living with the risk of confusion, or adding a custom control and bundling Blackhole with a small app (tiny GUI or cli tool) to access the control?

@HEnquist
Copy link
Contributor Author

Just had a (maybe) good idea. What if we add a ClockSource control, with two values called something like "InternalFixed" and "InternalAdjustable"? Default is "InternalFixed" where everything is the same as before, and "InternalAdjustable" enables adjusting via the pan control. And later the list can be extended with the other devices for syncing the clock to another device.

@devinroth
Copy link
Member

devinroth commented Apr 3, 2022 via email

@Wang-Yue
Copy link

Wang-Yue commented Jun 3, 2022

will there still be an update on this feature?

@HEnquist
Copy link
Contributor Author

HEnquist commented Jun 3, 2022

will there still be an update on this feature?

I will finish it! I just haven't had enough free time lately.

@HEnquist
Copy link
Contributor Author

Rebased on current master. I had to change the os target version back to 11.1 since my old Mac can't run the latest Xcode. This is a bit annoying since it meant I had to rename kAudioObjectPropertyElementMain back to the old kAudioObjectPropertyElementMaster to be able to compile. Is it ok if I leave it like this in the PR?

I haven't looked at the clock source selection yet. Should I continue using the balance control for the rate, and add the clock source selection as I suggested before? Just want to confirm this before I start any work.

@devinroth
Copy link
Member

Why not do something like define kAudioObjectPropertyElementMain = kAudioObjectPropertyElementMaster if target is below a certain macOS version? Then it works in all instances.

@HEnquist
Copy link
Contributor Author

Why not do something like define kAudioObjectPropertyElementMain = kAudioObjectPropertyElementMaster if target is below a certain macOS version? Then it works in all instances.

Good idea, done!

I'm still wondering about the clock source select and use of the pan control, can you please share your view on that?

@HEnquist
Copy link
Contributor Author

The changes for adding a clock source selection are here:
HEnquist#1
This adds the sources "Internal Fixed" and "Internal Adjustable", where the adjustable one activates the adjust via the pan control.

@Wang-Yue
Copy link

Hi, Devin any chance to prioritize the review and merge these changes in? Thanks!

@Wang-Yue
Copy link

A friendly ping

@devinroth devinroth marked this pull request as ready for review October 28, 2022 16:03
@HEnquist
Copy link
Contributor Author

I'm still wondering about the clock source select and use of the pan control, can you please share your view on that?

I'm still not sure what to do here. Should I merge the clock source select to this pr or not?

@devinroth
Copy link
Member

devinroth commented Nov 15, 2022 via email

@HEnquist
Copy link
Contributor Author

I did that a couple of weeks ago, I don't see any changes in master since then.

Then I leave this PR as is, and submit another one for the clock source select.

@HEnquist
Copy link
Contributor Author

Should I get rid of the changes in the other files than Blackhole.c?

@devinroth
Copy link
Member

devinroth commented Nov 23, 2022 via email

@HEnquist
Copy link
Contributor Author

done

@Wang-Yue
Copy link

Any chance this could be merged soon?

@devinroth
Copy link
Member

I'll be able to look at this over the next week.

@devinroth
Copy link
Member

Looking at this now!

devinroth
devinroth approved these changes Jan 16, 2023
View reviewed changes
Copy link
Member

@devinroth devinroth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Thanks for all the hard work on this. I hope it help some people out.

@devinroth devinroth merged commit 3bb87a8 into ExistentialAudio:master Jan 16, 2023
@devinroth
Copy link
Member

Only took me a year. Lol. Sorry about that.

@HEnquist
Copy link
Contributor Author

Sweet thanks!! Then I will follow up with another pr very soon, for adding a clock source selector.

@devinroth
Copy link
Member

devinroth commented Jan 16, 2023 via email

@HEnquist
Copy link
Contributor Author

HEnquist commented Jan 16, 2023 •
edited

Oh I'll look into that too then! I haven't noticed any glitches, but I usually change the value very little. How big of a change did you make?
EDIT: Thought a little more.. how did you test this? If you play with the rate while playing sound through the blackhole device, then onto a real device, it's very likely you get buffer underrruns. Could that be the glitches you noticed?

@Wang-Yue
Copy link

Good news that this is finally merged in!

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

@devinroth devinroth devinroth approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@HEnquist @CLAassistant @devinroth @Wang-Yue