All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] coreaudio: Drop support for macOS older than 10.6
@ 2021-03-01 11:45 Akihiko Odaki
  2021-03-01 11:45 ` [PATCH 2/2] coreaudio: Handle output device change Akihiko Odaki
  2021-03-03  9:22 ` [PATCH 1/2] coreaudio: Drop support for macOS older than 10.6 Gerd Hoffmann
  0 siblings, 2 replies; 6+ messages in thread
From: Akihiko Odaki @ 2021-03-01 11:45 UTC (permalink / raw)
  Cc: qemu-devel, Akihiko Odaki, Gerd Hoffmann

Mac OS X 10.6 was released in 2009.

Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com>
---
 audio/coreaudio.c | 103 ----------------------------------------------
 1 file changed, 103 deletions(-)

diff --git a/audio/coreaudio.c b/audio/coreaudio.c
index b7c02e0e516..c5f0b615d64 100644
--- a/audio/coreaudio.c
+++ b/audio/coreaudio.c
@@ -32,10 +32,6 @@
 #define AUDIO_CAP "coreaudio"
 #include "audio_int.h"
 
-#ifndef MAC_OS_X_VERSION_10_6
-#define MAC_OS_X_VERSION_10_6 1060
-#endif
-
 typedef struct coreaudioVoiceOut {
     HWVoiceOut hw;
     pthread_mutex_t mutex;
@@ -45,9 +41,6 @@ typedef struct coreaudioVoiceOut {
     AudioDeviceIOProcID ioprocid;
 } coreaudioVoiceOut;
 
-#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_6
-/* The APIs used here only become available from 10.6 */
-
 static OSStatus coreaudio_get_voice(AudioDeviceID *id)
 {
     UInt32 size = sizeof(*id);
@@ -169,102 +162,6 @@ static OSStatus coreaudio_get_isrunning(AudioDeviceID id, UInt32 *result)
                                       &size,
                                       result);
 }
-#else
-/* Legacy versions of functions using deprecated APIs */
-
-static OSStatus coreaudio_get_voice(AudioDeviceID *id)
-{
-    UInt32 size = sizeof(*id);
-
-    return AudioHardwareGetProperty(
-        kAudioHardwarePropertyDefaultOutputDevice,
-        &size,
-        id);
-}
-
-static OSStatus coreaudio_get_framesizerange(AudioDeviceID id,
-                                             AudioValueRange *framerange)
-{
-    UInt32 size = sizeof(*framerange);
-
-    return AudioDeviceGetProperty(
-        id,
-        0,
-        0,
-        kAudioDevicePropertyBufferFrameSizeRange,
-        &size,
-        framerange);
-}
-
-static OSStatus coreaudio_get_framesize(AudioDeviceID id, UInt32 *framesize)
-{
-    UInt32 size = sizeof(*framesize);
-
-    return AudioDeviceGetProperty(
-        id,
-        0,
-        false,
-        kAudioDevicePropertyBufferFrameSize,
-        &size,
-        framesize);
-}
-
-static OSStatus coreaudio_set_framesize(AudioDeviceID id, UInt32 *framesize)
-{
-    UInt32 size = sizeof(*framesize);
-
-    return AudioDeviceSetProperty(
-        id,
-        NULL,
-        0,
-        false,
-        kAudioDevicePropertyBufferFrameSize,
-        size,
-        framesize);
-}
-
-static OSStatus coreaudio_get_streamformat(AudioDeviceID id,
-                                           AudioStreamBasicDescription *d)
-{
-    UInt32 size = sizeof(*d);
-
-    return AudioDeviceGetProperty(
-        id,
-        0,
-        false,
-        kAudioDevicePropertyStreamFormat,
-        &size,
-        d);
-}
-
-static OSStatus coreaudio_set_streamformat(AudioDeviceID id,
-                                           AudioStreamBasicDescription *d)
-{
-    UInt32 size = sizeof(*d);
-
-    return AudioDeviceSetProperty(
-        id,
-        0,
-        0,
-        0,
-        kAudioDevicePropertyStreamFormat,
-        size,
-        d);
-}
-
-static OSStatus coreaudio_get_isrunning(AudioDeviceID id, UInt32 *result)
-{
-    UInt32 size = sizeof(*result);
-
-    return AudioDeviceGetProperty(
-        id,
-        0,
-        0,
-        kAudioDevicePropertyDeviceIsRunning,
-        &size,
-        result);
-}
-#endif
 
 static void coreaudio_logstatus (OSStatus status)
 {
-- 
2.24.3 (Apple Git-128)



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] coreaudio: Handle output device change
  2021-03-01 11:45 [PATCH 1/2] coreaudio: Drop support for macOS older than 10.6 Akihiko Odaki
@ 2021-03-01 11:45 ` Akihiko Odaki
  2021-03-03  9:27   ` Gerd Hoffmann
  2021-03-03  9:22 ` [PATCH 1/2] coreaudio: Drop support for macOS older than 10.6 Gerd Hoffmann
  1 sibling, 1 reply; 6+ messages in thread
From: Akihiko Odaki @ 2021-03-01 11:45 UTC (permalink / raw)
  Cc: qemu-devel, Akihiko Odaki, Gerd Hoffmann

An output device change can occur when plugging or unplugging an
earphone.

Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com>
---
 audio/coreaudio.c | 327 +++++++++++++++++++++++++++++++++-------------
 1 file changed, 236 insertions(+), 91 deletions(-)

diff --git a/audio/coreaudio.c b/audio/coreaudio.c
index c5f0b615d64..578ec9b8b2e 100644
--- a/audio/coreaudio.c
+++ b/audio/coreaudio.c
@@ -36,22 +36,26 @@ typedef struct coreaudioVoiceOut {
     HWVoiceOut hw;
     pthread_mutex_t mutex;
     AudioDeviceID outputDeviceID;
+    int frameSizeSetting;
+    uint32_t bufferCount;
     UInt32 audioDevicePropertyBufferFrameSize;
     AudioStreamBasicDescription outputStreamBasicDescription;
     AudioDeviceIOProcID ioprocid;
+    bool enabled;
 } coreaudioVoiceOut;
 
+static const AudioObjectPropertyAddress voice_addr = {
+    kAudioHardwarePropertyDefaultOutputDevice,
+    kAudioObjectPropertyScopeGlobal,
+    kAudioObjectPropertyElementMaster
+};
+
 static OSStatus coreaudio_get_voice(AudioDeviceID *id)
 {
     UInt32 size = sizeof(*id);
-    AudioObjectPropertyAddress addr = {
-        kAudioHardwarePropertyDefaultOutputDevice,
-        kAudioObjectPropertyScopeGlobal,
-        kAudioObjectPropertyElementMaster
-    };
 
     return AudioObjectGetPropertyData(kAudioObjectSystemObject,
-                                      &addr,
+                                      &voice_addr,
                                       0,
                                       NULL,
                                       &size,
@@ -253,17 +257,8 @@ static void GCC_FMT_ATTR (3, 4) coreaudio_logerr2 (
     coreaudio_logstatus (status);
 }
 
-static inline UInt32 isPlaying (AudioDeviceID outputDeviceID)
-{
-    OSStatus status;
-    UInt32 result = 0;
-    status = coreaudio_get_isrunning(outputDeviceID, &result);
-    if (status != kAudioHardwareNoError) {
-        coreaudio_logerr(status,
-                         "Could not determine whether Device is playing\n");
-    }
-    return result;
-}
+#define coreaudio_playback_logerr(status, ...) \
+    coreaudio_logerr2(status, "playback", __VA_ARGS__)
 
 static int coreaudio_lock (coreaudioVoiceOut *core, const char *fn_name)
 {
@@ -336,6 +331,11 @@ static OSStatus audioDeviceIOProc(
         return 0;
     }
 
+    if (inDevice != core->outputDeviceID) {
+        coreaudio_unlock (core, "audioDeviceIOProc(old device)");
+        return 0;
+    }
+
     frameCount = core->audioDevicePropertyBufferFrameSize;
     pending_frames = hw->pending_emul / hw->info.bytes_per_frame;
 
@@ -368,175 +368,320 @@ static OSStatus audioDeviceIOProc(
     return 0;
 }
 
-static int coreaudio_init_out(HWVoiceOut *hw, struct audsettings *as,
-                              void *drv_opaque)
+static OSStatus init_out_device(coreaudioVoiceOut *core)
 {
     OSStatus status;
-    coreaudioVoiceOut *core = (coreaudioVoiceOut *) hw;
-    int err;
-    const char *typ = "playback";
     AudioValueRange frameRange;
-    Audiodev *dev = drv_opaque;
-    AudiodevCoreaudioPerDirectionOptions *cpdo = dev->u.coreaudio.out;
-    int frames;
-    struct audsettings obt_as;
-
-    /* create mutex */
-    err = pthread_mutex_init(&core->mutex, NULL);
-    if (err) {
-        dolog("Could not create mutex\nReason: %s\n", strerror (err));
-        return -1;
-    }
-
-    obt_as = *as;
-    as = &obt_as;
-    as->fmt = AUDIO_FORMAT_F32;
-    audio_pcm_init_info (&hw->info, as);
 
     status = coreaudio_get_voice(&core->outputDeviceID);
     if (status != kAudioHardwareNoError) {
-        coreaudio_logerr2 (status, typ,
-                           "Could not get default output Device\n");
-        return -1;
+        coreaudio_playback_logerr (status,
+                                   "Could not get default output Device\n");
+        return status;
     }
     if (core->outputDeviceID == kAudioDeviceUnknown) {
-        dolog ("Could not initialize %s - Unknown Audiodevice\n", typ);
-        return -1;
+        dolog ("Could not initialize playback - Unknown Audiodevice\n");
+        return status;
     }
 
     /* get minimum and maximum buffer frame sizes */
     status = coreaudio_get_framesizerange(core->outputDeviceID,
                                           &frameRange);
+    if (status == kAudioHardwareBadObjectError) {
+        return 0;
+    }
     if (status != kAudioHardwareNoError) {
-        coreaudio_logerr2 (status, typ,
-                           "Could not get device buffer frame range\n");
-        return -1;
+        coreaudio_playback_logerr (status,
+                                    "Could not get device buffer frame range\n");
+        return status;
     }
 
-    frames = audio_buffer_frames(
-        qapi_AudiodevCoreaudioPerDirectionOptions_base(cpdo), as, 11610);
-    if (frameRange.mMinimum > frames) {
+    if (frameRange.mMinimum > core->frameSizeSetting) {
         core->audioDevicePropertyBufferFrameSize = (UInt32) frameRange.mMinimum;
         dolog ("warning: Upsizing Buffer Frames to %f\n", frameRange.mMinimum);
-    } else if (frameRange.mMaximum < frames) {
+    } else if (frameRange.mMaximum < core->frameSizeSetting) {
         core->audioDevicePropertyBufferFrameSize = (UInt32) frameRange.mMaximum;
         dolog ("warning: Downsizing Buffer Frames to %f\n", frameRange.mMaximum);
     } else {
-        core->audioDevicePropertyBufferFrameSize = frames;
+        core->audioDevicePropertyBufferFrameSize = core->frameSizeSetting;
     }
 
     /* set Buffer Frame Size */
     status = coreaudio_set_framesize(core->outputDeviceID,
                                      &core->audioDevicePropertyBufferFrameSize);
+    if (status == kAudioHardwareBadObjectError) {
+        return 0;
+    }
     if (status != kAudioHardwareNoError) {
-        coreaudio_logerr2 (status, typ,
-                           "Could not set device buffer frame size %" PRIu32 "\n",
-                           (uint32_t)core->audioDevicePropertyBufferFrameSize);
-        return -1;
+        coreaudio_playback_logerr (status,
+                                    "Could not set device buffer frame size %" PRIu32 "\n",
+                                    (uint32_t)core->audioDevicePropertyBufferFrameSize);
+        return status;
     }
 
     /* get Buffer Frame Size */
     status = coreaudio_get_framesize(core->outputDeviceID,
                                      &core->audioDevicePropertyBufferFrameSize);
+    if (status == kAudioHardwareBadObjectError) {
+        return 0;
+    }
     if (status != kAudioHardwareNoError) {
-        coreaudio_logerr2 (status, typ,
-                           "Could not get device buffer frame size\n");
-        return -1;
+        coreaudio_playback_logerr (status,
+                                    "Could not get device buffer frame size\n");
+        return status;
     }
-    hw->samples = (cpdo->has_buffer_count ? cpdo->buffer_count : 4) *
-        core->audioDevicePropertyBufferFrameSize;
+    core->hw.samples = core->bufferCount * core->audioDevicePropertyBufferFrameSize;
 
     /* get StreamFormat */
     status = coreaudio_get_streamformat(core->outputDeviceID,
                                         &core->outputStreamBasicDescription);
+    if (status == kAudioHardwareBadObjectError) {
+        return 0;
+    }
     if (status != kAudioHardwareNoError) {
-        coreaudio_logerr2 (status, typ,
-                           "Could not get Device Stream properties\n");
+        coreaudio_playback_logerr (status,
+                                    "Could not get Device Stream properties\n");
         core->outputDeviceID = kAudioDeviceUnknown;
-        return -1;
+        return status;
     }
 
     /* set Samplerate */
-    core->outputStreamBasicDescription.mSampleRate = (Float64) as->freq;
-
     status = coreaudio_set_streamformat(core->outputDeviceID,
                                         &core->outputStreamBasicDescription);
+    if (status == kAudioHardwareBadObjectError) {
+        return 0;
+    }
     if (status != kAudioHardwareNoError) {
-        coreaudio_logerr2 (status, typ, "Could not set samplerate %d\n",
-                           as->freq);
+        coreaudio_playback_logerr (status,
+                                   "Could not set samplerate %lf\n",
+                                   core->outputStreamBasicDescription.mSampleRate);
         core->outputDeviceID = kAudioDeviceUnknown;
-        return -1;
+        return status;
     }
 
     /* set Callback */
     core->ioprocid = NULL;
     status = AudioDeviceCreateIOProcID(core->outputDeviceID,
                                        audioDeviceIOProc,
-                                       hw,
+                                       &core->hw,
                                        &core->ioprocid);
+    if (status == kAudioHardwareBadDeviceError) {
+        return 0;
+    }
     if (status != kAudioHardwareNoError || core->ioprocid == NULL) {
-        coreaudio_logerr2 (status, typ, "Could not set IOProc\n");
+        coreaudio_playback_logerr (status, "Could not set IOProc\n");
         core->outputDeviceID = kAudioDeviceUnknown;
-        return -1;
+        return status;
     }
 
     return 0;
 }
 
-static void coreaudio_fini_out (HWVoiceOut *hw)
+static void fini_out_device(coreaudioVoiceOut *core)
 {
     OSStatus status;
-    int err;
-    coreaudioVoiceOut *core = (coreaudioVoiceOut *) hw;
+    UInt32 isrunning;
 
     /* stop playback */
-    if (isPlaying(core->outputDeviceID)) {
-        status = AudioDeviceStop(core->outputDeviceID, core->ioprocid);
+    status = coreaudio_get_isrunning(core->outputDeviceID, &isrunning);
+    if (status != kAudioHardwareBadObjectError) {
         if (status != kAudioHardwareNoError) {
-            coreaudio_logerr(status, "Could not stop playback\n");
+            coreaudio_logerr(status,
+                             "Could not determine whether Device is playing\n");
+        }
+
+        if (isrunning) {
+            status = AudioDeviceStop(core->outputDeviceID, core->ioprocid);
+            if (status != kAudioHardwareBadDeviceError && status != kAudioHardwareNoError) {
+                coreaudio_logerr(status, "Could not stop playback\n");
+            }
         }
     }
 
     /* remove callback */
     status = AudioDeviceDestroyIOProcID(core->outputDeviceID,
                                         core->ioprocid);
-    if (status != kAudioHardwareNoError) {
+    if (status != kAudioHardwareBadDeviceError && status != kAudioHardwareNoError) {
         coreaudio_logerr(status, "Could not remove IOProc\n");
     }
     core->outputDeviceID = kAudioDeviceUnknown;
-
-    /* destroy mutex */
-    err = pthread_mutex_destroy(&core->mutex);
-    if (err) {
-        dolog("Could not destroy mutex\nReason: %s\n", strerror (err));
-    }
 }
 
-static void coreaudio_enable_out(HWVoiceOut *hw, bool enable)
+static void update_device_playback_state(coreaudioVoiceOut *core)
 {
     OSStatus status;
-    coreaudioVoiceOut *core = (coreaudioVoiceOut *) hw;
+    UInt32 isrunning;
 
-    if (enable) {
+    status = coreaudio_get_isrunning(core->outputDeviceID, &isrunning);
+    if (status != kAudioHardwareNoError) {
+        if (status != kAudioHardwareBadObjectError) {
+            coreaudio_logerr(status,
+                             "Could not determine whether Device is playing\n");
+        }
+
+        return;
+    }
+
+    if (core->enabled) {
         /* start playback */
-        if (!isPlaying(core->outputDeviceID)) {
+        if (!isrunning) {
             status = AudioDeviceStart(core->outputDeviceID, core->ioprocid);
-            if (status != kAudioHardwareNoError) {
+            if (status != kAudioHardwareBadDeviceError && status != kAudioHardwareNoError) {
                 coreaudio_logerr (status, "Could not resume playback\n");
             }
         }
     } else {
         /* stop playback */
-        if (isPlaying(core->outputDeviceID)) {
+        if (isrunning) {
             status = AudioDeviceStop(core->outputDeviceID,
                                      core->ioprocid);
-            if (status != kAudioHardwareNoError) {
+            if (status != kAudioHardwareBadDeviceError && status != kAudioHardwareNoError) {
                 coreaudio_logerr(status, "Could not pause playback\n");
             }
         }
     }
 }
 
+static OSStatus handle_voice_change(
+    AudioObjectID in_object_id,
+    UInt32 in_number_addresses,
+    const AudioObjectPropertyAddress *in_addresses,
+    void *in_client_data)
+{
+    OSStatus status;
+    coreaudioVoiceOut *core = in_client_data;
+
+    if (coreaudio_lock(core, __func__)) {
+        abort();
+    }
+
+    if (core->outputDeviceID) {
+        fini_out_device(core);
+    }
+
+    status = init_out_device(core);
+    if (!status) {
+        update_device_playback_state(core);
+    }
+
+    coreaudio_unlock (core, __func__);
+    return status;
+}
+
+static int coreaudio_init_out(HWVoiceOut *hw, struct audsettings *as,
+                              void *drv_opaque)
+{
+    OSStatus status;
+    coreaudioVoiceOut *core = (coreaudioVoiceOut *) hw;
+    int err;
+    Audiodev *dev = drv_opaque;
+    AudiodevCoreaudioPerDirectionOptions *cpdo = dev->u.coreaudio.out;
+    struct audsettings obt_as;
+
+    /* create mutex */
+    err = pthread_mutex_init(&core->mutex, NULL);
+    if (err) {
+        dolog("Could not create mutex\nReason: %s\n", strerror (err));
+        goto mutex_error;
+    }
+
+    if (coreaudio_lock(core, __func__)) {
+        goto lock_error;
+    }
+
+    obt_as = *as;
+    as = &obt_as;
+    as->fmt = AUDIO_FORMAT_F32;
+    audio_pcm_init_info (&hw->info, as);
+
+    core->frameSizeSetting = audio_buffer_frames(
+        qapi_AudiodevCoreaudioPerDirectionOptions_base(cpdo), as, 11610);
+
+    core->bufferCount = cpdo->has_buffer_count ? cpdo->buffer_count : 4;
+    core->outputStreamBasicDescription.mSampleRate = (Float64) as->freq;
+
+    status = AudioObjectAddPropertyListener(kAudioObjectSystemObject,
+                                            &voice_addr, handle_voice_change,
+                                            core);
+    if (status != kAudioHardwareNoError) {
+        coreaudio_playback_logerr (status,
+                                   "Could not listen to voice property change\n");
+        goto listener_error;
+    }
+
+    if (init_out_device(core)) {
+        goto device_error;
+    }
+
+    coreaudio_unlock(core, __func__);
+    return 0;
+
+device_error:
+    status = AudioObjectRemovePropertyListener(kAudioObjectSystemObject,
+                                               &voice_addr,
+                                               handle_voice_change,
+                                               core);
+    if (status != kAudioHardwareNoError) {
+        coreaudio_playback_logerr(status,
+                                  "Could not remove voice property change listener\n");
+    }
+
+listener_error:
+    coreaudio_unlock(core, __func__);
+
+lock_error:
+    err = pthread_mutex_destroy(&core->mutex);
+    if (err) {
+        dolog("Could not destroy mutex\nReason: %s\n", strerror (err));
+    }
+
+mutex_error:
+    return -1;
+}
+
+static void coreaudio_fini_out (HWVoiceOut *hw)
+{
+    OSStatus status;
+    int err;
+    coreaudioVoiceOut *core = (coreaudioVoiceOut *) hw;
+
+    if (coreaudio_lock(core, __func__)) {
+        abort();
+    }
+
+    status = AudioObjectRemovePropertyListener(kAudioObjectSystemObject,
+                                               &voice_addr,
+                                               handle_voice_change,
+                                               core);
+    if (status != kAudioHardwareNoError) {
+        coreaudio_logerr(status, "Could not remove voice property change listener\n");
+    }
+
+    fini_out_device(core);
+
+    coreaudio_unlock(core, __func__);
+
+    /* destroy mutex */
+    err = pthread_mutex_destroy(&core->mutex);
+    if (err) {
+        dolog("Could not destroy mutex\nReason: %s\n", strerror (err));
+    }
+}
+
+static void coreaudio_enable_out(HWVoiceOut *hw, bool enable)
+{
+    coreaudioVoiceOut *core = (coreaudioVoiceOut *) hw;
+
+    if (coreaudio_lock(core, __func__)) {
+        abort();
+    }
+
+    core->enabled = enable;
+    update_device_playback_state(core);
+
+    coreaudio_unlock(core, __func__);
+}
+
 static void *coreaudio_audio_init(Audiodev *dev)
 {
     return dev;
-- 
2.24.3 (Apple Git-128)



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] coreaudio: Drop support for macOS older than 10.6
  2021-03-01 11:45 [PATCH 1/2] coreaudio: Drop support for macOS older than 10.6 Akihiko Odaki
  2021-03-01 11:45 ` [PATCH 2/2] coreaudio: Handle output device change Akihiko Odaki
@ 2021-03-03  9:22 ` Gerd Hoffmann
  1 sibling, 0 replies; 6+ messages in thread
From: Gerd Hoffmann @ 2021-03-03  9:22 UTC (permalink / raw)
  To: Akihiko Odaki; +Cc: qemu-devel

On Mon, Mar 01, 2021 at 08:45:53PM +0900, Akihiko Odaki wrote:
> Mac OS X 10.6 was released in 2009.

Also minimum version required my qemu is 10.13 (I think),
so any code for older macos versions is dead anyway.

take care,
  Gerd



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] coreaudio: Handle output device change
  2021-03-01 11:45 ` [PATCH 2/2] coreaudio: Handle output device change Akihiko Odaki
@ 2021-03-03  9:27   ` Gerd Hoffmann
  2021-03-03 13:20     ` Akihiko Odaki
  0 siblings, 1 reply; 6+ messages in thread
From: Gerd Hoffmann @ 2021-03-03  9:27 UTC (permalink / raw)
  To: Akihiko Odaki; +Cc: qemu-devel

  Hi,

>      status = coreaudio_get_voice(&core->outputDeviceID);
>      if (status != kAudioHardwareNoError) {
> -        coreaudio_logerr2 (status, typ,
> -                           "Could not get default output Device\n");
> -        return -1;
> +        coreaudio_playback_logerr (status,
> +                                   "Could not get default output Device\n");
> +        return status;

This "pass status code to caller" change can and should be splitted to a
separate patch.

Likewise with the logging changes.

That makes the patch with the actual functional change to deal with the
device changes smaller and easier to review.

thanks,
  Gerd



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] coreaudio: Handle output device change
  2021-03-03  9:27   ` Gerd Hoffmann
@ 2021-03-03 13:20     ` Akihiko Odaki
  2021-03-11 12:50       ` Gerd Hoffmann
  0 siblings, 1 reply; 6+ messages in thread
From: Akihiko Odaki @ 2021-03-03 13:20 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: qemu Developers

2021年3月3日(水) 18:27 Gerd Hoffmann <kraxel@redhat.com>:
>
>   Hi,
>
> >      status = coreaudio_get_voice(&core->outputDeviceID);
> >      if (status != kAudioHardwareNoError) {
> > -        coreaudio_logerr2 (status, typ,
> > -                           "Could not get default output Device\n");
> > -        return -1;
> > +        coreaudio_playback_logerr (status,
> > +                                   "Could not get default output Device\n");
> > +        return status;
>
> This "pass status code to caller" change can and should be splitted to a
> separate patch.
>
> Likewise with the logging changes.
>
> That makes the patch with the actual functional change to deal with the
> device changes smaller and easier to review.
>
> thanks,
>   Gerd
>

Actually the code was extracted from coreaudio_init_out to
init_out_device in this patch. init_out_device "passes status code to
caller", but coreaudio_init_out still doesn't. A new executor of the
procedure, handle_voice_change only uses the status code returned by
init_out_device.

Logging changes are alike; Now "playback" type logs are recorded by
both of coreaudio_init_out and handle_voice_change.
handle_voice_change is a function required only for device change.

Regards,
Akihiko Odaki


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] coreaudio: Handle output device change
  2021-03-03 13:20     ` Akihiko Odaki
@ 2021-03-11 12:50       ` Gerd Hoffmann
  0 siblings, 0 replies; 6+ messages in thread
From: Gerd Hoffmann @ 2021-03-11 12:50 UTC (permalink / raw)
  To: Akihiko Odaki; +Cc: qemu Developers

On Wed, Mar 03, 2021 at 10:20:20PM +0900, Akihiko Odaki wrote:
> 2021年3月3日(水) 18:27 Gerd Hoffmann <kraxel@redhat.com>:
> >
> >   Hi,
> >
> > >      status = coreaudio_get_voice(&core->outputDeviceID);
> > >      if (status != kAudioHardwareNoError) {
> > > -        coreaudio_logerr2 (status, typ,
> > > -                           "Could not get default output Device\n");
> > > -        return -1;
> > > +        coreaudio_playback_logerr (status,
> > > +                                   "Could not get default output Device\n");
> > > +        return status;
> >
> > This "pass status code to caller" change can and should be splitted to a
> > separate patch.
> >
> > Likewise with the logging changes.
> >
> > That makes the patch with the actual functional change to deal with the
> > device changes smaller and easier to review.
> >
> > thanks,
> >   Gerd
> >
> 
> Actually the code was extracted from coreaudio_init_out to
> init_out_device in this patch.

It is likewise good practice to handle such refactorings as separate
patch, i.e. have a patch which reorganizes the code (in this case move
code to the new init_out_device() function) without functional change,
then go add the new handle_voice_change which fixes the actual issue
in the next patch.  Finally send the two (or more) changes as patch
series.

take care,
  Gerd



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-03-11 12:56 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-01 11:45 [PATCH 1/2] coreaudio: Drop support for macOS older than 10.6 Akihiko Odaki
2021-03-01 11:45 ` [PATCH 2/2] coreaudio: Handle output device change Akihiko Odaki
2021-03-03  9:27   ` Gerd Hoffmann
2021-03-03 13:20     ` Akihiko Odaki
2021-03-11 12:50       ` Gerd Hoffmann
2021-03-03  9:22 ` [PATCH 1/2] coreaudio: Drop support for macOS older than 10.6 Gerd Hoffmann

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.