Commit 241d3ebe authored by Jeffrey Kardatzke's avatar Jeffrey Kardatzke Committed by Commit Bot

Revert "media/gpu/vaapiWrapper: Add WARN_UNUSED_RESULT to critical functions"

This reverts commit 8a1b573d.

Reason for revert: Incorrect condition on return value from CreateContext in ApplyResolutionChange.

Original change's description:
> media/gpu/vaapiWrapper: Add WARN_UNUSED_RESULT to critical functions
>
> This adds WARN_UNUSED_RESULT to VaapiWrapper functions whose
> result should have the critical impact enough to regard its
> failure as video processing fatal.
> VaapiVideoDecoder actually doesn't handle the failure of
> VaapiWrapper::CreateContext. This CL also fixes it.
>
> Bug: b:171536540
> Test: Build media_unittest and vaapi_unittest
> Change-Id: Iba6924849bfe8627fd328c3541c7a2c5cb0f0371
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2493742
> Commit-Queue: Hirokazu Honda <hiroh@chromium.org>
> Reviewed-by: Andres Calderon Jaramillo <andrescj@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#820164}

TBR=hiroh@chromium.org,andrescj@chromium.org

Change-Id: If3f3255475735f81786f64d686524947ec042a65
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: b:171536540
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2495980Reviewed-by: default avatarJeffrey Kardatzke <jkardatzke@google.com>
Commit-Queue: Jeffrey Kardatzke <jkardatzke@google.com>
Cr-Commit-Position: refs/heads/master@{#820424}
parent 45b379af
...@@ -506,11 +506,7 @@ void VaapiVideoDecoder::ApplyResolutionChange() { ...@@ -506,11 +506,7 @@ void VaapiVideoDecoder::ApplyResolutionChange() {
vaapi_wrapper_ = std::move(new_vaapi_wrapper); vaapi_wrapper_ = std::move(new_vaapi_wrapper);
} }
if (vaapi_wrapper_->CreateContext(pic_size)) { vaapi_wrapper_->CreateContext(pic_size);
VLOGF(1) << "Failed creating context";
SetState(State::kError);
return;
}
// If we reset during resolution change, then there is no decode tasks. In // If we reset during resolution change, then there is no decode tasks. In
// this case we do nothing and wait for next input. Otherwise, continue // this case we do nothing and wait for next input. Otherwise, continue
......
...@@ -18,7 +18,6 @@ ...@@ -18,7 +18,6 @@
#include <set> #include <set>
#include <vector> #include <vector>
#include "base/compiler_specific.h"
#include "base/files/file.h" #include "base/files/file.h"
#include "base/gtest_prod_util.h" #include "base/gtest_prod_util.h"
#include "base/macros.h" #include "base/macros.h"
...@@ -237,8 +236,7 @@ class MEDIA_GPU_EXPORT VaapiWrapper ...@@ -237,8 +236,7 @@ class MEDIA_GPU_EXPORT VaapiWrapper
const gfx::Size& size, const gfx::Size& size,
SurfaceUsageHint surface_usage_hint, SurfaceUsageHint surface_usage_hint,
size_t num_surfaces, size_t num_surfaces,
std::vector<VASurfaceID>* va_surfaces) std::vector<VASurfaceID>* va_surfaces);
WARN_UNUSED_RESULT;
// Creates a single ScopedVASurface of |va_format| and |size| and, if // Creates a single ScopedVASurface of |va_format| and |size| and, if
// successful, creates a |va_context_id_| of the same size. Returns nullptr if // successful, creates a |va_context_id_| of the same size. Returns nullptr if
...@@ -257,7 +255,7 @@ class MEDIA_GPU_EXPORT VaapiWrapper ...@@ -257,7 +255,7 @@ class MEDIA_GPU_EXPORT VaapiWrapper
// VPP VaapiWrapper, |size| is ignored and 0x0 is used to create the context. // VPP VaapiWrapper, |size| is ignored and 0x0 is used to create the context.
// The client is responsible for releasing it via DestroyContext() or // The client is responsible for releasing it via DestroyContext() or
// DestroyContextAndSurfaces(), or it will be released on dtor. // DestroyContextAndSurfaces(), or it will be released on dtor.
virtual bool CreateContext(const gfx::Size& size) WARN_UNUSED_RESULT; virtual bool CreateContext(const gfx::Size& size);
// Destroys the context identified by |va_context_id_|. // Destroys the context identified by |va_context_id_|.
virtual void DestroyContext(); virtual void DestroyContext();
...@@ -304,7 +302,7 @@ class MEDIA_GPU_EXPORT VaapiWrapper ...@@ -304,7 +302,7 @@ class MEDIA_GPU_EXPORT VaapiWrapper
// Synchronize the VASurface explicitly. This is useful when sharing a surface // Synchronize the VASurface explicitly. This is useful when sharing a surface
// between contexts. // between contexts.
bool SyncSurface(VASurfaceID va_surface_id) WARN_UNUSED_RESULT; bool SyncSurface(VASurfaceID va_surface_id);
// Calls SubmitBuffer_Locked() to request libva to allocate a new VABufferID // Calls SubmitBuffer_Locked() to request libva to allocate a new VABufferID
// of |va_buffer_type| and |size|, and to map-and-copy the |data| into it. The // of |va_buffer_type| and |size|, and to map-and-copy the |data| into it. The
...@@ -312,14 +310,11 @@ class MEDIA_GPU_EXPORT VaapiWrapper ...@@ -312,14 +310,11 @@ class MEDIA_GPU_EXPORT VaapiWrapper
// that this method does not submit the buffers for execution, they are simply // that this method does not submit the buffers for execution, they are simply
// stored until ExecuteAndDestroyPendingBuffers()/Execute_Locked(). The // stored until ExecuteAndDestroyPendingBuffers()/Execute_Locked(). The
// ownership of |data| stays with the caller. // ownership of |data| stays with the caller.
bool SubmitBuffer(VABufferType va_buffer_type, bool SubmitBuffer(VABufferType va_buffer_type, size_t size, const void* data);
size_t size,
const void* data) WARN_UNUSED_RESULT;
// Convenient templatized version of SubmitBuffer() where |size| is deduced to // Convenient templatized version of SubmitBuffer() where |size| is deduced to
// be the size of the type of |*data|. // be the size of the type of |*data|.
template <typename T> template <typename T>
bool SubmitBuffer(VABufferType va_buffer_type, bool SubmitBuffer(VABufferType va_buffer_type, const T* data) {
const T* data) WARN_UNUSED_RESULT {
return SubmitBuffer(va_buffer_type, sizeof(T), data); return SubmitBuffer(va_buffer_type, sizeof(T), data);
} }
// Batch-version of SubmitBuffer(), where the lock for accessing libva is // Batch-version of SubmitBuffer(), where the lock for accessing libva is
...@@ -329,8 +324,7 @@ class MEDIA_GPU_EXPORT VaapiWrapper ...@@ -329,8 +324,7 @@ class MEDIA_GPU_EXPORT VaapiWrapper
size_t size; size_t size;
const void* data; const void* data;
}; };
bool SubmitBuffers(const std::vector<VABufferDescriptor>& va_buffers) bool SubmitBuffers(const std::vector<VABufferDescriptor>& va_buffers);
WARN_UNUSED_RESULT;
// Destroys all |pending_va_buffers_| sent via SubmitBuffer*(). Useful when a // Destroys all |pending_va_buffers_| sent via SubmitBuffer*(). Useful when a
// pending job is to be cancelled (on reset or error). // pending job is to be cancelled (on reset or error).
...@@ -338,22 +332,20 @@ class MEDIA_GPU_EXPORT VaapiWrapper ...@@ -338,22 +332,20 @@ class MEDIA_GPU_EXPORT VaapiWrapper
// Executes job in hardware on target |va_surface_id| and destroys pending // Executes job in hardware on target |va_surface_id| and destroys pending
// buffers. Returns false if Execute() fails. // buffers. Returns false if Execute() fails.
virtual bool ExecuteAndDestroyPendingBuffers(VASurfaceID va_surface_id) virtual bool ExecuteAndDestroyPendingBuffers(VASurfaceID va_surface_id);
WARN_UNUSED_RESULT;
// Maps each |va_buffers| ID and copies the data described by the associated // Maps each |va_buffers| ID and copies the data described by the associated
// VABufferDescriptor into it; then calls Execute_Locked() on |va_surface_id|. // VABufferDescriptor into it; then calls Execute_Locked() on |va_surface_id|.
bool MapAndCopyAndExecute( bool MapAndCopyAndExecute(
VASurfaceID va_surface_id, VASurfaceID va_surface_id,
const std::vector<std::pair<VABufferID, VABufferDescriptor>>& va_buffers) const std::vector<std::pair<VABufferID, VABufferDescriptor>>& va_buffers);
WARN_UNUSED_RESULT;
#if defined(USE_X11) #if defined(USE_X11)
// Put data from |va_surface_id| into |x_pixmap| of size // Put data from |va_surface_id| into |x_pixmap| of size
// |dest_size|, converting/scaling to it. // |dest_size|, converting/scaling to it.
bool PutSurfaceIntoPixmap(VASurfaceID va_surface_id, bool PutSurfaceIntoPixmap(VASurfaceID va_surface_id,
Pixmap x_pixmap, Pixmap x_pixmap,
gfx::Size dest_size) WARN_UNUSED_RESULT; gfx::Size dest_size);
#endif // USE_X11 #endif // USE_X11
// Creates a ScopedVAImage from a VASurface |va_surface_id| and map it into // Creates a ScopedVAImage from a VASurface |va_surface_id| and map it into
...@@ -368,8 +360,7 @@ class MEDIA_GPU_EXPORT VaapiWrapper ...@@ -368,8 +360,7 @@ class MEDIA_GPU_EXPORT VaapiWrapper
// Uploads contents of |frame| into |va_surface_id| for encode. // Uploads contents of |frame| into |va_surface_id| for encode.
virtual bool UploadVideoFrameToSurface(const VideoFrame& frame, virtual bool UploadVideoFrameToSurface(const VideoFrame& frame,
VASurfaceID va_surface_id, VASurfaceID va_surface_id,
const gfx::Size& va_surface_size) const gfx::Size& va_surface_size);
WARN_UNUSED_RESULT;
// Creates a buffer of |size| bytes to be used as encode output. // Creates a buffer of |size| bytes to be used as encode output.
virtual std::unique_ptr<ScopedVABuffer> CreateVABuffer(VABufferType type, virtual std::unique_ptr<ScopedVABuffer> CreateVABuffer(VABufferType type,
...@@ -381,8 +372,7 @@ class MEDIA_GPU_EXPORT VaapiWrapper ...@@ -381,8 +372,7 @@ class MEDIA_GPU_EXPORT VaapiWrapper
// source surface passed to the encode job. Returns 0 if it fails for any // source surface passed to the encode job. Returns 0 if it fails for any
// reason. // reason.
virtual uint64_t GetEncodedChunkSize(VABufferID buffer_id, virtual uint64_t GetEncodedChunkSize(VABufferID buffer_id,
VASurfaceID sync_surface_id) VASurfaceID sync_surface_id);
WARN_UNUSED_RESULT;
// Downloads the contents of the buffer with given |buffer_id| into a buffer // Downloads the contents of the buffer with given |buffer_id| into a buffer
// of size |target_size|, pointed to by |target_ptr|. The number of bytes // of size |target_size|, pointed to by |target_ptr|. The number of bytes
...@@ -395,7 +385,7 @@ class MEDIA_GPU_EXPORT VaapiWrapper ...@@ -395,7 +385,7 @@ class MEDIA_GPU_EXPORT VaapiWrapper
VASurfaceID sync_surface_id, VASurfaceID sync_surface_id,
uint8_t* target_ptr, uint8_t* target_ptr,
size_t target_size, size_t target_size,
size_t* coded_data_size) WARN_UNUSED_RESULT; size_t* coded_data_size);
// Get the max number of reference frames for encoding supported by the // Get the max number of reference frames for encoding supported by the
// driver. // driver.
...@@ -403,8 +393,7 @@ class MEDIA_GPU_EXPORT VaapiWrapper ...@@ -403,8 +393,7 @@ class MEDIA_GPU_EXPORT VaapiWrapper
// frames for both the reference picture list 0 (bottom 16 bits) and the // frames for both the reference picture list 0 (bottom 16 bits) and the
// reference picture list 1 (top 16 bits). // reference picture list 1 (top 16 bits).
virtual bool GetVAEncMaxNumOfRefFrames(VideoCodecProfile profile, virtual bool GetVAEncMaxNumOfRefFrames(VideoCodecProfile profile,
size_t* max_ref_frames) size_t* max_ref_frames);
WARN_UNUSED_RESULT;
// Checks if the driver supports frame rotation. // Checks if the driver supports frame rotation.
bool IsRotationSupported(); bool IsRotationSupported();
...@@ -417,8 +406,7 @@ class MEDIA_GPU_EXPORT VaapiWrapper ...@@ -417,8 +406,7 @@ class MEDIA_GPU_EXPORT VaapiWrapper
const VASurface& va_surface_dest, const VASurface& va_surface_dest,
base::Optional<gfx::Rect> src_rect = base::nullopt, base::Optional<gfx::Rect> src_rect = base::nullopt,
base::Optional<gfx::Rect> dest_rect = base::nullopt, base::Optional<gfx::Rect> dest_rect = base::nullopt,
VideoRotation rotation = VIDEO_ROTATION_0) VideoRotation rotation = VIDEO_ROTATION_0);
WARN_UNUSED_RESULT;
// Initialize static data before sandbox is enabled. // Initialize static data before sandbox is enabled.
static void PreSandboxInitialization(); static void PreSandboxInitialization();
...@@ -438,10 +426,9 @@ class MEDIA_GPU_EXPORT VaapiWrapper ...@@ -438,10 +426,9 @@ class MEDIA_GPU_EXPORT VaapiWrapper
FRIEND_TEST_ALL_PREFIXES(VaapiUtilsTest, BadScopedVAImage); FRIEND_TEST_ALL_PREFIXES(VaapiUtilsTest, BadScopedVAImage);
FRIEND_TEST_ALL_PREFIXES(VaapiUtilsTest, BadScopedVABufferMapping); FRIEND_TEST_ALL_PREFIXES(VaapiUtilsTest, BadScopedVABufferMapping);
bool Initialize(CodecMode mode, VAProfile va_profile) WARN_UNUSED_RESULT; bool Initialize(CodecMode mode, VAProfile va_profile);
void Deinitialize(); void Deinitialize();
bool VaInitialize(const ReportErrorToUMACB& report_error_to_uma_cb) bool VaInitialize(const ReportErrorToUMACB& report_error_to_uma_cb);
WARN_UNUSED_RESULT;
// Tries to allocate |num_surfaces| VASurfaceIDs of |size| and |va_format|. // Tries to allocate |num_surfaces| VASurfaceIDs of |size| and |va_format|.
// Fills |va_surfaces| and returns true if successful, or returns false. // Fills |va_surfaces| and returns true if successful, or returns false.
...@@ -449,26 +436,26 @@ class MEDIA_GPU_EXPORT VaapiWrapper ...@@ -449,26 +436,26 @@ class MEDIA_GPU_EXPORT VaapiWrapper
const gfx::Size& size, const gfx::Size& size,
SurfaceUsageHint usage_hint, SurfaceUsageHint usage_hint,
size_t num_surfaces, size_t num_surfaces,
std::vector<VASurfaceID>* va_surfaces) WARN_UNUSED_RESULT; std::vector<VASurfaceID>* va_surfaces);
// Carries out the vaBeginPicture()-vaRenderPicture()-vaEndPicture() on target // Carries out the vaBeginPicture()-vaRenderPicture()-vaEndPicture() on target
// |va_surface_id|. Returns false if any of these calls fails. // |va_surface_id|. Returns false if any of these calls fails.
bool Execute_Locked(VASurfaceID va_surface_id, bool Execute_Locked(VASurfaceID va_surface_id,
const std::vector<VABufferID>& va_buffers) const std::vector<VABufferID>& va_buffers)
EXCLUSIVE_LOCKS_REQUIRED(va_lock_) WARN_UNUSED_RESULT; EXCLUSIVE_LOCKS_REQUIRED(va_lock_);
void DestroyPendingBuffers_Locked() EXCLUSIVE_LOCKS_REQUIRED(va_lock_); void DestroyPendingBuffers_Locked() EXCLUSIVE_LOCKS_REQUIRED(va_lock_);
// Requests libva to allocate a new VABufferID of type |va_buffer.type|, then // Requests libva to allocate a new VABufferID of type |va_buffer.type|, then
// maps-and-copies |va_buffer.size| contents of |va_buffer.data| to it. // maps-and-copies |va_buffer.size| contents of |va_buffer.data| to it.
bool SubmitBuffer_Locked(const VABufferDescriptor& va_buffer) bool SubmitBuffer_Locked(const VABufferDescriptor& va_buffer)
EXCLUSIVE_LOCKS_REQUIRED(va_lock_) WARN_UNUSED_RESULT; EXCLUSIVE_LOCKS_REQUIRED(va_lock_);
// Maps |va_buffer_id| and, if successful, copies the contents of |va_buffer| // Maps |va_buffer_id| and, if successful, copies the contents of |va_buffer|
// into it. // into it.
bool MapAndCopy_Locked(VABufferID va_buffer_id, bool MapAndCopy_Locked(VABufferID va_buffer_id,
const VABufferDescriptor& va_buffer) const VABufferDescriptor& va_buffer)
EXCLUSIVE_LOCKS_REQUIRED(va_lock_) WARN_UNUSED_RESULT; EXCLUSIVE_LOCKS_REQUIRED(va_lock_);
const CodecMode mode_; const CodecMode mode_;
......
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