Commit 134bd4b2 authored by Miguel Casas's avatar Miguel Casas Committed by Commit Bot

media/VaapiVideoDecoder: clarify threading

VaapiVideoDecoder lives in a single SequencedTaskRunner, so this CL
removes the comments' references to 'on [the] decoder thread'. Also
s/decoder_sequence_checker_/sequence_checker_/ since there's only one
TaskRunner. Finally, this CL also flips the DVLOG()s and the
DCHECK_CALLED_ON_VALID_SEQUENCE() so that a potential CHECK happens
after the printout, not before.

No new code intended.

Bug: b:162962069
Change-Id: I500acc380a0d3ac83fc5027217d6735198a2f1bc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2340365
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Commit-Queue: Andres Calderon Jaramillo <andrescj@chromium.org>
Reviewed-by: default avatarAndres Calderon Jaramillo <andrescj@chromium.org>
Auto-Submit: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#796155}
parent 6dcdeefe
...@@ -82,15 +82,16 @@ VaapiVideoDecoder::VaapiVideoDecoder( ...@@ -82,15 +82,16 @@ VaapiVideoDecoder::VaapiVideoDecoder(
: DecoderInterface(std::move(decoder_task_runner), std::move(client)), : DecoderInterface(std::move(decoder_task_runner), std::move(client)),
buffer_id_to_timestamp_(kTimestampCacheSize), buffer_id_to_timestamp_(kTimestampCacheSize),
weak_this_factory_(this) { weak_this_factory_(this) {
DCHECK_CALLED_ON_VALID_SEQUENCE(decoder_sequence_checker_);
VLOGF(2); VLOGF(2);
DCHECK(decoder_task_runner->RunsTasksInCurrentSequence());
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
weak_this_ = weak_this_factory_.GetWeakPtr(); weak_this_ = weak_this_factory_.GetWeakPtr();
} }
VaapiVideoDecoder::~VaapiVideoDecoder() { VaapiVideoDecoder::~VaapiVideoDecoder() {
DCHECK_CALLED_ON_VALID_SEQUENCE(decoder_sequence_checker_);
VLOGF(2); VLOGF(2);
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
// Abort all currently scheduled decode tasks. // Abort all currently scheduled decode tasks.
ClearDecodeTaskQueue(DecodeStatus::ABORTED); ClearDecodeTaskQueue(DecodeStatus::ABORTED);
...@@ -114,10 +115,10 @@ VaapiVideoDecoder::~VaapiVideoDecoder() { ...@@ -114,10 +115,10 @@ VaapiVideoDecoder::~VaapiVideoDecoder() {
void VaapiVideoDecoder::Initialize(const VideoDecoderConfig& config, void VaapiVideoDecoder::Initialize(const VideoDecoderConfig& config,
InitCB init_cb, InitCB init_cb,
const OutputCB& output_cb) { const OutputCB& output_cb) {
DCHECK_CALLED_ON_VALID_SEQUENCE(decoder_sequence_checker_); DVLOGF(2) << config.AsHumanReadableString();
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DCHECK(config.IsValidConfig()); DCHECK(config.IsValidConfig());
DCHECK(state_ == State::kUninitialized || state_ == State::kWaitingForInput); DCHECK(state_ == State::kUninitialized || state_ == State::kWaitingForInput);
DVLOGF(2) << config.AsHumanReadableString();
// Reinitializing the decoder is allowed if there are no pending decodes. // Reinitializing the decoder is allowed if there are no pending decodes.
if (current_decode_task_ || !decode_task_queue_.empty()) { if (current_decode_task_ || !decode_task_queue_.empty()) {
...@@ -177,7 +178,7 @@ void VaapiVideoDecoder::Initialize(const VideoDecoderConfig& config, ...@@ -177,7 +178,7 @@ void VaapiVideoDecoder::Initialize(const VideoDecoderConfig& config,
void VaapiVideoDecoder::Decode(scoped_refptr<DecoderBuffer> buffer, void VaapiVideoDecoder::Decode(scoped_refptr<DecoderBuffer> buffer,
DecodeCB decode_cb) { DecodeCB decode_cb) {
DCHECK_CALLED_ON_VALID_SEQUENCE(decoder_sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DVLOGF(4) << "Queuing input buffer, id: " << next_buffer_id_ << ", size: " DVLOGF(4) << "Queuing input buffer, id: " << next_buffer_id_ << ", size: "
<< (buffer->end_of_stream() ? 0 : buffer->data_size()); << (buffer->end_of_stream() ? 0 : buffer->data_size());
...@@ -205,7 +206,7 @@ void VaapiVideoDecoder::Decode(scoped_refptr<DecoderBuffer> buffer, ...@@ -205,7 +206,7 @@ void VaapiVideoDecoder::Decode(scoped_refptr<DecoderBuffer> buffer,
} }
void VaapiVideoDecoder::ScheduleNextDecodeTask() { void VaapiVideoDecoder::ScheduleNextDecodeTask() {
DCHECK_CALLED_ON_VALID_SEQUENCE(decoder_sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DCHECK_EQ(state_, State::kDecoding); DCHECK_EQ(state_, State::kDecoding);
DCHECK(!current_decode_task_); DCHECK(!current_decode_task_);
DCHECK(!decode_task_queue_.empty()); DCHECK(!decode_task_queue_.empty());
...@@ -224,8 +225,8 @@ void VaapiVideoDecoder::ScheduleNextDecodeTask() { ...@@ -224,8 +225,8 @@ void VaapiVideoDecoder::ScheduleNextDecodeTask() {
} }
void VaapiVideoDecoder::HandleDecodeTask() { void VaapiVideoDecoder::HandleDecodeTask() {
DCHECK_CALLED_ON_VALID_SEQUENCE(decoder_sequence_checker_);
DVLOGF(4); DVLOGF(4);
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
if (state_ == State::kError || state_ == State::kResetting) if (state_ == State::kError || state_ == State::kResetting)
return; return;
...@@ -284,8 +285,8 @@ void VaapiVideoDecoder::HandleDecodeTask() { ...@@ -284,8 +285,8 @@ void VaapiVideoDecoder::HandleDecodeTask() {
} }
void VaapiVideoDecoder::ClearDecodeTaskQueue(DecodeStatus status) { void VaapiVideoDecoder::ClearDecodeTaskQueue(DecodeStatus status) {
DCHECK_CALLED_ON_VALID_SEQUENCE(decoder_sequence_checker_);
DVLOGF(4); DVLOGF(4);
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
if (current_decode_task_) { if (current_decode_task_) {
std::move(current_decode_task_->decode_done_cb_).Run(status); std::move(current_decode_task_->decode_done_cb_).Run(status);
...@@ -299,10 +300,10 @@ void VaapiVideoDecoder::ClearDecodeTaskQueue(DecodeStatus status) { ...@@ -299,10 +300,10 @@ void VaapiVideoDecoder::ClearDecodeTaskQueue(DecodeStatus status) {
} }
scoped_refptr<VASurface> VaapiVideoDecoder::CreateSurface() { scoped_refptr<VASurface> VaapiVideoDecoder::CreateSurface() {
DCHECK_CALLED_ON_VALID_SEQUENCE(decoder_sequence_checker_); DVLOGF(4);
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DCHECK_EQ(state_, State::kDecoding); DCHECK_EQ(state_, State::kDecoding);
DCHECK(current_decode_task_); DCHECK(current_decode_task_);
DVLOGF(4);
// Get a video frame from the video frame pool. // Get a video frame from the video frame pool.
scoped_refptr<VideoFrame> frame = frame_pool_->GetFrame(); scoped_refptr<VideoFrame> frame = frame_pool_->GetFrame();
...@@ -364,9 +365,9 @@ void VaapiVideoDecoder::SurfaceReady(scoped_refptr<VASurface> va_surface, ...@@ -364,9 +365,9 @@ void VaapiVideoDecoder::SurfaceReady(scoped_refptr<VASurface> va_surface,
int32_t buffer_id, int32_t buffer_id,
const gfx::Rect& visible_rect, const gfx::Rect& visible_rect,
const VideoColorSpace& color_space) { const VideoColorSpace& color_space) {
DCHECK_CALLED_ON_VALID_SEQUENCE(decoder_sequence_checker_);
DCHECK_EQ(state_, State::kDecoding);
DVLOGF(4); DVLOGF(4);
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DCHECK_EQ(state_, State::kDecoding);
// Find the timestamp associated with |buffer_id|. It's possible that a // Find the timestamp associated with |buffer_id|. It's possible that a
// surface is output multiple times for different |buffer_id|s (e.g. VP9 // surface is output multiple times for different |buffer_id|s (e.g. VP9
...@@ -410,7 +411,7 @@ void VaapiVideoDecoder::SurfaceReady(scoped_refptr<VASurface> va_surface, ...@@ -410,7 +411,7 @@ void VaapiVideoDecoder::SurfaceReady(scoped_refptr<VASurface> va_surface,
} }
void VaapiVideoDecoder::ApplyResolutionChange() { void VaapiVideoDecoder::ApplyResolutionChange() {
DCHECK_CALLED_ON_VALID_SEQUENCE(decoder_sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DCHECK(state_ == State::kChangingResolution || DCHECK(state_ == State::kChangingResolution ||
state_ == State::kWaitingForInput); state_ == State::kWaitingForInput);
DCHECK(output_frames_.empty()); DCHECK(output_frames_.empty());
...@@ -469,8 +470,8 @@ void VaapiVideoDecoder::ApplyResolutionChange() { ...@@ -469,8 +470,8 @@ void VaapiVideoDecoder::ApplyResolutionChange() {
} }
void VaapiVideoDecoder::ReleaseVideoFrame(VASurfaceID surface_id) { void VaapiVideoDecoder::ReleaseVideoFrame(VASurfaceID surface_id) {
DCHECK_CALLED_ON_VALID_SEQUENCE(decoder_sequence_checker_);
DVLOGF(4); DVLOGF(4);
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
// The decoder has finished using the frame associated with |surface_id| for // The decoder has finished using the frame associated with |surface_id| for
// output or reference, so it's safe to drop our reference here. Once the // output or reference, so it's safe to drop our reference here. Once the
...@@ -481,8 +482,8 @@ void VaapiVideoDecoder::ReleaseVideoFrame(VASurfaceID surface_id) { ...@@ -481,8 +482,8 @@ void VaapiVideoDecoder::ReleaseVideoFrame(VASurfaceID surface_id) {
} }
void VaapiVideoDecoder::NotifyFrameAvailable() { void VaapiVideoDecoder::NotifyFrameAvailable() {
DCHECK_CALLED_ON_VALID_SEQUENCE(decoder_sequence_checker_);
DVLOGF(4); DVLOGF(4);
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
// If we were waiting for output buffers, retry the current decode task. // If we were waiting for output buffers, retry the current decode task.
if (state_ == State::kWaitingForOutput) { if (state_ == State::kWaitingForOutput) {
...@@ -495,12 +496,12 @@ void VaapiVideoDecoder::NotifyFrameAvailable() { ...@@ -495,12 +496,12 @@ void VaapiVideoDecoder::NotifyFrameAvailable() {
} }
void VaapiVideoDecoder::Flush() { void VaapiVideoDecoder::Flush() {
DCHECK_CALLED_ON_VALID_SEQUENCE(decoder_sequence_checker_); DVLOGF(2);
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DCHECK_EQ(state_, State::kDecoding); DCHECK_EQ(state_, State::kDecoding);
DCHECK(current_decode_task_); DCHECK(current_decode_task_);
DCHECK(current_decode_task_->buffer_->end_of_stream()); DCHECK(current_decode_task_->buffer_->end_of_stream());
DCHECK(decode_task_queue_.empty()); DCHECK(decode_task_queue_.empty());
DVLOGF(2);
// Flush will block until SurfaceReady() has been called for every frame // Flush will block until SurfaceReady() has been called for every frame
// currently decoding. // currently decoding.
...@@ -524,8 +525,8 @@ void VaapiVideoDecoder::Flush() { ...@@ -524,8 +525,8 @@ void VaapiVideoDecoder::Flush() {
} }
void VaapiVideoDecoder::Reset(base::OnceClosure reset_cb) { void VaapiVideoDecoder::Reset(base::OnceClosure reset_cb) {
DCHECK_CALLED_ON_VALID_SEQUENCE(decoder_sequence_checker_);
DVLOGF(2); DVLOGF(2);
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
// If we encountered an error, skip reset and notify client. // If we encountered an error, skip reset and notify client.
if (state_ == State::kError) { if (state_ == State::kError) {
...@@ -558,8 +559,8 @@ void VaapiVideoDecoder::Reset(base::OnceClosure reset_cb) { ...@@ -558,8 +559,8 @@ void VaapiVideoDecoder::Reset(base::OnceClosure reset_cb) {
} }
bool VaapiVideoDecoder::CreateAcceleratedVideoDecoder() { bool VaapiVideoDecoder::CreateAcceleratedVideoDecoder() {
DCHECK_CALLED_ON_VALID_SEQUENCE(decoder_sequence_checker_);
DVLOGF(2); DVLOGF(2);
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
if (profile_ >= H264PROFILE_MIN && profile_ <= H264PROFILE_MAX) { if (profile_ >= H264PROFILE_MIN && profile_ <= H264PROFILE_MAX) {
auto accelerator = auto accelerator =
...@@ -589,20 +590,20 @@ bool VaapiVideoDecoder::CreateAcceleratedVideoDecoder() { ...@@ -589,20 +590,20 @@ bool VaapiVideoDecoder::CreateAcceleratedVideoDecoder() {
} }
void VaapiVideoDecoder::ResetDone(base::OnceClosure reset_cb) { void VaapiVideoDecoder::ResetDone(base::OnceClosure reset_cb) {
DCHECK_CALLED_ON_VALID_SEQUENCE(decoder_sequence_checker_); DVLOGF(2);
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DCHECK_EQ(state_, State::kResetting); DCHECK_EQ(state_, State::kResetting);
DCHECK(!current_decode_task_); DCHECK(!current_decode_task_);
DCHECK(decode_task_queue_.empty()); DCHECK(decode_task_queue_.empty());
DVLOGF(2);
std::move(reset_cb).Run(); std::move(reset_cb).Run();
SetState(State::kWaitingForInput); SetState(State::kWaitingForInput);
} }
void VaapiVideoDecoder::SetState(State state) { void VaapiVideoDecoder::SetState(State state) {
DCHECK_CALLED_ON_VALID_SEQUENCE(decoder_sequence_checker_);
DVLOGF(3) << static_cast<int>(state) DVLOGF(3) << static_cast<int>(state)
<< ", current state: " << static_cast<int>(state_); << ", current state: " << static_cast<int>(state_);
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
// Check whether the state change is valid. // Check whether the state change is valid.
switch (state) { switch (state) {
......
...@@ -21,7 +21,6 @@ ...@@ -21,7 +21,6 @@
#include "base/memory/weak_ptr.h" #include "base/memory/weak_ptr.h"
#include "base/optional.h" #include "base/optional.h"
#include "base/sequence_checker.h" #include "base/sequence_checker.h"
#include "base/threading/thread.h"
#include "base/time/time.h" #include "base/time/time.h"
#include "media/base/video_codecs.h" #include "media/base/video_codecs.h"
#include "media/base/video_frame_layout.h" #include "media/base/video_frame_layout.h"
...@@ -98,23 +97,23 @@ class VaapiVideoDecoder : public DecoderInterface, ...@@ -98,23 +97,23 @@ class VaapiVideoDecoder : public DecoderInterface,
// Schedule the next decode task in the queue to be executed. // Schedule the next decode task in the queue to be executed.
void ScheduleNextDecodeTask(); void ScheduleNextDecodeTask();
// Try to decode a single input buffer on the decoder thread. // Try to decode a single input buffer.
void HandleDecodeTask(); void HandleDecodeTask();
// Clear the decode task queue on the decoder thread. This is done when // Clear the decode task queue. This is done when resetting or destroying the
// resetting or destroying the decoder, or encountering an error. // decoder, or encountering an error.
void ClearDecodeTaskQueue(DecodeStatus status); void ClearDecodeTaskQueue(DecodeStatus status);
// Releases the local reference to the VideoFrame associated with the // Releases the local reference to the VideoFrame associated with the
// specified |surface_id| on the decoder thread. This is called when // specified |surface_id|. This is called when |decoder_| has outputted the
// |decoder_| has outputted the VideoFrame and stopped using it as a // VideoFrame and stopped using it as a reference frame. Note that this
// reference frame. Note that this doesn't mean the frame can be reused // doesn't mean the frame can be reused immediately, as it might still be used
// immediately, as it might still be used by the client. // by the client.
void ReleaseVideoFrame(VASurfaceID surface_id); void ReleaseVideoFrame(VASurfaceID surface_id);
// Callback for |frame_pool_| to notify of available resources. // Callback for |frame_pool_| to notify of available resources.
void NotifyFrameAvailable(); void NotifyFrameAvailable();
// Flushes |decoder_| on the decoder thread, blocking until all pending decode // Flushes |decoder_|, blocking until all pending decode tasks have been
// tasks have been executed and all frames have been output. // executed and all frames have been output.
void Flush(); void Flush();
// Called when resetting the decoder is finished, to execute |reset_cb|. // Called when resetting the decoder is finished, to execute |reset_cb|.
...@@ -123,7 +122,7 @@ class VaapiVideoDecoder : public DecoderInterface, ...@@ -123,7 +122,7 @@ class VaapiVideoDecoder : public DecoderInterface,
// Create codec-specific AcceleratedVideoDecoder and reset related variables. // Create codec-specific AcceleratedVideoDecoder and reset related variables.
bool CreateAcceleratedVideoDecoder(); bool CreateAcceleratedVideoDecoder();
// Change the current |state_| to the specified |state| on the decoder thread. // Change the current |state_| to the specified |state|.
void SetState(State state); void SetState(State state);
// The video decoder's state. // The video decoder's state.
...@@ -175,7 +174,7 @@ class VaapiVideoDecoder : public DecoderInterface, ...@@ -175,7 +174,7 @@ class VaapiVideoDecoder : public DecoderInterface,
// the pointer from AcceleratedVideoDecoder. // the pointer from AcceleratedVideoDecoder.
VaapiVideoDecoderDelegate* decoder_delegate_ = nullptr; VaapiVideoDecoderDelegate* decoder_delegate_ = nullptr;
SEQUENCE_CHECKER(decoder_sequence_checker_); SEQUENCE_CHECKER(sequence_checker_);
base::WeakPtr<VaapiVideoDecoder> weak_this_; base::WeakPtr<VaapiVideoDecoder> weak_this_;
base::WeakPtrFactory<VaapiVideoDecoder> weak_this_factory_; base::WeakPtrFactory<VaapiVideoDecoder> weak_this_factory_;
......
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