Bug 1438134 - Make the return value of MediaEngineSource::Reconfigure well defined. r?jib draft
authorAndreas Pehrson <pehrsons@mozilla.com>
Thu, 22 Feb 2018 15:35:16 +0100
changeset 767915 badd0800a1d1ec69eb851b54bc18e75f3823ac76
parent 767914 76f33757de70deb544803bbd5519c7caf4858b0d
child 767916 d912a5268474aae507e79ded68564e58bd4cecf1
child 768123 d23b608ba74ab79f875e87f72c74246a666ce398
push id102738
push userbmo:apehrson@mozilla.com
push dateThu, 15 Mar 2018 10:50:20 +0000
reviewersjib
bugs1438134
milestone60.0a1
Bug 1438134 - Make the return value of MediaEngineSource::Reconfigure well defined. r?jib MozReview-Commit-ID: DR3sdtdZkob
dom/media/webrtc/MediaEngineRemoteVideoSource.cpp
dom/media/webrtc/MediaEngineSource.h
dom/media/webrtc/MediaEngineWebRTCAudio.cpp
--- a/dom/media/webrtc/MediaEngineRemoteVideoSource.cpp
+++ b/dom/media/webrtc/MediaEngineRemoteVideoSource.cpp
@@ -4,16 +4,17 @@
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "MediaEngineRemoteVideoSource.h"
 
 #include "AllocationHandle.h"
 #include "CamerasChild.h"
 #include "MediaManager.h"
 #include "MediaTrackConstraints.h"
+#include "mozilla/ErrorNames.h"
 #include "mozilla/RefPtr.h"
 #include "nsIPrefService.h"
 #include "VideoFrameUtils.h"
 #include "VideoUtils.h"
 #include "webrtc/common_video/include/video_frame_buffer.h"
 #include "webrtc/common_video/libyuv/include/webrtc_libyuv.h"
 
 mozilla::LogModule* GetMediaManagerLog();
@@ -328,16 +329,17 @@ MediaEngineRemoteVideoSource::Stop(const
     return NS_OK;
   }
 
   MOZ_ASSERT(mState == kStarted);
 
   if (camera::GetChildAndCall(&camera::CamerasChild::StopCapture,
                               mCapEngine, mCaptureIndex)) {
     MOZ_DIAGNOSTIC_ASSERT(false, "Stopping a started capture failed");
+    return NS_ERROR_FAILURE;
   }
 
   {
     MutexAutoLock lock(mMutex);
     mState = kStopped;
 
     // Drop any cached image so we don't start with a stale image on next
     // usage.  Also, gfx gets very upset if these are held until this object
@@ -361,44 +363,52 @@ MediaEngineRemoteVideoSource::Reconfigur
   MOZ_ASSERT(mInitDone);
 
   NormalizedConstraints constraints(aConstraints);
   webrtc::CaptureCapability newCapability;
   LOG(("ChooseCapability(kFitness) for mTargetCapability (Reconfigure) ++"));
   if (!ChooseCapability(constraints, aPrefs, aDeviceId, newCapability, kFitness)) {
     *aOutBadConstraint =
       MediaConstraintsHelper::FindBadConstraint(constraints, this, aDeviceId);
-    return NS_ERROR_FAILURE;
+    return NS_ERROR_INVALID_ARG;
   }
   LOG(("ChooseCapability(kFitness) for mTargetCapability (Reconfigure) --"));
 
   if (mCapability == newCapability) {
     return NS_OK;
   }
 
   bool started = mState == kStarted;
   if (started) {
     // Allocate always returns a null AllocationHandle.
     // We can safely pass nullptr below.
     nsresult rv = Stop(nullptr);
     if (NS_WARN_IF(NS_FAILED(rv))) {
-      return rv;
+      nsAutoCString name;
+      GetErrorName(rv, name);
+      LOG(("Video source %p for video device %d Reconfigure() failed "
+           "unexpectedly in Stop(). rv=%s", this, mCaptureIndex, name.Data()));
+      return NS_ERROR_UNEXPECTED;
     }
   }
 
   {
     MutexAutoLock lock(mMutex);
     // Start() applies mCapability on the device.
     mCapability = newCapability;
   }
 
   if (started) {
     nsresult rv = Start(nullptr);
     if (NS_WARN_IF(NS_FAILED(rv))) {
-      return rv;
+      nsAutoCString name;
+      GetErrorName(rv, name);
+      LOG(("Video source %p for video device %d Reconfigure() failed "
+           "unexpectedly in Start(). rv=%s", this, mCaptureIndex, name.Data()));
+      return NS_ERROR_UNEXPECTED;
     }
   }
 
   return NS_OK;
 }
 
 size_t
 MediaEngineRemoteVideoSource::NumCapabilities() const
--- a/dom/media/webrtc/MediaEngineSource.h
+++ b/dom/media/webrtc/MediaEngineSource.h
@@ -153,16 +153,24 @@ public:
 
   /**
    * Applies new constraints to the capability selection for the underlying
    * device.
    *
    * Should the constraints lead to choosing a new capability while the device
    * is actively being captured, the device will restart using the new
    * capability.
+   *
+   * We return one of the following:
+   * NS_OK                - Successful reconfigure.
+   * NS_ERROR_INVALID_ARG - Couldn't find a capability fitting aConstraints.
+   *                        See aBadConstraint for details.
+   * NS_ERROR_UNEXPECTED  - Reconfiguring the underlying device failed
+   *                        unexpectedly. This leaves the device in a stopped
+   *                        state.
    */
   virtual nsresult Reconfigure(const RefPtr<AllocationHandle>& aHandle,
                                const dom::MediaTrackConstraints& aConstraints,
                                const MediaEnginePrefs& aPrefs,
                                const nsString& aDeviceId,
                                const char** aOutBadConstraint) = 0;
 
   /**
--- a/dom/media/webrtc/MediaEngineWebRTCAudio.cpp
+++ b/dom/media/webrtc/MediaEngineWebRTCAudio.cpp
@@ -9,16 +9,17 @@
 #include <algorithm>
 
 #include "AllocationHandle.h"
 #include "AudioConverter.h"
 #include "MediaManager.h"
 #include "MediaStreamGraphImpl.h"
 #include "MediaTrackConstraints.h"
 #include "mozilla/Assertions.h"
+#include "mozilla/ErrorNames.h"
 #include "mtransport/runnable_utils.h"
 #include "nsAutoPtr.h"
 
 // scoped_ptr.h uses FF
 #ifdef FF
 #undef FF
 #endif
 #include "webrtc/modules/audio_device/opensl/single_rw_fifo.h"
@@ -227,18 +228,32 @@ MediaEngineWebRTCMicrophoneSource::Recon
                                                const char** aOutBadConstraint)
 {
   AssertIsOnOwningThread();
   MOZ_ASSERT(aHandle);
 
   LOG(("Mic source %p allocation %p Reconfigure()", this, aHandle.get()));
 
   NormalizedConstraints constraints(aConstraints);
-  return ReevaluateAllocation(aHandle, &constraints, aPrefs, aDeviceId,
-                              aOutBadConstraint);
+  nsresult rv = ReevaluateAllocation(aHandle, &constraints, aPrefs, aDeviceId,
+                                     aOutBadConstraint);
+  if (NS_FAILED(rv)) {
+    if (aOutBadConstraint) {
+      return NS_ERROR_INVALID_ARG;
+    }
+
+    nsAutoCString name;
+    GetErrorName(rv, name);
+    LOG(("Mic source %p Reconfigure() failed unexpectedly. rv=%s",
+         this, name.Data()));
+    Stop(aHandle);
+    return NS_ERROR_UNEXPECTED;
+  }
+
+  return NS_OK;
 }
 
 bool operator == (const MediaEnginePrefs& a, const MediaEnginePrefs& b)
 {
   return !memcmp(&a, &b, sizeof(MediaEnginePrefs));
 };
 
 // This does an early return in case of error.