Merge "SF: Fix ADPF regression due to wrong present fence" into udc-dev
diff --git a/libs/ultrahdr/include/ultrahdr/jpegr.h b/libs/ultrahdr/include/ultrahdr/jpegr.h
index 88038f1..00b66ae 100644
--- a/libs/ultrahdr/include/ultrahdr/jpegr.h
+++ b/libs/ultrahdr/include/ultrahdr/jpegr.h
@@ -373,14 +373,41 @@
jr_uncompressed_ptr dest);
/*
- * This method will check the validity of the input images.
+ * This method will check the validity of the input arguments.
*
* @param uncompressed_p010_image uncompressed HDR image in P010 color format
* @param uncompressed_yuv_420_image uncompressed SDR image in YUV_420 color format
- * @return NO_ERROR if the input images are valid, error code is not valid.
+ * @param hdr_tf transfer function of the HDR image
+ * @param dest destination of the compressed JPEGR image. Please note that {@code maxLength}
+ * represents the maximum available size of the desitination buffer, and it must be
+ * set before calling this method. If the encoded JPEGR size exceeds
+ * {@code maxLength}, this method will return {@code ERROR_JPEGR_BUFFER_TOO_SMALL}.
+ * @return NO_ERROR if the input args are valid, error code is not valid.
*/
- status_t areInputImagesValid(jr_uncompressed_ptr uncompressed_p010_image,
- jr_uncompressed_ptr uncompressed_yuv_420_image);
+ status_t areInputArgumentsValid(jr_uncompressed_ptr uncompressed_p010_image,
+ jr_uncompressed_ptr uncompressed_yuv_420_image,
+ ultrahdr_transfer_function hdr_tf,
+ jr_compressed_ptr dest);
+
+ /*
+ * This method will check the validity of the input arguments.
+ *
+ * @param uncompressed_p010_image uncompressed HDR image in P010 color format
+ * @param uncompressed_yuv_420_image uncompressed SDR image in YUV_420 color format
+ * @param hdr_tf transfer function of the HDR image
+ * @param dest destination of the compressed JPEGR image. Please note that {@code maxLength}
+ * represents the maximum available size of the desitination buffer, and it must be
+ * set before calling this method. If the encoded JPEGR size exceeds
+ * {@code maxLength}, this method will return {@code ERROR_JPEGR_BUFFER_TOO_SMALL}.
+ * @param quality target quality of the JPEG encoding, must be in range of 0-100 where 100 is
+ * the highest quality
+ * @return NO_ERROR if the input args are valid, error code is not valid.
+ */
+ status_t areInputArgumentsValid(jr_uncompressed_ptr uncompressed_p010_image,
+ jr_uncompressed_ptr uncompressed_yuv_420_image,
+ ultrahdr_transfer_function hdr_tf,
+ jr_compressed_ptr dest,
+ int quality);
};
} // namespace android::ultrahdr
diff --git a/libs/ultrahdr/include/ultrahdr/ultrahdr.h b/libs/ultrahdr/include/ultrahdr/ultrahdr.h
index f970936..e87a025 100644
--- a/libs/ultrahdr/include/ultrahdr/ultrahdr.h
+++ b/libs/ultrahdr/include/ultrahdr/ultrahdr.h
@@ -24,6 +24,7 @@
ULTRAHDR_COLORGAMUT_BT709,
ULTRAHDR_COLORGAMUT_P3,
ULTRAHDR_COLORGAMUT_BT2100,
+ ULTRAHDR_COLORGAMUT_MAX = ULTRAHDR_COLORGAMUT_BT2100,
} ultrahdr_color_gamut;
// Transfer functions for image data
@@ -33,6 +34,7 @@
ULTRAHDR_TF_HLG = 1,
ULTRAHDR_TF_PQ = 2,
ULTRAHDR_TF_SRGB = 3,
+ ULTRAHDR_TF_MAX = ULTRAHDR_TF_SRGB,
} ultrahdr_transfer_function;
// Target output formats for decoder
diff --git a/libs/ultrahdr/jpegr.cpp b/libs/ultrahdr/jpegr.cpp
index cb8197c..da25726 100644
--- a/libs/ultrahdr/jpegr.cpp
+++ b/libs/ultrahdr/jpegr.cpp
@@ -89,23 +89,69 @@
return cpuCoreCount;
}
-status_t JpegR::areInputImagesValid(jr_uncompressed_ptr uncompressed_p010_image,
- jr_uncompressed_ptr uncompressed_yuv_420_image) {
- if (uncompressed_p010_image == nullptr) {
+status_t JpegR::areInputArgumentsValid(jr_uncompressed_ptr uncompressed_p010_image,
+ jr_uncompressed_ptr uncompressed_yuv_420_image,
+ ultrahdr_transfer_function hdr_tf,
+ jr_compressed_ptr dest) {
+ if (uncompressed_p010_image == nullptr || uncompressed_p010_image->data == nullptr) {
+ ALOGE("received nullptr for uncompressed p010 image");
return ERROR_JPEGR_INVALID_NULL_PTR;
}
+ if (uncompressed_p010_image->width % 2 != 0
+ || uncompressed_p010_image->height % 2 != 0) {
+ ALOGE("Image dimensions cannot be odd, image dimensions %dx%d",
+ uncompressed_p010_image->width, uncompressed_p010_image->height);
+ return ERROR_JPEGR_INVALID_INPUT_TYPE;
+ }
+
+ if (uncompressed_p010_image->width == 0
+ || uncompressed_p010_image->height == 0) {
+ ALOGE("Image dimensions cannot be zero, image dimensions %dx%d",
+ uncompressed_p010_image->width, uncompressed_p010_image->height);
+ return ERROR_JPEGR_INVALID_INPUT_TYPE;
+ }
+
+ if (uncompressed_p010_image->colorGamut <= ULTRAHDR_COLORGAMUT_UNSPECIFIED
+ || uncompressed_p010_image->colorGamut > ULTRAHDR_COLORGAMUT_MAX) {
+ ALOGE("Unrecognized p010 color gamut %d", uncompressed_p010_image->colorGamut);
+ return ERROR_JPEGR_INVALID_INPUT_TYPE;
+ }
+
if (uncompressed_p010_image->luma_stride != 0
&& uncompressed_p010_image->luma_stride < uncompressed_p010_image->width) {
- ALOGE("Image stride can not be smaller than width, stride=%d, width=%d",
+ ALOGE("Luma stride can not be smaller than width, stride=%d, width=%d",
uncompressed_p010_image->luma_stride, uncompressed_p010_image->width);
return ERROR_JPEGR_INVALID_INPUT_TYPE;
}
+ if (uncompressed_p010_image->chroma_data != nullptr
+ && uncompressed_p010_image->chroma_stride < uncompressed_p010_image->width) {
+ ALOGE("Chroma stride can not be smaller than width, stride=%d, width=%d",
+ uncompressed_p010_image->chroma_stride,
+ uncompressed_p010_image->width);
+ return ERROR_JPEGR_INVALID_INPUT_TYPE;
+ }
+
+ if (dest == nullptr || dest->data == nullptr) {
+ ALOGE("received nullptr for destination");
+ return ERROR_JPEGR_INVALID_NULL_PTR;
+ }
+
+ if (hdr_tf <= ULTRAHDR_TF_UNSPECIFIED || hdr_tf > ULTRAHDR_TF_MAX) {
+ ALOGE("Invalid hdr transfer function %d", hdr_tf);
+ return ERROR_JPEGR_INVALID_INPUT_TYPE;
+ }
+
if (uncompressed_yuv_420_image == nullptr) {
return NO_ERROR;
}
+ if (uncompressed_yuv_420_image->data == nullptr) {
+ ALOGE("received nullptr for uncompressed 420 image");
+ return ERROR_JPEGR_INVALID_NULL_PTR;
+ }
+
if (uncompressed_yuv_420_image->luma_stride != 0) {
ALOGE("Stride is not supported for YUV420 image");
return ERROR_JPEGR_UNSUPPORTED_FEATURE;
@@ -127,6 +173,30 @@
return ERROR_JPEGR_RESOLUTION_MISMATCH;
}
+ if (uncompressed_yuv_420_image->colorGamut <= ULTRAHDR_COLORGAMUT_UNSPECIFIED
+ || uncompressed_yuv_420_image->colorGamut > ULTRAHDR_COLORGAMUT_MAX) {
+ ALOGE("Unrecognized 420 color gamut %d", uncompressed_yuv_420_image->colorGamut);
+ return ERROR_JPEGR_INVALID_INPUT_TYPE;
+ }
+
+ return NO_ERROR;
+}
+
+status_t JpegR::areInputArgumentsValid(jr_uncompressed_ptr uncompressed_p010_image,
+ jr_uncompressed_ptr uncompressed_yuv_420_image,
+ ultrahdr_transfer_function hdr_tf,
+ jr_compressed_ptr dest,
+ int quality) {
+ if (status_t ret = areInputArgumentsValid(
+ uncompressed_p010_image, uncompressed_yuv_420_image, hdr_tf, dest) != NO_ERROR) {
+ return ret;
+ }
+
+ if (quality < 0 || quality > 100) {
+ ALOGE("quality factor is out side range [0-100], quality factor : %d", quality);
+ return ERROR_JPEGR_INVALID_INPUT_TYPE;
+ }
+
return NO_ERROR;
}
@@ -136,19 +206,17 @@
jr_compressed_ptr dest,
int quality,
jr_exif_ptr exif) {
- if (uncompressed_p010_image == nullptr || dest == nullptr) {
- return ERROR_JPEGR_INVALID_NULL_PTR;
- }
-
- if (quality < 0 || quality > 100) {
- return ERROR_JPEGR_INVALID_INPUT_TYPE;
- }
-
- if (status_t ret = areInputImagesValid(
- uncompressed_p010_image, /* uncompressed_yuv_420_image */ nullptr) != NO_ERROR) {
+ if (status_t ret = areInputArgumentsValid(
+ uncompressed_p010_image, /* uncompressed_yuv_420_image */ nullptr,
+ hdr_tf, dest, quality) != NO_ERROR) {
return ret;
}
+ if (exif != nullptr && exif->data == nullptr) {
+ ALOGE("received nullptr for exif metadata");
+ return ERROR_JPEGR_INVALID_NULL_PTR;
+ }
+
ultrahdr_metadata_struct metadata;
metadata.version = kJpegrVersion;
@@ -201,18 +269,19 @@
jr_compressed_ptr dest,
int quality,
jr_exif_ptr exif) {
- if (uncompressed_p010_image == nullptr
- || uncompressed_yuv_420_image == nullptr
- || dest == nullptr) {
+ if (uncompressed_yuv_420_image == nullptr) {
+ ALOGE("received nullptr for uncompressed 420 image");
return ERROR_JPEGR_INVALID_NULL_PTR;
}
- if (quality < 0 || quality > 100) {
- return ERROR_JPEGR_INVALID_INPUT_TYPE;
+ if (exif != nullptr && exif->data == nullptr) {
+ ALOGE("received nullptr for exif metadata");
+ return ERROR_JPEGR_INVALID_NULL_PTR;
}
- if (status_t ret = areInputImagesValid(
- uncompressed_p010_image, uncompressed_yuv_420_image) != NO_ERROR) {
+ if (status_t ret = areInputArgumentsValid(
+ uncompressed_p010_image, uncompressed_yuv_420_image, hdr_tf,
+ dest, quality) != NO_ERROR) {
return ret;
}
@@ -256,15 +325,18 @@
jr_compressed_ptr compressed_jpeg_image,
ultrahdr_transfer_function hdr_tf,
jr_compressed_ptr dest) {
- if (uncompressed_p010_image == nullptr
- || uncompressed_yuv_420_image == nullptr
- || compressed_jpeg_image == nullptr
- || dest == nullptr) {
+ if (uncompressed_yuv_420_image == nullptr) {
+ ALOGE("received nullptr for uncompressed 420 image");
return ERROR_JPEGR_INVALID_NULL_PTR;
}
- if (status_t ret = areInputImagesValid(
- uncompressed_p010_image, uncompressed_yuv_420_image) != NO_ERROR) {
+ if (compressed_jpeg_image == nullptr || compressed_jpeg_image->data == nullptr) {
+ ALOGE("received nullptr for compressed jpeg image");
+ return ERROR_JPEGR_INVALID_NULL_PTR;
+ }
+
+ if (status_t ret = areInputArgumentsValid(
+ uncompressed_p010_image, uncompressed_yuv_420_image, hdr_tf, dest) != NO_ERROR) {
return ret;
}
@@ -293,14 +365,14 @@
jr_compressed_ptr compressed_jpeg_image,
ultrahdr_transfer_function hdr_tf,
jr_compressed_ptr dest) {
- if (uncompressed_p010_image == nullptr
- || compressed_jpeg_image == nullptr
- || dest == nullptr) {
+ if (compressed_jpeg_image == nullptr || compressed_jpeg_image->data == nullptr) {
+ ALOGE("received nullptr for compressed jpeg image");
return ERROR_JPEGR_INVALID_NULL_PTR;
}
- if (status_t ret = areInputImagesValid(
- uncompressed_p010_image, /* uncompressed_yuv_420_image */ nullptr) != NO_ERROR) {
+ if (status_t ret = areInputArgumentsValid(
+ uncompressed_p010_image, /* uncompressed_yuv_420_image */ nullptr,
+ hdr_tf, dest) != NO_ERROR) {
return ret;
}
@@ -344,13 +416,34 @@
jr_compressed_ptr compressed_gainmap,
ultrahdr_metadata_ptr metadata,
jr_compressed_ptr dest) {
+ if (compressed_jpeg_image == nullptr || compressed_jpeg_image->data == nullptr) {
+ ALOGE("received nullptr for compressed jpeg image");
+ return ERROR_JPEGR_INVALID_NULL_PTR;
+ }
+
+ if (compressed_gainmap == nullptr || compressed_gainmap->data == nullptr) {
+ ALOGE("received nullptr for compressed gain map");
+ return ERROR_JPEGR_INVALID_NULL_PTR;
+ }
+
+ if (dest == nullptr || dest->data == nullptr) {
+ ALOGE("received nullptr for destination");
+ return ERROR_JPEGR_INVALID_NULL_PTR;
+ }
+
JPEGR_CHECK(appendGainMap(compressed_jpeg_image, compressed_gainmap, /* exif */ nullptr,
metadata, dest));
return NO_ERROR;
}
status_t JpegR::getJPEGRInfo(jr_compressed_ptr compressed_jpegr_image, jr_info_ptr jpegr_info) {
- if (compressed_jpegr_image == nullptr || jpegr_info == nullptr) {
+ if (compressed_jpegr_image == nullptr || compressed_jpegr_image->data == nullptr) {
+ ALOGE("received nullptr for compressed jpegr image");
+ return ERROR_JPEGR_INVALID_NULL_PTR;
+ }
+
+ if (jpegr_info == nullptr) {
+ ALOGE("received nullptr for compressed jpegr info struct");
return ERROR_JPEGR_INVALID_NULL_PTR;
}
diff --git a/services/surfaceflinger/BackgroundExecutor.cpp b/services/surfaceflinger/BackgroundExecutor.cpp
index a15de2b..6ddf790 100644
--- a/services/surfaceflinger/BackgroundExecutor.cpp
+++ b/services/surfaceflinger/BackgroundExecutor.cpp
@@ -28,29 +28,19 @@
ANDROID_SINGLETON_STATIC_INSTANCE(BackgroundExecutor);
BackgroundExecutor::BackgroundExecutor() : Singleton<BackgroundExecutor>() {
+ // mSemaphore must be initialized before any calls to
+ // BackgroundExecutor::sendCallbacks. For this reason, we initialize it
+ // within the constructor instead of within mThread.
+ LOG_ALWAYS_FATAL_IF(sem_init(&mSemaphore, 0, 0), "sem_init failed");
mThread = std::thread([&]() {
- LOG_ALWAYS_FATAL_IF(sem_init(&mSemaphore, 0, 0), "sem_init failed");
while (!mDone) {
LOG_ALWAYS_FATAL_IF(sem_wait(&mSemaphore), "sem_wait failed (%d)", errno);
-
- ftl::SmallVector<Work*, 10> workItems;
-
- Work* work = mWorks.pop();
- while (work) {
- workItems.push_back(work);
- work = mWorks.pop();
+ auto callbacks = mCallbacksQueue.pop();
+ if (!callbacks) {
+ continue;
}
-
- // Sequence numbers are guaranteed to be in intended order, as we assume a single
- // producer and single consumer.
- std::stable_sort(workItems.begin(), workItems.end(), [](Work* left, Work* right) {
- return left->sequence < right->sequence;
- });
- for (Work* work : workItems) {
- for (auto& task : work->tasks) {
- task();
- }
- delete work;
+ for (auto& callback : *callbacks) {
+ callback();
}
}
});
@@ -66,12 +56,8 @@
}
void BackgroundExecutor::sendCallbacks(Callbacks&& tasks) {
- Work* work = new Work();
- work->sequence = mSequence;
- work->tasks = std::move(tasks);
- mWorks.push(work);
- mSequence++;
+ mCallbacksQueue.push(std::move(tasks));
LOG_ALWAYS_FATAL_IF(sem_post(&mSemaphore), "sem_post failed");
}
-} // namespace android
\ No newline at end of file
+} // namespace android
diff --git a/services/surfaceflinger/BackgroundExecutor.h b/services/surfaceflinger/BackgroundExecutor.h
index eeaf3bd..0fae5a5 100644
--- a/services/surfaceflinger/BackgroundExecutor.h
+++ b/services/surfaceflinger/BackgroundExecutor.h
@@ -16,15 +16,13 @@
#pragma once
-#include <Tracing/LocklessStack.h>
-#include <android-base/thread_annotations.h>
#include <ftl/small_vector.h>
#include <semaphore.h>
#include <utils/Singleton.h>
-#include <mutex>
-#include <queue>
#include <thread>
+#include "LocklessQueue.h"
+
namespace android {
// Executes tasks off the main thread.
@@ -34,24 +32,14 @@
~BackgroundExecutor();
using Callbacks = ftl::SmallVector<std::function<void()>, 10>;
// Queues callbacks onto a work queue to be executed by a background thread.
- // Note that this is not thread-safe - a single producer is assumed.
+ // This is safe to call from multiple threads.
void sendCallbacks(Callbacks&& tasks);
private:
sem_t mSemaphore;
std::atomic_bool mDone = false;
- // Sequence number for work items.
- // Work items are batched by sequence number. Work items for earlier sequence numbers are
- // executed first. Work items with the same sequence number are executed in the same order they
- // were added to the stack (meaning the stack must reverse the order after popping from the
- // queue)
- int32_t mSequence = 0;
- struct Work {
- int32_t sequence = 0;
- Callbacks tasks;
- };
- LocklessStack<Work> mWorks;
+ LocklessQueue<Callbacks> mCallbacksQueue;
std::thread mThread;
};
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 0612f5a..b1d4b3c 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -7049,9 +7049,9 @@
}
RenderAreaFuture renderAreaFuture = ftl::defer([=] {
- return DisplayRenderArea::create(displayWeak, args.sourceCrop, reqSize,
- ui::Dataspace::UNKNOWN, args.useIdentityTransform,
- args.hintForSeamlessTransition, args.captureSecureLayers);
+ return DisplayRenderArea::create(displayWeak, args.sourceCrop, reqSize, args.dataspace,
+ args.useIdentityTransform, args.hintForSeamlessTransition,
+ args.captureSecureLayers);
});
GetLayerSnapshotsFunction getLayerSnapshots;
diff --git a/services/surfaceflinger/tests/unittests/Android.bp b/services/surfaceflinger/tests/unittests/Android.bp
index 70f8a83..881b362 100644
--- a/services/surfaceflinger/tests/unittests/Android.bp
+++ b/services/surfaceflinger/tests/unittests/Android.bp
@@ -71,6 +71,7 @@
":libsurfaceflinger_sources",
"libsurfaceflinger_unittest_main.cpp",
"ActiveDisplayRotationFlagsTest.cpp",
+ "BackgroundExecutorTest.cpp",
"CompositionTest.cpp",
"DisplayIdGeneratorTest.cpp",
"DisplayTransactionTest.cpp",
diff --git a/services/surfaceflinger/tests/unittests/BackgroundExecutorTest.cpp b/services/surfaceflinger/tests/unittests/BackgroundExecutorTest.cpp
new file mode 100644
index 0000000..5413bae
--- /dev/null
+++ b/services/surfaceflinger/tests/unittests/BackgroundExecutorTest.cpp
@@ -0,0 +1,57 @@
+#include <gtest/gtest.h>
+#include <condition_variable>
+
+#include "BackgroundExecutor.h"
+
+namespace android {
+
+class BackgroundExecutorTest : public testing::Test {};
+
+namespace {
+
+TEST_F(BackgroundExecutorTest, singleProducer) {
+ std::mutex mutex;
+ std::condition_variable condition_variable;
+ bool backgroundTaskComplete = false;
+
+ BackgroundExecutor::getInstance().sendCallbacks(
+ {[&mutex, &condition_variable, &backgroundTaskComplete]() {
+ std::lock_guard<std::mutex> lock{mutex};
+ condition_variable.notify_one();
+ backgroundTaskComplete = true;
+ }});
+
+ std::unique_lock<std::mutex> lock{mutex};
+ condition_variable.wait(lock, [&backgroundTaskComplete]() { return backgroundTaskComplete; });
+ ASSERT_TRUE(backgroundTaskComplete);
+}
+
+TEST_F(BackgroundExecutorTest, multipleProducers) {
+ std::mutex mutex;
+ std::condition_variable condition_variable;
+ const int backgroundTaskCount = 10;
+ int backgroundTaskCompleteCount = 0;
+
+ for (int i = 0; i < backgroundTaskCount; i++) {
+ std::thread([&mutex, &condition_variable, &backgroundTaskCompleteCount]() {
+ BackgroundExecutor::getInstance().sendCallbacks(
+ {[&mutex, &condition_variable, &backgroundTaskCompleteCount]() {
+ std::lock_guard<std::mutex> lock{mutex};
+ backgroundTaskCompleteCount++;
+ if (backgroundTaskCompleteCount == backgroundTaskCount) {
+ condition_variable.notify_one();
+ }
+ }});
+ }).detach();
+ }
+
+ std::unique_lock<std::mutex> lock{mutex};
+ condition_variable.wait(lock, [&backgroundTaskCompleteCount]() {
+ return backgroundTaskCompleteCount == backgroundTaskCount;
+ });
+ ASSERT_EQ(backgroundTaskCount, backgroundTaskCompleteCount);
+}
+
+} // namespace
+
+} // namespace android