Bug 199559 – [GStreamer] Ensure that “volume = foo; assert(volume == foo)” always succeeds

• Status: NEW
• Component: Media

[Charlie Turner]

See https://bugs.webkit.org/show_bug.cgi?id=199505. It would be nice if for any media element E, E.volume = x does not undergo and transformations that cannot be reversed when reading the volume back. Currently in the pulseaudio backend, this is not the case due to the following PA code,


// converting to PA integer volume code
uint32_t pa_sw_volume_from_linear(double v) {
 
  if (v <= 0.0)
    return PA_VOLUME_MUTED;
 
  /*
   * We use a cubic mapping here, as suggested and discussed here:
   *
   * http://www.robotplanet.dk/audio/audio_gui_design/
   * http://lists.linuxaudio.org/pipermail/linux-audio-dev/2009-May/thread.html#23151
   *
   * We make sure that the conversion to linear and back yields the
   * same volume value! That's why we need the lround() below!
   */
 
// clamps between 0 and UINT32_MAX/2
  return (uint32_t) PA_CLAMP_VOLUME((uint64_t) lround(cbrt(v) * PA_VOLUME_NORM));
}

// converting back to double from PA integer volume code
double pa_sw_volume_to_linear(uint32_t v) {
  double f;
 
  if (v <= PA_VOLUME_MUTED)
    return 0.0;
 
  if (v == PA_VOLUME_NORM)
    return 1.0;
 
  f = ((double) v / PA_VOLUME_NORM);

  return (f*f*f);
}



The lround there means it's lossy. I'm not sure how to fix that when the PA devs do not consider it a bug.

[Charlie Turner]

The upstream PA bug is at https://gitlab.freedesktop.org/pulseaudio/pulseaudio/issues/697

[Xabier Rodríguez Calvar]

If PA devs don't fix this, which is unlikely, I think we should cache the volume we set somehow and use this unless we get a volume change signal from GStreamer, which would invalidate that cache.

[Philippe Normand]

In MediaPlayerPrivateGStreamerBase::notifyPlayerOfVolumeChange() why do we cast the value to float? m_player->volumeChanged() expects a double.

[Philippe Normand]

Also, I wonder if we should override MediaPlayerPrivate::setVolumeDouble(double) instead of MediaPlayer::setVolume(float).

[Charlie Turner]

(In reply to Philippe Normand from comment #3)
> In MediaPlayerPrivateGStreamerBase::notifyPlayerOfVolumeChange() why do we
> cast the value to float? m_player->volumeChanged() expects a double.

It's old code, perhaps the MediaPlayer class changed since it was written. Regardless, it doesn't matter since the error from PA has already crept in before we start casting. Same goes for the setVolumeDouble, PA behavior still introduces this issue.