Use RAII for locking/unlocking buffers.
Bug: 324383963
Test: atest virtual_camera_tests
Test: atest VirtualCameraTest
Change-Id: I8d8071250f7847334a75e261c7f69b5022e04507
diff --git a/services/camera/virtualcamera/VirtualCameraRenderThread.cc b/services/camera/virtualcamera/VirtualCameraRenderThread.cc
index 218e114..615e449 100644
--- a/services/camera/virtualcamera/VirtualCameraRenderThread.cc
+++ b/services/camera/virtualcamera/VirtualCameraRenderThread.cc
@@ -510,12 +510,10 @@
return {};
}
- android_ycbcr ycbcr;
- status_t status =
- gBuffer->lockYCbCr(AHARDWAREBUFFER_USAGE_CPU_READ_OFTEN, &ycbcr);
- if (status != NO_ERROR) {
+ YCbCrLockGuard yCbCrLock(inHwBuffer, AHARDWAREBUFFER_USAGE_CPU_READ_OFTEN);
+ if (yCbCrLock.getStatus() != NO_ERROR) {
ALOGE("%s: Failed to lock graphic buffer while generating thumbnail: %d",
- __func__, status);
+ __func__, yCbCrLock.getStatus());
return {};
}
@@ -523,9 +521,9 @@
compressedThumbnail.resize(kJpegThumbnailBufferSize);
ALOGE("%s: Compressing thumbnail %d x %d", __func__, gBuffer->getWidth(),
gBuffer->getHeight());
- std::optional<size_t> compressedSize =
- compressJpeg(gBuffer->getWidth(), gBuffer->getHeight(), quality, ycbcr,
- {}, compressedThumbnail.size(), compressedThumbnail.data());
+ std::optional<size_t> compressedSize = compressJpeg(
+ gBuffer->getWidth(), gBuffer->getHeight(), quality, *yCbCrLock, {},
+ compressedThumbnail.size(), compressedThumbnail.data());
if (!compressedSize.has_value()) {
ALOGE("%s: Failed to compress jpeg thumbnail", __func__);
return {};
@@ -570,14 +568,9 @@
return status;
}
- AHardwareBuffer_Planes planes_info;
-
- int32_t rawFence = fence != nullptr ? fence->get() : -1;
- int result = AHardwareBuffer_lockPlanes(hwBuffer.get(),
- AHARDWAREBUFFER_USAGE_CPU_READ_RARELY,
- rawFence, nullptr, &planes_info);
- if (result != OK) {
- ALOGE("%s: Failed to lock planes for BLOB buffer: %d", __func__, result);
+ PlanesLockGuard planesLock(hwBuffer, AHARDWAREBUFFER_USAGE_CPU_READ_RARELY,
+ fence);
+ if (planesLock.getStatus() != OK) {
return cameraStatus(Status::INTERNAL_ERROR);
}
@@ -586,22 +579,16 @@
std::optional<size_t> compressedSize;
if (gBuffer != nullptr) {
- android_ycbcr ycbcr;
if (gBuffer->getPixelFormat() != HAL_PIXEL_FORMAT_YCbCr_420_888) {
// This should never happen since we're allocating the temporary buffer
// with YUV420 layout above.
ALOGE("%s: Cannot compress non-YUV buffer (pixelFormat %d)", __func__,
gBuffer->getPixelFormat());
- AHardwareBuffer_unlock(hwBuffer.get(), nullptr);
return cameraStatus(Status::INTERNAL_ERROR);
}
- status_t status =
- gBuffer->lockYCbCr(AHARDWAREBUFFER_USAGE_CPU_READ_OFTEN, &ycbcr);
- ALOGV("Locked buffers");
- if (status != NO_ERROR) {
- AHardwareBuffer_unlock(hwBuffer.get(), nullptr);
- ALOGE("%s: Failed to lock graphic buffer: %d", __func__, status);
+ YCbCrLockGuard yCbCrLock(inHwBuffer, AHARDWAREBUFFER_USAGE_CPU_READ_OFTEN);
+ if (yCbCrLock.getStatus() != OK) {
return cameraStatus(Status::INTERNAL_ERROR);
}
@@ -611,24 +598,18 @@
requestSettings.thumbnailJpegQuality));
compressedSize = compressJpeg(
gBuffer->getWidth(), gBuffer->getHeight(), requestSettings.jpegQuality,
- ycbcr, app1ExifData, stream->bufferSize - sizeof(CameraBlob),
- planes_info.planes[0].data);
-
- status_t res = gBuffer->unlock();
- if (res != NO_ERROR) {
- ALOGE("Failed to unlock graphic buffer: %d", res);
- }
+ *yCbCrLock, app1ExifData, stream->bufferSize - sizeof(CameraBlob),
+ (*planesLock).planes[0].data);
} else {
std::vector<uint8_t> app1ExifData =
createExif(Resolution(stream->width, stream->height));
compressedSize = compressBlackJpeg(
stream->width, stream->height, requestSettings.jpegQuality, app1ExifData,
- stream->bufferSize - sizeof(CameraBlob), planes_info.planes[0].data);
+ stream->bufferSize - sizeof(CameraBlob), (*planesLock).planes[0].data);
}
if (!compressedSize.has_value()) {
ALOGE("%s: Failed to compress JPEG image", __func__);
- AHardwareBuffer_unlock(hwBuffer.get(), nullptr);
return cameraStatus(Status::INTERNAL_ERROR);
}
@@ -636,12 +617,10 @@
.blobId = CameraBlobId::JPEG,
.blobSizeBytes = static_cast<int32_t>(compressedSize.value())};
- memcpy(reinterpret_cast<uint8_t*>(planes_info.planes[0].data) +
+ memcpy(reinterpret_cast<uint8_t*>((*planesLock).planes[0].data) +
(stream->bufferSize - sizeof(cameraBlob)),
&cameraBlob, sizeof(cameraBlob));
- AHardwareBuffer_unlock(hwBuffer.get(), nullptr);
-
ALOGV("%s: Successfully compressed JPEG image, resulting size %zu B",
__func__, compressedSize.value());
diff --git a/services/camera/virtualcamera/util/Util.cc b/services/camera/virtualcamera/util/Util.cc
index ef986a6..b2048bc 100644
--- a/services/camera/virtualcamera/util/Util.cc
+++ b/services/camera/virtualcamera/util/Util.cc
@@ -20,8 +20,13 @@
#include <algorithm>
#include <array>
+#include <cstdint>
+#include <memory>
+#include "android/hardware_buffer.h"
#include "jpeglib.h"
+#include "ui/GraphicBuffer.h"
+#include "utils/Errors.h"
namespace android {
namespace companion {
@@ -40,6 +45,82 @@
constexpr std::array<Format, 2> kSupportedFormats{Format::YUV_420_888,
Format::RGBA_8888};
+YCbCrLockGuard::YCbCrLockGuard(std::shared_ptr<AHardwareBuffer> hwBuffer,
+ const uint32_t usageFlags)
+ : mHwBuffer(hwBuffer) {
+ GraphicBuffer* gBuffer = GraphicBuffer::fromAHardwareBuffer(mHwBuffer.get());
+ if (gBuffer == nullptr) {
+ ALOGE("%s: Attempting to lock nullptr buffer.", __func__);
+ return;
+ }
+ mLockStatus = gBuffer->lockYCbCr(usageFlags, &mYCbCr);
+ if (mLockStatus != OK) {
+ ALOGE("%s: Failed to lock graphic buffer: %s", __func__,
+ statusToString(mLockStatus).c_str());
+ }
+}
+
+YCbCrLockGuard::~YCbCrLockGuard() {
+ if (getStatus() != OK) {
+ return;
+ }
+
+ GraphicBuffer* gBuffer = GraphicBuffer::fromAHardwareBuffer(mHwBuffer.get());
+ if (gBuffer == nullptr) {
+ return;
+ }
+ gBuffer->unlock();
+ status_t status = gBuffer->unlock();
+ if (status != NO_ERROR) {
+ ALOGE("Failed to unlock graphic buffer: %s", statusToString(status).c_str());
+ }
+}
+
+status_t YCbCrLockGuard::getStatus() const {
+ return mLockStatus;
+}
+
+const android_ycbcr& YCbCrLockGuard::operator*() const {
+ LOG_ALWAYS_FATAL_IF(getStatus() != OK,
+ "Dereferencing unlocked YCbCrLockGuard, status is %s",
+ statusToString(mLockStatus).c_str());
+ return mYCbCr;
+}
+
+PlanesLockGuard::PlanesLockGuard(std::shared_ptr<AHardwareBuffer> hwBuffer,
+ const uint64_t usageFlags, sp<Fence> fence) {
+ if (hwBuffer == nullptr) {
+ ALOGE("%s: Attempting to lock nullptr buffer.", __func__);
+ return;
+ }
+
+ const int32_t rawFence = fence != nullptr ? fence->get() : -1;
+ mLockStatus = static_cast<status_t>(AHardwareBuffer_lockPlanes(
+ hwBuffer.get(), usageFlags, rawFence, nullptr, &mPlanes));
+ if (mLockStatus != OK) {
+ ALOGE("%s: Failed to lock graphic buffer: %s", __func__,
+ statusToString(mLockStatus).c_str());
+ }
+}
+
+PlanesLockGuard::~PlanesLockGuard() {
+ if (getStatus() != OK || mHwBuffer == nullptr) {
+ return;
+ }
+ AHardwareBuffer_unlock(mHwBuffer.get(), /*fence=*/nullptr);
+}
+
+int PlanesLockGuard::getStatus() const {
+ return mLockStatus;
+}
+
+const AHardwareBuffer_Planes& PlanesLockGuard::operator*() const {
+ LOG_ALWAYS_FATAL_IF(getStatus() != OK,
+ "Dereferencing unlocked PlanesLockGuard, status is %s",
+ statusToString(mLockStatus).c_str());
+ return mPlanes;
+}
+
sp<Fence> importFence(const NativeHandle& aidlHandle) {
if (aidlHandle.fds.size() != 1) {
return sp<Fence>::make();
diff --git a/services/camera/virtualcamera/util/Util.h b/services/camera/virtualcamera/util/Util.h
index 15f7969..d5b0b1f 100644
--- a/services/camera/virtualcamera/util/Util.h
+++ b/services/camera/virtualcamera/util/Util.h
@@ -18,17 +18,80 @@
#define ANDROID_COMPANION_VIRTUALCAMERA_UTIL_H
#include <cstdint>
+#include <memory>
#include "aidl/android/companion/virtualcamera/Format.h"
#include "aidl/android/hardware/camera/common/Status.h"
#include "aidl/android/hardware/camera/device/StreamBuffer.h"
#include "android/binder_auto_utils.h"
+#include "android/hardware_buffer.h"
+#include "system/graphics.h"
#include "ui/Fence.h"
namespace android {
namespace companion {
namespace virtualcamera {
+// RAII utility class to safely lock AHardwareBuffer and obtain android_ycbcr
+// structure describing YUV plane layout.
+//
+// Access to the buffer is locked immediatelly afer construction.
+class YCbCrLockGuard {
+ public:
+ YCbCrLockGuard(std::shared_ptr<AHardwareBuffer> hwBuffer, uint32_t usageFlags);
+ YCbCrLockGuard(YCbCrLockGuard&& other) = default;
+ ~YCbCrLockGuard();
+
+ // Returns OK if the buffer is successfully locked.
+ status_t getStatus() const;
+
+ // Dereferencing instance of this guard returns android_ycbcr structure
+ // describing the layout.
+ // Caller needs to check whether the buffer was successfully locked
+ // before dereferencing.
+ const android_ycbcr& operator*() const;
+
+ // Disable copy.
+ YCbCrLockGuard(const YCbCrLockGuard&) = delete;
+ YCbCrLockGuard& operator=(const YCbCrLockGuard&) = delete;
+
+ private:
+ std::shared_ptr<AHardwareBuffer> mHwBuffer;
+ android_ycbcr mYCbCr = {};
+ status_t mLockStatus = DEAD_OBJECT;
+};
+
+// RAII utility class to safely lock AHardwareBuffer and obtain
+// AHardwareBuffer_Planes (Suitable for interacting with RGBA / BLOB buffers.
+//
+// Access to the buffer is locked immediatelly afer construction.
+class PlanesLockGuard {
+ public:
+ PlanesLockGuard(std::shared_ptr<AHardwareBuffer> hwBuffer,
+ uint64_t usageFlags, sp<Fence> fence = nullptr);
+ PlanesLockGuard(PlanesLockGuard&& other) = default;
+ ~PlanesLockGuard();
+
+ // Returns OK if the buffer is successfully locked.
+ status_t getStatus() const;
+
+ // Dereferencing instance of this guard returns AHardwareBuffer_Planes
+ // structure describing the layout.
+ //
+ // Caller needs to check whether the buffer was successfully locked
+ // before dereferencing.
+ const AHardwareBuffer_Planes& operator*() const;
+
+ // Disable copy.
+ PlanesLockGuard(const PlanesLockGuard&) = delete;
+ PlanesLockGuard& operator=(const YCbCrLockGuard&) = delete;
+
+ private:
+ std::shared_ptr<AHardwareBuffer> mHwBuffer;
+ AHardwareBuffer_Planes mPlanes;
+ status_t mLockStatus = DEAD_OBJECT;
+};
+
// Converts camera AIDL status to ndk::ScopedAStatus
inline ndk::ScopedAStatus cameraStatus(
const ::aidl::android::hardware::camera::common::Status status) {