On 27/01/2026 18:24, [email protected] wrote:

From: Marc-André Lureau <[email protected]>

Migrate the PipeWire audio backend from the legacy driver init/fini
callbacks to proper QOM realize and finalize methods.

The pwaudio struct fields are now embedded directly in the AudioPw
QOM object instead of being allocated separately as drv_opaque. This
allows accessing the backend state through proper QOM type casting
with AUDIO_PW() rather than casting drv_opaque pointers.

The PipeWire thread loop and context are now managed through the QOM
lifecycle, with initialization in realize and cleanup in finalize.

Signed-off-by: Marc-André Lureau <[email protected]>
---
  audio/pwaudio.c | 119 +++++++++++++++++++++++-------------------------
  1 file changed, 58 insertions(+), 61 deletions(-)

diff --git a/audio/pwaudio.c b/audio/pwaudio.c
index ed48a9754fe..ddf5f2c70f7 100644
--- a/audio/pwaudio.c
+++ b/audio/pwaudio.c
@@ -31,36 +31,27 @@
  #define TYPE_AUDIO_PW "audio-pipewire"
  OBJECT_DECLARE_SIMPLE_TYPE(AudioPw, AUDIO_PW)
+static AudioBackendClass *audio_pw_parent_class;
+
  struct AudioPw {
      AudioMixengBackend parent;
-};
-static struct audio_driver pw_audio_driver;
+    struct pw_thread_loop *thread_loop;
+    struct pw_context *context;
-static void audio_pw_class_init(ObjectClass *klass, const void *data)
-{
-    AudioMixengBackendClass *k = AUDIO_MIXENG_BACKEND_CLASS(klass);
+    struct pw_core *core;
+    struct spa_hook core_listener;
+    int last_seq, pending_seq, error;
+};
- k->driver = &pw_audio_driver;
-}
+static struct audio_driver pw_audio_driver;
typedef struct pwvolume {
      uint32_t channels;
      float values[SPA_AUDIO_MAX_CHANNELS];
  } pwvolume;
-typedef struct pwaudio {
-    Audiodev *dev;
-    struct pw_thread_loop *thread_loop;
-    struct pw_context *context;
-
-    struct pw_core *core;
-    struct spa_hook core_listener;
-    int last_seq, pending_seq, error;
-} pwaudio;
-
  typedef struct PWVoice {
-    pwaudio *g;
      struct pw_stream *stream;
      struct spa_hook stream_listener;
      struct spa_audio_info_raw info;
@@ -236,9 +227,9 @@ static const struct pw_stream_events playback_stream_events 
= {
  static size_t
  qpw_read(HWVoiceIn *hw, void *data, size_t len)
  {
+    AudioPw *c = AUDIO_PW(hw->s);
      PWVoiceIn *pw = (PWVoiceIn *) hw;
      PWVoice *v = &pw->v;
-    pwaudio *c = v->g;
      const char *error = NULL;
      size_t l;
      int32_t avail;
@@ -273,9 +264,9 @@ done_unlock:
static size_t qpw_buffer_get_free(HWVoiceOut *hw)
  {
+    AudioPw *c = AUDIO_PW(hw->s);
      PWVoiceOut *pw = (PWVoiceOut *)hw;
      PWVoice *v = &pw->v;
-    pwaudio *c = v->g;
      const char *error = NULL;
      int32_t filled, avail;
      uint32_t index;
@@ -298,9 +289,9 @@ done_unlock:
  static size_t
  qpw_write(HWVoiceOut *hw, void *data, size_t len)
  {
+    AudioPw *c = AUDIO_PW(hw->s);
      PWVoiceOut *pw = (PWVoiceOut *) hw;
      PWVoice *v = &pw->v;
-    pwaudio *c = v->g;
      const char *error = NULL;
      int32_t filled, avail;
      uint32_t index;
@@ -434,7 +425,7 @@ pw_to_audfmt(enum spa_audio_format fmt, int *endianness,
  }
static int
-qpw_stream_new(pwaudio *c, PWVoice *v, const char *stream_name,
+qpw_stream_new(AudioPw *c, PWVoice *v, const char *stream_name,
                 const char *name, enum spa_direction dir)
  {
      int res;
@@ -452,8 +443,8 @@ qpw_stream_new(pwaudio *c, PWVoice *v, const char 
*stream_name,
      }
/* 75% of the timer period for faster updates */
-    buf_samples = (uint64_t)v->g->dev->timer_period * v->info.rate
-                    * 3 / 4 / 1000000;
+    buf_samples = (uint64_t)AUDIO_MIXENG_BACKEND(c)->dev->timer_period
+        * v->info.rate * 3 / 4 / 1000000;
      pw_properties_setf(props, PW_KEY_NODE_LATENCY, "%" PRIu64 "/%u",
                         buf_samples, v->info.rate);
@@ -535,11 +526,11 @@ qpw_set_position(uint32_t channels, uint32_t position[SPA_AUDIO_MAX_CHANNELS])
  static int
  qpw_init_out(HWVoiceOut *hw, struct audsettings *as, void *drv_opaque)
  {
+    AudioPw *c = AUDIO_PW(hw->s);
      PWVoiceOut *pw = (PWVoiceOut *) hw;
      PWVoice *v = &pw->v;
      struct audsettings obt_as = *as;
-    pwaudio *c = v->g = drv_opaque;
-    AudiodevPipewireOptions *popts = &c->dev->u.pipewire;
+    AudiodevPipewireOptions *popts = &AUDIO_MIXENG_BACKEND(c)->dev->u.pipewire;
      AudiodevPipewirePerDirectionOptions *ppdo = popts->out;
      int r;
@@ -554,11 +545,11 @@ qpw_init_out(HWVoiceOut *hw, struct audsettings *as, void *drv_opaque)
          pw_to_audfmt(v->info.format, &obt_as.endianness, &v->frame_size);
      v->frame_size *= as->nchannels;
- v->req = (uint64_t)c->dev->timer_period * v->info.rate
+    v->req = (uint64_t)AUDIO_MIXENG_BACKEND(c)->dev->timer_period * 
v->info.rate
          * 1 / 2 / 1000000 * v->frame_size;
/* call the function that creates a new stream for playback */
-    r = qpw_stream_new(c, v, ppdo->stream_name ? : c->dev->id,
+    r = qpw_stream_new(c, v, ppdo->stream_name ?: 
AUDIO_MIXENG_BACKEND(c)->dev->id,
                         ppdo->name, SPA_DIRECTION_OUTPUT);
      if (r < 0) {
          pw_thread_loop_unlock(c->thread_loop);
@@ -582,11 +573,11 @@ qpw_init_out(HWVoiceOut *hw, struct audsettings *as, void 
*drv_opaque)
  static int
  qpw_init_in(HWVoiceIn *hw, struct audsettings *as, void *drv_opaque)
  {
+    AudioPw *c = AUDIO_PW(hw->s);
      PWVoiceIn *pw = (PWVoiceIn *) hw;
      PWVoice *v = &pw->v;
      struct audsettings obt_as = *as;
-    pwaudio *c = v->g = drv_opaque;
-    AudiodevPipewireOptions *popts = &c->dev->u.pipewire;
+    AudiodevPipewireOptions *popts = &AUDIO_MIXENG_BACKEND(c)->dev->u.pipewire;
      AudiodevPipewirePerDirectionOptions *ppdo = popts->in;
      int r;
@@ -602,7 +593,7 @@ qpw_init_in(HWVoiceIn *hw, struct audsettings *as, void *drv_opaque)
      v->frame_size *= as->nchannels;
/* call the function that creates a new stream for recording */
-    r = qpw_stream_new(c, v, ppdo->stream_name ? : c->dev->id,
+    r = qpw_stream_new(c, v, ppdo->stream_name ? : 
AUDIO_MIXENG_BACKEND(c)->dev->id,
                         ppdo->name, SPA_DIRECTION_INPUT);
      if (r < 0) {
          pw_thread_loop_unlock(c->thread_loop);
@@ -621,10 +612,8 @@ qpw_init_in(HWVoiceIn *hw, struct audsettings *as, void 
*drv_opaque)
  }
static void
-qpw_voice_fini(PWVoice *v)
+qpw_voice_fini(AudioPw *c, PWVoice *v)
  {
-    pwaudio *c = v->g;
-
      if (!v->stream) {
          return;
      }
@@ -637,19 +626,18 @@ qpw_voice_fini(PWVoice *v)
  static void
  qpw_fini_out(HWVoiceOut *hw)
  {
-    qpw_voice_fini(&PW_VOICE_OUT(hw)->v);
+    qpw_voice_fini(AUDIO_PW(hw->s), &PW_VOICE_OUT(hw)->v);
  }
static void
  qpw_fini_in(HWVoiceIn *hw)
  {
-    qpw_voice_fini(&PW_VOICE_IN(hw)->v);
+    qpw_voice_fini(AUDIO_PW(hw->s), &PW_VOICE_IN(hw)->v);
  }
static void
-qpw_voice_set_enabled(PWVoice *v, bool enable)
+qpw_voice_set_enabled(AudioPw *c, PWVoice *v, bool enable)
  {
-    pwaudio *c = v->g;
      pw_thread_loop_lock(c->thread_loop);
      pw_stream_set_active(v->stream, enable);
      pw_thread_loop_unlock(c->thread_loop);
@@ -658,19 +646,18 @@ qpw_voice_set_enabled(PWVoice *v, bool enable)
  static void
  qpw_enable_out(HWVoiceOut *hw, bool enable)
  {
-    qpw_voice_set_enabled(&PW_VOICE_OUT(hw)->v, enable);
+    qpw_voice_set_enabled(AUDIO_PW(hw->s), &PW_VOICE_OUT(hw)->v, enable);
  }
static void
  qpw_enable_in(HWVoiceIn *hw, bool enable)
  {
-    qpw_voice_set_enabled(&PW_VOICE_IN(hw)->v, enable);
+    qpw_voice_set_enabled(AUDIO_PW(hw->s), &PW_VOICE_IN(hw)->v, enable);
  }
static void
-qpw_voice_set_volume(PWVoice *v, Volume *vol)
+qpw_voice_set_volume(AudioPw *c, PWVoice *v, Volume *vol)
  {
-    pwaudio *c = v->g;
      int i, ret;
pw_thread_loop_lock(c->thread_loop);
@@ -693,16 +680,16 @@ qpw_voice_set_volume(PWVoice *v, Volume *vol)
  static void
  qpw_volume_out(HWVoiceOut *hw, Volume *vol)
  {
-    qpw_voice_set_volume(&PW_VOICE_OUT(hw)->v, vol);
+    qpw_voice_set_volume(AUDIO_PW(hw->s), &PW_VOICE_OUT(hw)->v, vol);
  }
static void
  qpw_volume_in(HWVoiceIn *hw, Volume *vol)
  {
-    qpw_voice_set_volume(&PW_VOICE_IN(hw)->v, vol);
+    qpw_voice_set_volume(AUDIO_PW(hw->s), &PW_VOICE_IN(hw)->v, vol);
  }
-static int wait_resync(pwaudio *pw)
+static int wait_resync(AudioPw *pw)
  {
      int res;
      pw->pending_seq = pw_core_sync(pw->core, PW_ID_CORE, pw->pending_seq);
@@ -725,7 +712,7 @@ static int wait_resync(pwaudio *pw)
  static void
  on_core_error(void *data, uint32_t id, int seq, int res, const char *message)
  {
-    pwaudio *pw = data;
+    AudioPw *pw = data;
error_report("error id:%u seq:%d res:%d (%s): %s",
                  id, seq, res, spa_strerror(res), message);
@@ -737,7 +724,7 @@ on_core_error(void *data, uint32_t id, int seq, int res, 
const char *message)
  static void
  on_core_done(void *data, uint32_t id, int seq)
  {
-    pwaudio *pw = data;
+    AudioPw *pw = data;
      assert(id == PW_ID_CORE);
      pw->last_seq = seq;
      if (pw->pending_seq == seq) {
@@ -752,17 +739,20 @@ static const struct pw_core_events core_events = {
      .error = on_core_error,
  };
-static void *
-qpw_audio_init(Audiodev *dev, Error **errp)
+static bool
+audio_pw_realize(AudioBackend *abe, Audiodev *dev, Error **errp)
  {
-    g_autofree pwaudio *pw = g_new0(pwaudio, 1);
+    AudioPw *pw = AUDIO_PW(abe);
assert(dev->driver == AUDIODEV_DRIVER_PIPEWIRE);
      trace_pw_audio_init();
+ if (!audio_pw_parent_class->realize(abe, dev, errp)) {
+        return false;
+    }
+
      pw_init(NULL, NULL);
- pw->dev = dev;
      pw->thread_loop = pw_thread_loop_new("PipeWire thread loop", NULL);
      if (pw->thread_loop == NULL) {
          error_setg_errno(errp, errno, "Could not create PipeWire loop");
@@ -801,8 +791,7 @@ qpw_audio_init(Audiodev *dev, Error **errp)
      }
pw_thread_loop_unlock(pw->thread_loop);
-
-    return g_steal_pointer(&pw);
+    return true;

Unrelated to this patch directly, however when viewing the file at this commit I see the end of audio_pw_realize() looks like this:

    if (wait_resync(pw) < 0) {
        pw_thread_loop_unlock(pw->thread_loop);
    }

    pw_thread_loop_unlock(pw->thread_loop);
    return true;

If pw_thread_loop_unlock() is always called regardless, why do we need the wait_resync() logic? Is there a refcounted mutex involved?

  fail:
      if (pw->thread_loop) {
@@ -810,13 +799,13 @@ fail:
      }
      g_clear_pointer(&pw->context, pw_context_destroy);
      g_clear_pointer(&pw->thread_loop, pw_thread_loop_destroy);
-    return NULL;
+    return false;
  }
static void
-qpw_audio_fini(void *opaque)
+audio_pw_finalize(Object *obj)
  {
-    pwaudio *pw = opaque;
+    AudioPw *pw = AUDIO_PW(obj);
if (pw->thread_loop) {
          pw_thread_loop_stop(pw->thread_loop);
@@ -831,9 +820,7 @@ qpw_audio_fini(void *opaque)
      if (pw->context) {
          pw_context_destroy(pw->context);
      }
-    pw_thread_loop_destroy(pw->thread_loop);
-
-    g_free(pw);
+    g_clear_pointer(&pw->thread_loop, pw_thread_loop_destroy);
  }

Is there any way that pw->thread_loop can be locked when _finalize() is invoked given that there is no explicit unlock here? Or is this done by one of the other functions?

  static struct audio_pcm_ops qpw_pcm_ops = {
@@ -855,8 +842,6 @@ static struct audio_pcm_ops qpw_pcm_ops = {
static struct audio_driver pw_audio_driver = {
      .name = "pipewire",
-    .init = qpw_audio_init,
-    .fini = qpw_audio_fini,
      .pcm_ops = &qpw_pcm_ops,
      .max_voices_out = INT_MAX,
      .max_voices_in = INT_MAX,
@@ -864,11 +849,23 @@ static struct audio_driver pw_audio_driver = {
      .voice_size_in = sizeof(PWVoiceIn),
  };
+static void audio_pw_class_init(ObjectClass *klass, const void *data)
+{
+    AudioBackendClass *b = AUDIO_BACKEND_CLASS(klass);
+    AudioMixengBackendClass *k = AUDIO_MIXENG_BACKEND_CLASS(klass);
+
+    audio_pw_parent_class = 
AUDIO_BACKEND_CLASS(object_class_get_parent(klass));
+
+    b->realize = audio_pw_realize;
+    k->driver = &pw_audio_driver;
+}
+
  static const TypeInfo audio_pw_info = {
      .name = TYPE_AUDIO_PW,
      .parent = TYPE_AUDIO_MIXENG_BACKEND,
      .instance_size = sizeof(AudioPw),
      .class_init = audio_pw_class_init,
+    .instance_finalize = audio_pw_finalize,
  };
static void

Regardless:

Reviewed-by: Mark Cave-Ayland <[email protected]>


ATB,

Mark.


Reply via email to