Bug 1515549 - Avoid resetting volume in every AudioSink restart in case volume has changed outside firefox. r=jya
authorAlex Chronopoulos <achronop@gmail.com>
Thu, 10 Jan 2019 11:24:29 +0000
changeset 510337 849f81f21979e1eceec29e2b3f7dd769d64496e4
parent 510336 876faba9c48f5bee522dae19d91acde4b60efc86
child 510338 34d4a5081960a1623e0ebdf9e4fb60c7b167b8f4
push id10547
push userffxbld-merge
push dateMon, 21 Jan 2019 13:03:58 +0000
treeherdermozilla-beta@24ec1916bffe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjya
bugs1515549
milestone66.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1515549 - Avoid resetting volume in every AudioSink restart in case volume has changed outside firefox. r=jya Avoid resetting volume in every AudioSink restart to stop unexpected volume change during play/pause and seek. Volume changes could occur if the volume has been modified outside firefox (for example pavucontrol in Linux). Differential Revision: https://phabricator.services.mozilla.com/D15210
dom/media/mediasink/AudioSink.cpp
dom/media/mediasink/AudioSinkWrapper.cpp
dom/media/mediasink/DecodedStream.cpp
dom/media/mediasink/MediaSink.h
--- a/dom/media/mediasink/AudioSink.cpp
+++ b/dom/media/mediasink/AudioSink.cpp
@@ -182,17 +182,19 @@ nsresult AudioSink::InitializeAudioStrea
   if (NS_FAILED(rv)) {
     mAudioStream->Shutdown();
     mAudioStream = nullptr;
     return rv;
   }
 
   // Set playback params before calling Start() so they can take effect
   // as soon as the 1st DataCallback of the AudioStream fires.
-  mAudioStream->SetVolume(aParams.mVolume);
+  if (aParams.mVolume) {
+    mAudioStream->SetVolume(*aParams.mVolume);
+  }
   mAudioStream->SetPlaybackRate(aParams.mPlaybackRate);
   mAudioStream->SetPreservesPitch(aParams.mPreservesPitch);
   return mAudioStream->Start();
 }
 
 TimeUnit AudioSink::GetEndTime() const {
   int64_t written;
   {
--- a/dom/media/mediasink/AudioSinkWrapper.cpp
+++ b/dom/media/mediasink/AudioSinkWrapper.cpp
@@ -24,17 +24,19 @@ void AudioSinkWrapper::Shutdown() {
 const MediaSink::PlaybackParams& AudioSinkWrapper::GetPlaybackParams() const {
   AssertOwnerThread();
   return mParams;
 }
 
 void AudioSinkWrapper::SetPlaybackParams(const PlaybackParams& aParams) {
   AssertOwnerThread();
   if (mAudioSink) {
-    mAudioSink->SetVolume(aParams.mVolume);
+    if (aParams.mVolume) {
+      mAudioSink->SetVolume(*aParams.mVolume);
+    }
     mAudioSink->SetPlaybackRate(aParams.mPlaybackRate);
     mAudioSink->SetPreservesPitch(aParams.mPreservesPitch);
   }
   mParams = aParams;
 }
 
 RefPtr<MediaSink::EndedPromise> AudioSinkWrapper::OnEnded(TrackType aType) {
   AssertOwnerThread();
@@ -91,17 +93,17 @@ TimeUnit AudioSinkWrapper::GetPosition(T
 
 bool AudioSinkWrapper::HasUnplayedFrames(TrackType aType) const {
   AssertOwnerThread();
   return mAudioSink ? mAudioSink->HasUnplayedFrames() : false;
 }
 
 void AudioSinkWrapper::SetVolume(double aVolume) {
   AssertOwnerThread();
-  mParams.mVolume = aVolume;
+  mParams.mVolume = Some(aVolume);
   if (mAudioSink) {
     mAudioSink->SetVolume(aVolume);
   }
 }
 
 void AudioSinkWrapper::SetPlaybackRate(double aPlaybackRate) {
   AssertOwnerThread();
   if (!mAudioEnded) {
@@ -191,16 +193,20 @@ void AudioSinkWrapper::Stop() {
   AssertOwnerThread();
   MOZ_ASSERT(mIsStarted, "playback not started.");
 
   mIsStarted = false;
   mAudioEnded = true;
 
   if (mAudioSink) {
     mAudioSinkEndedPromise.DisconnectIfExists();
+    // Reset volume to signal that it should
+    // not be updated, in case the volume
+    // has been changed outside MediaElement.
+    mParams.mVolume.reset();
     mAudioSink->Shutdown();
     mAudioSink = nullptr;
     mEndedPromise = nullptr;
   }
 }
 
 bool AudioSinkWrapper::IsStarted() const {
   AssertOwnerThread();
--- a/dom/media/mediasink/DecodedStream.cpp
+++ b/dom/media/mediasink/DecodedStream.cpp
@@ -474,17 +474,17 @@ void DecodedStream::SetPlaying(bool aPla
     return;
   }
 
   mPlaying = aPlaying;
 }
 
 void DecodedStream::SetVolume(double aVolume) {
   AssertOwnerThread();
-  mParams.mVolume = aVolume;
+  mParams.mVolume = Some(aVolume);
 }
 
 void DecodedStream::SetPlaybackRate(double aPlaybackRate) {
   AssertOwnerThread();
   mParams.mPlaybackRate = aPlaybackRate;
 }
 
 void DecodedStream::SetPreservesPitch(bool aPreservesPitch) {
@@ -725,17 +725,18 @@ void DecodedStream::SendData() {
   AssertOwnerThread();
   MOZ_ASSERT(mStartTime.isSome(), "Must be called after StartPlayback()");
 
   // Not yet created on the main thread. MDSM will try again later.
   if (!mData) {
     return;
   }
 
-  SendAudio(mParams.mVolume, mSameOrigin, mPrincipalHandle);
+  MOZ_ASSERT(mParams.mVolume.isSome(), "Volume should exist at that point");
+  SendAudio(mParams.mVolume.value(), mSameOrigin, mPrincipalHandle);
   SendVideo(mSameOrigin, mPrincipalHandle);
 }
 
 TimeUnit DecodedStream::GetEndTime(TrackType aType) const {
   AssertOwnerThread();
   if (aType == TrackInfo::kAudioTrack && mInfo.HasAudio() && mData) {
     auto t = mStartTime.ref() +
              FramesToTimeUnit(mData->mAudioFramesWritten, mInfo.mAudio.mRate);
--- a/dom/media/mediasink/MediaSink.h
+++ b/dom/media/mediasink/MediaSink.h
@@ -34,18 +34,18 @@ class TimeStamp;
  */
 class MediaSink {
  public:
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(MediaSink);
   typedef mozilla::TrackInfo::TrackType TrackType;
 
   struct PlaybackParams {
     PlaybackParams()
-        : mVolume(1.0), mPlaybackRate(1.0), mPreservesPitch(true) {}
-    double mVolume;
+        : mVolume(Some(1.0)), mPlaybackRate(1.0), mPreservesPitch(true) {}
+    Maybe<double> mVolume;
     double mPlaybackRate;
     bool mPreservesPitch;
     RefPtr<AudioDeviceInfo> mSink;
   };
 
   // Return the playback parameters of this sink.
   // Can be called in any state.
   virtual const PlaybackParams& GetPlaybackParams() const = 0;