Fix potentially bad rate control with libaom av1 encoder.

This can happen when the encoder uses real presentation timestamps that
originate with the input frames. By using those, the encoder can bypass
webrtc frame dropping logic and may severely over/under-shoot if the
timestamps are very precise. In practice, this seems rather common on
Chrome on Windows.

Bug: aomedia:3391
Change-Id: I2be5eed4fabc86dac8a6c7bfdd068c2dcb5a3743
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/294740
Commit-Queue: Erik Språng <sprang@webrtc.org>
Reviewed-by: Philip Eliasson <philipel@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39382}

modules/video_coding/codecs/av1/BUILD.gn

@@ -96,6 +96,8 @@ if (rtc_include_tests) {
         ":libaom_av1_encoder",
         "../..:encoded_video_frame_producer",
         "../..:video_codec_interface",
+        "../../../../api:create_frame_generator",
+        "../../../../api:frame_generator_api",
         "../../../../api:mock_video_encoder",
         "../../../../api/units:data_size",
         "../../../../api/units:time_delta",

modules/video_coding/codecs/av1/libaom_av1_encoder.cc

@@ -33,7 +33,6 @@
 #include "modules/video_coding/svc/scalable_video_controller_no_layering.h"
 #include "rtc_base/checks.h"
 #include "rtc_base/logging.h"
-#include "rtc_base/numerics/sequence_number_unwrapper.h"
 #include "third_party/libaom/source/libaom/aom/aom_codec.h"
 #include "third_party/libaom/source/libaom/aom/aom_encoder.h"
 #include "third_party/libaom/source/libaom/aom/aomcx.h"
@@ -120,7 +119,7 @@ class LibaomAv1Encoder final : public VideoEncoder {
   aom_codec_ctx_t ctx_;
   aom_codec_enc_cfg_t cfg_;
   EncodedImageCallback* encoded_image_callback_;
-  SeqNumUnwrapper<uint32_t> rtp_timestamp_unwrapper_;
+  int64_t timestamp_;
 };
 
 int32_t VerifyCodecSettings(const VideoCodec& codec_settings) {
@@ -154,7 +153,8 @@ LibaomAv1Encoder::LibaomAv1Encoder(
       rates_configured_(false),
       aux_config_(aux_config),
       frame_for_encode_(nullptr),
-      encoded_image_callback_(nullptr) {}
+      encoded_image_callback_(nullptr),
+      timestamp_(0) {}
 
 LibaomAv1Encoder::~LibaomAv1Encoder() {
   Release();
@@ -606,6 +606,7 @@ int32_t LibaomAv1Encoder::Encode(
 
   const uint32_t duration =
       kRtpTicksPerSecond / static_cast<float>(encoder_settings_.maxFramerate);
+  timestamp_ += duration;
 
   const size_t num_spatial_layers =
       svc_params_ ? svc_params_->number_spatial_layers : 1;
@@ -639,11 +640,11 @@ int32_t LibaomAv1Encoder::Encode(
                                         layer_frame->TemporalId() > 0 ? 1 : 0);
     }
 
-    // Encode a frame. The presentation timestamp `pts` should never wrap, hence
-    // the unwrapping.
-    aom_codec_err_t ret = aom_codec_encode(
-        &ctx_, frame_for_encode_,
-        rtp_timestamp_unwrapper_.Unwrap(frame.timestamp()), duration, flags);
+    // Encode a frame. The presentation timestamp `pts` should not use real
+    // timestamps from frames or the wall clock, as that can cause the rate
+    // controller to misbehave.
+    aom_codec_err_t ret =
+        aom_codec_encode(&ctx_, frame_for_encode_, timestamp_, duration, flags);
     if (ret != AOM_CODEC_OK) {
       RTC_LOG(LS_WARNING) << "LibaomAv1Encoder::Encode returned " << ret
                           << " on aom_codec_encode.";

modules/video_coding/codecs/av1/libaom_av1_encoder_unittest.cc

@@ -15,6 +15,8 @@
 #include <vector>
 
 #include "absl/types/optional.h"
+#include "api/test/create_frame_generator.h"
+#include "api/test/frame_generator_interface.h"
 #include "api/video_codecs/video_codec.h"
 #include "api/video_codecs/video_encoder.h"
 #include "modules/video_coding/codecs/test/encoded_video_frame_producer.h"
@@ -295,5 +297,83 @@ TEST(LibaomAv1EncoderTest, TestCaptureTimeId) {
             capture_time_id.us());
 }
 
+TEST(LibaomAv1EncoderTest, AdheresToTargetBitrateDespiteUnevenFrameTiming) {
+  std::unique_ptr<VideoEncoder> encoder = CreateLibaomAv1Encoder();
+  VideoCodec codec_settings = DefaultCodecSettings();
+  codec_settings.SetScalabilityMode(ScalabilityMode::kL1T1);
+  codec_settings.maxBitrate = 300;  // kbps
+  codec_settings.width = 320;
+  codec_settings.height = 180;
+  ASSERT_EQ(encoder->InitEncode(&codec_settings, DefaultEncoderSettings()),
+            WEBRTC_VIDEO_CODEC_OK);
+
+  const int kFps = 30;
+  const int kTargetBitrateBps = codec_settings.maxBitrate * 1000;
+  VideoEncoder::RateControlParameters rate_parameters;
+  rate_parameters.framerate_fps = kFps;
+  rate_parameters.bitrate.SetBitrate(/*spatial_index=*/0, 0, kTargetBitrateBps);
+  encoder->SetRates(rate_parameters);
+
+  class EncoderCallback : public EncodedImageCallback {
+   public:
+    EncoderCallback() = default;
+    DataSize BytesEncoded() const { return bytes_encoded_; }
+
+   private:
+    Result OnEncodedImage(
+        const EncodedImage& encoded_image,
+        const CodecSpecificInfo* codec_specific_info) override {
+      bytes_encoded_ += DataSize::Bytes(encoded_image.size());
+      return Result(Result::Error::OK);
+    }
+
+    DataSize bytes_encoded_ = DataSize::Zero();
+  } callback;
+  encoder->RegisterEncodeCompleteCallback(&callback);
+
+  // Insert frames with too low rtp timestamp delta compared to what is expected
+  // based on the framerate, then insert on with 2x the delta it should - making
+  // the average correct.
+  const uint32_t kHighTimestampDelta =
+      static_cast<uint32_t>((90000.0 / kFps) * 2 + 0.5);
+  const uint32_t kLowTimestampDelta =
+      static_cast<uint32_t>((90000.0 - kHighTimestampDelta) / (kFps - 1));
+
+  std::unique_ptr<test::FrameGeneratorInterface> frame_buffer_generator =
+      test::CreateSquareFrameGenerator(
+          codec_settings.width, codec_settings.height,
+          test::FrameGeneratorInterface::OutputType::kI420, /*num_squares=*/20);
+
+  uint32_t rtp_timestamp = 1000;
+  std::vector<VideoFrameType> frame_types = {VideoFrameType::kVideoFrameKey};
+
+  const int kRunTimeSeconds = 3;
+  for (int i = 0; i < kRunTimeSeconds; ++i) {
+    for (int j = 0; j < kFps; ++j) {
+      if (j < kFps - 1) {
+        rtp_timestamp += kLowTimestampDelta;
+      } else {
+        rtp_timestamp += kHighTimestampDelta;
+      }
+      VideoFrame frame = VideoFrame::Builder()
+                             .set_video_frame_buffer(
+                                 frame_buffer_generator->NextFrame().buffer)
+                             .set_timestamp_rtp(rtp_timestamp)
+                             .build();
+
+      RTC_CHECK_EQ(encoder->Encode(frame, &frame_types), WEBRTC_VIDEO_CODEC_OK);
+      frame_types[0] = VideoFrameType::kVideoFrameDelta;
+    }
+  }
+
+  // Expect produced bitrate to match, to within 10%.
+  // This catches an issue that was seen when real frame timestamps with jitter
+  // was used. It resulted in the overall produced bitrate to be overshot by
+  // ~30% even though the averages should have been ok.
+  EXPECT_NEAR(
+      (callback.BytesEncoded() / TimeDelta::Seconds(kRunTimeSeconds)).bps(),
+      kTargetBitrateBps, kTargetBitrateBps / 10);
+}
+
 }  // namespace
 }  // namespace webrtc