Merge pull request #4045 from khaneliman/pulse

audio_backend: fix crash

src/util/audio_backend.cpp

@@ -24,6 +24,8 @@ AudioBackend::AudioBackend(std::function<void()> on_updated_cb, private_construc
       source_volume_(0),
       source_muted_(false),
       on_updated_cb_(std::move(on_updated_cb)) {
+  // Initialize pa_volume_ with safe defaults
+  pa_cvolume_init(&pa_volume_);
   mainloop_ = pa_threaded_mainloop_new();
   if (mainloop_ == nullptr) {
     throw std::runtime_error("pa_mainloop_new() failed.");
@@ -131,7 +133,12 @@ void AudioBackend::subscribeCb(pa_context *context, pa_subscription_event_type_t
 void AudioBackend::volumeModifyCb(pa_context *c, int success, void *data) {
   auto *backend = static_cast<AudioBackend *>(data);
   if (success != 0) {
-    pa_context_get_sink_info_by_index(backend->context_, backend->sink_idx_, sinkInfoCb, data);
+    if ((backend->context_ != nullptr) &&
+        pa_context_get_state(backend->context_) == PA_CONTEXT_READY) {
+      pa_context_get_sink_info_by_index(backend->context_, backend->sink_idx_, sinkInfoCb, data);
+    }
+  } else {
+    spdlog::debug("Volume modification failed");
   }
 }
 
@@ -180,11 +187,20 @@ void AudioBackend::sinkInfoCb(pa_context * /*context*/, const pa_sink_info *i, i
   }
 
   if (backend->current_sink_name_ == i->name) {
-    backend->pa_volume_ = i->volume;
+    // Safely copy the volume structure
-    float volume =
+    if (pa_cvolume_valid(&i->volume) != 0) {
-        static_cast<float>(pa_cvolume_avg(&(backend->pa_volume_))) / float{PA_VOLUME_NORM};
+      backend->pa_volume_ = i->volume;
-    backend->sink_idx_ = i->index;
+      float volume =
-    backend->volume_ = std::round(volume * 100.0F);
+          static_cast<float>(pa_cvolume_avg(&(backend->pa_volume_))) / float{PA_VOLUME_NORM};
+      backend->sink_idx_ = i->index;
+      backend->volume_ = std::round(volume * 100.0F);
+    } else {
+      spdlog::error("Invalid volume structure received from PulseAudio");
+      // Initialize with safe defaults
+      pa_cvolume_init(&backend->pa_volume_);
+      backend->volume_ = 0;
+    }
+
     backend->muted_ = i->mute != 0;
     backend->desc_ = i->description;
     backend->monitor_ = i->monitor_source_name;
@@ -230,43 +246,109 @@ void AudioBackend::serverInfoCb(pa_context *context, const pa_server_info *i, vo
 }
 
 void AudioBackend::changeVolume(uint16_t volume, uint16_t min_volume, uint16_t max_volume) {
-  double volume_tick = static_cast<double>(PA_VOLUME_NORM) / 100;
+  // Early return if context is not ready
-  pa_cvolume pa_volume = pa_volume_;
+  if ((context_ == nullptr) || pa_context_get_state(context_) != PA_CONTEXT_READY) {
+    spdlog::error("PulseAudio context not ready");
+    return;
+  }
 
+  // Prepare volume structure
+  pa_cvolume pa_volume;
+
+  pa_cvolume_init(&pa_volume);
+
+  // Use existing volume structure if valid, otherwise create a safe default
+  if ((pa_cvolume_valid(&pa_volume_) != 0) && (pa_channels_valid(pa_volume_.channels) != 0)) {
+    pa_volume = pa_volume_;
+  } else {
+    // Set stereo as a safe default
+    pa_volume.channels = 2;
+    spdlog::debug("Using default stereo volume structure");
+  }
+
+  // Set the volume safely
   volume = std::clamp(volume, min_volume, max_volume);
-  pa_cvolume_set(&pa_volume, pa_volume_.channels, volume * volume_tick);
+  pa_volume_t vol = volume * (static_cast<double>(PA_VOLUME_NORM) / 100);
 
+  // Set all channels to the same volume manually to avoid pa_cvolume_set
+  for (uint8_t i = 0; i < pa_volume.channels; i++) {
+    pa_volume.values[i] = vol;
+  }
+
+  // Apply the volume change
   pa_threaded_mainloop_lock(mainloop_);
   pa_context_set_sink_volume_by_index(context_, sink_idx_, &pa_volume, volumeModifyCb, this);
   pa_threaded_mainloop_unlock(mainloop_);
 }
 
 void AudioBackend::changeVolume(ChangeType change_type, double step, uint16_t max_volume) {
-  double volume_tick = static_cast<double>(PA_VOLUME_NORM) / 100;
+  // Early return if context is not ready
-  pa_volume_t change = volume_tick;
+  if ((context_ == nullptr) || pa_context_get_state(context_) != PA_CONTEXT_READY) {
-  pa_cvolume pa_volume = pa_volume_;
+    spdlog::error("PulseAudio context not ready");
+    return;
+  }
 
+  // Prepare volume structure
+  pa_cvolume pa_volume;
+  pa_cvolume_init(&pa_volume);
+
+  // Use existing volume structure if valid, otherwise create a safe default
+  if ((pa_cvolume_valid(&pa_volume_) != 0) && (pa_channels_valid(pa_volume_.channels) != 0)) {
+    pa_volume = pa_volume_;
+  } else {
+    // Set stereo as a safe default
+    pa_volume.channels = 2;
+    spdlog::debug("Using default stereo volume structure");
+
+    // Initialize all channels to current volume level
+    double volume_tick = static_cast<double>(PA_VOLUME_NORM) / 100;
+    pa_volume_t vol = volume_ * volume_tick;
+    for (uint8_t i = 0; i < pa_volume.channels; i++) {
+      pa_volume.values[i] = vol;
+    }
+
+    // No need to continue with volume change if we had to create a new structure
+    pa_threaded_mainloop_lock(mainloop_);
+    pa_context_set_sink_volume_by_index(context_, sink_idx_, &pa_volume, volumeModifyCb, this);
+    pa_threaded_mainloop_unlock(mainloop_);
+    return;
+  }
+
+  // Calculate volume change
+  double volume_tick = static_cast<double>(PA_VOLUME_NORM) / 100;
+  pa_volume_t change;
   max_volume = std::min(max_volume, static_cast<uint16_t>(PA_VOLUME_UI_MAX));
 
-  if (change_type == ChangeType::Increase) {
+  if (change_type == ChangeType::Increase && volume_ < max_volume) {
-    if (volume_ < max_volume) {
+    // Calculate how much to increase
-      if (volume_ + step > max_volume) {
+    if (volume_ + step > max_volume) {
-        change = round((max_volume - volume_) * volume_tick);
+      change = round((max_volume - volume_) * volume_tick);
-      } else {
+    } else {
-        change = round(step * volume_tick);
+      change = round(step * volume_tick);
-      }
-      pa_cvolume_inc(&pa_volume, change);
     }
-  } else if (change_type == ChangeType::Decrease) {
+
-    if (volume_ > 0) {
+    // Manually increase each channel's volume
-      if (volume_ - step < 0) {
+    for (uint8_t i = 0; i < pa_volume.channels; i++) {
-        change = round(volume_ * volume_tick);
+      pa_volume.values[i] = std::min(pa_volume.values[i] + change, PA_VOLUME_MAX);
-      } else {
-        change = round(step * volume_tick);
-      }
-      pa_cvolume_dec(&pa_volume, change);
     }
+  } else if (change_type == ChangeType::Decrease && volume_ > 0) {
+    // Calculate how much to decrease
+    if (volume_ - step < 0) {
+      change = round(volume_ * volume_tick);
+    } else {
+      change = round(step * volume_tick);
+    }
+
+    // Manually decrease each channel's volume
+    for (uint8_t i = 0; i < pa_volume.channels; i++) {
+      pa_volume.values[i] = (pa_volume.values[i] > change) ? (pa_volume.values[i] - change) : 0;
+    }
+  } else {
+    // No change needed
+    return;
   }
+
+  // Apply the volume change
   pa_threaded_mainloop_lock(mainloop_);
   pa_context_set_sink_volume_by_index(context_, sink_idx_, &pa_volume, volumeModifyCb, this);
   pa_threaded_mainloop_unlock(mainloop_);