On Wednesday, January 15, 2025 1:06:55 PM CET Akihiko Odaki wrote:
> init_out_device may only commit some part of the result and leave the
> state inconsistent when it encounters an error. Commit the result in
> the end of the function so that it commits the result iff it sees no
Typo "if".
> error.
>
> With this change, handle_voice_change can rely on core->outputDeviceID
> to know whether the output device is initialized after calling
> init_out_device.
>
> Signed-off-by: Akihiko Odaki <akihiko.od...@daynix.com>
> ---
> audio/coreaudio.m | 49 +++++++++++++++++++++++++++----------------------
> 1 file changed, 27 insertions(+), 22 deletions(-)
>
> diff --git a/audio/coreaudio.m b/audio/coreaudio.m
> index cadd729d5053..b9e1a952ed37 100644
> --- a/audio/coreaudio.m
> +++ b/audio/coreaudio.m
> @@ -355,7 +355,10 @@ static OSStatus audioDeviceIOProc(
> static OSStatus init_out_device(coreaudioVoiceOut *core)
> {
> OSStatus status;
> + AudioDeviceID deviceID;
I would probably preserve the name 'outputDeviceID' to make it more clear that
it's for output.
> AudioValueRange frameRange;
> + UInt32 audioDevicePropertyBufferFrameSize;
> + AudioDeviceIOProcID ioprocid;
>
> AudioStreamBasicDescription streamBasicDescription = {
> .mBitsPerChannel = core->hw.info.bits,
> @@ -368,20 +371,19 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
> .mSampleRate = core->hw.info.freq
> };
>
> - status = coreaudio_get_voice(&core->outputDeviceID);
> + status = coreaudio_get_voice(&deviceID);
> if (status != kAudioHardwareNoError) {
> coreaudio_playback_logerr (status,
> "Could not get default output Device\n");
> return status;
> }
> - if (core->outputDeviceID == kAudioDeviceUnknown) {
> + if (deviceID == kAudioDeviceUnknown) {
> dolog ("Could not initialize playback - Unknown Audiodevice\n");
> return status;
> }
>
> /* get minimum and maximum buffer frame sizes */
> - status = coreaudio_get_framesizerange(core->outputDeviceID,
> - &frameRange);
> + status = coreaudio_get_framesizerange(deviceID, &frameRange);
> if (status == kAudioHardwareBadObjectError) {
> return 0;
> }
> @@ -392,31 +394,31 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
> }
>
> if (frameRange.mMinimum > core->frameSizeSetting) {
> - core->audioDevicePropertyBufferFrameSize = (UInt32)
> frameRange.mMinimum;
> + audioDevicePropertyBufferFrameSize = (UInt32) frameRange.mMinimum;
> dolog ("warning: Upsizing Buffer Frames to %f\n",
> frameRange.mMinimum);
> } else if (frameRange.mMaximum < core->frameSizeSetting) {
> - core->audioDevicePropertyBufferFrameSize = (UInt32)
> frameRange.mMaximum;
> + audioDevicePropertyBufferFrameSize = (UInt32) frameRange.mMaximum;
> dolog ("warning: Downsizing Buffer Frames to %f\n",
> frameRange.mMaximum);
> } else {
> - core->audioDevicePropertyBufferFrameSize = core->frameSizeSetting;
> + audioDevicePropertyBufferFrameSize = core->frameSizeSetting;
> }
>
> /* set Buffer Frame Size */
> - status = coreaudio_set_framesize(core->outputDeviceID,
> -
> &core->audioDevicePropertyBufferFrameSize);
> + status = coreaudio_set_framesize(deviceID,
> + &audioDevicePropertyBufferFrameSize);
> if (status == kAudioHardwareBadObjectError) {
> return 0;
> }
> if (status != kAudioHardwareNoError) {
> coreaudio_playback_logerr (status,
> "Could not set device buffer frame size
> %" PRIu32 "\n",
> -
> (uint32_t)core->audioDevicePropertyBufferFrameSize);
> +
> (uint32_t)audioDevicePropertyBufferFrameSize);
'audioDevicePropertyBufferFrameSize' is declared as UInt32, so I guess the
cast can be dropped.
> return status;
> }
>
> /* get Buffer Frame Size */
> - status = coreaudio_get_framesize(core->outputDeviceID,
> -
> &core->audioDevicePropertyBufferFrameSize);
> + status = coreaudio_get_framesize(deviceID,
> + &audioDevicePropertyBufferFrameSize);
> if (status == kAudioHardwareBadObjectError) {
> return 0;
> }
> @@ -425,11 +427,9 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
> "Could not get device buffer frame
> size\n");
> return status;
> }
> - core->hw.samples = core->bufferCount *
> core->audioDevicePropertyBufferFrameSize;
Are you sure this should be deferred to the end of the function?
>
> /* set Samplerate */
> - status = coreaudio_set_streamformat(core->outputDeviceID,
> - &streamBasicDescription);
> + status = coreaudio_set_streamformat(deviceID, &streamBasicDescription);
> if (status == kAudioHardwareBadObjectError) {
> return 0;
> }
> @@ -437,7 +437,6 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
> coreaudio_playback_logerr (status,
> "Could not set samplerate %lf\n",
> streamBasicDescription.mSampleRate);
> - core->outputDeviceID = kAudioDeviceUnknown;
> return status;
> }
>
> @@ -451,20 +450,24 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
> * Therefore, the specified callback must be designed to avoid a deadlock
> * with the callers of AudioObjectGetPropertyData.
> */
> - core->ioprocid = NULL;
> - status = AudioDeviceCreateIOProcID(core->outputDeviceID,
> + ioprocid = NULL;
> + status = AudioDeviceCreateIOProcID(deviceID,
> audioDeviceIOProc,
> &core->hw,
> - &core->ioprocid);
> + &ioprocid);
> if (status == kAudioHardwareBadDeviceError) {
> return 0;
> }
> - if (status != kAudioHardwareNoError || core->ioprocid == NULL) {
> + if (status != kAudioHardwareNoError || ioprocid == NULL) {
> coreaudio_playback_logerr (status, "Could not set IOProc\n");
> - core->outputDeviceID = kAudioDeviceUnknown;
> return status;
> }
>
> + core->outputDeviceID = deviceID;
> + core->audioDevicePropertyBufferFrameSize =
> audioDevicePropertyBufferFrameSize;
> + core->hw.samples = core->bufferCount *
> core->audioDevicePropertyBufferFrameSize;
> + core->ioprocid = ioprocid;
> +
> return 0;
> }
>
> @@ -548,7 +551,9 @@ static OSStatus handle_voice_change(
> fini_out_device(core);
> }
>
> - if (!init_out_device(core)) {
> + init_out_device(core);
> +
> + if (core->outputDeviceID) {
> update_device_playback_state(core);
> }
>
>
>
