Merge pull request #5231 from fritsch/pa-volume-fix

AESinkPULSE: Fix linear volume to do the same as other PA apps

2 files changed, 90 insertions, 106 deletions

diff --git a/xbmc/cores/AudioEngine/Sinks/AESinkPULSE.cpp b/xbmc/cores/AudioEngine/Sinks/AESinkPULSE.cpp
index eb36ad1615..4626f166df 100644
--- a/xbmc/cores/AudioEngine/Sinks/AESinkPULSE.cpp
+++ b/xbmc/cores/AudioEngine/Sinks/AESinkPULSE.cpp
@@ -175,6 +175,33 @@ static void StreamLatencyUpdateCallback(pa_stream *s, void *userdata)
   pa_threaded_mainloop_signal(m, 0);
 }
 
+
+static void SinkInputInfoCallback(pa_context *c, const pa_sink_input_info *i, int eol, void *userdata)
+{
+  CAESinkPULSE *p = (CAESinkPULSE*) userdata;
+  if (!p || !p->IsInitialized())
+    return;
+
+  if(i && i->has_volume)
+    p->UpdateInternalVolume(i->volume);
+}
+
+static void SinkInputInfoChangedCallback(pa_context *c, pa_subscription_event_type_t t, uint32_t idx, void *userdata)
+{
+  CAESinkPULSE* p = (CAESinkPULSE*) userdata;
+  if (!p || !p->IsInitialized())
+    return;
+  
+   if (idx != pa_stream_get_index(p->GetInternalStream()))
+     return;
+   
+   pa_operation* op = pa_context_get_sink_input_info(c, idx, SinkInputInfoCallback, p);
+   if (op == NULL)
+     CLog::Log(LOGERROR, "PulseAudio: Failed to sync volume");
+   else
+    pa_operation_unref(op);
+}
+
 static void SinkChangedCallback(pa_context *c, pa_subscription_event_type_t t, uint32_t idx, void *userdata)
 {
   CAESinkPULSE* p = (CAESinkPULSE*) userdata;
@@ -219,21 +246,6 @@ struct SinkInfoStruct
   }
 };
 
-struct SinkInputInfoStruct
-{
-  bool is_valid;
-  int mute;
-  int index;
-  pa_cvolume volume;
-  pa_threaded_mainloop *mainloop;
-  SinkInputInfoStruct()
-  {
-    is_valid = false;
-    mute = 0;
-    mainloop = NULL;
-  }
-};
-
 static void SinkInfoCallback(pa_context *c, const pa_sink_info *i, int eol, void *userdata)
 {
   SinkInfoStruct *sinkStruct = (SinkInfoStruct *)userdata;
@@ -248,19 +260,6 @@ static void SinkInfoCallback(pa_context *c, const pa_sink_info *i, int eol, void
   pa_threaded_mainloop_signal(sinkStruct->mainloop, 0);
 }
 
-static void SinkInputInfoCallback(pa_context *c, const pa_sink_input_info *i, int eol, void *userdata)
-{
-  SinkInputInfoStruct *siiStruct = (SinkInputInfoStruct *)userdata;
-  if(i && i->has_volume)
-  {
-    siiStruct->is_valid = true;
-    siiStruct->volume = i->volume;
-    siiStruct->mute = i->mute;
-    siiStruct->index = i->index;
-  }
-  pa_threaded_mainloop_signal(siiStruct->mainloop, 0);
-}
-
 static AEChannel PAChannelToAEChannel(pa_channel_position_t channel)
 {
   AEChannel ae_channel;
@@ -442,6 +441,7 @@ CAESinkPULSE::CAESinkPULSE()
   m_Stream = NULL;
   m_Context = NULL;
   m_IsStreamPaused = false;
+  m_volume_needs_update = false;
 }
 
 CAESinkPULSE::~CAESinkPULSE()
@@ -471,16 +471,6 @@ bool CAESinkPULSE::Initialize(AEAudioFormat &format, std::string &device)
 
   pa_threaded_mainloop_lock(m_MainLoop);
 
-  {
-    // Register Callback for Sink changes
-    CSingleLock lock(m_sec);
-    pa_context_set_subscribe_callback(m_Context, SinkChangedCallback, this);
-    const pa_subscription_mask_t mask = PA_SUBSCRIPTION_MASK_SINK;
-    pa_operation *op = pa_context_subscribe(m_Context, mask, NULL, this);
-    if (op != NULL)
-      pa_operation_unref(op);
-  }
-
   struct pa_channel_map map;
   pa_channel_map_init(&map);
 
@@ -633,29 +623,6 @@ bool CAESinkPULSE::Initialize(AEAudioFormat &format, std::string &device)
     return false;
   }
 
-  //update local volume if we are in non passthrough mode
-  if (!m_passthrough)
-  {
-    unsigned int sink_input_idx = pa_stream_get_index(m_Stream);
-    SinkInputInfoStruct sii;
-    sii.mainloop = m_MainLoop;
-    bool success = WaitForOperation(pa_context_get_sink_input_info(m_Context, sink_input_idx, SinkInputInfoCallback, &sii), m_MainLoop, "Get Sink Input Info");
-    if(success && sii.is_valid)
-    {
-      // we don't have per channel values so use avg of them
-      pa_volume_t p_vol = pa_cvolume_avg(&sii.volume);
-      // store it internally
-      m_Volume = sii.volume;
-      float sValue = (float) pa_sw_volume_to_linear(p_vol);
-      CLog::Log(LOGDEBUG, "Restored Stream value to %f", sValue);
-      g_application.SetVolume(sValue, false);
-      if (sii.mute && sValue > 0)
-      {
-        CLog::Log(LOGDEBUG, "PulseAudio: Stream is muted - perhaps was a user wish - if volume is changed we unmute");
-      }
-    } 
-  }
-
   const pa_buffer_attr *a;
 
   if (!(a = pa_stream_get_buffer_attr(m_Stream)))
@@ -673,6 +640,23 @@ bool CAESinkPULSE::Initialize(AEAudioFormat &format, std::string &device)
     format.m_frames = packetSize / frameSize;
   }
 
+  {
+    CSingleLock lock(m_sec);
+    // Register Callback for Sink changes
+    pa_context_set_subscribe_callback(m_Context, SinkChangedCallback, this);
+    const pa_subscription_mask_t mask = PA_SUBSCRIPTION_MASK_SINK;
+    pa_operation *op = pa_context_subscribe(m_Context, mask, NULL, this);
+    if (op != NULL)
+      pa_operation_unref(op);
+
+    // Register Callback for Sink Info changes - this handles volume
+    pa_context_set_subscribe_callback(m_Context, SinkInputInfoChangedCallback, this);
+    const pa_subscription_mask_t mask_input = PA_SUBSCRIPTION_MASK_SINK_INPUT;
+    pa_operation* op_sinfo = pa_context_subscribe(m_Context, mask_input, NULL, this);
+    if (op_sinfo != NULL)
+      pa_operation_unref(op_sinfo);
+  }
+
   pa_threaded_mainloop_unlock(m_MainLoop);
   
   format.m_frameSize = frameSize;
@@ -804,62 +788,59 @@ void CAESinkPULSE::Drain()
   pa_threaded_mainloop_unlock(m_MainLoop);
 }
 
+// This is a helper to get stream info during the PA callbacks
+// it shall never be called from real outside
+pa_stream* CAESinkPULSE::GetInternalStream()
+{
+  return m_Stream;
+}
+
+void CAESinkPULSE::UpdateInternalVolume(pa_cvolume nVol)
+{
+  pa_volume_t o_vol = pa_cvolume_avg(&m_Volume);
+  pa_volume_t n_vol = pa_cvolume_avg(&nVol);
+
+  if (o_vol != n_vol)
+  {
+    pa_cvolume_set(&m_Volume, m_Channels, n_vol);
+    m_volume_needs_update = true;
+  }
+}
+
 void CAESinkPULSE::SetVolume(float volume)
 {
   if (m_IsAllocated && !m_passthrough)
   {
+    pa_threaded_mainloop_lock(m_MainLoop);
     // clamp possibly too large / low values
     float per_cent_volume = std::max(0.0f, std::min(volume, 1.0f));
-
-    pa_threaded_mainloop_lock(m_MainLoop);
-    bool external_change = false;
-    //check if internal volume and sink input volume do not match
-    unsigned int sink_input_idx = pa_stream_get_index(m_Stream);
-    SinkInputInfoStruct sii;
-    sii.mainloop = m_MainLoop;
-    bool success = WaitForOperation(pa_context_get_sink_input_info(m_Context, sink_input_idx, SinkInputInfoCallback, &sii), m_MainLoop, "Get Sink Input Info");
-    float sValue = 0.0f;
-    if(success && sii.is_valid)
-    {
-      // we don't have per channel values so use avg of them
-      pa_volume_t n_vol = pa_cvolume_avg(&sii.volume);
-      pa_volume_t o_vol = pa_cvolume_avg(&m_Volume);
-      sValue = (float) pa_sw_volume_to_linear(n_vol);
-      if (n_vol != o_vol)
-      {
-        external_change = true;
-        // update internal volume
-        m_Volume = sii.volume;
-        CLog::Log(LOGDEBUG, "Restored Volume cause of external change to value to %f", sValue);
-        g_application.SetVolume(sValue, false);
-      }
-      // unmute if we should not be muted
-      if (sii.mute && sValue > 0)
-      {
-        pa_operation *op = pa_context_set_sink_input_mute(m_Context, sii.index, 0, NULL, NULL);
-        if (op == NULL)
-          CLog::Log(LOGERROR, "PulseAudio: Failed to unmute the stream");
-        else
-          pa_operation_unref(op);
-      }
-    }
-    else // we don't know stream volume so don't change anything
+    
+    if (m_volume_needs_update)
     {
-      external_change = true;
+       m_volume_needs_update = false;
+       pa_volume_t n_vol = pa_cvolume_avg(&m_Volume); 
+       n_vol = std::min(n_vol, PA_VOLUME_NORM);
+       per_cent_volume = (float) n_vol / PA_VOLUME_NORM; 
+       // only update internal volume
+       pa_threaded_mainloop_unlock(m_MainLoop);
+       g_application.SetVolume(per_cent_volume, false);
+       return;
     }
-    if (!external_change)
-    {
-      pa_volume_t pavolume = pa_sw_volume_from_linear(per_cent_volume);
-      if ( pavolume <= 0 )
-        pa_cvolume_mute(&m_Volume, m_Channels);
-      else
-        pa_cvolume_set(&m_Volume, m_Channels, pavolume);
+    
+    pa_volume_t pavolume = per_cent_volume * PA_VOLUME_NORM;
+    unsigned int sink_input_idx = pa_stream_get_index(m_Stream);
+    
+    if ( pavolume <= 0 )
+      pa_cvolume_mute(&m_Volume, m_Channels);
+    else
+      pa_cvolume_set(&m_Volume, m_Channels, pavolume);
+        
       pa_operation *op = pa_context_set_sink_input_volume(m_Context, sink_input_idx, &m_Volume, NULL, NULL);
       if (op == NULL)
         CLog::Log(LOGERROR, "PulseAudio: Failed to set volume");
       else
         pa_operation_unref(op);
-    }
+
     pa_threaded_mainloop_unlock(m_MainLoop);
   }
 }
diff --git a/xbmc/cores/AudioEngine/Sinks/AESinkPULSE.h b/xbmc/cores/AudioEngine/Sinks/AESinkPULSE.h
index 44753874c6..0ff733050e 100644
--- a/xbmc/cores/AudioEngine/Sinks/AESinkPULSE.h
+++ b/xbmc/cores/AudioEngine/Sinks/AESinkPULSE.h
@@ -50,6 +50,8 @@ public:
 
   static void EnumerateDevicesEx(AEDeviceInfoList &list, bool force = false);
   bool IsInitialized();
+  void UpdateInternalVolume(pa_cvolume nVol);
+  pa_stream* GetInternalStream();
   CCriticalSection m_sec;
 private:
   void Pause(bool pause);
@@ -64,9 +66,10 @@ private:
   unsigned int m_BytesPerSecond;
   unsigned int m_BufferSize;
   unsigned int m_Channels;
-  
-  pa_stream *m_Stream;
+
+  pa_stream *m_Stream; 
   pa_cvolume m_Volume;
+  bool m_volume_needs_update;
 
   pa_context *m_Context;
   pa_threaded_mainloop *m_MainLoop;