From 895556e19c081ba9a5f0e015f0fe95745d677b55 Mon Sep 17 00:00:00 2001 From: Evan Shrubsole Date: Mon, 5 Oct 2020 09:15:13 +0200 Subject: [PATCH] Avoid converting frames to I420 in VideoStreamEncoder This needs to be done still for kNative frames, but all other frame types can be passed in. I have checked all VideoEncoder implementations in Chromium and confirmed they either convert the frame to their preferred pixel format, or just forward the frame to a delegate encoder. Tested: - video_loopback with NV12 generated frames for VP9, the only codec supporting NV12, as well as VP8 which only accepts I420 frames. - internal_tests tryrun Bug: webrtc:11976,webrtc:11635 Change-Id: If39a815fb0c5636fceb1040c8946c3db2fb350a1 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/185803 Commit-Queue: Evan Shrubsole Reviewed-by: Ilya Nikolaevskiy Reviewed-by: Niels Moller Cr-Commit-Position: refs/heads/master@{#32306} --- api/video_codecs/video_encoder.h | 2 +- video/BUILD.gn | 1 + video/video_stream_encoder.cc | 12 +--- video/video_stream_encoder_unittest.cc | 93 ++++++++++++++++++++++++++ 4 files changed, 98 insertions(+), 10 deletions(-) diff --git a/api/video_codecs/video_encoder.h b/api/video_codecs/video_encoder.h index 9ef8021190..ed46691023 100644 --- a/api/video_codecs/video_encoder.h +++ b/api/video_codecs/video_encoder.h @@ -378,7 +378,7 @@ class RTC_EXPORT VideoEncoder { // Return value : WEBRTC_VIDEO_CODEC_OK if OK, < 0 otherwise. virtual int32_t Release() = 0; - // Encode an I420 image (as a part of a video stream). The encoded image + // Encode an image (as a part of a video stream). The encoded image // will be returned to the user through the encode complete callback. // // Input: diff --git a/video/BUILD.gn b/video/BUILD.gn index 2e41981c74..71c6714e27 100644 --- a/video/BUILD.gn +++ b/video/BUILD.gn @@ -601,6 +601,7 @@ if (rtc_include_tests) { "../api/video:video_bitrate_allocation", "../api/video:video_frame", "../api/video:video_frame_i420", + "../api/video:video_frame_nv12", "../api/video:video_frame_type", "../api/video:video_rtp_headers", "../api/video_codecs:video_codecs_api", diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc index 1ecbef108c..234a26a56f 100644 --- a/video/video_stream_encoder.cc +++ b/video/video_stream_encoder.cc @@ -1311,15 +1311,9 @@ void VideoStreamEncoder::EncodeVideoFrame(const VideoFrame& video_frame, last_encode_info_ms_ = clock_->TimeInMilliseconds(); VideoFrame out_frame(video_frame); - - const VideoFrameBuffer::Type buffer_type = - out_frame.video_frame_buffer()->type(); - const bool is_buffer_type_supported = - buffer_type == VideoFrameBuffer::Type::kI420 || - (buffer_type == VideoFrameBuffer::Type::kNative && - info.supports_native_handle); - - if (!is_buffer_type_supported) { + if (out_frame.video_frame_buffer()->type() == + VideoFrameBuffer::Type::kNative && + !info.supports_native_handle) { // This module only supports software encoding. rtc::scoped_refptr converted_buffer( out_frame.video_frame_buffer()->ToI420()); diff --git a/video/video_stream_encoder_unittest.cc b/video/video_stream_encoder_unittest.cc index 21a609794e..63788c0044 100644 --- a/video/video_stream_encoder_unittest.cc +++ b/video/video_stream_encoder_unittest.cc @@ -21,6 +21,7 @@ #include "api/test/mock_video_encoder.h" #include "api/video/builtin_video_bitrate_allocator_factory.h" #include "api/video/i420_buffer.h" +#include "api/video/nv12_buffer.h" #include "api/video/video_adaptation_reason.h" #include "api/video/video_bitrate_allocation.h" #include "api/video_codecs/video_encoder.h" @@ -131,6 +132,30 @@ class FakeNativeBuffer : public webrtc::VideoFrameBuffer { const int height_; }; +// A fake native buffer that is backed by an NV12 buffer. +class FakeNV12NativeBuffer : public webrtc::VideoFrameBuffer { + public: + FakeNV12NativeBuffer(rtc::Event* event, int width, int height) + : nv12_buffer_(NV12Buffer::Create(width, height)), event_(event) {} + + webrtc::VideoFrameBuffer::Type type() const override { return Type::kNative; } + int width() const override { return nv12_buffer_->width(); } + int height() const override { return nv12_buffer_->height(); } + rtc::scoped_refptr ToI420() override { + return nv12_buffer_->ToI420(); + } + const NV12BufferInterface* GetNV12() const { return nv12_buffer_; } + + private: + friend class rtc::RefCountedObject; + ~FakeNV12NativeBuffer() override { + if (event_) + event_->Set(); + } + rtc::scoped_refptr nv12_buffer_; + rtc::Event* const event_; +}; + class CpuOveruseDetectorProxy : public OveruseFrameDetector { public: explicit CpuOveruseDetectorProxy(CpuOveruseMetricsObserver* metrics_observer) @@ -734,6 +759,19 @@ class VideoStreamEncoderTest : public ::testing::Test { return frame; } + VideoFrame CreateNV12Frame(int64_t ntp_time_ms, int width, int height) const { + VideoFrame frame = + VideoFrame::Builder() + .set_video_frame_buffer(NV12Buffer::Create(width, height)) + .set_timestamp_rtp(99) + .set_timestamp_ms(99) + .set_rotation(kVideoRotation_0) + .build(); + frame.set_ntp_time_ms(ntp_time_ms); + frame.set_timestamp_us(ntp_time_ms * 1000); + return frame; + } + VideoFrame CreateFakeNativeFrame(int64_t ntp_time_ms, rtc::Event* destruction_event, int width, @@ -750,6 +788,22 @@ class VideoStreamEncoderTest : public ::testing::Test { return frame; } + VideoFrame CreateFakeNV12NativeFrame(int64_t ntp_time_ms, + rtc::Event* destruction_event, + int width, + int height) const { + VideoFrame frame = VideoFrame::Builder() + .set_video_frame_buffer( + new rtc::RefCountedObject( + destruction_event, width, height)) + .set_timestamp_rtp(99) + .set_timestamp_ms(99) + .set_rotation(kVideoRotation_0) + .build(); + frame.set_ntp_time_ms(ntp_time_ms); + return frame; + } + VideoFrame CreateFakeNativeFrame(int64_t ntp_time_ms, rtc::Event* destruction_event) const { return CreateFakeNativeFrame(ntp_time_ms, destruction_event, codec_width_, @@ -948,6 +1002,11 @@ class VideoStreamEncoderTest : public ::testing::Test { return settings; } + absl::optional GetLastInputPixelFormat() { + MutexLock lock(&local_mutex_); + return last_input_pixel_format_; + } + int GetNumEncoderInitializations() const { MutexLock lock(&local_mutex_); return num_encoder_initializations_; @@ -988,6 +1047,7 @@ class VideoStreamEncoderTest : public ::testing::Test { block_next_encode_ = false; last_update_rect_ = input_image.update_rect(); last_frame_types_ = *frame_types; + last_input_pixel_format_ = input_image.video_frame_buffer()->type(); } int32_t result = FakeEncoder::Encode(input_image, frame_types); if (block_encode) @@ -1107,6 +1167,8 @@ class VideoStreamEncoderTest : public ::testing::Test { RTC_GUARDED_BY(local_mutex_); int num_set_rates_ RTC_GUARDED_BY(local_mutex_) = 0; VideoCodec video_codec_ RTC_GUARDED_BY(local_mutex_); + absl::optional last_input_pixel_format_ + RTC_GUARDED_BY(local_mutex_); }; class TestSink : public VideoStreamEncoder::EncoderSink { @@ -1485,6 +1547,37 @@ TEST_F(VideoStreamEncoderTest, DropFrameWithFailedI420ConversionWithCrop) { video_stream_encoder_->Stop(); } +TEST_F(VideoStreamEncoderTest, NonI420FramesShouldNotBeConvertedToI420) { + video_stream_encoder_->OnBitrateUpdatedAndWaitForManagedResources( + DataRate::BitsPerSec(kTargetBitrateBps), + DataRate::BitsPerSec(kTargetBitrateBps), + DataRate::BitsPerSec(kTargetBitrateBps), 0, 0, 0); + + video_source_.IncomingCapturedFrame( + CreateNV12Frame(1, codec_width_, codec_height_)); + WaitForEncodedFrame(1); + EXPECT_EQ(VideoFrameBuffer::Type::kNV12, + fake_encoder_.GetLastInputPixelFormat()); + video_stream_encoder_->Stop(); +} + +// TODO(webrtc:11977): When a native frame backed by an NV12 image is possible, +// the frame should be encoded in NV12. +TEST_F(VideoStreamEncoderTest, NativeFrameBackedByNV12FrameIsEncodedFromI420) { + video_stream_encoder_->OnBitrateUpdatedAndWaitForManagedResources( + DataRate::BitsPerSec(kTargetBitrateBps), + DataRate::BitsPerSec(kTargetBitrateBps), + DataRate::BitsPerSec(kTargetBitrateBps), 0, 0, 0); + + rtc::Event frame_destroyed_event; + video_source_.IncomingCapturedFrame(CreateFakeNV12NativeFrame( + 1, &frame_destroyed_event, codec_width_, codec_height_)); + WaitForEncodedFrame(1); + EXPECT_EQ(VideoFrameBuffer::Type::kI420, + fake_encoder_.GetLastInputPixelFormat()); + video_stream_encoder_->Stop(); +} + TEST_F(VideoStreamEncoderTest, DropsFramesWhenCongestionWindowPushbackSet) { video_stream_encoder_->OnBitrateUpdatedAndWaitForManagedResources( DataRate::BitsPerSec(kTargetBitrateBps),