Commit 5eec3679 authored by Miguel Casas's avatar Miguel Casas Committed by Commit Bot

RELAND: Vaapi: Remove ConditionVariables and use PostTask

This CL relands the attached CL that got reverted due to scaling
issues, i.e. when the decode bitstream changed resolution, there
was the chance that before marking |awaiting_va_surfaces_recycle_|,
we could get an "old" resolution frame, that would confuse |decoder_|.

The solution is to move |awaiting_va_surfaces_recycle_| marking to
DecodeTask(), and hold off any more Decode() operations until the
"old" resolution frames are processed. This in turn makes
InitiateSurfaceSetChange() superfluous, so it's moved into DecodeTask().
The rest is comment update :-)
** Changes can be found here: crrev.com/c/973684/3..9

TEST=http://crosvideo.appspot.com/?codec=vp9&cycle=true&loop=true&mute=true
and letting it roll through many many many resolution changes.

Original CL description ------------------------------------------------

This CL removes the use of the ConditionVariables inside VaVDA and
uses instead PostTask()ing. This removes blocking the worker
thread, which allows for moving other tasks there, e.g. the
Surface Blit (as done in the follow up WIP CL crrev.com/c/947341)

For all that goodness, this CL:

- Refactors the waiting logic that on ToT is spread between
 GetCurrInputBuffer_Locked() and WaitForSurfaces_Locked() and
 simplifies it in DecodeTask() itself.

- Moves the |curr_input_buffer_| filling logic in
 GetCurrInputBuffer_Locked() into DecodeTask() proper.

- Reduces the scoped of the |lock_| in DecodeTask() (note that
 |curr_input_buffer_| and |decode_| are only used on the decoder
 thread -- added a .h comment).

Minor stuff: This CL also moves together the declaration of the
related member vars |input_buffers_|, |curr_input_buffer_| and
|available_va_surfaces_|

TEST=simplechrome+crosvideo (including seeks) on soraka, and
v_d_a_unittest with h264, vp8, vp9 (with dcheck_always_on=true).

Bug: 717265
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I86d179ac4d48c63b36787f5c7bdcb0da3839ed69
Reviewed-on: https://chromium-review.googlesource.com/949283
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Reviewed-by: default avatarDaniele Castagna <dcastagna@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#542955}
Reviewed-on: https://chromium-review.googlesource.com/973684
Cr-Commit-Position: refs/heads/master@{#546827}
parent 5c217469
......@@ -23,7 +23,6 @@
#include "base/memory/linked_ptr.h"
#include "base/memory/weak_ptr.h"
#include "base/single_thread_task_runner.h"
#include "base/synchronization/condition_variable.h"
#include "base/synchronization/lock.h"
#include "base/threading/thread.h"
#include "media/base/bitstream_buffer.h"
......@@ -116,21 +115,6 @@ class MEDIA_GPU_EXPORT VaapiVideoDecodeAccelerator
// Queue a input buffer for decode.
void QueueInputBuffer(const BitstreamBuffer& bitstream_buffer);
// Gets a new |current_input_buffer_| from |input_buffers_| and sets it up in
// |decoder_|. This method will sleep if no |input_buffers_| are available.
// Returns true if a new buffer has been set up, false if an early exit has
// been requested (due to initiated reset/flush/destroy).
bool GetCurrInputBuffer_Locked();
// Signals the client that |curr_input_buffer_| has been read and can be
// returned. Will also release the mapping.
void ReturnCurrInputBuffer_Locked();
// Waits for more surfaces to become available. Returns true once they do or
// false if an early exit has been requested (due to an initiated
// reset/flush/destroy).
bool WaitForSurfaces_Locked();
// Continue decoding given input buffers and sleep waiting for input/output
// as needed. Will exit if a new set of surfaces or reset/flush/destroy
// is requested.
......@@ -185,10 +169,7 @@ class MEDIA_GPU_EXPORT VaapiVideoDecodeAccelerator
// Initiate wait cycle for surfaces to be released before we release them
// and allocate new ones, as requested by the decoder.
void InitiateSurfaceSetChange(size_t num_pics, gfx::Size size);
// Check if the surfaces have been released or post ourselves for later.
void TryFinishSurfaceSetChange();
void TryFinishSurfaceSetChange(size_t num_pics, const gfx::Size& size);
// VAVDA state.
enum State {
......@@ -204,19 +185,21 @@ class MEDIA_GPU_EXPORT VaapiVideoDecodeAccelerator
kDestroying,
};
// Protects input buffer and surface queues and state_.
// Protects input buffer and surface queues and |state_|.
base::Lock lock_;
State state_;
Config::OutputMode output_mode_;
// Queue of available InputBuffers (picture_buffer_ids).
// Queue of input InputBuffers (PictureBuffer ids) to decode.
base::queue<std::unique_ptr<InputBuffer>> input_buffers_;
// Signalled when input buffers are queued onto |input_buffers_| queue.
base::ConditionVariable input_ready_;
// Current input buffer at decoder.
// Current InputBuffer at |decoder_|.
// Only accessed on |decoder_thread_task_runner_| (needs no |lock_|)
std::unique_ptr<InputBuffer> curr_input_buffer_;
// VA Surfaces no longer in use that can be passed back to the decoder for
// reuse, once it requests them.
std::list<VASurfaceID> available_va_surfaces_;
// Queue for incoming output buffers (texture ids).
using OutputBuffers = base::queue<int32_t>;
OutputBuffers output_buffers_;
......@@ -237,13 +220,6 @@ class MEDIA_GPU_EXPORT VaapiVideoDecodeAccelerator
// Return a VaapiPicture associated with given client-provided id.
VaapiPicture* PictureById(int32_t picture_buffer_id);
// VA Surfaces no longer in use that can be passed back to the decoder for
// reuse, once it requests them.
std::list<VASurfaceID> available_va_surfaces_;
// Signalled when output surfaces are queued onto the available_va_surfaces_
// queue.
base::ConditionVariable surfaces_available_;
// Pending output requests from the decoder. When it indicates that we should
// output a surface and we have an available Picture (i.e. texture) ready
// to use, we'll execute the callback passing the Picture. The callback
......
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