Commit 1bba741f authored by Miguel Casas's avatar Miguel Casas Committed by Chromium LUCI CQ

media::V4L2VideoDecoder: introduce kMaxNumOfInstances

MS Teams on ARM-based ChromeOS with the new VideoDecoder (not launched)
tries to create many direct video decoders (I see e.g. 42 on kukui);
some succeed but eventually they don't and the tab ends up hitting an
OOM. The process is not well understood and needs further debugging.

This CL replicates the kMaxNumOfInstances [1] logic from the legacy
VideoDecodeAccelerator to the new direct VideoDecoder, so the new
behaves like the old, allowing us to unblock the linked bug.

[1] https://source.chromium.org/search?q=kMaxNumOfInstances&sq=&ss=chromium%2Fchromium%2Fsrc:media%2Fgpu%2Fv4l2%2F

Landing this provision on Kukui was already discussed on
.

https: //chromium-review.googlesource.com/c/chromium/src/+/2372926/10#message-ca9039b5fb3eb9e1fbcd9701ddc476b257f088d6
Bug: b/170870476
Change-Id: Ice0d9f16e7ca0063c03cccdafe54e29ad5602cd0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2628069
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Reviewed-by: default avatarAndres Calderon Jaramillo <andrescj@chromium.org>
Reviewed-by: default avatarFritz Koenig <frkoenig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#843377}
parent cdcaa4a7
...@@ -46,6 +46,9 @@ constexpr size_t kDpbOutputBufferExtraCount = limits::kMaxVideoFrames + 1; ...@@ -46,6 +46,9 @@ constexpr size_t kDpbOutputBufferExtraCount = limits::kMaxVideoFrames + 1;
} // namespace } // namespace
// static
base::AtomicRefCount V4L2VideoDecoder::num_instances_(0);
// static // static
std::unique_ptr<DecoderInterface> V4L2VideoDecoder::Create( std::unique_ptr<DecoderInterface> V4L2VideoDecoder::Create(
scoped_refptr<base::SequencedTaskRunner> decoder_task_runner, scoped_refptr<base::SequencedTaskRunner> decoder_task_runner,
...@@ -80,6 +83,7 @@ V4L2VideoDecoder::V4L2VideoDecoder( ...@@ -80,6 +83,7 @@ V4L2VideoDecoder::V4L2VideoDecoder(
base::WeakPtr<DecoderInterface::Client> client, base::WeakPtr<DecoderInterface::Client> client,
scoped_refptr<V4L2Device> device) scoped_refptr<V4L2Device> device)
: DecoderInterface(std::move(decoder_task_runner), std::move(client)), : DecoderInterface(std::move(decoder_task_runner), std::move(client)),
can_use_decoder_(num_instances_.Increment() < kMaxNumOfInstances),
device_(std::move(device)), device_(std::move(device)),
weak_this_factory_(this) { weak_this_factory_(this) {
DCHECK_CALLED_ON_VALID_SEQUENCE(decoder_sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(decoder_sequence_checker_);
...@@ -110,6 +114,7 @@ V4L2VideoDecoder::~V4L2VideoDecoder() { ...@@ -110,6 +114,7 @@ V4L2VideoDecoder::~V4L2VideoDecoder() {
} }
weak_this_factory_.InvalidateWeakPtrs(); weak_this_factory_.InvalidateWeakPtrs();
num_instances_.Decrement();
} }
void V4L2VideoDecoder::Initialize(const VideoDecoderConfig& config, void V4L2VideoDecoder::Initialize(const VideoDecoderConfig& config,
...@@ -121,6 +126,13 @@ void V4L2VideoDecoder::Initialize(const VideoDecoderConfig& config, ...@@ -121,6 +126,13 @@ void V4L2VideoDecoder::Initialize(const VideoDecoderConfig& config,
DCHECK(state_ == State::kUninitialized || state_ == State::kDecoding); DCHECK(state_ == State::kUninitialized || state_ == State::kDecoding);
DVLOGF(3); DVLOGF(3);
if (!can_use_decoder_) {
VLOGF(1) << "Reached maximum number of decoder instances ("
<< kMaxNumOfInstances << ")";
std::move(init_cb).Run(StatusCode::kDecoderCreationFailed);
return;
}
if (cdm_context || config.is_encrypted()) { if (cdm_context || config.is_encrypted()) {
VLOGF(1) << "V4L2 decoder does not support encrypted stream"; VLOGF(1) << "V4L2 decoder does not support encrypted stream";
std::move(init_cb).Run(StatusCode::kEncryptedContentUnsupported); std::move(init_cb).Run(StatusCode::kEncryptedContentUnsupported);
......
...@@ -135,6 +135,16 @@ class MEDIA_GPU_EXPORT V4L2VideoDecoder ...@@ -135,6 +135,16 @@ class MEDIA_GPU_EXPORT V4L2VideoDecoder
// Change the state and check the state transition is valid. // Change the state and check the state transition is valid.
void SetState(State new_state); void SetState(State new_state);
// Pages with multiple V4L2VideoDecoder instances might run out of memory
// (e.g. b/170870476) or crash (e.g. crbug.com/1109312). To avoid that and
// while the investigation goes on, limit the maximum number of simultaneous
// decoder instances for now. |num_instances_| tracks the number of
// simultaneous decoders. |can_use_decoder_| is true iff we haven't reached
// the maximum number of instances at the time this decoder is created.
static constexpr int kMaxNumOfInstances = 10;
static base::AtomicRefCount num_instances_;
const bool can_use_decoder_;
// The V4L2 backend, i.e. the part of the decoder that sends // The V4L2 backend, i.e. the part of the decoder that sends
// decoding jobs to the kernel. // decoding jobs to the kernel.
std::unique_ptr<V4L2VideoDecoderBackend> backend_; std::unique_ptr<V4L2VideoDecoderBackend> backend_;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment