Non-integer frame rate in Android HW encoder

Bug: webrtc:10812
Change-Id: I4443dcfea851114bd5fbb10f11ca8a51cda12da8
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/229025
Commit-Queue: Sergey Silkin <ssilkin@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34813}
diff --git a/sdk/android/BUILD.gn b/sdk/android/BUILD.gn
index eb5527c..633f491 100644
--- a/sdk/android/BUILD.gn
+++ b/sdk/android/BUILD.gn
@@ -1615,6 +1615,7 @@
       "tests/src/org/webrtc/CodecTestHelper.java",
       "tests/src/org/webrtc/CryptoOptionsTest.java",
       "tests/src/org/webrtc/FakeMediaCodecWrapper.java",
+      "tests/src/org/webrtc/FramerateBitrateAdjusterTest.java",
       "tests/src/org/webrtc/GlGenericDrawerTest.java",
       "tests/src/org/webrtc/HardwareVideoEncoderTest.java",
       "tests/src/org/webrtc/IceCandidateTest.java",
diff --git a/sdk/android/api/org/webrtc/VideoEncoder.java b/sdk/android/api/org/webrtc/VideoEncoder.java
index 352c702..cd897d0 100644
--- a/sdk/android/api/org/webrtc/VideoEncoder.java
+++ b/sdk/android/api/org/webrtc/VideoEncoder.java
@@ -236,6 +236,28 @@
     }
   }
 
+  /** Rate control parameters. */
+  public class RateControlParameters {
+    /**
+     * Adjusted target bitrate, per spatial/temporal layer. May be lower or higher than the target
+     * depending on encoder behaviour.
+     */
+    public final BitrateAllocation bitrate;
+
+    /**
+     * Target framerate, in fps. A value <= 0.0 is invalid and should be interpreted as framerate
+     * target not available. In this case the encoder should fall back to the max framerate
+     * specified in `codec_settings` of the last InitEncode() call.
+     */
+    public final double framerateFps;
+
+    @CalledByNative("RateControlParameters")
+    public RateControlParameters(BitrateAllocation bitrate, double framerateFps) {
+      this.bitrate = bitrate;
+      this.framerateFps = framerateFps;
+    }
+  }
+
   public interface Callback {
     /**
      * Old encoders assume that the byte buffer held by `frame` is not accessed after the call to
@@ -296,7 +318,14 @@
   @CalledByNative VideoCodecStatus encode(VideoFrame frame, EncodeInfo info);
 
   /** Sets the bitrate allocation and the target framerate for the encoder. */
-  @CalledByNative VideoCodecStatus setRateAllocation(BitrateAllocation allocation, int framerate);
+  VideoCodecStatus setRateAllocation(BitrateAllocation allocation, int framerate);
+
+  /** Sets the bitrate allocation and the target framerate for the encoder. */
+  default @CalledByNative VideoCodecStatus setRates(RateControlParameters rcParameters) {
+    // Round frame rate up to avoid overshoots.
+    int framerateFps = (int) Math.ceil(rcParameters.framerateFps);
+    return setRateAllocation(rcParameters.bitrate, framerateFps);
+  }
 
   /** Any encoder that wants to use WebRTC provided quality scaler must implement this method. */
   @CalledByNative ScalingSettings getScalingSettings();
diff --git a/sdk/android/src/java/org/webrtc/BaseBitrateAdjuster.java b/sdk/android/src/java/org/webrtc/BaseBitrateAdjuster.java
index 43a6d2a..3b5f5d2 100644
--- a/sdk/android/src/java/org/webrtc/BaseBitrateAdjuster.java
+++ b/sdk/android/src/java/org/webrtc/BaseBitrateAdjuster.java
@@ -13,10 +13,10 @@
 /** BitrateAdjuster that tracks bitrate and framerate but does not adjust them. */
 class BaseBitrateAdjuster implements BitrateAdjuster {
   protected int targetBitrateBps;
-  protected int targetFramerateFps;
+  protected double targetFramerateFps;
 
   @Override
-  public void setTargets(int targetBitrateBps, int targetFramerateFps) {
+  public void setTargets(int targetBitrateBps, double targetFramerateFps) {
     this.targetBitrateBps = targetBitrateBps;
     this.targetFramerateFps = targetFramerateFps;
   }
@@ -32,7 +32,7 @@
   }
 
   @Override
-  public int getAdjustedFramerateFps() {
+  public double getAdjustedFramerateFps() {
     return targetFramerateFps;
   }
 }
diff --git a/sdk/android/src/java/org/webrtc/BitrateAdjuster.java b/sdk/android/src/java/org/webrtc/BitrateAdjuster.java
index 89a682f..bfa08ba 100644
--- a/sdk/android/src/java/org/webrtc/BitrateAdjuster.java
+++ b/sdk/android/src/java/org/webrtc/BitrateAdjuster.java
@@ -15,7 +15,7 @@
   /**
    * Sets the target bitrate in bits per second and framerate in frames per second.
    */
-  void setTargets(int targetBitrateBps, int targetFramerateFps);
+  void setTargets(int targetBitrateBps, double targetFramerateFps);
 
   /**
    * Should be used to report the size of an encoded frame to the bitrate adjuster. Use
@@ -27,5 +27,5 @@
   int getAdjustedBitrateBps();
 
   /** Gets the current framerate. */
-  int getAdjustedFramerateFps();
+  double getAdjustedFramerateFps();
 }
diff --git a/sdk/android/src/java/org/webrtc/DynamicBitrateAdjuster.java b/sdk/android/src/java/org/webrtc/DynamicBitrateAdjuster.java
index b2a6592..96a15bb 100644
--- a/sdk/android/src/java/org/webrtc/DynamicBitrateAdjuster.java
+++ b/sdk/android/src/java/org/webrtc/DynamicBitrateAdjuster.java
@@ -31,7 +31,7 @@
   private int bitrateAdjustmentScaleExp;
 
   @Override
-  public void setTargets(int targetBitrateBps, int targetFramerateFps) {
+  public void setTargets(int targetBitrateBps, double targetFramerateFps) {
     if (this.targetBitrateBps > 0 && targetBitrateBps < this.targetBitrateBps) {
       // Rescale the accumulator level if the accumulator max decreases
       deviationBytes = deviationBytes * targetBitrateBps / this.targetBitrateBps;
diff --git a/sdk/android/src/java/org/webrtc/FramerateBitrateAdjuster.java b/sdk/android/src/java/org/webrtc/FramerateBitrateAdjuster.java
index 6c0182d..e28b7b5 100644
--- a/sdk/android/src/java/org/webrtc/FramerateBitrateAdjuster.java
+++ b/sdk/android/src/java/org/webrtc/FramerateBitrateAdjuster.java
@@ -18,9 +18,9 @@
   private static final int DEFAULT_FRAMERATE_FPS = 30;
 
   @Override
-  public void setTargets(int targetBitrateBps, int targetFramerateFps) {
+  public void setTargets(int targetBitrateBps, double targetFramerateFps) {
     // Keep frame rate unchanged and adjust bit rate.
     this.targetFramerateFps = DEFAULT_FRAMERATE_FPS;
-    this.targetBitrateBps = targetBitrateBps * DEFAULT_FRAMERATE_FPS / targetFramerateFps;
+    this.targetBitrateBps = (int) (targetBitrateBps * DEFAULT_FRAMERATE_FPS / targetFramerateFps);
   }
 }
diff --git a/sdk/android/src/java/org/webrtc/HardwareVideoEncoder.java b/sdk/android/src/java/org/webrtc/HardwareVideoEncoder.java
index 995c9d4..7119ed2 100644
--- a/sdk/android/src/java/org/webrtc/HardwareVideoEncoder.java
+++ b/sdk/android/src/java/org/webrtc/HardwareVideoEncoder.java
@@ -241,7 +241,8 @@
       format.setInteger(MediaFormat.KEY_BIT_RATE, adjustedBitrate);
       format.setInteger(KEY_BITRATE_MODE, VIDEO_ControlRateConstant);
       format.setInteger(MediaFormat.KEY_COLOR_FORMAT, colorFormat);
-      format.setInteger(MediaFormat.KEY_FRAME_RATE, bitrateAdjuster.getAdjustedFramerateFps());
+      format.setFloat(
+          MediaFormat.KEY_FRAME_RATE, (float) bitrateAdjuster.getAdjustedFramerateFps());
       format.setInteger(MediaFormat.KEY_I_FRAME_INTERVAL, keyFrameIntervalSec);
       if (codecType == VideoCodecMimeType.H264) {
         String profileLevelId = params.get(VideoCodecInfo.H264_FMTP_PROFILE_LEVEL_ID);
@@ -469,6 +470,13 @@
   }
 
   @Override
+  public VideoCodecStatus setRates(RateControlParameters rcParameters) {
+    encodeThreadChecker.checkIsOnValidThread();
+    bitrateAdjuster.setTargets(rcParameters.bitrate.getSum(), rcParameters.framerateFps);
+    return VideoCodecStatus.OK;
+  }
+
+  @Override
   public ScalingSettings getScalingSettings() {
     encodeThreadChecker.checkIsOnValidThread();
     if (automaticResizeOn) {
diff --git a/sdk/android/src/jni/video_encoder_wrapper.cc b/sdk/android/src/jni/video_encoder_wrapper.cc
index cb5a7f4..5c585c8 100644
--- a/sdk/android/src/jni/video_encoder_wrapper.cc
+++ b/sdk/android/src/jni/video_encoder_wrapper.cc
@@ -167,15 +167,14 @@
   return HandleReturnCode(jni, ret, "encode");
 }
 
-void VideoEncoderWrapper::SetRates(const RateControlParameters& parameters) {
+void VideoEncoderWrapper::SetRates(const RateControlParameters& rc_parameters) {
   JNIEnv* jni = AttachCurrentThreadIfNeeded();
 
-  ScopedJavaLocalRef<jobject> j_bitrate_allocation =
-      ToJavaBitrateAllocation(jni, parameters.bitrate);
-  ScopedJavaLocalRef<jobject> ret = Java_VideoEncoder_setRateAllocation(
-      jni, encoder_, j_bitrate_allocation,
-      (jint)(parameters.framerate_fps + 0.5));
-  HandleReturnCode(jni, ret, "setRateAllocation");
+  ScopedJavaLocalRef<jobject> j_rc_parameters =
+      ToJavaRateControlParameters(jni, rc_parameters);
+  ScopedJavaLocalRef<jobject> ret =
+      Java_VideoEncoder_setRates(jni, encoder_, j_rc_parameters);
+  HandleReturnCode(jni, ret, "setRates");
 }
 
 VideoEncoder::EncoderInfo VideoEncoderWrapper::GetEncoderInfo() const {
@@ -410,6 +409,16 @@
   return Java_BitrateAllocation_Constructor(jni, j_allocation_array);
 }
 
+ScopedJavaLocalRef<jobject> VideoEncoderWrapper::ToJavaRateControlParameters(
+    JNIEnv* jni,
+    const VideoEncoder::RateControlParameters& rc_parameters) {
+  ScopedJavaLocalRef<jobject> j_bitrate_allocation =
+      ToJavaBitrateAllocation(jni, rc_parameters.bitrate);
+
+  return Java_RateControlParameters_Constructor(jni, j_bitrate_allocation,
+                                                rc_parameters.framerate_fps);
+}
+
 std::unique_ptr<VideoEncoder> JavaToNativeVideoEncoder(
     JNIEnv* jni,
     const JavaRef<jobject>& j_encoder) {
diff --git a/sdk/android/src/jni/video_encoder_wrapper.h b/sdk/android/src/jni/video_encoder_wrapper.h
index 2cd77df..9cf5c5a 100644
--- a/sdk/android/src/jni/video_encoder_wrapper.h
+++ b/sdk/android/src/jni/video_encoder_wrapper.h
@@ -46,7 +46,7 @@
   int32_t Encode(const VideoFrame& frame,
                  const std::vector<VideoFrameType>* frame_types) override;
 
-  void SetRates(const RateControlParameters& parameters) override;
+  void SetRates(const RateControlParameters& rc_parameters) override;
 
   EncoderInfo GetEncoderInfo() const override;
 
@@ -70,12 +70,19 @@
                            const char* method_name);
 
   int ParseQp(rtc::ArrayView<const uint8_t> buffer);
+
   CodecSpecificInfo ParseCodecSpecificInfo(const EncodedImage& frame);
+
   ScopedJavaLocalRef<jobject> ToJavaBitrateAllocation(
       JNIEnv* jni,
       const VideoBitrateAllocation& allocation);
 
+  ScopedJavaLocalRef<jobject> ToJavaRateControlParameters(
+      JNIEnv* jni,
+      const VideoEncoder::RateControlParameters& rc_parameters);
+
   void UpdateEncoderInfo(JNIEnv* jni);
+
   ScalingSettings GetScalingSettingsInternal(JNIEnv* jni) const;
   std::vector<ResolutionBitrateLimits> GetResolutionBitrateLimits(
       JNIEnv* jni) const;
diff --git a/sdk/android/tests/src/org/webrtc/FramerateBitrateAdjusterTest.java b/sdk/android/tests/src/org/webrtc/FramerateBitrateAdjusterTest.java
new file mode 100644
index 0000000..0b21089
--- /dev/null
+++ b/sdk/android/tests/src/org/webrtc/FramerateBitrateAdjusterTest.java
@@ -0,0 +1,46 @@
+/*
+ *  Copyright 2021 The WebRTC Project Authors. All rights reserved.
+ *
+ *  Use of this source code is governed by a BSD-style license
+ *  that can be found in the LICENSE file in the root of the source
+ *  tree. An additional intellectual property rights grant can be found
+ *  in the file PATENTS.  All contributing project authors may
+ *  be found in the AUTHORS file in the root of the source tree.
+ */
+
+package org.webrtc;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import org.chromium.testing.local.LocalRobolectricTestRunner;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.robolectric.annotation.Config;
+import org.webrtc.VideoEncoder.ScalingSettings;
+
+@RunWith(LocalRobolectricTestRunner.class)
+@Config(manifest = Config.NONE)
+public class FramerateBitrateAdjusterTest {
+  @Test
+  public void getAdjustedFramerate_alwaysReturnsDefault() {
+    FramerateBitrateAdjuster bitrateAdjuster = new FramerateBitrateAdjuster();
+    bitrateAdjuster.setTargets(1000, 15);
+    assertThat(bitrateAdjuster.getAdjustedFramerateFps()).isEqualTo(30.0);
+  }
+
+  @Test
+  public void getAdjustedBitrate_defaultFramerate_returnsTargetBitrate() {
+    FramerateBitrateAdjuster bitrateAdjuster = new FramerateBitrateAdjuster();
+    bitrateAdjuster.setTargets(1000, 30);
+    assertThat(bitrateAdjuster.getAdjustedBitrateBps()).isEqualTo(1000);
+  }
+
+  @Test
+  public void getAdjustedBitrate_nonDefaultFramerate_returnsAdjustedBitrate() {
+    FramerateBitrateAdjuster bitrateAdjuster = new FramerateBitrateAdjuster();
+    bitrateAdjuster.setTargets(1000, 7.5);
+    // Target frame frame is x4 times smaller than the adjusted one (30fps). Adjusted bitrate should
+    // be x4 times larger then the target one.
+    assertThat(bitrateAdjuster.getAdjustedBitrateBps()).isEqualTo(4000);
+  }
+}
diff --git a/sdk/android/tests/src/org/webrtc/HardwareVideoEncoderTest.java b/sdk/android/tests/src/org/webrtc/HardwareVideoEncoderTest.java
index 05eea35..145c0cc 100644
--- a/sdk/android/tests/src/org/webrtc/HardwareVideoEncoderTest.java
+++ b/sdk/android/tests/src/org/webrtc/HardwareVideoEncoderTest.java
@@ -298,7 +298,7 @@
         mockEncoderCallback);
 
     MediaFormat mediaFormat = fakeMediaCodecWrapper.getConfiguredFormat();
-    assertThat(mediaFormat.getInteger(MediaFormat.KEY_FRAME_RATE)).isEqualTo(30);
+    assertThat(mediaFormat.getFloat(MediaFormat.KEY_FRAME_RATE)).isEqualTo(30.0f);
   }
 
   @Test