Bug 1393687 - Use VideoAdapter to handle max-fs; r=jesup
authorDan Minor <dminor@mozilla.com>
Mon, 11 Sep 2017 13:43:32 -0400
changeset 430182 17637a9cb3db623a41e2dc29a0f56f9a2cefa35f
parent 430181 142c2979af94204afd0963202c4b8f5282dfd38e
child 430183 0140b94180037fb8a02bdbb19760dc9f566c83e9
push id7761
push userjlund@mozilla.com
push dateFri, 15 Sep 2017 00:19:52 +0000
treeherdermozilla-beta@c38455951db4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjesup
bugs1393687
milestone57.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 1393687 - Use VideoAdapter to handle max-fs; r=jesup This uses the VideoAdapter to handle the max-fs constraint rather than attempting to calculate and enforce it manually within the VideoConduit. The resolution is taken as the small of the max-fs constraint and the latest request from the video sink. The old unit tests for max-fs and max-fr are removed as the size calculation has now been delegated to the VideoAdapter and the tests do not verify whether or not video has actually been scaled. MozReview-Commit-ID: 18khmiNageR
media/webrtc/signaling/gtest/mediaconduit_unittests.cpp
media/webrtc/signaling/src/media-conduit/MediaConduitInterface.h
media/webrtc/signaling/src/media-conduit/VideoConduit.cpp
media/webrtc/signaling/src/media-conduit/VideoConduit.h
--- a/media/webrtc/signaling/gtest/mediaconduit_unittests.cpp
+++ b/media/webrtc/signaling/gtest/mediaconduit_unittests.cpp
@@ -536,168 +536,16 @@ class TransportConduitTest : public ::te
     cerr << "   *************************************************" << endl;
     cerr << "    3. Null Codec Parameter  " << endl;
     cerr << "   *************************************************" << endl;
 
     err = videoSession->ConfigureSendMediaCodec(nullptr);
     EXPECT_TRUE(err != mozilla::kMediaConduitNoError);
   }
 
-  void DumpMaxFs(int orig_width, int orig_height, int max_fs,
-                 int new_width, int new_height)
-  {
-    cerr << "Applying max_fs=" << max_fs << " to input resolution " <<
-                 orig_width << "x" << orig_height << endl;
-    cerr << "New resolution: " << new_width << "x" << new_height << endl;
-    cerr << endl;
-  }
-
-  // Calculate new resolution for sending video by applying max-fs constraint.
-  void GetVideoResolutionWithMaxFs(unsigned short orig_width,
-                                   unsigned short orig_height,
-                                   int max_fs,
-                                   unsigned short &new_width,
-                                   unsigned short &new_height)
-  {
-    int err = 0;
-
-    // Get pointer to VideoSessionConduit.
-    mVideoSession = VideoSessionConduit::Create(WebRtcCallWrapper::Create());
-    if( !mVideoSession ) {
-      ASSERT_NE(mVideoSession, (void*)nullptr);
-    }
-
-    std::vector<unsigned int> ssrcs = {SSRC};
-    mVideoSession->SetLocalSSRCs(ssrcs);
-
-    mozilla::EncodingConstraints constraints;
-    constraints.maxFs = max_fs;
-    // Configure send codecs on the conduit.
-    mozilla::VideoCodecConfig cinst1(120, "VP8", constraints);
-    VideoCodecConfig::SimulcastEncoding encoding;
-    cinst1.mSimulcastEncodings.push_back(encoding);
-
-    err = mVideoSession->ConfigureSendMediaCodec(&cinst1);
-    ASSERT_EQ(mozilla::kMediaConduitNoError, err);
-    err = mVideoSession->StartTransmitting();
-    ASSERT_EQ(mozilla::kMediaConduitNoError, err);
-
-    // Send one frame.
-    MOZ_ASSERT(!(orig_width & 1));
-    MOZ_ASSERT(!(orig_height & 1));
-
-    // Get the new resolution as adjusted by the max-fs constraint.
-    mVideoSession->SetSendingWidthAndHeight(orig_width, orig_height,
-                                            new_width, new_height);
-  }
-
-  void TestVideoConduitMaxFs()
-  {
-    unsigned short orig_width, orig_height, width, height;
-    int max_fs;
-
-    // No limitation.
-    cerr << "Test no max-fs limition" << endl;
-    orig_width = 640;
-    orig_height = 480;
-    max_fs = 0;
-    GetVideoResolutionWithMaxFs(orig_width, orig_height, max_fs, width, height);
-    DumpMaxFs(orig_width, orig_height, max_fs, width, height);
-    ASSERT_EQ(width, 640);
-    ASSERT_EQ(height, 480);
-
-    // VGA to QVGA.
-    cerr << "Test resizing from VGA to QVGA" << endl;
-    orig_width = 640;
-    orig_height = 480;
-    max_fs = 300;
-    GetVideoResolutionWithMaxFs(orig_width, orig_height, max_fs, width, height);
-    DumpMaxFs(orig_width, orig_height, max_fs, width, height);
-    ASSERT_EQ(width, 320);
-    ASSERT_EQ(height, 240);
-
-    // Extreme input resolution.
-    cerr << "Test extreme input resolution" << endl;
-    orig_width = 3072;
-    orig_height = 100;
-    max_fs = 300;
-    GetVideoResolutionWithMaxFs(orig_width, orig_height, max_fs, width, height);
-    DumpMaxFs(orig_width, orig_height, max_fs, width, height);
-    ASSERT_EQ(width, 768);
-    ASSERT_EQ(height, 25);
-
-    // Small max-fs.
-    cerr << "Test small max-fs (case 1)" << endl;
-    orig_width = 8;
-    orig_height = 32;
-    max_fs = 1;
-    GetVideoResolutionWithMaxFs(orig_width, orig_height, max_fs, width, height);
-    DumpMaxFs(orig_width, orig_height, max_fs, width, height);
-    ASSERT_EQ(width, 4);
-    ASSERT_EQ(height, 16);
-
-    // Small max-fs.
-    cerr << "Test small max-fs (case 2)" << endl;
-    orig_width = 4;
-    orig_height = 50;
-    max_fs = 1;
-    GetVideoResolutionWithMaxFs(orig_width, orig_height, max_fs, width, height);
-    DumpMaxFs(orig_width, orig_height, max_fs, width, height);
-    ASSERT_EQ(width, 1);
-    ASSERT_EQ(height, 16);
-
-    // Small max-fs.
-    cerr << "Test small max-fs (case 3)" << endl;
-    orig_width = 872;
-    orig_height = 136;
-    max_fs = 3;
-    GetVideoResolutionWithMaxFs(orig_width, orig_height, max_fs, width, height);
-    DumpMaxFs(orig_width, orig_height, max_fs, width, height);
-    ASSERT_EQ(width, 48);
-    ASSERT_EQ(height, 7);
-
-    // Small max-fs.
-    cerr << "Test small max-fs (case 4)" << endl;
-    orig_width = 160;
-    orig_height = 8;
-    max_fs = 5;
-    GetVideoResolutionWithMaxFs(orig_width, orig_height, max_fs, width, height);
-    DumpMaxFs(orig_width, orig_height, max_fs, width, height);
-    ASSERT_EQ(width, 80);
-    ASSERT_EQ(height, 4);
-
-     // Extremely small width and height(see bug 919979).
-    cerr << "Test with extremely small width and height" << endl;
-    orig_width = 2;
-    orig_height = 2;
-    max_fs = 5;
-    GetVideoResolutionWithMaxFs(orig_width, orig_height, max_fs, width, height);
-    DumpMaxFs(orig_width, orig_height, max_fs, width, height);
-    ASSERT_EQ(width, 2);
-    ASSERT_EQ(height, 2);
-
-    // Random values.
-    cerr << "Test with random values" << endl;
-    for (int i = 0; i < 30; i++) {
-      cerr << ".";
-      max_fs = rand() % 1000;
-      orig_width = ((rand() % 2000) & ~1) + 2;
-      orig_height = ((rand() % 2000) & ~1) + 2;
-
-      GetVideoResolutionWithMaxFs(orig_width, orig_height, max_fs,
-                                  width, height);
-      if (max_fs > 0 &&
-          ceil(width / 16.) * ceil(height / 16.) > max_fs) {
-        DumpMaxFs(orig_width, orig_height, max_fs, width, height);
-        ADD_FAILURE();
-      }
-    }
-    cerr << endl;
- }
-
  private:
   //Audio Conduit Test Objects
   RefPtr<mozilla::AudioSessionConduit> mAudioSession;
   RefPtr<mozilla::AudioSessionConduit> mAudioSession2;
   RefPtr<mozilla::TransportInterface> mAudioTransport;
   AudioSendAndReceive audioTester;
 
   //Video Conduit Test Objects
@@ -715,15 +563,11 @@ class TransportConduitTest : public ::te
 
 // Disabled, see Bug 1319121
 TEST_F(TransportConduitTest, DISABLED_TestDummyAudioWithTransport) {
   TestDummyAudioAndTransport();
 }
 
 TEST_F(TransportConduitTest, TestVideoConduitCodecAPI) {
   TestVideoConduitCodecAPI();
- }
-
-TEST_F(TransportConduitTest, TestVideoConduitMaxFs) {
-  TestVideoConduitMaxFs();
- }
+}
 
 }  // end namespace
--- a/media/webrtc/signaling/src/media-conduit/MediaConduitInterface.h
+++ b/media/webrtc/signaling/src/media-conduit/MediaConduitInterface.h
@@ -389,25 +389,16 @@ public:
    * @param sendSessionConfig: CodecConfiguration
    * NOTE: This API can be invoked multiple time. Invoking this API may involve restarting
    *        reception sub-system on the engine
    *
    */
   virtual MediaConduitErrorCode ConfigureRecvMediaCodecs(
       const std::vector<VideoCodecConfig* >& recvCodecConfigList) = 0;
 
-  /**
-   * This method allows unit tests to double-check that the
-   * max-fs and max-fr related settings are as expected.
-   */
-  virtual void SetSendingWidthAndHeight(unsigned short frame_width,
-                                        unsigned short frame_height,
-                                        unsigned short &result_width,
-                                        unsigned short &result_height) = 0;
-
   virtual unsigned int SendingMaxFs() = 0;
 
   virtual unsigned int SendingMaxFr() = 0;
 
   /**
     * These methods allow unit tests to double-check that the
     * rtcp-fb settings are as expected.
     */
--- a/media/webrtc/signaling/src/media-conduit/VideoConduit.cpp
+++ b/media/webrtc/signaling/src/media-conduit/VideoConduit.cpp
@@ -1686,60 +1686,28 @@ WebrtcVideoConduit::SelectSendResolution
     uint16_t max_width = mCurSendCodecConfig->mEncodingConstraints.maxWidth;
     uint16_t max_height = mCurSendCodecConfig->mEncodingConstraints.maxHeight;
     if (max_width || max_height) {
       max_width = max_width ? max_width : UINT16_MAX;
       max_height = max_height ? max_height : UINT16_MAX;
       ConstrainPreservingAspectRatio(max_width, max_height, &width, &height);
     }
 
-    // Limit resolution to max-fs while keeping same aspect ratio as the
-    // incoming image.
+    // Limit resolution to max-fs
     if (mCurSendCodecConfig->mEncodingConstraints.maxFs) {
-      uint32_t max_fs = mCurSendCodecConfig->mEncodingConstraints.maxFs;
-      unsigned int cur_fs, mb_width, mb_height, mb_max;
-
-      // Could we make this simpler by picking the larger of width and height,
-      // calculating a max for just that value based on the scale parameter,
-      // and then let ConstrainPreservingAspectRatio do the rest?
-      mb_width = (width + 15) >> 4;
-      mb_height = (height + 15) >> 4;
-
-      cur_fs = mb_width * mb_height;
-
-      // Limit resolution to max_fs, but don't scale up.
-      if (cur_fs > max_fs) {
-        double scale_ratio;
-
-        scale_ratio = sqrt((double)max_fs / (double)cur_fs);
-
-        mb_width = mb_width * scale_ratio;
-        mb_height = mb_height * scale_ratio;
-
-        // Adjust mb_width and mb_height if they were truncated to zero.
-        if (mb_width == 0) {
-          mb_width = 1;
-          mb_height = std::min(mb_height, max_fs);
-        }
-        if (mb_height == 0) {
-          mb_height = 1;
-          mb_width = std::min(mb_width, max_fs);
-        }
+      // max-fs is in macroblocks, convert to pixels
+      int max_fs(mCurSendCodecConfig->mEncodingConstraints.maxFs*(16*16));
+      if (max_fs > mLastSinkWanted.max_pixel_count.value_or(max_fs)) {
+        max_fs = mLastSinkWanted.max_pixel_count.value_or(max_fs);
       }
-
-      // Limit width/height seperately to limit effect of extreme aspect ratios.
-      mb_max = (unsigned)sqrt(8 * (double)max_fs);
-
-      max_width = 16 * std::min(mb_width, mb_max);
-      max_height = 16 * std::min(mb_height, mb_max);
-      ConstrainPreservingAspectRatio(max_width, max_height, &width, &height);
+      mVideoAdapter.OnResolutionRequest(rtc::Optional<int>(max_fs),
+                                        rtc::Optional<int>());
     }
   }
 
-
   // Adapt to getUserMedia resolution changes
   // check if we need to reconfigure the sending resolution.
   // NOTE: mSendingWidth != mLastWidth, because of maxwidth/height/etc above
   bool changed = false;
   if (mSendingWidth != width || mSendingHeight != height) {
     CSFLogDebug(logTag, "%s: resolution changing to %ux%u (from %ux%u)",
                 __FUNCTION__, width, height, mSendingWidth, mSendingHeight);
     // This will avoid us continually retrying this operation if it fails.
@@ -1929,18 +1897,33 @@ WebrtcVideoConduit::RemoveSink(
   OnSinkWantsChanged(mVideoBroadcaster.wants());
 }
 
 void
 WebrtcVideoConduit::OnSinkWantsChanged(
   const rtc::VideoSinkWants& wants) {
   NS_ASSERTION(NS_IsMainThread(), "Only call on main thread");
   if (!mLockScaling) {
-    mVideoAdapter.OnResolutionRequest(wants.max_pixel_count,
-                                      wants.max_pixel_count_step_up);
+    mLastSinkWanted = wants;
+
+    // limit sink wants based upon max-fs constraint
+    int max_fs = mCurSendCodecConfig->mEncodingConstraints.maxFs*(16*16);
+    rtc::Optional<int> max_pixel_count = wants.max_pixel_count;
+    rtc::Optional<int> max_pixel_count_step_up = wants.max_pixel_count_step_up;
+
+    if (max_pixel_count.value_or(max_fs) > max_fs) {
+      max_pixel_count = rtc::Optional<int>(max_fs);
+    }
+
+    if (max_pixel_count_step_up.value_or(max_fs) > max_fs) {
+      max_pixel_count_step_up = rtc::Optional<int>(max_fs);
+    }
+
+    mVideoAdapter.OnResolutionRequest(max_pixel_count,
+                                      max_pixel_count_step_up);
   }
 }
 
 MediaConduitErrorCode
 WebrtcVideoConduit::SendVideoFrame(webrtc::VideoFrame& frame)
 {
   // XXX Google uses a "timestamp_aligner" to translate timestamps from the
   // camera via TranslateTimestamp(); we should look at doing the same.  This
@@ -2412,30 +2395,16 @@ WebrtcVideoConduit::CodecPluginID()
   }
   if (mRecvCodecPlugin) {
     return mRecvCodecPlugin->PluginID();
   }
 
   return 0;
 }
 
-void
-WebrtcVideoConduit::SetSendingWidthAndHeight(unsigned short frame_width,
-                                             unsigned short frame_height,
-                                             unsigned short &result_width,
-                                             unsigned short &result_height)
-{
-  MutexAutoLock lock(mCodecMutex);
-  result_width = 0;
-  result_height = 0;
-  (void)SelectSendResolution(frame_width, frame_height, nullptr);
-  result_width = mSendingWidth;
-  result_height = mSendingHeight;
-}
-
 bool
 WebrtcVideoConduit::RequiresNewSendStream(const VideoCodecConfig& newConfig) const
 {
   return !mCurSendCodecConfig
     || mCurSendCodecConfig->mName != newConfig.mName
     || mCurSendCodecConfig->mType != newConfig.mType
     || mCurSendCodecConfig->RtcpFbNackIsSet("") != newConfig.RtcpFbNackIsSet("")
     || mCurSendCodecConfig->RtcpFbFECIsSet() != newConfig.RtcpFbFECIsSet()
--- a/media/webrtc/signaling/src/media-conduit/VideoConduit.h
+++ b/media/webrtc/signaling/src/media-conduit/VideoConduit.h
@@ -251,25 +251,16 @@ public:
   void OnSinkWantsChanged(const rtc::VideoSinkWants& wants);
 
   virtual uint64_t CodecPluginID() override;
 
   virtual void SetPCHandle(const std::string& aPCHandle) override {
     mPCHandle = aPCHandle;
   }
 
-  /**
-   * This method allows unit tests to double-check that the
-   * max-fs and max-fr related settings are as expected.
-   */
-  virtual void SetSendingWidthAndHeight(unsigned short frame_width,
-                                        unsigned short frame_height,
-                                        unsigned short &result_width,
-                                        unsigned short &result_height) override;
-
   unsigned int SendingMaxFs() override {
     if(mCurSendCodecConfig) {
       return mCurSendCodecConfig->mEncodingConstraints.maxFs;
     }
     return 0;
   }
 
   unsigned int SendingMaxFr() override {
@@ -544,16 +535,18 @@ private:
   int mPrefMaxBitrate;
   int mNegotiatedMaxBitrate;
   int mMinBitrateEstimate;
   bool mDenoising;
   bool mLockScaling; // for tests that care about output resolution
   uint8_t mSpatialLayers;
   uint8_t mTemporalLayers;
 
+  rtc::VideoSinkWants mLastSinkWanted;
+
   static const unsigned int sAlphaNum = 7;
   static const unsigned int sAlphaDen = 8;
   static const unsigned int sRoundingPadding = 1024;
 
   RefPtr<WebrtcAudioConduit> mSyncedTo;
 
   webrtc::VideoCodecMode mCodecMode;