From 41d8343603225bed1a6cc016c2ba83f337d3b0e1 Mon Sep 17 00:00:00 2001 From: Wonsik Kim Date: Mon, 27 Apr 2020 16:40:49 -0700 Subject: [PATCH] CCodec: remove dependency to # of slots for allocating local buffers In addition, a little build tweak for the test. Bug: 146168540 Test: atest ccodec_unit_test:RawGraphicOutputBuffersTest Change-Id: Ia1a0969e87ca1729bb38cf5cf1c7ec8df3029bc7 --- media/codec2/sfplugin/CCodecBufferChannel.cpp | 5 +- media/codec2/sfplugin/CCodecBuffers.cpp | 30 ++++-- media/codec2/sfplugin/CCodecBuffers.h | 17 ++-- media/codec2/sfplugin/Codec2Buffer.cpp | 3 + media/codec2/sfplugin/Codec2Buffer.h | 19 +++- media/codec2/sfplugin/tests/Android.bp | 13 ++- .../sfplugin/tests/CCodecBuffers_test.cpp | 99 +++++++++++++++++++ 7 files changed, 161 insertions(+), 25 deletions(-) create mode 100644 media/codec2/sfplugin/tests/CCodecBuffers_test.cpp diff --git a/media/codec2/sfplugin/CCodecBufferChannel.cpp b/media/codec2/sfplugin/CCodecBufferChannel.cpp index e2be9911a2..5fe7a9c051 100644 --- a/media/codec2/sfplugin/CCodecBufferChannel.cpp +++ b/media/codec2/sfplugin/CCodecBufferChannel.cpp @@ -44,6 +44,7 @@ #include #include #include +#include #include #include "CCodecBufferChannel.h" @@ -982,7 +983,7 @@ status_t CCodecBufferChannel::start( // TODO: handle this without going into array mode forceArrayMode = true; } else { - input->buffers.reset(new GraphicInputBuffers(numInputSlots, mName)); + input->buffers.reset(new GraphicInputBuffers(mName)); } } else { if (hasCryptoOrDescrambler()) { @@ -1150,7 +1151,7 @@ status_t CCodecBufferChannel::start( if (outputSurface || !buffersBoundToCodec) { output->buffers.reset(new GraphicOutputBuffers(mName)); } else { - output->buffers.reset(new RawGraphicOutputBuffers(numOutputSlots, mName)); + output->buffers.reset(new RawGraphicOutputBuffers(mName)); } } else { output->buffers.reset(new LinearOutputBuffers(mName)); diff --git a/media/codec2/sfplugin/CCodecBuffers.cpp b/media/codec2/sfplugin/CCodecBuffers.cpp index 4ce13aabe9..1af6b3ed97 100644 --- a/media/codec2/sfplugin/CCodecBuffers.cpp +++ b/media/codec2/sfplugin/CCodecBuffers.cpp @@ -24,6 +24,7 @@ #include #include #include +#include #include "CCodecBuffers.h" @@ -122,6 +123,11 @@ sp InputBuffers::cloneAndReleaseBuffer(const sp // OutputBuffers +OutputBuffers::OutputBuffers(const char *componentName, const char *name) + : CCodecBuffers(componentName, name) { } + +OutputBuffers::~OutputBuffers() = default; + void OutputBuffers::initSkipCutBuffer( int32_t delay, int32_t padding, int32_t sampleRate, int32_t channelCount) { CHECK(mSkipCutBuffer == nullptr); @@ -354,8 +360,11 @@ bool OutputBuffers::less( // LocalBufferPool -std::shared_ptr LocalBufferPool::Create(size_t poolCapacity) { - return std::shared_ptr(new LocalBufferPool(poolCapacity)); +constexpr size_t kInitialPoolCapacity = kMaxLinearBufferSize; +constexpr size_t kMaxPoolCapacity = kMaxLinearBufferSize * 32; + +std::shared_ptr LocalBufferPool::Create() { + return std::shared_ptr(new LocalBufferPool(kInitialPoolCapacity)); } sp LocalBufferPool::newBuffer(size_t capacity) { @@ -375,6 +384,11 @@ sp LocalBufferPool::newBuffer(size_t capacity) { mUsedSize -= mPool.back().capacity(); mPool.pop_back(); } + while (mUsedSize + capacity > mPoolCapacity && mPoolCapacity * 2 <= kMaxPoolCapacity) { + ALOGD("Increasing local buffer pool capacity from %zu to %zu", + mPoolCapacity, mPoolCapacity * 2); + mPoolCapacity *= 2; + } if (mUsedSize + capacity > mPoolCapacity) { ALOGD("mUsedSize = %zu, capacity = %zu, mPoolCapacity = %zu", mUsedSize, capacity, mPoolCapacity); @@ -960,11 +974,10 @@ sp GraphicMetadataInputBuffers::createNewBuffer() { // GraphicInputBuffers GraphicInputBuffers::GraphicInputBuffers( - size_t numInputSlots, const char *componentName, const char *name) + const char *componentName, const char *name) : InputBuffers(componentName, name), mImpl(mName), - mLocalBufferPool(LocalBufferPool::Create( - kMaxLinearBufferSize * numInputSlots)) { } + mLocalBufferPool(LocalBufferPool::Create()) { } bool GraphicInputBuffers::requestNewBuffer(size_t *index, sp *buffer) { sp newBuffer = createNewBuffer(); @@ -1125,7 +1138,7 @@ void OutputBuffersArray::realloc(const std::shared_ptr &c2buffer) { case C2BufferData::GRAPHIC: { // This is only called for RawGraphicOutputBuffers. mAlloc = [format = mFormat, - lbp = LocalBufferPool::Create(kMaxLinearBufferSize * mImpl.arraySize())] { + lbp = LocalBufferPool::Create()] { return ConstGraphicBlockBuffer::AllocateEmpty( format, [lbp](size_t capacity) { @@ -1271,10 +1284,9 @@ std::function()> GraphicOutputBuffers::getAlloc() { // RawGraphicOutputBuffers RawGraphicOutputBuffers::RawGraphicOutputBuffers( - size_t numOutputSlots, const char *componentName, const char *name) + const char *componentName, const char *name) : FlexOutputBuffers(componentName, name), - mLocalBufferPool(LocalBufferPool::Create( - kMaxLinearBufferSize * numOutputSlots)) { } + mLocalBufferPool(LocalBufferPool::Create()) { } sp RawGraphicOutputBuffers::wrap(const std::shared_ptr &buffer) { if (buffer == nullptr) { diff --git a/media/codec2/sfplugin/CCodecBuffers.h b/media/codec2/sfplugin/CCodecBuffers.h index cadc4d8a65..b863a45c28 100644 --- a/media/codec2/sfplugin/CCodecBuffers.h +++ b/media/codec2/sfplugin/CCodecBuffers.h @@ -28,6 +28,8 @@ namespace android { +struct ICrypto; +class MemoryDealer; class SkipCutBuffer; constexpr size_t kLinearBufferSize = 1048576; @@ -158,9 +160,8 @@ class OutputBuffersArray; class OutputBuffers : public CCodecBuffers { public: - OutputBuffers(const char *componentName, const char *name = "Output") - : CCodecBuffers(componentName, name) { } - virtual ~OutputBuffers() = default; + OutputBuffers(const char *componentName, const char *name = "Output"); + virtual ~OutputBuffers(); /** * Register output C2Buffer from the component and obtain corresponding @@ -471,11 +472,9 @@ public: /** * Create a new LocalBufferPool object. * - * \param poolCapacity max total size of buffers managed by this pool. - * * \return a newly created pool object. */ - static std::shared_ptr Create(size_t poolCapacity); + static std::shared_ptr Create(); /** * Return an ABuffer object whose size is at least |capacity|. @@ -907,8 +906,7 @@ private: class GraphicInputBuffers : public InputBuffers { public: - GraphicInputBuffers( - size_t numInputSlots, const char *componentName, const char *name = "2D-BB-Input"); + GraphicInputBuffers(const char *componentName, const char *name = "2D-BB-Input"); ~GraphicInputBuffers() override = default; bool requestNewBuffer(size_t *index, sp *buffer) override; @@ -1126,8 +1124,7 @@ public: class RawGraphicOutputBuffers : public FlexOutputBuffers { public: - RawGraphicOutputBuffers( - size_t numOutputSlots, const char *componentName, const char *name = "2D-BB-Output"); + RawGraphicOutputBuffers(const char *componentName, const char *name = "2D-BB-Output"); ~RawGraphicOutputBuffers() override = default; sp wrap(const std::shared_ptr &buffer) override; diff --git a/media/codec2/sfplugin/Codec2Buffer.cpp b/media/codec2/sfplugin/Codec2Buffer.cpp index 5b3a62f37a..25e7da9206 100644 --- a/media/codec2/sfplugin/Codec2Buffer.cpp +++ b/media/codec2/sfplugin/Codec2Buffer.cpp @@ -18,6 +18,8 @@ #define LOG_TAG "Codec2Buffer" #include +#include +#include #include #include #include @@ -25,6 +27,7 @@ #include #include #include +#include #include #include diff --git a/media/codec2/sfplugin/Codec2Buffer.h b/media/codec2/sfplugin/Codec2Buffer.h index ff79946524..dc788cd87e 100644 --- a/media/codec2/sfplugin/Codec2Buffer.h +++ b/media/codec2/sfplugin/Codec2Buffer.h @@ -20,16 +20,29 @@ #include -#include -#include #include #include #include #include -#include namespace android { +namespace hardware { +class HidlMemory; +namespace cas { +namespace native { +namespace V1_0 { +struct SharedBuffer; +} // namespace V1_0 +} // namespace native +} // namespace cas +namespace drm { +namespace V1_0 { +struct SharedBuffer; +} // namespace V1_0 +} // namespace drm +} // namespace hardware + /** * Copies a graphic view into a media image. * diff --git a/media/codec2/sfplugin/tests/Android.bp b/media/codec2/sfplugin/tests/Android.bp index fe5fa682de..8d1a9c313a 100644 --- a/media/codec2/sfplugin/tests/Android.bp +++ b/media/codec2/sfplugin/tests/Android.bp @@ -2,12 +2,13 @@ cc_test { name: "ccodec_unit_test", srcs: [ + "CCodecBuffers_test.cpp", "CCodecConfig_test.cpp", "ReflectedParamUpdater_test.cpp", ], defaults: [ - "libcodec2-hidl-defaults@1.0", + "libcodec2-impl-defaults", "libcodec2-internal-defaults", ], @@ -16,14 +17,24 @@ cc_test { ], shared_libs: [ + "android.hardware.media.bufferpool@2.0", + "android.hardware.media.c2@1.0", "libcodec2", "libcodec2_client", + "libhidlbase", + "libfmq", + "libmedia_omx", "libsfplugin_ccodec", "libsfplugin_ccodec_utils", "libstagefright_foundation", "libutils", ], + static_libs: [ + "libcodec2_hidl@1.0", + "libstagefright_bufferpool@2.0", + ], + cflags: [ "-Werror", "-Wall", diff --git a/media/codec2/sfplugin/tests/CCodecBuffers_test.cpp b/media/codec2/sfplugin/tests/CCodecBuffers_test.cpp new file mode 100644 index 0000000000..5bee605276 --- /dev/null +++ b/media/codec2/sfplugin/tests/CCodecBuffers_test.cpp @@ -0,0 +1,99 @@ +/* + * Copyright 2020 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "CCodecBuffers.h" + +#include + +#include + +#include + +namespace android { + +TEST(RawGraphicOutputBuffersTest, ChangeNumSlots) { + constexpr int32_t kWidth = 3840; + constexpr int32_t kHeight = 2160; + + std::shared_ptr buffers = + std::make_shared("test"); + sp format{new AMessage}; + format->setInt32("width", kWidth); + format->setInt32("height", kHeight); + buffers->setFormat(format); + + std::shared_ptr pool; + ASSERT_EQ(OK, GetCodec2BlockPool(C2BlockPool::BASIC_GRAPHIC, nullptr, &pool)); + + // Register 4 buffers + std::vector> clientBuffers; + auto registerBuffer = [&buffers, &clientBuffers, &pool] { + std::shared_ptr block; + ASSERT_EQ(OK, pool->fetchGraphicBlock( + kWidth, kHeight, HAL_PIXEL_FORMAT_YCbCr_420_888, + C2MemoryUsage{C2MemoryUsage::CPU_READ, C2MemoryUsage::CPU_WRITE}, &block)); + std::shared_ptr c2Buffer = C2Buffer::CreateGraphicBuffer(block->share( + block->crop(), C2Fence{})); + size_t index; + sp clientBuffer; + ASSERT_EQ(OK, buffers->registerBuffer(c2Buffer, &index, &clientBuffer)); + ASSERT_NE(nullptr, clientBuffer); + while (clientBuffers.size() <= index) { + clientBuffers.emplace_back(); + } + ASSERT_EQ(nullptr, clientBuffers[index]) << "index = " << index; + clientBuffers[index] = clientBuffer; + }; + for (int i = 0; i < 4; ++i) { + registerBuffer(); + } + + // Release 2 buffers + auto releaseBuffer = [&buffers, &clientBuffers, kWidth, kHeight](int index) { + std::shared_ptr c2Buffer; + ASSERT_TRUE(buffers->releaseBuffer(clientBuffers[index], &c2Buffer)) + << "index = " << index; + clientBuffers[index] = nullptr; + // Sanity checks + ASSERT_TRUE(c2Buffer->data().linearBlocks().empty()); + ASSERT_EQ(1u, c2Buffer->data().graphicBlocks().size()); + C2ConstGraphicBlock block = c2Buffer->data().graphicBlocks().front(); + ASSERT_EQ(kWidth, block.width()); + ASSERT_EQ(kHeight, block.height()); + }; + for (int i = 0, index = 0; i < 2 && index < clientBuffers.size(); ++index) { + if (clientBuffers[index] == nullptr) { + continue; + } + releaseBuffer(index); + ++i; + } + + // Simulate # of slots 4->16 + for (int i = 2; i < 16; ++i) { + registerBuffer(); + } + + // Release everything + for (int index = 0; index < clientBuffers.size(); ++index) { + if (clientBuffers[index] == nullptr) { + continue; + } + releaseBuffer(index); + } +} + +} // namespace android