mackron / miniaudio

Audio playback and capture library written in C, in a single source file.
https://miniaud.io
Other
3.96k stars 348 forks source link

Question: macOS code explicitly overrides sampleRate #201

Closed oviano closed 3 years ago

oviano commented 3 years ago
    /* From what I can see, Apple's documentation implies that we should keep the sample rate consistent. */
    origFormatSize = sizeof(origFormat);
    if (deviceType == ma_device_type_playback) {
        status = ((ma_AudioUnitGetProperty_proc)pContext->coreaudio.AudioUnitGetProperty)(pData->audioUnit, kAudioUnitProperty_StreamFormat, kAudioUnitScope_Output, MA_COREAUDIO_OUTPUT_BUS, &origFormat, &origFormatSize);
    } else {
        status = ((ma_AudioUnitGetProperty_proc)pContext->coreaudio.AudioUnitGetProperty)(pData->audioUnit, kAudioUnitProperty_StreamFormat, kAudioUnitScope_Input, MA_COREAUDIO_INPUT_BUS, &origFormat, &origFormatSize);
    }

    if (status != noErr) {
        ((ma_AudioComponentInstanceDispose_proc)pContext->coreaudio.AudioComponentInstanceDispose)(pData->audioUnit);
        return result;
    }

    bestFormat.mSampleRate = origFormat.mSampleRate;

Can you shed some more light on this, as it seems a bit of a shame according to my test at the moment I can't feed it 48Khz without it resampling to 44.1Khz?

It goes through all the code for finding the best format, matching sample rate and channels....and then ditches it.

mackron commented 3 years ago

In hindsight I should have quoted the section of documentation where I read that, because for the life of me I can't remember what that was, nor can I find it again. I'm happy to remove that and use the sample rate returned by ma_find_best_format__coreaudio() and see what the feedback from the community is like.

mackron commented 3 years ago

I've gone ahead and commented out that section. If the community reports issues about this I can easily revert it back again. Currently in the dev branch, but will get this out to the master branch soon.

Commit: 0ae8adc1c0256be5bb981dff05bae0ee927b73f1

oviano commented 3 years ago

Did you mean to wrap the error check too?

It seems like the error check on "status" belongs to the code block above and only the actual assignment of bestFormat.mSampleRate = origFormat.mSampleRate needed wrapping, but maybe I've got that wrong!

mackron commented 3 years ago

No, you're right. Fixed. https://github.com/mackron/miniaudio/commit/7090c1b7734f77f96dbc8add3a7324cb717f7626

kritzikratzi commented 3 years ago

hope this fits, but i just notized that the "use default sample rate" flag in osx goes through miniaudio's favourite samplerates in order. instead it could check the current sample rate of the device using:

    // ma_find_best_format__coreaudio ... 
    // ... if (usingDefaultSampleRate) { 
Float64 sampleRate; 
AudioObjectPropertyAddress propertyAddress;
UInt32 propertySize;
propertyAddress.mSelector = kAudioDevicePropertyNominalSampleRate;
propertyAddress.mScope = kAudioObjectPropertyScopeGlobal;
propertyAddress.mElement = 0;

propertySize = sizeof(Float64);
error = AudioHardwareServiceGetPropertyData(deviceObjectID,
                                            &propertyAddress,
                                            0,
                                            nullptr,
                                            &propertySize,
                                            &sampleRate);

if(!error){} // ... we got something meaningful! 

works perfectly fine for both capture and playback devices on osx 10.14 maybe my understanding of "default samplerate" is different, but i'd expected it to use the current sample rate if i pass in zero. or at least: i really want to be able to say "just use the current samplerate, don't resample, don't mess with my system settings".

ps. just like with my other report, if you're interested i'm happy to try to make a PR.

mackron commented 3 years ago

@kritzikratzi Yes, I completely agree. A value of 0 is used as an indicator to use defaults - makes much more sense to use the current sample rate defined by the system rather than miniaudio's preferences. A pull-request would be appreciated!

mackron commented 3 years ago

I've gone ahead and improved the format selection logic, including sample rate. Now it should use the system defined format/channels/rate when using defaults rather than miniaudio's preferences. It's actually much simpler now as well as an added bonus.

@kritzikratzi I wasn't able to use AudioHardwareServiceGetPropertyData() as it has been deprecated, but I think my solution is cleaner anyway. When you get a chance are you able to confirm that's working for you?

Commit: https://github.com/mackron/miniaudio/commit/25bd2576bfe0146907de38c9d13a6ca16dfaf9bb

mackron commented 3 years ago

I'll go ahead a close this one, but feel free to open a new issue if there's any more problems.

mackron commented 3 years ago

Unfortunately I need to reopen this one. The removal of this line has caused my capture and duplex tests to fail if I set the sample rate to anything other than the original sample rate, even when the sample rate is reported as being supported:

bestFormat.mSampleRate = origFormat.mSampleRate;

I'm therefore going to have to revert this, at least for capture devices. I'm wondering if this could have been the original motivation for that line.

mackron commented 3 years ago

So an update on this one. I found where I read that original documentation from Apple. I've found a way to actually change the same rate properly. The only problem is that it changes it system-wide. These are the comments I made in the sample rate selection part of the code (the miniaudio.h file is too big to link to directly 😒):

/*
Technical Note TN2091: Device input using the HAL Output Audio Unit
    https://developer.apple.com/library/archive/technotes/tn2091/_index.html

This documentation says the following:

    The internal AudioConverter can handle any *simple* conversion. Typically, this means that a client can specify ANY
    variant of the PCM formats. Consequently, the device's sample rate should match the desired sample rate. If sample rate
    conversion is needed, it can be accomplished by buffering the input and converting the data on a separate thread with
    another AudioConverter.

The important part here is the mention that it can handle *simple* conversions, which does *not* include sample rate. We
therefore want to ensure the sample rate stays consistent. This document is specifically for input, but I'm going to play it
safe and apply the same rule to output as well.

I have tried going against the documentation by setting the sample rate anyway, but this just results in AudioUnitRender()
returning a result code of -10863. I have also tried changing the format directly on the input scope on the input bus, but
this just results in `ca_require: IsStreamFormatWritable(inScope, inElement) NotWritable` when trying to set the format.

Something that does seem to work, however, has been setting the nominal sample rate on the deivce object. The problem with
this, however, is that it actually changes the sample rate at the operating system level and not just the application. This
could be intrusive to the user, however, so I don't think it's wise to make this the default. Instead I'm making this a
configuration option. When the `coreaudio.allowNominalSampleRateChange` config option is set to true, changing the sample
rate will be allowed. Otherwise it'll be fixed to the current sample rate. To check the system-defined sample rate, run
the Audio MIDI Setup program that comes installed on macOS and observe how the sample rate changes as the sample rate is
changed by miniaudio.
*/
if (pData->allowNominalSampleRateChange) {
    AudioValueRange sampleRateRange;
    AudioObjectPropertyAddress propAddress;

    sampleRateRange.mMinimum = bestFormat.mSampleRate;
    sampleRateRange.mMaximum = bestFormat.mSampleRate;

    propAddress.mSelector = kAudioDevicePropertyNominalSampleRate;
    propAddress.mScope    = (deviceType == ma_device_type_playback) ? kAudioObjectPropertyScopeOutput : kAudioObjectPropertyScopeInput;
    propAddress.mElement  = kAudioObjectPropertyElementMaster;

    status = ((ma_AudioObjectSetPropertyData_proc)pContext->coreaudio.AudioObjectSetPropertyData)(deviceObjectID, &propAddress, 0, NULL, sizeof(sampleRateRange), &sampleRateRange);
    if (status != noErr) {
        bestFormat.mSampleRate = origFormat.mSampleRate;
    }
} else {
    bestFormat.mSampleRate = origFormat.mSampleRate;
}

If you're able to give that a test that'd be handy, but I'll be releasing these changes probably tomorrow regardless.

mackron commented 3 years ago

This change has been released.