Skip to content

Commit

Permalink
media/gpu/v4l2: Guard MT21Decompressor correctly
Browse files Browse the repository at this point in the history
The MT21Decompressor was recently introduced for ChromeOS. This function
is written in NEON intrinisics and so is ARM specific. However references
to it added to V4L2VDA are not correctly guarded. This breaks the build
on Linux, on both ARM64 and x86_64.

Nothing limits the function to ChromeOS. A user running Chrome on an
MT8173 device running Linux could theoretically take advantage of it.
The implementation, due to its use of intrinsics, is still limited to
ARM.

Add guards around references to MT21Decompressor in V4L2VDA. Also
loosen the requirements so that it can be built on Linux.

Bug: b:284994884, b:258331312, b:291169645
Change-Id: Ib1164c2286735a3ebd30bd1cd53857d01234da74
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4710354
Commit-Queue: Chen-Yu Tsai <wenst@chromium.org>
Reviewed-by: Justin Green <greenjustin@google.com>
Reviewed-by: Miguel Casas-Sanchez <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1175283}
  • Loading branch information
wens authored and Chromium LUCI CQ committed Jul 26, 2023
1 parent c2820cb commit f23c835
Show file tree
Hide file tree
Showing 6 changed files with 79 additions and 33 deletions.
18 changes: 9 additions & 9 deletions media/gpu/v4l2/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,14 @@ source_set("v4l2") {
]
}

if (current_cpu == "arm" || current_cpu == "arm64") {
sources += [
"mt21/mt21_decompressor.cc",
"mt21/mt21_decompressor.h",
"mt21/mt21_util.h",
]
}

if (is_chromeos) {
sources += [
# AV1 delegate depends on header files only in ChromeOS SDK
Expand All @@ -75,14 +83,6 @@ source_set("v4l2") {
"v4l2_video_encode_accelerator.cc",
"v4l2_video_encode_accelerator.h",
]

if (current_cpu == "arm" || current_cpu == "arm64") {
sources += [
"mt21/mt21_decompressor.cc",
"mt21/mt21_decompressor.h",
"mt21/mt21_util.h",
]
}
}

libs = [
Expand Down Expand Up @@ -224,7 +224,7 @@ test("v4l2_unittest") {
]
}

if (is_chromeos && (current_cpu == "arm" || current_cpu == "arm64")) {
if (current_cpu == "arm" || current_cpu == "arm64") {
test("mt21_util_unittest") {
testonly = true
sources = [
Expand Down
52 changes: 43 additions & 9 deletions media/gpu/v4l2/legacy/v4l2_video_decode_accelerator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -413,18 +413,22 @@ void V4L2VideoDecodeAccelerator::AssignPictureBuffersTask(
return;
}

const bool prefer_software_mt21 =
#ifdef SUPPORT_MT21_PIXEL_FORMAT_SOFTWARE_DECOMPRESSION
base::FeatureList::IsEnabled(media::kPreferSoftwareMT21);
#else
false;
#endif
enum v4l2_memory memory;
if (!image_processor_device_ &&
!base::FeatureList::IsEnabled(media::kPreferSoftwareMT21) &&
if (!image_processor_device_ && !prefer_software_mt21 &&
output_mode_ == Config::OutputMode::IMPORT) {
memory = V4L2_MEMORY_DMABUF;
} else {
memory = V4L2_MEMORY_MMAP;
}

if (output_queue_->AllocateBuffers(
buffers.size(), memory,
base::FeatureList::IsEnabled(media::kPreferSoftwareMT21)) == 0) {
if (output_queue_->AllocateBuffers(buffers.size(), memory,
prefer_software_mt21) == 0) {
LOG(ERROR) << "Failed to request buffers!";
NOTIFY_ERROR(PLATFORM_FAILURE);
return;
Expand Down Expand Up @@ -573,7 +577,11 @@ void V4L2VideoDecodeAccelerator::AssignEGLImage(size_t buffer_index,

// Make ourselves available if CreateEGLImageFor has been called from
// ImportBufferForPictureTask.
if (!image_processor_ && !mt21_decompressor_) {
if (!image_processor_
#ifdef SUPPORT_MT21_PIXEL_FORMAT_SOFTWARE_DECOMPRESSION
&& !mt21_decompressor_
#endif
) {
DCHECK_EQ(output_wait_map_.count(picture_buffer_id), 1u);
output_wait_map_.erase(picture_buffer_id);
if (decoder_state_ != kChangingResolution) {
Expand Down Expand Up @@ -677,16 +685,22 @@ void V4L2VideoDecodeAccelerator::ImportBufferForPictureTask(
}
DCHECK_EQ(egl_image_size_, handle_size);

#ifdef SUPPORT_MT21_PIXEL_FORMAT_SOFTWARE_DECOMPRESSION
if (base::FeatureList::IsEnabled(media::kPreferSoftwareMT21) &&
!mt21_decompressor_) {
mt21_decompressor_ = std::make_unique<MT21Decompressor>(coded_size_);
}
#endif

// For allocate mode, the IP will already have been created in
// AssignPictureBuffersTask.
// Note: usage of the MT21 software decompressor disables the image
// processor.
if (image_processor_device_ && !image_processor_ && !mt21_decompressor_) {
if (image_processor_device_ && !image_processor_
#ifdef SUPPORT_MT21_PIXEL_FORMAT_SOFTWARE_DECOMPRESSION
&& !mt21_decompressor_
#endif
) {
DCHECK_EQ(kAwaitingPictureBuffers, decoder_state_);
// This is the first buffer import. Create the image processor and change
// the decoder state. The client may adjust the coded width. We don't have
Expand Down Expand Up @@ -752,7 +766,11 @@ void V4L2VideoDecodeAccelerator::ImportBufferForPictureTask(
// time since we already have its DMABUF fds. It is guaranteed that
// CreateEGLImageFor will run before the picture is passed to the client
// because the picture will need to be cleared on the child thread first.
if (!image_processor_ && !mt21_decompressor_) {
if (!image_processor_
#ifdef SUPPORT_MT21_PIXEL_FORMAT_SOFTWARE_DECOMPRESSION
&& !mt21_decompressor_
#endif
) {
DCHECK_GT(handle.planes.size(), 0u);
size_t index = iter - output_buffer_map_.begin();

Expand Down Expand Up @@ -1513,7 +1531,11 @@ bool V4L2VideoDecodeAccelerator::DequeueOutputBuffer() {
DCHECK_GE(bitstream_buffer_id, 0);
DVLOGF(4) << "Dequeue output buffer: dqbuf index=" << buf->BufferId()
<< " bitstream input_id=" << bitstream_buffer_id;
if (image_processor_device_ || mt21_decompressor_) {
if (image_processor_device_
#ifdef SUPPORT_MT21_PIXEL_FORMAT_SOFTWARE_DECOMPRESSION
|| mt21_decompressor_
#endif
) {
if (!ProcessFrame(bitstream_buffer_id, buf)) {
LOG(ERROR) << "Processing frame failed";
NOTIFY_ERROR(PLATFORM_FAILURE);
Expand Down Expand Up @@ -1896,7 +1918,10 @@ void V4L2VideoDecodeAccelerator::DestroyTask() {
image_processor_ = nullptr;
while (!buffers_at_ip_.empty())
buffers_at_ip_.pop();

#ifdef SUPPORT_MT21_PIXEL_FORMAT_SOFTWARE_DECOMPRESSION
mt21_decompressor_ = nullptr;
#endif

DestroyInputBuffers();
DestroyOutputBuffers();
Expand Down Expand Up @@ -2025,7 +2050,10 @@ void V4L2VideoDecodeAccelerator::StartResolutionChange() {
buffers_at_client_.clear();

image_processor_ = nullptr;

#ifdef SUPPORT_MT21_PIXEL_FORMAT_SOFTWARE_DECOMPRESSION
mt21_decompressor_ = nullptr;
#endif

if (!DestroyOutputBuffers()) {
LOG(ERROR) << "Failed destroying output buffers.";
Expand Down Expand Up @@ -2305,10 +2333,14 @@ bool V4L2VideoDecodeAccelerator::SetupFormats() {
DCHECK(!image_processor_device_);
if (!output_format_fourcc_) {
VLOGF(2) << "Could not find a usable output format. Try image processor";
#ifdef SUPPORT_MT21_PIXEL_FORMAT_SOFTWARE_DECOMPRESSION
if (base::FeatureList::IsEnabled(media::kPreferSoftwareMT21)) {
output_format_fourcc_ = Fourcc(Fourcc::MT21);
egl_image_format_fourcc_ = Fourcc(Fourcc::NV12);
} else {
#else
{
#endif
if (!V4L2ImageProcessorBackend::IsSupported()) {
VLOGF(1) << "Image processor not available";
return false;
Expand Down Expand Up @@ -2402,6 +2434,7 @@ bool V4L2VideoDecodeAccelerator::ProcessFrame(int32_t bitstream_buffer_id,
// Keep reference to the IP input until the frame is processed
buffers_at_ip_.push(std::make_pair(bitstream_buffer_id, buf));

#ifdef SUPPORT_MT21_PIXEL_FORMAT_SOFTWARE_DECOMPRESSION
if (base::FeatureList::IsEnabled(media::kPreferSoftwareMT21)) {
if (!mt21_decompressor_) {
LOG(ERROR) << "PreferSoftwareMT21 enabled, but MT21 decompressor was not "
Expand Down Expand Up @@ -2457,6 +2490,7 @@ bool V4L2VideoDecodeAccelerator::ProcessFrame(int32_t bitstream_buffer_id,

return true;
}
#endif

scoped_refptr<VideoFrame> input_frame = buf->GetVideoFrame();
if (!input_frame) {
Expand Down
12 changes: 12 additions & 0 deletions media/gpu/v4l2/legacy/v4l2_video_decode_accelerator.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,14 @@
#include <utility>
#include <vector>

#include "build/build_config.h"

#if defined(ARCH_CPU_ARM_FAMILY)
// The MT21C software decompressor is tightly coupled to the MT8173.
// See mt21_decompressor.h
#define SUPPORT_MT21_PIXEL_FORMAT_SOFTWARE_DECOMPRESSION
#endif

#include "base/cancelable_callback.h"
#include "base/containers/queue.h"
#include "base/functional/callback_forward.h"
Expand All @@ -32,7 +40,9 @@
#include "media/gpu/chromeos/image_processor.h"
#include "media/gpu/gpu_video_decode_accelerator_helpers.h"
#include "media/gpu/media_gpu_export.h"
#ifdef SUPPORT_MT21_PIXEL_FORMAT_SOFTWARE_DECOMPRESSION
#include "media/gpu/v4l2/mt21/mt21_decompressor.h"
#endif
#include "media/gpu/v4l2/v4l2_device.h"
#include "media/video/picture.h"
#include "media/video/video_decode_accelerator.h"
Expand Down Expand Up @@ -616,7 +626,9 @@ class MEDIA_GPU_EXPORT V4L2VideoDecodeAccelerator
// Image processor. Accessed on |decoder_thread_|.
std::unique_ptr<ImageProcessor> image_processor_;

#ifdef SUPPORT_MT21_PIXEL_FORMAT_SOFTWARE_DECOMPRESSION
std::unique_ptr<MT21Decompressor> mt21_decompressor_;
#endif

// The format of EGLImage.
absl::optional<Fourcc> egl_image_format_fourcc_;
Expand Down
8 changes: 1 addition & 7 deletions media/gpu/v4l2/mt21/mt21_decompressor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,11 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "media/gpu/v4l2/mt21/mt21_decompressor.h"

#if BUILDFLAG(IS_CHROMEOS) && defined(ARCH_CPU_ARM_FAMILY) && \
(defined(COMPILER_GCC) || defined(__clang__))

#include <sched.h>
#include <stdlib.h>

#include "base/bits.h"
#include "media/gpu/v4l2/mt21/mt21_decompressor.h"
#include "media/gpu/v4l2/mt21/mt21_util.h"
#include "third_party/libyuv/include/libyuv/planar_functions.h"

Expand Down Expand Up @@ -249,5 +245,3 @@ void MT21Decompressor::MT21ToNV12(const uint8_t* src_y,
}

} // namespace media

#endif
11 changes: 7 additions & 4 deletions media/gpu/v4l2/mt21/mt21_decompressor.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,13 @@

#include "build/build_config.h"

#if BUILDFLAG(IS_CHROMEOS) && defined(ARCH_CPU_ARM_FAMILY) && \
(defined(COMPILER_GCC) || defined(__clang__))
#if !defined(ARCH_CPU_ARM_FAMILY)
#error "MT21Decompressor is only intended to run on MT8173 (ARM)"
#endif

#if !(defined(COMPILER_GCC) || defined(__clang__))
#error "MT21Decompressor is only intended to be built with GCC or Clang"
#endif

#include <stdint.h>

Expand Down Expand Up @@ -117,6 +122,4 @@ class MT21Decompressor {

} // namespace media

#endif

#endif // MEDIA_GPU_V4L2_MT21_MT21_DECOMPRESSOR_H_
11 changes: 7 additions & 4 deletions media/gpu/v4l2/mt21/mt21_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,13 @@

#include "build/build_config.h"

#if BUILDFLAG(IS_CHROMEOS) && defined(ARCH_CPU_ARM_FAMILY) && \
(defined(COMPILER_GCC) || defined(__clang__))
#if !defined(ARCH_CPU_ARM_FAMILY)
#error "MT21Decompressor is only intended to run on MT8173 (ARM)"
#endif

#if !(defined(COMPILER_GCC) || defined(__clang__))
#error "MT21Decompressor is only intended to be built with GCC or Clang"
#endif

#include <arm_neon.h>
#include <stdint.h>
Expand Down Expand Up @@ -1115,6 +1120,4 @@ void BinSubblocks(const uint8_t* src,

} // namespace media

#endif

#endif // MEDIA_GPU_V4L2_MT21_MT21_UTIL_H_

0 comments on commit f23c835

Please sign in to comment.