From 8525a8028a0ad622f9b3574dd2b6babb0fba2ec6 Mon Sep 17 00:00:00 2001 From: Artem Titov Date: Mon, 16 Dec 2019 11:41:57 +0100 Subject: [PATCH] Add ability to resize buffers pool in decoder and use it in IVF generator Bug: webrtc:10138 Change-Id: I452f08f1d9af57de789bd947a1fcb95536845f80 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/162183 Commit-Queue: Artem Titov Reviewed-by: Karl Wiberg Reviewed-by: Ilya Nikolaevskiy Cr-Commit-Position: refs/heads/master@{#30098} --- api/video_codecs/video_codec.h | 7 +++++ common_video/i420_buffer_pool.cc | 30 +++++++++++++++++++ common_video/include/i420_buffer_pool.h | 7 ++++- .../codecs/h264/h264_decoder_impl.cc | 6 ++++ .../codecs/vp8/libvpx_vp8_decoder.cc | 5 ++++ .../codecs/vp9/vp9_frame_buffer_pool.cc | 30 +++++++++++++++++++ .../codecs/vp9/vp9_frame_buffer_pool.h | 26 +++++++++------- modules/video_coding/codecs/vp9/vp9_impl.cc | 5 ++++ test/testsupport/ivf_video_frame_generator.cc | 6 ++++ 9 files changed, 111 insertions(+), 11 deletions(-) diff --git a/api/video_codecs/video_codec.h b/api/video_codecs/video_codec.h index a248d19382..330bbbce19 100644 --- a/api/video_codecs/video_codec.h +++ b/api/video_codecs/video_codec.h @@ -16,6 +16,7 @@ #include +#include "absl/types/optional.h" #include "api/video/video_bitrate_allocation.h" #include "api/video/video_codec_type.h" #include "common_types.h" // NOLINT(build/include) @@ -126,6 +127,12 @@ class RTC_EXPORT VideoCodec { VideoCodecMode mode; bool expect_encode_from_texture; + // The size of pool which is used to store video frame buffers inside decoder. + // If value isn't present some codec-default value will be used. + // If value is present and decoder doesn't have buffer pool the + // value will be ignored. + absl::optional buffer_pool_size; + // Timing frames configuration. There is delay of delay_ms between two // consequent timing frames, excluding outliers. Frame is always made a // timing frame if it's at least outlier_ratio in percent of "ideal" average diff --git a/common_video/i420_buffer_pool.cc b/common_video/i420_buffer_pool.cc index e970419ba6..d13da6a172 100644 --- a/common_video/i420_buffer_pool.cc +++ b/common_video/i420_buffer_pool.cc @@ -29,6 +29,36 @@ void I420BufferPool::Release() { buffers_.clear(); } +bool I420BufferPool::Resize(size_t max_number_of_buffers) { + RTC_DCHECK_RUNS_SERIALIZED(&race_checker_); + size_t used_buffers_count = 0; + for (const rtc::scoped_refptr& buffer : buffers_) { + // If the buffer is in use, the ref count will be >= 2, one from the list we + // are looping over and one from the application. If the ref count is 1, + // then the list we are looping over holds the only reference and it's safe + // to reuse. + if (!buffer->HasOneRef()) { + used_buffers_count++; + } + } + if (used_buffers_count > max_number_of_buffers) { + return false; + } + max_number_of_buffers_ = max_number_of_buffers; + + size_t buffers_to_purge = buffers_.size() - max_number_of_buffers_; + auto iter = buffers_.begin(); + while (iter != buffers_.end() && buffers_to_purge > 0) { + if ((*iter)->HasOneRef()) { + iter = buffers_.erase(iter); + buffers_to_purge--; + } else { + ++iter; + } + } + return true; +} + rtc::scoped_refptr I420BufferPool::CreateBuffer(int width, int height) { // Default stride_y is width, default uv stride is width / 2 (rounding up). diff --git a/common_video/include/i420_buffer_pool.h b/common_video/include/i420_buffer_pool.h index d0ccee5742..44f4821798 100644 --- a/common_video/include/i420_buffer_pool.h +++ b/common_video/include/i420_buffer_pool.h @@ -48,6 +48,11 @@ class I420BufferPool { int stride_u, int stride_v); + // Changes the max amount of buffers in the pool to the new value. + // Returns true if change was successful and false if the amount of already + // allocated buffers is bigger than new value. + bool Resize(size_t max_number_of_buffers); + // Clears buffers_ and detaches the thread checker so that it can be reused // later from another thread. void Release(); @@ -66,7 +71,7 @@ class I420BufferPool { // has to do with "Use-of-uninitialized-value" on "Linux_msan_chrome". const bool zero_initialize_; // Max number of buffers this pool can have pending. - const size_t max_number_of_buffers_; + size_t max_number_of_buffers_; }; } // namespace webrtc diff --git a/modules/video_coding/codecs/h264/h264_decoder_impl.cc b/modules/video_coding/codecs/h264/h264_decoder_impl.cc index fa5af98ef2..6725a3b7c7 100644 --- a/modules/video_coding/codecs/h264/h264_decoder_impl.cc +++ b/modules/video_coding/codecs/h264/h264_decoder_impl.cc @@ -218,6 +218,12 @@ int32_t H264DecoderImpl::InitDecode(const VideoCodec* codec_settings, } av_frame_.reset(av_frame_alloc()); + + if (codec_settings && codec_settings->buffer_pool_size) { + if (!pool_.Resize(*codec_settings->buffer_pool_size)) { + return WEBRTC_VIDEO_CODEC_UNINITIALIZED; + } + } return WEBRTC_VIDEO_CODEC_OK; } diff --git a/modules/video_coding/codecs/vp8/libvpx_vp8_decoder.cc b/modules/video_coding/codecs/vp8/libvpx_vp8_decoder.cc index bc4ddf65e9..a3ee2c0c41 100644 --- a/modules/video_coding/codecs/vp8/libvpx_vp8_decoder.cc +++ b/modules/video_coding/codecs/vp8/libvpx_vp8_decoder.cc @@ -149,6 +149,11 @@ int LibvpxVp8Decoder::InitDecode(const VideoCodec* inst, int number_of_cores) { // Always start with a complete key frame. key_frame_required_ = true; + if (inst && inst->buffer_pool_size) { + if (!buffer_pool_.Resize(*inst->buffer_pool_size)) { + return WEBRTC_VIDEO_CODEC_UNINITIALIZED; + } + } return WEBRTC_VIDEO_CODEC_OK; } diff --git a/modules/video_coding/codecs/vp9/vp9_frame_buffer_pool.cc b/modules/video_coding/codecs/vp9/vp9_frame_buffer_pool.cc index 372a813760..4e475076ed 100644 --- a/modules/video_coding/codecs/vp9/vp9_frame_buffer_pool.cc +++ b/modules/video_coding/codecs/vp9/vp9_frame_buffer_pool.cc @@ -97,6 +97,36 @@ int Vp9FrameBufferPool::GetNumBuffersInUse() const { return num_buffers_in_use; } +bool Vp9FrameBufferPool::Resize(size_t max_number_of_buffers) { + rtc::CritScope cs(&buffers_lock_); + size_t used_buffers_count = 0; + for (const auto& buffer : allocated_buffers_) { + // If the buffer is in use, the ref count will be >= 2, one from the list we + // are looping over and one from the application. If the ref count is 1, + // then the list we are looping over holds the only reference and it's safe + // to reuse. + if (!buffer->HasOneRef()) { + used_buffers_count++; + } + } + if (used_buffers_count > max_number_of_buffers) { + return false; + } + max_num_buffers_ = max_number_of_buffers; + + size_t buffers_to_purge = allocated_buffers_.size() - max_num_buffers_; + auto iter = allocated_buffers_.begin(); + while (iter != allocated_buffers_.end() && buffers_to_purge > 0) { + if ((*iter)->HasOneRef()) { + iter = allocated_buffers_.erase(iter); + buffers_to_purge--; + } else { + ++iter; + } + } + return true; +} + void Vp9FrameBufferPool::ClearPool() { rtc::CritScope cs(&buffers_lock_); allocated_buffers_.clear(); diff --git a/modules/video_coding/codecs/vp9/vp9_frame_buffer_pool.h b/modules/video_coding/codecs/vp9/vp9_frame_buffer_pool.h index df6d45106c..02d2b26273 100644 --- a/modules/video_coding/codecs/vp9/vp9_frame_buffer_pool.h +++ b/modules/video_coding/codecs/vp9/vp9_frame_buffer_pool.h @@ -26,6 +26,17 @@ struct vpx_codec_frame_buffer; namespace webrtc { +// If more buffers than this are allocated we print warnings and crash if in +// debug mode. VP9 is defined to have 8 reference buffers, of which 3 can be +// referenced by any frame, see +// https://tools.ietf.org/html/draft-grange-vp9-bitstream-00#section-2.2.2. +// Assuming VP9 holds on to at most 8 buffers, any more buffers than that +// would have to be by application code. Decoded frames should not be +// referenced for longer than necessary. If we allow ~60 additional buffers +// then the application has ~1 second to e.g. render each frame of a 60 fps +// video. +constexpr size_t kDefaultMaxNumBuffers = 68; + // This memory pool is used to serve buffers to libvpx for decoding purposes in // VP9, which is set up in InitializeVPXUsePool. After the initialization any // time libvpx wants to decode a frame it will use buffers provided and released @@ -77,6 +88,10 @@ class Vp9FrameBufferPool { rtc::scoped_refptr GetFrameBuffer(size_t min_size); // Gets the number of buffers currently in use (not ready to be recycled). int GetNumBuffersInUse() const; + // Changes the max amount of buffers in the pool to the new value. + // Returns true if change was successful and false if the amount of already + // allocated buffers is bigger than new value. + bool Resize(size_t max_number_of_buffers); // Releases allocated buffers, deleting available buffers. Buffers in use are // not deleted until they are no longer referenced. void ClearPool(); @@ -108,16 +123,7 @@ class Vp9FrameBufferPool { // All buffers, in use or ready to be recycled. std::vector> allocated_buffers_ RTC_GUARDED_BY(buffers_lock_); - // If more buffers than this are allocated we print warnings and crash if in - // debug mode. VP9 is defined to have 8 reference buffers, of which 3 can be - // referenced by any frame, see - // https://tools.ietf.org/html/draft-grange-vp9-bitstream-00#section-2.2.2. - // Assuming VP9 holds on to at most 8 buffers, any more buffers than that - // would have to be by application code. Decoded frames should not be - // referenced for longer than necessary. If we allow ~60 additional buffers - // then the application has ~1 second to e.g. render each frame of a 60 fps - // video. - static const size_t max_num_buffers_ = 68; + size_t max_num_buffers_ = kDefaultMaxNumBuffers; }; } // namespace webrtc diff --git a/modules/video_coding/codecs/vp9/vp9_impl.cc b/modules/video_coding/codecs/vp9/vp9_impl.cc index b926fe3040..06b7fe1a2e 100644 --- a/modules/video_coding/codecs/vp9/vp9_impl.cc +++ b/modules/video_coding/codecs/vp9/vp9_impl.cc @@ -1652,6 +1652,11 @@ int VP9DecoderImpl::InitDecode(const VideoCodec* inst, int number_of_cores) { inited_ = true; // Always start with a complete key frame. key_frame_required_ = true; + if (inst && inst->buffer_pool_size) { + if (!frame_buffer_pool_.Resize(*inst->buffer_pool_size)) { + return WEBRTC_VIDEO_CODEC_UNINITIALIZED; + } + } return WEBRTC_VIDEO_CODEC_OK; } diff --git a/test/testsupport/ivf_video_frame_generator.cc b/test/testsupport/ivf_video_frame_generator.cc index 9fb057f1c9..81155f80ff 100644 --- a/test/testsupport/ivf_video_frame_generator.cc +++ b/test/testsupport/ivf_video_frame_generator.cc @@ -10,6 +10,8 @@ #include "test/testsupport/ivf_video_frame_generator.h" +#include + #include "api/video/encoded_image.h" #include "api/video/i420_buffer.h" #include "api/video_codecs/video_codec.h" @@ -40,6 +42,10 @@ IvfVideoFrameGenerator::IvfVideoFrameGenerator(const std::string& file_name) codec_settings.codecType = file_reader_->GetVideoCodecType(); codec_settings.width = file_reader_->GetFrameWidth(); codec_settings.height = file_reader_->GetFrameHeight(); + // Set buffer pool size to max value to ensure that if users of generator, + // ex. test frameworks, will retain frames for quite a long time, decoder + // won't crash with buffers pool overflow error. + codec_settings.buffer_pool_size = std::numeric_limits::max(); RTC_CHECK_EQ(video_decoder_->RegisterDecodeCompleteCallback(&callback_), WEBRTC_VIDEO_CODEC_OK); RTC_CHECK_EQ(