Modified the simulcast encoder adapter to correctly handle encoded frames from sub encoders even if the encoder is unable to (temporarily or permanently) produce frames of the exactly matching resolution. This is done by using a different EncodedImageCallback for each encoder, which remembers which VideoEncoder it is registered to and forwards that on to SimulcastEncoderAdapter::Encoded.

BUG=
R=pbos@webrtc.org, stefan@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/45949004

Cr-Commit-Position: refs/heads/master@{#9011}

webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter.cc

@@ -112,6 +112,28 @@ struct ScreenshareTemporalLayersFactory : webrtc::TemporalLayers::Factory {
   mutable webrtc::FrameDropper tl1_frame_dropper_;
 };
 
+// An EncodedImageCallback implementation that forwards on calls to a
+// SimulcastEncoderAdapter, but with the stream index it's registered with as
+// the first parameter to Encoded.
+class AdapterEncodedImageCallback : public webrtc::EncodedImageCallback {
+ public:
+  AdapterEncodedImageCallback(webrtc::SimulcastEncoderAdapter* adapter,
+                              size_t stream_idx)
+      : adapter_(adapter), stream_idx_(stream_idx) {}
+
+  int32_t Encoded(
+      const webrtc::EncodedImage& encodedImage,
+      const webrtc::CodecSpecificInfo* codecSpecificInfo = NULL,
+      const webrtc::RTPFragmentationHeader* fragmentation = NULL) override {
+    return adapter_->Encoded(stream_idx_, encodedImage, codecSpecificInfo,
+                             fragmentation);
+  }
+
+ private:
+  webrtc::SimulcastEncoderAdapter* const adapter_;
+  const size_t stream_idx_;
+};
+
 }  // namespace
 
 namespace webrtc {
@@ -133,7 +155,9 @@ int SimulcastEncoderAdapter::Release() {
   // ~SimulcastEncoderAdapter().
   while (!streaminfos_.empty()) {
     VideoEncoder* encoder = streaminfos_.back().encoder;
+    EncodedImageCallback* callback = streaminfos_.back().callback;
     factory_->Destroy(encoder);
+    delete callback;
     streaminfos_.pop_back();
   }
   return WEBRTC_VIDEO_CODEC_OK;
@@ -199,11 +223,10 @@ int SimulcastEncoderAdapter::InitEncode(const VideoCodec* inst,
       Release();
       return ret;
     }
-    encoder->RegisterEncodeCompleteCallback(this);
-    streaminfos_.push_back(StreamInfo(encoder,
-                                      stream_codec.width,
-                                      stream_codec.height,
-                                      send_stream));
+    EncodedImageCallback* callback = new AdapterEncodedImageCallback(this, i);
+    encoder->RegisterEncodeCompleteCallback(callback);
+    streaminfos_.push_back(StreamInfo(encoder, callback, stream_codec.width,
+                                      stream_codec.height, send_stream));
   }
   return WEBRTC_VIDEO_CODEC_OK;
 }
@@ -362,11 +385,10 @@ int SimulcastEncoderAdapter::SetRates(uint32_t new_bitrate_kbit,
 }
 
 int32_t SimulcastEncoderAdapter::Encoded(
+    size_t stream_idx,
     const EncodedImage& encodedImage,
     const CodecSpecificInfo* codecSpecificInfo,
     const RTPFragmentationHeader* fragmentation) {
-  size_t stream_idx = GetStreamIndex(encodedImage);
-
   CodecSpecificInfo stream_codec_specific = *codecSpecificInfo;
   CodecSpecificInfoVP8* vp8Info = &(stream_codec_specific.codecSpecific.VP8);
   vp8Info->simulcastIdx = stream_idx;
@@ -475,21 +497,6 @@ void SimulcastEncoderAdapter::PopulateStreamCodec(
   stream_codec->startBitrate = stream_bitrate;
 }
 
-size_t SimulcastEncoderAdapter::GetStreamIndex(
-    const EncodedImage& encodedImage) {
-  uint32_t width = encodedImage._encodedWidth;
-  uint32_t height = encodedImage._encodedHeight;
-  for (size_t stream_idx = 0; stream_idx < streaminfos_.size(); ++stream_idx) {
-    if (streaminfos_[stream_idx].width == width &&
-        streaminfos_[stream_idx].height == height) {
-      return stream_idx;
-    }
-  }
-  // should not be here
-  assert(false);
-  return 0;
-}
-
 bool SimulcastEncoderAdapter::Initialized() const {
   return !streaminfos_.empty();
 }

webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter.h

@@ -30,8 +30,7 @@ class VideoEncoderFactory {
 // webrtc::VideoEncoder instances with the given VideoEncoderFactory.
 // All the public interfaces are expected to be called from the same thread,
 // e.g the encoder thread.
-class SimulcastEncoderAdapter : public VP8Encoder,
-                                public EncodedImageCallback {
+class SimulcastEncoderAdapter : public VP8Encoder {
  public:
   explicit SimulcastEncoderAdapter(VideoEncoderFactory* factory);
   virtual ~SimulcastEncoderAdapter();
@@ -48,27 +47,37 @@ class SimulcastEncoderAdapter : public VP8Encoder,
   int SetChannelParameters(uint32_t packet_loss, int64_t rtt) override;
   int SetRates(uint32_t new_bitrate_kbit, uint32_t new_framerate) override;
 
-  // Implements EncodedImageCallback
-  int32_t Encoded(const EncodedImage& encodedImage,
+  // Eventual handler for the contained encoders' EncodedImageCallbacks, but
+  // called from an internal helper that also knows the correct stream
+  // index.
+  int32_t Encoded(size_t stream_idx,
+                  const EncodedImage& encodedImage,
                   const CodecSpecificInfo* codecSpecificInfo = NULL,
-                  const RTPFragmentationHeader* fragmentation = NULL) override;
+                  const RTPFragmentationHeader* fragmentation = NULL);
 
  private:
   struct StreamInfo {
     StreamInfo()
-        : encoder(NULL), width(0), height(0),
-          key_frame_request(false), send_stream(true) {}
+        : encoder(NULL),
+          callback(NULL),
+          width(0),
+          height(0),
+          key_frame_request(false),
+          send_stream(true) {}
     StreamInfo(VideoEncoder* encoder,
+               EncodedImageCallback* callback,
                unsigned short width,
                unsigned short height,
                bool send_stream)
         : encoder(encoder),
+          callback(callback),
           width(width),
           height(height),
           key_frame_request(false),
           send_stream(send_stream) {}
     // Deleted by SimulcastEncoderAdapter::Release().
     VideoEncoder* encoder;
+    EncodedImageCallback* callback;
     unsigned short width;
     unsigned short height;
     bool key_frame_request;
@@ -89,9 +98,6 @@ class SimulcastEncoderAdapter : public VP8Encoder,
                            webrtc::VideoCodec* stream_codec,
                            bool* send_stream);
 
-  // Get the stream index according to |encodedImage|.
-  size_t GetStreamIndex(const EncodedImage& encodedImage);
-
   bool Initialized() const;
 
   rtc::scoped_ptr<VideoEncoderFactory> factory_;

webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter_unittest.cc

@@ -120,6 +120,7 @@ class MockVideoEncoder : public VideoEncoder {
                  const std::vector<VideoFrameType>* frame_types) { return 0; }
 
   int32_t RegisterEncodeCompleteCallback(EncodedImageCallback* callback) {
+    callback_ = callback;
     return 0;
   }
 
@@ -139,8 +140,19 @@ class MockVideoEncoder : public VideoEncoder {
 
   const VideoCodec& codec() const { return codec_; }
 
+  void SendEncodedImage(int width, int height) {
+    // Sends a fake image of the given width/height.
+    EncodedImage image;
+    image._encodedWidth = width;
+    image._encodedHeight = height;
+    CodecSpecificInfo codecSpecificInfo;
+    memset(&codecSpecificInfo, 0, sizeof(codecSpecificInfo));
+    callback_->Encoded(image, &codecSpecificInfo, NULL);
+  }
+
  private:
   VideoCodec codec_;
+  EncodedImageCallback* callback_;
 };
 
 class MockVideoEncoderFactory : public VideoEncoderFactory {
@@ -188,18 +200,47 @@ class TestSimulcastEncoderAdapterFakeHelper {
 
 static const int kTestTemporalLayerProfile[3] = {3, 2, 1};
 
-class TestSimulcastEncoderAdapterFake : public ::testing::Test {
+class TestSimulcastEncoderAdapterFake : public ::testing::Test,
+                                        public EncodedImageCallback {
  public:
   TestSimulcastEncoderAdapterFake()
-     : helper_(new TestSimulcastEncoderAdapterFakeHelper()),
-       adapter_(helper_->CreateMockEncoderAdapter()) {}
+      : helper_(new TestSimulcastEncoderAdapterFakeHelper()),
+        adapter_(helper_->CreateMockEncoderAdapter()),
+        last_encoded_image_width_(-1),
+        last_encoded_image_height_(-1),
+        last_encoded_image_simulcast_index_(-1) {}
   virtual ~TestSimulcastEncoderAdapterFake() {}
 
+  int32_t Encoded(const EncodedImage& encodedImage,
+                  const CodecSpecificInfo* codecSpecificInfo = NULL,
+                  const RTPFragmentationHeader* fragmentation = NULL) override {
+    last_encoded_image_width_ = encodedImage._encodedWidth;
+    last_encoded_image_height_ = encodedImage._encodedHeight;
+    if (codecSpecificInfo) {
+      last_encoded_image_simulcast_index_ =
+          codecSpecificInfo->codecSpecific.VP8.simulcastIdx;
+    }
+    return 0;
+  }
+
+  bool GetLastEncodedImageInfo(int* out_width,
+                               int* out_height,
+                               int* out_simulcast_index) {
+    if (last_encoded_image_width_ == -1) {
+      return false;
+    }
+    *out_width = last_encoded_image_width_;
+    *out_height = last_encoded_image_height_;
+    *out_simulcast_index = last_encoded_image_simulcast_index_;
+    return true;
+  }
+
   void SetupCodec() {
     TestVp8Simulcast::DefaultSettings(
       &codec_,
       static_cast<const int*>(kTestTemporalLayerProfile));
     EXPECT_EQ(0, adapter_->InitEncode(&codec_, 1, 1200));
+    adapter_->RegisterEncodeCompleteCallback(this);
   }
 
   void VerifyCodec(const VideoCodec& ref, int stream_index) {
@@ -282,6 +323,9 @@ class TestSimulcastEncoderAdapterFake : public ::testing::Test {
   rtc::scoped_ptr<TestSimulcastEncoderAdapterFakeHelper> helper_;
   rtc::scoped_ptr<VP8Encoder> adapter_;
   VideoCodec codec_;
+  int last_encoded_image_width_;
+  int last_encoded_image_height_;
+  int last_encoded_image_simulcast_index_;
 };
 
 TEST_F(TestSimulcastEncoderAdapterFake, InitEncode) {
@@ -297,5 +341,36 @@ TEST_F(TestSimulcastEncoderAdapterFake, SetChannelParameters) {
   adapter_->SetChannelParameters(packetLoss, rtt);
 }
 
+TEST_F(TestSimulcastEncoderAdapterFake, EncodedCallbackForDifferentEncoders) {
+  SetupCodec();
+
+  // At this point, the simulcast encoder adapter should have 3 streams: HD,
+  // quarter HD, and quarter quarter HD. We're going to mostly ignore the exact
+  // resolutions, to test that the adapter forwards on the correct resolution
+  // and simulcast index values, going only off the encoder that generates the
+  // image.
+  EXPECT_EQ(3u, helper_->factory()->encoders().size());
+  helper_->factory()->encoders()[0]->SendEncodedImage(1152, 704);
+  int width;
+  int height;
+  int simulcast_index;
+  EXPECT_TRUE(GetLastEncodedImageInfo(&width, &height, &simulcast_index));
+  EXPECT_EQ(1152, width);
+  EXPECT_EQ(704, height);
+  EXPECT_EQ(0, simulcast_index);
+
+  helper_->factory()->encoders()[1]->SendEncodedImage(300, 620);
+  EXPECT_TRUE(GetLastEncodedImageInfo(&width, &height, &simulcast_index));
+  EXPECT_EQ(300, width);
+  EXPECT_EQ(620, height);
+  EXPECT_EQ(1, simulcast_index);
+
+  helper_->factory()->encoders()[2]->SendEncodedImage(120, 240);
+  EXPECT_TRUE(GetLastEncodedImageInfo(&width, &height, &simulcast_index));
+  EXPECT_EQ(120, width);
+  EXPECT_EQ(240, height);
+  EXPECT_EQ(2, simulcast_index);
+}
+
 }  // namespace testing
 }  // namespace webrtc