From 26c7997dea97345a81bcabd21fa07050c8ff0a57 Mon Sep 17 00:00:00 2001 From: Yin-Chia Yeh Date: Tue, 25 Jun 2019 10:53:03 -0700 Subject: [PATCH 01/34] Camera: listen to provider instance names from hwservicemanager Also remove the expected argument in addProviderLocked method as we now can query from device manifest for instance names. Test: manually modify Pixel3 manifest to test, CTS Bug: 136010319 Merged-In: Ib57fd84ad8f22aac2a82920e03148cff2592daae Change-Id: Ib57fd84ad8f22aac2a82920e03148cff2592daae --- .../common/CameraProviderManager.cpp | 37 +++++++++++-------- .../common/CameraProviderManager.h | 5 ++- .../tests/CameraProviderManagerTest.cpp | 34 +++++++---------- 3 files changed, 39 insertions(+), 37 deletions(-) diff --git a/services/camera/libcameraservice/common/CameraProviderManager.cpp b/services/camera/libcameraservice/common/CameraProviderManager.cpp index 98f93286a6..c72029ff5a 100644 --- a/services/camera/libcameraservice/common/CameraProviderManager.cpp +++ b/services/camera/libcameraservice/common/CameraProviderManager.cpp @@ -29,6 +29,7 @@ #include #include #include +#include #include #include #include @@ -47,10 +48,6 @@ using namespace ::android::hardware::camera::common::V1_0; using std::literals::chrono_literals::operator""s; namespace { -// Hardcoded name for the passthrough HAL implementation, since it can't be discovered via the -// service manager -const std::string kLegacyProviderName("legacy/0"); -const std::string kExternalProviderName("external/0"); const bool kEnableLazyHal(property_get_bool("ro.camera.enableLazyHal", false)); } // anonymous namespace @@ -62,6 +59,19 @@ CameraProviderManager::sHardwareServiceInteractionProxy{}; CameraProviderManager::~CameraProviderManager() { } +hardware::hidl_vec +CameraProviderManager::HardwareServiceInteractionProxy::listServices() { + hardware::hidl_vec ret; + auto manager = hardware::defaultServiceManager1_2(); + if (manager != nullptr) { + manager->listManifestByInterface(provider::V2_4::ICameraProvider::descriptor, + [&ret](const hardware::hidl_vec ®istered) { + ret = registered; + }); + } + return ret; +} + status_t CameraProviderManager::initialize(wp listener, ServiceInteractionProxy* proxy) { std::lock_guard lock(mInterfaceMutex); @@ -84,9 +94,10 @@ status_t CameraProviderManager::initialize(wplistServices()) { + this->addProviderLocked(instance); + } IPCThreadState::self()->flushCommands(); @@ -1087,7 +1098,7 @@ bool CameraProviderManager::isHiddenPhysicalCamera(const std::string& cameraId) return false; } -status_t CameraProviderManager::addProviderLocked(const std::string& newProvider, bool expected) { +status_t CameraProviderManager::addProviderLocked(const std::string& newProvider) { for (const auto& providerInfo : mProviders) { if (providerInfo->mProviderName == newProvider) { ALOGW("%s: Camera provider HAL with name '%s' already registered", __FUNCTION__, @@ -1100,13 +1111,9 @@ status_t CameraProviderManager::addProviderLocked(const std::string& newProvider interface = mServiceProxy->getService(newProvider); if (interface == nullptr) { - if (expected) { - ALOGE("%s: Camera provider HAL '%s' is not actually available", __FUNCTION__, - newProvider.c_str()); - return BAD_VALUE; - } else { - return OK; - } + ALOGE("%s: Camera provider HAL '%s' is not actually available", __FUNCTION__, + newProvider.c_str()); + return BAD_VALUE; } sp providerInfo = new ProviderInfo(newProvider, this); diff --git a/services/camera/libcameraservice/common/CameraProviderManager.h b/services/camera/libcameraservice/common/CameraProviderManager.h index a42fb4d60b..8cdfc24a9c 100644 --- a/services/camera/libcameraservice/common/CameraProviderManager.h +++ b/services/camera/libcameraservice/common/CameraProviderManager.h @@ -78,6 +78,7 @@ public: ¬ification) = 0; virtual sp getService( const std::string &serviceName) = 0; + virtual hardware::hidl_vec listServices() = 0; virtual ~ServiceInteractionProxy() {} }; @@ -95,6 +96,8 @@ public: const std::string &serviceName) override { return hardware::camera::provider::V2_4::ICameraProvider::getService(serviceName); } + + virtual hardware::hidl_vec listServices() override; }; /** @@ -567,7 +570,7 @@ private: hardware::hidl_version minVersion = hardware::hidl_version{0,0}, hardware::hidl_version maxVersion = hardware::hidl_version{1000,0}) const; - status_t addProviderLocked(const std::string& newProvider, bool expected = true); + status_t addProviderLocked(const std::string& newProvider); status_t removeProvider(const std::string& provider); sp getStatusListener() const; diff --git a/services/camera/libcameraservice/tests/CameraProviderManagerTest.cpp b/services/camera/libcameraservice/tests/CameraProviderManagerTest.cpp index f47e5a581e..78d737d1b2 100644 --- a/services/camera/libcameraservice/tests/CameraProviderManagerTest.cpp +++ b/services/camera/libcameraservice/tests/CameraProviderManagerTest.cpp @@ -205,6 +205,11 @@ struct TestInteractionProxy : public CameraProviderManager::ServiceInteractionPr return mTestCameraProvider; } + virtual hardware::hidl_vec listServices() override { + hardware::hidl_vec ret = {"test/0"}; + return ret; + } + }; struct TestStatusListener : public CameraProviderManager::StatusListener { @@ -231,37 +236,24 @@ TEST(CameraProviderManagerTest, InitializeTest) { vendorSection); serviceProxy.setProvider(provider); + int numProviders = static_cast(serviceProxy.listServices().size()); + res = providerManager->initialize(statusListener, &serviceProxy); ASSERT_EQ(res, OK) << "Unable to initialize provider manager"; // Check that both "legacy" and "external" providers (really the same object) are called // once for all the init methods - EXPECT_EQ(provider->mCalledCounter[TestICameraProvider::SET_CALLBACK], 2) << + EXPECT_EQ(provider->mCalledCounter[TestICameraProvider::SET_CALLBACK], numProviders) << "Only one call to setCallback per provider expected during init"; - EXPECT_EQ(provider->mCalledCounter[TestICameraProvider::GET_VENDOR_TAGS], 2) << + EXPECT_EQ(provider->mCalledCounter[TestICameraProvider::GET_VENDOR_TAGS], numProviders) << "Only one call to getVendorTags per provider expected during init"; - EXPECT_EQ(provider->mCalledCounter[TestICameraProvider::IS_SET_TORCH_MODE_SUPPORTED], 2) << + EXPECT_EQ(provider->mCalledCounter[TestICameraProvider::IS_SET_TORCH_MODE_SUPPORTED], + numProviders) << "Only one call to isSetTorchModeSupported per provider expected during init"; - EXPECT_EQ(provider->mCalledCounter[TestICameraProvider::GET_CAMERA_ID_LIST], 2) << + EXPECT_EQ(provider->mCalledCounter[TestICameraProvider::GET_CAMERA_ID_LIST], numProviders) << "Only one call to getCameraIdList per provider expected during init"; - EXPECT_EQ(provider->mCalledCounter[TestICameraProvider::NOTIFY_DEVICE_STATE], 2) << + EXPECT_EQ(provider->mCalledCounter[TestICameraProvider::NOTIFY_DEVICE_STATE], numProviders) << "Only one call to notifyDeviceState per provider expected during init"; - std::string legacyInstanceName = "legacy/0"; - std::string externalInstanceName = "external/0"; - bool gotLegacy = false; - bool gotExternal = false; - EXPECT_EQ(2u, serviceProxy.mLastRequestedServiceNames.size()) << - "Only two service queries expected to be seen by hardware service manager"; - - for (auto& serviceName : serviceProxy.mLastRequestedServiceNames) { - if (serviceName == legacyInstanceName) gotLegacy = true; - if (serviceName == externalInstanceName) gotExternal = true; - } - ASSERT_TRUE(gotLegacy) << - "Legacy instance not requested from service manager"; - ASSERT_TRUE(gotExternal) << - "External instance not requested from service manager"; - hardware::hidl_string testProviderFqInterfaceName = "android.hardware.camera.provider@2.4::ICameraProvider"; hardware::hidl_string testProviderInstanceName = "test/0"; From 03ffd0acf77309f0d7fa4cd07d388516727fe6e4 Mon Sep 17 00:00:00 2001 From: Harish Mahendrakar Date: Wed, 31 Jul 2019 13:28:03 -0700 Subject: [PATCH 02/34] SoftVorbisDec: Fix isConfigured() in vorbis decoder plugin isConfigured() in soft vorbis decoder omx plugin was using mInputBufferCount to detect if the decoder was configured. But mInputBufferCount was reset to 0 after flush. This was prone to a race condition, when flush was called immediately after start(), before codec could signal correct number of channels and sampling rate. isConfigured() is now fixed to check decoder state rather than mInputBufferCount. Bug: 137736256 Bug: 138213973 Test: atest android.media.cts.DecoderTest#testDecodeWithEOSOnLastBuffer Change-Id: Ia1e6b88eca45b46fe20f36627f79cd767b73cc1a --- media/libstagefright/codecs/vorbis/dec/SoftVorbis.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/media/libstagefright/codecs/vorbis/dec/SoftVorbis.cpp b/media/libstagefright/codecs/vorbis/dec/SoftVorbis.cpp index 76a400c0d8..1293a74ddf 100644 --- a/media/libstagefright/codecs/vorbis/dec/SoftVorbis.cpp +++ b/media/libstagefright/codecs/vorbis/dec/SoftVorbis.cpp @@ -290,7 +290,7 @@ OMX_ERRORTYPE SoftVorbis::internalSetParameter( } bool SoftVorbis::isConfigured() const { - return mInputBufferCount >= 2; + return (mState != NULL && mVi != NULL); } static void makeBitReader( From 47897d5ff923f830bfc322a87c2f48670d35571c Mon Sep 17 00:00:00 2001 From: Sungtak Lee Date: Thu, 25 Jul 2019 14:22:36 -0700 Subject: [PATCH 03/34] bufferpool2: fix native handle race condition Return cloned handle as output parameter in order to use it after closing the bufferpool connection. Bug: 138171841 Test: atest CtsMediaTestCases -- --module-arg CtsMediaTestCases:size:small Merged-In: I4ac142fcb7e3ecdb3fb02792f516b343d71bfc38 Change-Id: I4ac142fcb7e3ecdb3fb02792f516b343d71bfc38 (cherry picked from commit dc5cb62ff4528ac52a15fb525874ab47e377e8ac) --- media/bufferpool/2.0/ClientManager.cpp | 25 +++++- .../2.0/include/bufferpool/ClientManager.h | 8 +- media/codec2/hidl/1.0/utils/types.cpp | 5 ++ media/codec2/vndk/C2Buffer.cpp | 80 ++++++++----------- 4 files changed, 68 insertions(+), 50 deletions(-) diff --git a/media/bufferpool/2.0/ClientManager.cpp b/media/bufferpool/2.0/ClientManager.cpp index c31d313ab1..48c2da4be4 100644 --- a/media/bufferpool/2.0/ClientManager.cpp +++ b/media/bufferpool/2.0/ClientManager.cpp @@ -351,7 +351,17 @@ ResultStatus ClientManager::Impl::allocate( } client = it->second; } - return client->allocate(params, handle, buffer); + native_handle_t *origHandle; + ResultStatus res = client->allocate(params, &origHandle, buffer); + if (res != ResultStatus::OK) { + return res; + } + *handle = native_handle_clone(origHandle); + if (handle == NULL) { + buffer->reset(); + return ResultStatus::NO_MEMORY; + } + return ResultStatus::OK; } ResultStatus ClientManager::Impl::receive( @@ -367,7 +377,18 @@ ResultStatus ClientManager::Impl::receive( } client = it->second; } - return client->receive(transactionId, bufferId, timestampUs, handle, buffer); + native_handle_t *origHandle; + ResultStatus res = client->receive( + transactionId, bufferId, timestampUs, &origHandle, buffer); + if (res != ResultStatus::OK) { + return res; + } + *handle = native_handle_clone(origHandle); + if (handle == NULL) { + buffer->reset(); + return ResultStatus::NO_MEMORY; + } + return ResultStatus::OK; } ResultStatus ClientManager::Impl::postSend( diff --git a/media/bufferpool/2.0/include/bufferpool/ClientManager.h b/media/bufferpool/2.0/include/bufferpool/ClientManager.h index 953c3049a4..24b61f4fb5 100644 --- a/media/bufferpool/2.0/include/bufferpool/ClientManager.h +++ b/media/bufferpool/2.0/include/bufferpool/ClientManager.h @@ -104,7 +104,9 @@ struct ClientManager : public IClientManager { ResultStatus flush(ConnectionId connectionId); /** - * Allocates a buffer from the specified connection. + * Allocates a buffer from the specified connection. The output parameter + * handle is cloned from the internal handle. So it is safe to use directly, + * and it should be deleted and destroyed after use. * * @param connectionId The id of the connection. * @param params The allocation parameters. @@ -123,7 +125,9 @@ struct ClientManager : public IClientManager { std::shared_ptr *buffer); /** - * Receives a buffer for the transaction. + * Receives a buffer for the transaction. The output parameter handle is + * cloned from the internal handle. So it is safe to use directly, and it + * should be deleted and destoyed after use. * * @param connectionId The id of the receiving connection. * @param transactionId The id for the transaction. diff --git a/media/codec2/hidl/1.0/utils/types.cpp b/media/codec2/hidl/1.0/utils/types.cpp index 07dbf6777f..04fa59ccd0 100644 --- a/media/codec2/hidl/1.0/utils/types.cpp +++ b/media/codec2/hidl/1.0/utils/types.cpp @@ -1434,6 +1434,11 @@ bool objcpy(C2BaseBlock* d, const BaseBlock& s) { d->type = C2BaseBlock::GRAPHIC; return true; } + if (cHandle) { + // Though we got cloned handle, creating block failed. + native_handle_close(cHandle); + native_handle_delete(cHandle); + } LOG(ERROR) << "Unknown handle type in BaseBlock::pooledBlock."; return false; diff --git a/media/codec2/vndk/C2Buffer.cpp b/media/codec2/vndk/C2Buffer.cpp index 710b536ef7..2d99b53936 100644 --- a/media/codec2/vndk/C2Buffer.cpp +++ b/media/codec2/vndk/C2Buffer.cpp @@ -413,17 +413,14 @@ std::shared_ptr _C2BlockFactory::CreateLinearBlock( std::shared_ptr alloc; if (C2AllocatorIon::isValid(cHandle)) { - native_handle_t *handle = native_handle_clone(cHandle); - if (handle) { - c2_status_t err = sAllocator->priorLinearAllocation(handle, &alloc); - const std::shared_ptr poolData = - std::make_shared(data); - if (err == C2_OK && poolData) { - // TODO: config params? - std::shared_ptr block = - _C2BlockFactory::CreateLinearBlock(alloc, poolData); - return block; - } + c2_status_t err = sAllocator->priorLinearAllocation(cHandle, &alloc); + const std::shared_ptr poolData = + std::make_shared(data); + if (err == C2_OK && poolData) { + // TODO: config params? + std::shared_ptr block = + _C2BlockFactory::CreateLinearBlock(alloc, poolData); + return block; } } return nullptr; @@ -674,17 +671,14 @@ public: ResultStatus status = mBufferPoolManager->allocate( mConnectionId, params, &cHandle, &bufferPoolData); if (status == ResultStatus::OK) { - native_handle_t *handle = native_handle_clone(cHandle); - if (handle) { - std::shared_ptr alloc; - std::shared_ptr poolData = - std::make_shared(bufferPoolData); - c2_status_t err = mAllocator->priorLinearAllocation(handle, &alloc); - if (err == C2_OK && poolData && alloc) { - *block = _C2BlockFactory::CreateLinearBlock(alloc, poolData, 0, capacity); - if (*block) { - return C2_OK; - } + std::shared_ptr alloc; + std::shared_ptr poolData = + std::make_shared(bufferPoolData); + c2_status_t err = mAllocator->priorLinearAllocation(cHandle, &alloc); + if (err == C2_OK && poolData && alloc) { + *block = _C2BlockFactory::CreateLinearBlock(alloc, poolData, 0, capacity); + if (*block) { + return C2_OK; } } return C2_NO_MEMORY; @@ -710,19 +704,16 @@ public: ResultStatus status = mBufferPoolManager->allocate( mConnectionId, params, &cHandle, &bufferPoolData); if (status == ResultStatus::OK) { - native_handle_t *handle = native_handle_clone(cHandle); - if (handle) { - std::shared_ptr alloc; - std::shared_ptr poolData = - std::make_shared(bufferPoolData); - c2_status_t err = mAllocator->priorGraphicAllocation( - handle, &alloc); - if (err == C2_OK && poolData && alloc) { - *block = _C2BlockFactory::CreateGraphicBlock( - alloc, poolData, C2Rect(width, height)); - if (*block) { - return C2_OK; - } + std::shared_ptr alloc; + std::shared_ptr poolData = + std::make_shared(bufferPoolData); + c2_status_t err = mAllocator->priorGraphicAllocation( + cHandle, &alloc); + if (err == C2_OK && poolData && alloc) { + *block = _C2BlockFactory::CreateGraphicBlock( + alloc, poolData, C2Rect(width, height)); + if (*block) { + return C2_OK; } } return C2_NO_MEMORY; @@ -1117,17 +1108,14 @@ std::shared_ptr _C2BlockFactory::CreateGraphicBlock( std::shared_ptr alloc; if (C2AllocatorGralloc::isValid(cHandle)) { - native_handle_t *handle = native_handle_clone(cHandle); - if (handle) { - c2_status_t err = sAllocator->priorGraphicAllocation(handle, &alloc); - const std::shared_ptr poolData = - std::make_shared(data); - if (err == C2_OK && poolData) { - // TODO: config setup? - std::shared_ptr block = - _C2BlockFactory::CreateGraphicBlock(alloc, poolData); - return block; - } + c2_status_t err = sAllocator->priorGraphicAllocation(cHandle, &alloc); + const std::shared_ptr poolData = + std::make_shared(data); + if (err == C2_OK && poolData) { + // TODO: config setup? + std::shared_ptr block = + _C2BlockFactory::CreateGraphicBlock(alloc, poolData); + return block; } } return nullptr; From ee33d64bf82f3781ab826863f69d089d0a7400da Mon Sep 17 00:00:00 2001 From: Chong Zhang Date: Thu, 8 Aug 2019 14:26:43 -0700 Subject: [PATCH 04/34] Report video battery on/off from mediaserver We can't track video stats accurately if it's reported from app side MediaCodec. If the app dies, video stats get stuck in On state forever. This can be easily triggered by forcefully kill and app that uses MediaCodec from app side (instead of through mediaserver's Recorder or Player service), eg. launch GoogleCamera app and switch to Video tab, and kill it from adb shell. In order to track MediaCodec usage from apps we need to move the battery stats reporting from MediaCodec into ResourceManager. bug: 138381810 test: 1. test if app uses MediaCodec directly and it dies off, the video off is received: launch GoogleCamera app, swipe to Video tab, "dumpsys batterystats --history" and see +video; now adb shell kill GoogleCamera, dumpsys should show -video. 2. test app that uses MediaCodec through mediaserver: use Chrome (which uses MediaPlayer) to play some website with video, kills Chrome from adb shell, and check -video is received. In anycase it shouldn't stuck in On state. 3. ResourceManagerService_test Change-Id: I164b31681c4e72e8cce02342641dbec14b8df374 (cherry picked from commit 47db2ff19c3ceb2d3b39ff9315a463984e55dec1) --- media/libmedia/IResourceManagerService.cpp | 5 ++- .../include/media/IResourceManagerService.h | 1 + media/libmedia/include/media/MediaResource.h | 3 +- media/libstagefright/MediaCodec.cpp | 33 +++++++------------ .../include/media/stagefright/MediaCodec.h | 4 +-- .../ResourceManagerService.cpp | 24 +++++++++++--- .../ResourceManagerService.h | 3 ++ .../test/ResourceManagerService_test.cpp | 24 +++++++++----- 8 files changed, 58 insertions(+), 39 deletions(-) diff --git a/media/libmedia/IResourceManagerService.cpp b/media/libmedia/IResourceManagerService.cpp index 9724fc1a12..00f9b88aad 100644 --- a/media/libmedia/IResourceManagerService.cpp +++ b/media/libmedia/IResourceManagerService.cpp @@ -72,12 +72,14 @@ public: virtual void addResource( int pid, + int uid, int64_t clientId, const sp client, const Vector &resources) { Parcel data, reply; data.writeInterfaceToken(IResourceManagerService::getInterfaceDescriptor()); data.writeInt32(pid); + data.writeInt32(uid); data.writeInt64(clientId); data.writeStrongBinder(IInterface::asBinder(client)); writeToParcel(&data, resources); @@ -129,6 +131,7 @@ status_t BnResourceManagerService::onTransact( case ADD_RESOURCE: { CHECK_INTERFACE(IResourceManagerService, data, reply); int pid = data.readInt32(); + int uid = data.readInt32(); int64_t clientId = data.readInt64(); sp client( interface_cast(data.readStrongBinder())); @@ -137,7 +140,7 @@ status_t BnResourceManagerService::onTransact( } Vector resources; readFromParcel(data, &resources); - addResource(pid, clientId, client, resources); + addResource(pid, uid, clientId, client, resources); return NO_ERROR; } break; diff --git a/media/libmedia/include/media/IResourceManagerService.h b/media/libmedia/include/media/IResourceManagerService.h index 1e4f6dea74..404519b2a6 100644 --- a/media/libmedia/include/media/IResourceManagerService.h +++ b/media/libmedia/include/media/IResourceManagerService.h @@ -39,6 +39,7 @@ public: virtual void addResource( int pid, + int uid, int64_t clientId, const sp client, const Vector &resources) = 0; diff --git a/media/libmedia/include/media/MediaResource.h b/media/libmedia/include/media/MediaResource.h index e1fdb9be3f..10d0e3b1b1 100644 --- a/media/libmedia/include/media/MediaResource.h +++ b/media/libmedia/include/media/MediaResource.h @@ -31,12 +31,13 @@ public: kNonSecureCodec, kGraphicMemory, kCpuBoost, + kBattery, }; enum SubType { kUnspecifiedSubType = 0, kAudioCodec, - kVideoCodec + kVideoCodec, }; MediaResource(); diff --git a/media/libstagefright/MediaCodec.cpp b/media/libstagefright/MediaCodec.cpp index f579e9de6b..a6a856c3f8 100644 --- a/media/libstagefright/MediaCodec.cpp +++ b/media/libstagefright/MediaCodec.cpp @@ -57,7 +57,6 @@ #include #include #include -#include #include #include @@ -166,8 +165,9 @@ private: DISALLOW_EVIL_CONSTRUCTORS(ResourceManagerClient); }; -MediaCodec::ResourceManagerServiceProxy::ResourceManagerServiceProxy(pid_t pid) - : mPid(pid) { +MediaCodec::ResourceManagerServiceProxy::ResourceManagerServiceProxy( + pid_t pid, uid_t uid) + : mPid(pid), mUid(uid) { if (mPid == MediaCodec::kNoPid) { mPid = IPCThreadState::self()->getCallingPid(); } @@ -204,7 +204,7 @@ void MediaCodec::ResourceManagerServiceProxy::addResource( if (mService == NULL) { return; } - mService->addResource(mPid, clientId, client, resources); + mService->addResource(mPid, mUid, clientId, client, resources); } void MediaCodec::ResourceManagerServiceProxy::removeResource(int64_t clientId) { @@ -517,8 +517,6 @@ MediaCodec::MediaCodec(const sp &looper, pid_t pid, uid_t uid) mStickyError(OK), mSoftRenderer(NULL), mAnalyticsItem(NULL), - mResourceManagerClient(new ResourceManagerClient(this)), - mResourceManagerService(new ResourceManagerServiceProxy(pid)), mBatteryStatNotified(false), mIsVideo(false), mVideoWidth(0), @@ -537,6 +535,8 @@ MediaCodec::MediaCodec(const sp &looper, pid_t pid, uid_t uid) } else { mUid = uid; } + mResourceManagerClient = new ResourceManagerClient(this); + mResourceManagerService = new ResourceManagerServiceProxy(pid, mUid); initAnalyticsItem(); } @@ -1977,6 +1977,11 @@ void MediaCodec::onMessageReceived(const sp &msg) { if (mIsVideo) { // audio codec is currently ignored. addResource(resourceType, MediaResource::kVideoCodec, 1); + // TODO: track battery on/off by actual queueing/dequeueing + // For now, keep existing behavior and request battery on/off + // together with codec init/uninit. We'll improve the tracking + // later by adding/removing this based on queue/dequeue timing. + addResource(MediaResource::kBattery, MediaResource::kVideoCodec, 1); } (new AMessage)->postReply(mReplyID); @@ -3126,8 +3131,6 @@ void MediaCodec::setState(State newState) { mState = newState; cancelPendingDequeueOperations(); - - updateBatteryStat(); } void MediaCodec::returnBuffersToCodec(bool isReclaim) { @@ -3631,20 +3634,6 @@ status_t MediaCodec::amendOutputFormatWithCodecSpecificData( return OK; } -void MediaCodec::updateBatteryStat() { - if (!mIsVideo) { - return; - } - - if (mState == CONFIGURED && !mBatteryStatNotified) { - BatteryNotifier::getInstance().noteStartVideo(mUid); - mBatteryStatNotified = true; - } else if (mState == UNINITIALIZED && mBatteryStatNotified) { - BatteryNotifier::getInstance().noteStopVideo(mUid); - mBatteryStatNotified = false; - } -} - std::string MediaCodec::stateString(State state) { const char *rval = NULL; char rawbuffer[16]; // room for "%d" diff --git a/media/libstagefright/include/media/stagefright/MediaCodec.h b/media/libstagefright/include/media/stagefright/MediaCodec.h index 89cca63dc6..0218a88af7 100644 --- a/media/libstagefright/include/media/stagefright/MediaCodec.h +++ b/media/libstagefright/include/media/stagefright/MediaCodec.h @@ -283,7 +283,7 @@ private: }; struct ResourceManagerServiceProxy : public IBinder::DeathRecipient { - ResourceManagerServiceProxy(pid_t pid); + ResourceManagerServiceProxy(pid_t pid, uid_t uid); ~ResourceManagerServiceProxy(); void init(); @@ -304,6 +304,7 @@ private: Mutex mLock; sp mService; pid_t mPid; + uid_t mUid; }; State mState; @@ -425,7 +426,6 @@ private: status_t onSetParameters(const sp ¶ms); status_t amendOutputFormatWithCodecSpecificData(const sp &buffer); - void updateBatteryStat(); bool isExecuting() const; uint64_t getGraphicBufferSize(); diff --git a/services/mediaresourcemanager/ResourceManagerService.cpp b/services/mediaresourcemanager/ResourceManagerService.cpp index 28bfd3ff51..117a2114e1 100644 --- a/services/mediaresourcemanager/ResourceManagerService.cpp +++ b/services/mediaresourcemanager/ResourceManagerService.cpp @@ -21,8 +21,11 @@ #include #include +#include #include #include +#include +#include #include #include #include @@ -31,8 +34,7 @@ #include "ResourceManagerService.h" #include "ServiceLog.h" -#include "mediautils/SchedulingPolicyService.h" -#include + namespace android { namespace { @@ -101,6 +103,7 @@ static ResourceInfos& getResourceInfosForEdit( } static ResourceInfo& getResourceInfoForEdit( + uid_t uid, int64_t clientId, const sp& client, ResourceInfos& infos) { @@ -110,9 +113,11 @@ static ResourceInfo& getResourceInfoForEdit( } } ResourceInfo info; + info.uid = uid; info.clientId = clientId; info.client = client; info.cpuBoost = false; + info.batteryNoted = false; infos.push_back(info); return infos.editItemAt(infos.size() - 1); } @@ -204,7 +209,9 @@ ResourceManagerService::ResourceManagerService(sp processI mServiceLog(new ServiceLog()), mSupportsMultipleSecureCodecs(true), mSupportsSecureWithNonSecureCodec(true), - mCpuBoostCount(0) {} + mCpuBoostCount(0) { + BatteryNotifier::getInstance().noteResetVideo(); +} ResourceManagerService::~ResourceManagerService() {} @@ -226,6 +233,7 @@ void ResourceManagerService::config(const Vector &policies) void ResourceManagerService::addResource( int pid, + int uid, int64_t clientId, const sp client, const Vector &resources) { @@ -239,7 +247,7 @@ void ResourceManagerService::addResource( return; } ResourceInfos& infos = getResourceInfosForEdit(pid, mMap); - ResourceInfo& info = getResourceInfoForEdit(clientId, client, infos); + ResourceInfo& info = getResourceInfoForEdit(uid, clientId, client, infos); // TODO: do the merge instead of append. info.resources.appendVector(resources); @@ -253,6 +261,11 @@ void ResourceManagerService::addResource( ALOGW("couldn't request cpuset boost"); } mCpuBoostCount++; + } else if (resources[i].mType == MediaResource::kBattery + && resources[i].mSubType == MediaResource::kVideoCodec + && !info.batteryNoted) { + info.batteryNoted = true; + BatteryNotifier::getInstance().noteStartVideo(info.uid); } } if (info.deathNotifier == nullptr) { @@ -291,6 +304,9 @@ void ResourceManagerService::removeResource(int pid, int64_t clientId, bool chec requestCpusetBoost(false, this); } } + if (infos[j].batteryNoted) { + BatteryNotifier::getInstance().noteStopVideo(infos[j].uid); + } IInterface::asBinder(infos[j].client)->unlinkToDeath(infos[j].deathNotifier); j = infos.removeAt(j); found = true; diff --git a/services/mediaresourcemanager/ResourceManagerService.h b/services/mediaresourcemanager/ResourceManagerService.h index 82d2a0b369..741ef73c61 100644 --- a/services/mediaresourcemanager/ResourceManagerService.h +++ b/services/mediaresourcemanager/ResourceManagerService.h @@ -35,10 +35,12 @@ struct ProcessInfoInterface; struct ResourceInfo { int64_t clientId; + uid_t uid; sp client; sp deathNotifier; Vector resources; bool cpuBoost; + bool batteryNoted; }; typedef Vector ResourceInfos; @@ -61,6 +63,7 @@ public: virtual void addResource( int pid, + int uid, int64_t clientId, const sp client, const Vector &resources); diff --git a/services/mediaresourcemanager/test/ResourceManagerService_test.cpp b/services/mediaresourcemanager/test/ResourceManagerService_test.cpp index ed0b0efa63..8a3987ace8 100644 --- a/services/mediaresourcemanager/test/ResourceManagerService_test.cpp +++ b/services/mediaresourcemanager/test/ResourceManagerService_test.cpp @@ -86,7 +86,10 @@ private: }; static const int kTestPid1 = 30; +static const int kTestUid1 = 1010; + static const int kTestPid2 = 20; +static const int kTestUid2 = 1011; static const int kLowPriorityPid = 40; static const int kMidPriorityPid = 25; @@ -115,8 +118,11 @@ protected: return true; } - static void expectEqResourceInfo(const ResourceInfo &info, sp client, + static void expectEqResourceInfo(const ResourceInfo &info, + int uid, + sp client, const Vector &resources) { + EXPECT_EQ(uid, info.uid); EXPECT_EQ(client, info.client); EXPECT_TRUE(isEqualResources(resources, info.resources)); } @@ -153,24 +159,24 @@ protected: // kTestPid1 mTestClient1 Vector resources1; resources1.push_back(MediaResource(MediaResource::kSecureCodec, 1)); - mService->addResource(kTestPid1, getId(mTestClient1), mTestClient1, resources1); + mService->addResource(kTestPid1, kTestUid1, getId(mTestClient1), mTestClient1, resources1); resources1.push_back(MediaResource(MediaResource::kGraphicMemory, 200)); Vector resources11; resources11.push_back(MediaResource(MediaResource::kGraphicMemory, 200)); - mService->addResource(kTestPid1, getId(mTestClient1), mTestClient1, resources11); + mService->addResource(kTestPid1, kTestUid1, getId(mTestClient1), mTestClient1, resources11); // kTestPid2 mTestClient2 Vector resources2; resources2.push_back(MediaResource(MediaResource::kNonSecureCodec, 1)); resources2.push_back(MediaResource(MediaResource::kGraphicMemory, 300)); - mService->addResource(kTestPid2, getId(mTestClient2), mTestClient2, resources2); + mService->addResource(kTestPid2, kTestUid2, getId(mTestClient2), mTestClient2, resources2); // kTestPid2 mTestClient3 Vector resources3; - mService->addResource(kTestPid2, getId(mTestClient3), mTestClient3, resources3); + mService->addResource(kTestPid2, kTestUid2, getId(mTestClient3), mTestClient3, resources3); resources3.push_back(MediaResource(MediaResource::kSecureCodec, 1)); resources3.push_back(MediaResource(MediaResource::kGraphicMemory, 100)); - mService->addResource(kTestPid2, getId(mTestClient3), mTestClient3, resources3); + mService->addResource(kTestPid2, kTestUid2, getId(mTestClient3), mTestClient3, resources3); const PidResourceInfosMap &map = mService->mMap; EXPECT_EQ(2u, map.size()); @@ -178,14 +184,14 @@ protected: ASSERT_GE(index1, 0); const ResourceInfos &infos1 = map[index1]; EXPECT_EQ(1u, infos1.size()); - expectEqResourceInfo(infos1[0], mTestClient1, resources1); + expectEqResourceInfo(infos1[0], kTestUid1, mTestClient1, resources1); ssize_t index2 = map.indexOfKey(kTestPid2); ASSERT_GE(index2, 0); const ResourceInfos &infos2 = map[index2]; EXPECT_EQ(2u, infos2.size()); - expectEqResourceInfo(infos2[0], mTestClient2, resources2); - expectEqResourceInfo(infos2[1], mTestClient3, resources3); + expectEqResourceInfo(infos2[0], kTestUid2, mTestClient2, resources2); + expectEqResourceInfo(infos2[1], kTestUid2, mTestClient3, resources3); } void testConfig() { From fb092d3b720923efaf0f9d4f4c16c57da6d657ac Mon Sep 17 00:00:00 2001 From: Chong Zhang Date: Mon, 12 Aug 2019 09:45:44 -0700 Subject: [PATCH 05/34] Add support for combining and remove individual resource - Fix ResourceManagerSerivce handling of adding same-type resource in multiple calls, combine entries with the same . This is not required in existing usage, bug could be required if we allow same type to be added incrementally (eg. memory usage). - Add api for removing some resources. Currently we can add resources in multiple calls, but removeResource always remove all resources of that client. We need this to toggle battery stat On/Off based on actual usage of the codec. - Add unit tests for the above. bug: 138381810 test: ResourceManagerService_test; CTS ResourceManangerTest. Change-Id: Icdba6c6b4c517755291668c52764169aac3071ea --- media/libmedia/IResourceManagerService.cpp | 25 ++- .../include/media/IResourceManagerService.h | 5 +- media/libmedia/include/media/MediaResource.h | 2 + media/libstagefright/MediaCodec.cpp | 2 +- .../ResourceManagerService.cpp | 176 ++++++++++++------ .../ResourceManagerService.h | 16 +- .../test/ResourceManagerService_test.cpp | 110 +++++++++-- 7 files changed, 254 insertions(+), 82 deletions(-) diff --git a/media/libmedia/IResourceManagerService.cpp b/media/libmedia/IResourceManagerService.cpp index 00f9b88aad..f8a0a143e1 100644 --- a/media/libmedia/IResourceManagerService.cpp +++ b/media/libmedia/IResourceManagerService.cpp @@ -32,6 +32,7 @@ enum { CONFIG = IBinder::FIRST_CALL_TRANSACTION, ADD_RESOURCE, REMOVE_RESOURCE, + REMOVE_CLIENT, RECLAIM_RESOURCE, }; @@ -87,15 +88,25 @@ public: remote()->transact(ADD_RESOURCE, data, &reply); } - virtual void removeResource(int pid, int64_t clientId) { + virtual void removeResource(int pid, int64_t clientId, const Vector &resources) { Parcel data, reply; data.writeInterfaceToken(IResourceManagerService::getInterfaceDescriptor()); data.writeInt32(pid); data.writeInt64(clientId); + writeToParcel(&data, resources); remote()->transact(REMOVE_RESOURCE, data, &reply); } + virtual void removeClient(int pid, int64_t clientId) { + Parcel data, reply; + data.writeInterfaceToken(IResourceManagerService::getInterfaceDescriptor()); + data.writeInt32(pid); + data.writeInt64(clientId); + + remote()->transact(REMOVE_CLIENT, data, &reply); + } + virtual bool reclaimResource(int callingPid, const Vector &resources) { Parcel data, reply; data.writeInterfaceToken(IResourceManagerService::getInterfaceDescriptor()); @@ -148,7 +159,17 @@ status_t BnResourceManagerService::onTransact( CHECK_INTERFACE(IResourceManagerService, data, reply); int pid = data.readInt32(); int64_t clientId = data.readInt64(); - removeResource(pid, clientId); + Vector resources; + readFromParcel(data, &resources); + removeResource(pid, clientId, resources); + return NO_ERROR; + } break; + + case REMOVE_CLIENT: { + CHECK_INTERFACE(IResourceManagerService, data, reply); + int pid = data.readInt32(); + int64_t clientId = data.readInt64(); + removeClient(pid, clientId); return NO_ERROR; } break; diff --git a/media/libmedia/include/media/IResourceManagerService.h b/media/libmedia/include/media/IResourceManagerService.h index 404519b2a6..8992f8bc15 100644 --- a/media/libmedia/include/media/IResourceManagerService.h +++ b/media/libmedia/include/media/IResourceManagerService.h @@ -44,7 +44,10 @@ public: const sp client, const Vector &resources) = 0; - virtual void removeResource(int pid, int64_t clientId) = 0; + virtual void removeResource(int pid, int64_t clientId, + const Vector &resources) = 0; + + virtual void removeClient(int pid, int64_t clientId) = 0; virtual bool reclaimResource( int callingPid, diff --git a/media/libmedia/include/media/MediaResource.h b/media/libmedia/include/media/MediaResource.h index 10d0e3b1b1..10a07bb639 100644 --- a/media/libmedia/include/media/MediaResource.h +++ b/media/libmedia/include/media/MediaResource.h @@ -63,6 +63,8 @@ inline static const char *asString(MediaResource::Type i, const char *def = "??" case MediaResource::kSecureCodec: return "secure-codec"; case MediaResource::kNonSecureCodec: return "non-secure-codec"; case MediaResource::kGraphicMemory: return "graphic-memory"; + case MediaResource::kCpuBoost: return "cpu-boost"; + case MediaResource::kBattery: return "battery"; default: return def; } } diff --git a/media/libstagefright/MediaCodec.cpp b/media/libstagefright/MediaCodec.cpp index a6a856c3f8..4b52591aea 100644 --- a/media/libstagefright/MediaCodec.cpp +++ b/media/libstagefright/MediaCodec.cpp @@ -212,7 +212,7 @@ void MediaCodec::ResourceManagerServiceProxy::removeResource(int64_t clientId) { if (mService == NULL) { return; } - mService->removeResource(mPid, clientId); + mService->removeClient(mPid, clientId); } bool MediaCodec::ResourceManagerServiceProxy::reclaimResource( diff --git a/services/mediaresourcemanager/ResourceManagerService.cpp b/services/mediaresourcemanager/ResourceManagerService.cpp index 117a2114e1..5a52b3d5dc 100644 --- a/services/mediaresourcemanager/ResourceManagerService.cpp +++ b/services/mediaresourcemanager/ResourceManagerService.cpp @@ -71,9 +71,9 @@ static String8 getString(const Vector &items) { return itemsStr; } -static bool hasResourceType(MediaResource::Type type, const Vector& resources) { - for (size_t i = 0; i < resources.size(); ++i) { - if (resources[i].mType == type) { +static bool hasResourceType(MediaResource::Type type, const ResourceList& resources) { + for (auto it = resources.begin(); it != resources.end(); it++) { + if (it->second.mType == type) { return true; } } @@ -107,19 +107,18 @@ static ResourceInfo& getResourceInfoForEdit( int64_t clientId, const sp& client, ResourceInfos& infos) { - for (size_t i = 0; i < infos.size(); ++i) { - if (infos[i].clientId == clientId) { - return infos.editItemAt(i); - } + ssize_t index = infos.indexOfKey(clientId); + + if (index < 0) { + ResourceInfo info; + info.uid = uid; + info.clientId = clientId; + info.client = client; + + index = infos.add(clientId, info); } - ResourceInfo info; - info.uid = uid; - info.clientId = clientId; - info.client = client; - info.cpuBoost = false; - info.batteryNoted = false; - infos.push_back(info); - return infos.editItemAt(infos.size() - 1); + + return infos.editValueAt(index); } static void notifyResourceGranted(int pid, const Vector &resources) { @@ -186,10 +185,10 @@ status_t ResourceManagerService::dump(int fd, const Vector& /* args */ snprintf(buffer, SIZE, " Name: %s\n", infos[j].client->getName().string()); result.append(buffer); - Vector resources = infos[j].resources; + const ResourceList &resources = infos[j].resources; result.append(" Resources:\n"); - for (size_t k = 0; k < resources.size(); ++k) { - snprintf(buffer, SIZE, " %s\n", resources[k].toString().string()); + for (auto it = resources.begin(); it != resources.end(); it++) { + snprintf(buffer, SIZE, " %s\n", it->second.toString().string()); result.append(buffer); } } @@ -231,6 +230,38 @@ void ResourceManagerService::config(const Vector &policies) } } +void ResourceManagerService::onFirstAdded( + const MediaResource& resource, const ResourceInfo& clientInfo) { + // first time added + if (resource.mType == MediaResource::kCpuBoost + && resource.mSubType == MediaResource::kUnspecifiedSubType) { + // Request it on every new instance of kCpuBoost, as the media.codec + // could have died, if we only do it the first time subsequent instances + // never gets the boost. + if (requestCpusetBoost(true, this) != OK) { + ALOGW("couldn't request cpuset boost"); + } + mCpuBoostCount++; + } else if (resource.mType == MediaResource::kBattery + && resource.mSubType == MediaResource::kVideoCodec) { + BatteryNotifier::getInstance().noteStartVideo(clientInfo.uid); + } +} + +void ResourceManagerService::onLastRemoved( + const MediaResource& resource, const ResourceInfo& clientInfo) { + if (resource.mType == MediaResource::kCpuBoost + && resource.mSubType == MediaResource::kUnspecifiedSubType + && mCpuBoostCount > 0) { + if (--mCpuBoostCount == 0) { + requestCpusetBoost(false, this); + } + } else if (resource.mType == MediaResource::kBattery + && resource.mSubType == MediaResource::kVideoCodec) { + BatteryNotifier::getInstance().noteStopVideo(clientInfo.uid); + } +} + void ResourceManagerService::addResource( int pid, int uid, @@ -248,24 +279,14 @@ void ResourceManagerService::addResource( } ResourceInfos& infos = getResourceInfosForEdit(pid, mMap); ResourceInfo& info = getResourceInfoForEdit(uid, clientId, client, infos); - // TODO: do the merge instead of append. - info.resources.appendVector(resources); for (size_t i = 0; i < resources.size(); ++i) { - if (resources[i].mType == MediaResource::kCpuBoost && !info.cpuBoost) { - info.cpuBoost = true; - // Request it on every new instance of kCpuBoost, as the media.codec - // could have died, if we only do it the first time subsequent instances - // never gets the boost. - if (requestCpusetBoost(true, this) != OK) { - ALOGW("couldn't request cpuset boost"); - } - mCpuBoostCount++; - } else if (resources[i].mType == MediaResource::kBattery - && resources[i].mSubType == MediaResource::kVideoCodec - && !info.batteryNoted) { - info.batteryNoted = true; - BatteryNotifier::getInstance().noteStartVideo(info.uid); + const auto resType = std::make_pair(resources[i].mType, resources[i].mSubType); + if (info.resources.find(resType) == info.resources.end()) { + onFirstAdded(resources[i], info); + info.resources[resType] = resources[i]; + } else { + info.resources[resType].mValue += resources[i].mValue; } } if (info.deathNotifier == nullptr) { @@ -275,7 +296,48 @@ void ResourceManagerService::addResource( notifyResourceGranted(pid, resources); } -void ResourceManagerService::removeResource(int pid, int64_t clientId) { +void ResourceManagerService::removeResource(int pid, int64_t clientId, + const Vector &resources) { + String8 log = String8::format("removeResource(pid %d, clientId %lld, resources %s)", + pid, (long long) clientId, getString(resources).string()); + mServiceLog->add(log); + + Mutex::Autolock lock(mLock); + if (!mProcessInfo->isValidPid(pid)) { + ALOGE("Rejected removeResource call with invalid pid."); + return; + } + ssize_t index = mMap.indexOfKey(pid); + if (index < 0) { + ALOGV("removeResource: didn't find pid %d for clientId %lld", pid, (long long) clientId); + return; + } + ResourceInfos &infos = mMap.editValueAt(index); + + index = infos.indexOfKey(clientId); + if (index < 0) { + ALOGV("removeResource: didn't find clientId %lld", (long long) clientId); + return; + } + + ResourceInfo &info = infos.editValueAt(index); + + for (size_t i = 0; i < resources.size(); ++i) { + const auto resType = std::make_pair(resources[i].mType, resources[i].mSubType); + // ignore if we don't have it + if (info.resources.find(resType) != info.resources.end()) { + MediaResource &resource = info.resources[resType]; + if (resource.mValue > resources[i].mValue) { + resource.mValue -= resources[i].mValue; + } else { + onLastRemoved(resources[i], info); + info.resources.erase(resType); + } + } + } +} + +void ResourceManagerService::removeClient(int pid, int64_t clientId) { removeResource(pid, clientId, true); } @@ -295,27 +357,22 @@ void ResourceManagerService::removeResource(int pid, int64_t clientId, bool chec ALOGV("removeResource: didn't find pid %d for clientId %lld", pid, (long long) clientId); return; } - bool found = false; ResourceInfos &infos = mMap.editValueAt(index); - for (size_t j = 0; j < infos.size(); ++j) { - if (infos[j].clientId == clientId) { - if (infos[j].cpuBoost && mCpuBoostCount > 0) { - if (--mCpuBoostCount == 0) { - requestCpusetBoost(false, this); - } - } - if (infos[j].batteryNoted) { - BatteryNotifier::getInstance().noteStopVideo(infos[j].uid); - } - IInterface::asBinder(infos[j].client)->unlinkToDeath(infos[j].deathNotifier); - j = infos.removeAt(j); - found = true; - break; - } + + index = infos.indexOfKey(clientId); + if (index < 0) { + ALOGV("removeResource: didn't find clientId %lld", (long long) clientId); + return; } - if (!found) { - ALOGV("didn't find client"); + + const ResourceInfo &info = infos[index]; + for (auto it = info.resources.begin(); it != info.resources.end(); it++) { + onLastRemoved(it->second, info); } + + IInterface::asBinder(info.client)->unlinkToDeath(info.deathNotifier); + + infos.removeItemsAt(index); } void ResourceManagerService::getClientForResource_l( @@ -426,7 +483,7 @@ bool ResourceManagerService::reclaimResource( ResourceInfos &infos = mMap.editValueAt(i); for (size_t j = 0; j < infos.size();) { if (infos[j].client == failedClient) { - j = infos.removeAt(j); + j = infos.removeItemsAt(j); found = true; } else { ++j; @@ -554,11 +611,12 @@ bool ResourceManagerService::getBiggestClient_l( uint64_t largestValue = 0; const ResourceInfos &infos = mMap.valueAt(index); for (size_t i = 0; i < infos.size(); ++i) { - Vector resources = infos[i].resources; - for (size_t j = 0; j < resources.size(); ++j) { - if (resources[j].mType == type) { - if (resources[j].mValue > largestValue) { - largestValue = resources[j].mValue; + const ResourceList &resources = infos[i].resources; + for (auto it = resources.begin(); it != resources.end(); it++) { + const MediaResource &resource = it->second; + if (resource.mType == type) { + if (resource.mValue > largestValue) { + largestValue = resource.mValue; clientTemp = infos[i].client; } } diff --git a/services/mediaresourcemanager/ResourceManagerService.h b/services/mediaresourcemanager/ResourceManagerService.h index 741ef73c61..b9147ffbbb 100644 --- a/services/mediaresourcemanager/ResourceManagerService.h +++ b/services/mediaresourcemanager/ResourceManagerService.h @@ -33,17 +33,17 @@ namespace android { class ServiceLog; struct ProcessInfoInterface; +typedef std::map, MediaResource> ResourceList; struct ResourceInfo { int64_t clientId; uid_t uid; sp client; sp deathNotifier; - Vector resources; - bool cpuBoost; - bool batteryNoted; + ResourceList resources; }; -typedef Vector ResourceInfos; +// TODO: convert these to std::map +typedef KeyedVector ResourceInfos; typedef KeyedVector PidResourceInfosMap; class ResourceManagerService @@ -68,7 +68,10 @@ public: const sp client, const Vector &resources); - virtual void removeResource(int pid, int64_t clientId); + virtual void removeResource(int pid, int64_t clientId, + const Vector &resources); + + virtual void removeClient(int pid, int64_t clientId); // Tries to reclaim resource from processes with lower priority than the calling process // according to the requested resources. @@ -110,6 +113,9 @@ private: void getClientForResource_l( int callingPid, const MediaResource *res, Vector> *clients); + void onFirstAdded(const MediaResource& res, const ResourceInfo& clientInfo); + void onLastRemoved(const MediaResource& res, const ResourceInfo& clientInfo); + mutable Mutex mLock; sp mProcessInfo; sp mServiceLog; diff --git a/services/mediaresourcemanager/test/ResourceManagerService_test.cpp b/services/mediaresourcemanager/test/ResourceManagerService_test.cpp index 8a3987ace8..be592f537a 100644 --- a/services/mediaresourcemanager/test/ResourceManagerService_test.cpp +++ b/services/mediaresourcemanager/test/ResourceManagerService_test.cpp @@ -58,7 +58,7 @@ struct TestClient : public BnResourceManagerClient { virtual bool reclaimResource() { sp client(this); - mService->removeResource(mPid, (int64_t) client.get()); + mService->removeClient(mPid, (int64_t) client.get()); mReclaimed = true; return true; } @@ -106,16 +106,14 @@ public: protected: static bool isEqualResources(const Vector &resources1, - const Vector &resources2) { - if (resources1.size() != resources2.size()) { - return false; - } + const ResourceList &resources2) { + // convert resource1 to ResourceList + ResourceList r1; for (size_t i = 0; i < resources1.size(); ++i) { - if (resources1[i] != resources2[i]) { - return false; - } + const auto resType = std::make_pair(resources1[i].mType, resources1[i].mSubType); + r1[resType] = resources1[i]; } - return true; + return r1 == resources2; } static void expectEqResourceInfo(const ResourceInfo &info, @@ -184,14 +182,14 @@ protected: ASSERT_GE(index1, 0); const ResourceInfos &infos1 = map[index1]; EXPECT_EQ(1u, infos1.size()); - expectEqResourceInfo(infos1[0], kTestUid1, mTestClient1, resources1); + expectEqResourceInfo(infos1.valueFor(getId(mTestClient1)), kTestUid1, mTestClient1, resources1); ssize_t index2 = map.indexOfKey(kTestPid2); ASSERT_GE(index2, 0); const ResourceInfos &infos2 = map[index2]; EXPECT_EQ(2u, infos2.size()); - expectEqResourceInfo(infos2[0], kTestUid2, mTestClient2, resources2); - expectEqResourceInfo(infos2[1], kTestUid2, mTestClient3, resources3); + expectEqResourceInfo(infos2.valueFor(getId(mTestClient2)), kTestUid2, mTestClient2, resources2); + expectEqResourceInfo(infos2.valueFor(getId(mTestClient3)), kTestUid2, mTestClient3, resources3); } void testConfig() { @@ -225,10 +223,84 @@ protected: EXPECT_TRUE(mService->mSupportsSecureWithNonSecureCodec); } + void testCombineResource() { + // kTestPid1 mTestClient1 + Vector resources1; + resources1.push_back(MediaResource(MediaResource::kSecureCodec, 1)); + mService->addResource(kTestPid1, kTestUid1, getId(mTestClient1), mTestClient1, resources1); + + Vector resources11; + resources11.push_back(MediaResource(MediaResource::kGraphicMemory, 200)); + mService->addResource(kTestPid1, kTestUid1, getId(mTestClient1), mTestClient1, resources11); + + const PidResourceInfosMap &map = mService->mMap; + EXPECT_EQ(1u, map.size()); + ssize_t index1 = map.indexOfKey(kTestPid1); + ASSERT_GE(index1, 0); + const ResourceInfos &infos1 = map[index1]; + EXPECT_EQ(1u, infos1.size()); + + // test adding existing types to combine values + resources1.push_back(MediaResource(MediaResource::kGraphicMemory, 100)); + mService->addResource(kTestPid1, kTestUid1, getId(mTestClient1), mTestClient1, resources1); + + Vector expected; + expected.push_back(MediaResource(MediaResource::kSecureCodec, 2)); + expected.push_back(MediaResource(MediaResource::kGraphicMemory, 300)); + expectEqResourceInfo(infos1.valueFor(getId(mTestClient1)), kTestUid1, mTestClient1, expected); + + // test adding new types (including types that differs only in subType) + resources11.push_back(MediaResource(MediaResource::kNonSecureCodec, 1)); + resources11.push_back(MediaResource(MediaResource::kSecureCodec, MediaResource::kVideoCodec, 1)); + mService->addResource(kTestPid1, kTestUid1, getId(mTestClient1), mTestClient1, resources11); + + expected.clear(); + expected.push_back(MediaResource(MediaResource::kSecureCodec, 2)); + expected.push_back(MediaResource(MediaResource::kNonSecureCodec, 1)); + expected.push_back(MediaResource(MediaResource::kSecureCodec, MediaResource::kVideoCodec, 1)); + expected.push_back(MediaResource(MediaResource::kGraphicMemory, 500)); + expectEqResourceInfo(infos1.valueFor(getId(mTestClient1)), kTestUid1, mTestClient1, expected); + } + void testRemoveResource() { + // kTestPid1 mTestClient1 + Vector resources1; + resources1.push_back(MediaResource(MediaResource::kSecureCodec, 1)); + mService->addResource(kTestPid1, kTestUid1, getId(mTestClient1), mTestClient1, resources1); + + Vector resources11; + resources11.push_back(MediaResource(MediaResource::kGraphicMemory, 200)); + mService->addResource(kTestPid1, kTestUid1, getId(mTestClient1), mTestClient1, resources11); + + const PidResourceInfosMap &map = mService->mMap; + EXPECT_EQ(1u, map.size()); + ssize_t index1 = map.indexOfKey(kTestPid1); + ASSERT_GE(index1, 0); + const ResourceInfos &infos1 = map[index1]; + EXPECT_EQ(1u, infos1.size()); + + // test partial removal + resources11.editItemAt(0).mValue = 100; + mService->removeResource(kTestPid1, getId(mTestClient1), resources11); + + Vector expected; + expected.push_back(MediaResource(MediaResource::kSecureCodec, 1)); + expected.push_back(MediaResource(MediaResource::kGraphicMemory, 100)); + expectEqResourceInfo(infos1.valueFor(getId(mTestClient1)), kTestUid1, mTestClient1, expected); + + // test complete removal with overshoot value + resources11.editItemAt(0).mValue = 1000; + mService->removeResource(kTestPid1, getId(mTestClient1), resources11); + + expected.clear(); + expected.push_back(MediaResource(MediaResource::kSecureCodec, 1)); + expectEqResourceInfo(infos1.valueFor(getId(mTestClient1)), kTestUid1, mTestClient1, expected); + } + + void testRemoveClient() { addResource(); - mService->removeResource(kTestPid2, getId(mTestClient2)); + mService->removeClient(kTestPid2, getId(mTestClient2)); const PidResourceInfosMap &map = mService->mMap; EXPECT_EQ(2u, map.size()); @@ -237,6 +309,7 @@ protected: EXPECT_EQ(1u, infos1.size()); EXPECT_EQ(1u, infos2.size()); // mTestClient2 has been removed. + // (OK to use infos2[0] as there is only 1 entry) EXPECT_EQ(mTestClient3, infos2[0].client); } @@ -252,6 +325,7 @@ protected: EXPECT_TRUE(mService->getAllClients_l(kHighPriorityPid, type, &clients)); EXPECT_EQ(2u, clients.size()); + // (OK to require ordering in clients[], as the pid map is sorted) EXPECT_EQ(mTestClient3, clients[0]); EXPECT_EQ(mTestClient1, clients[1]); } @@ -444,7 +518,7 @@ protected: verifyClients(true /* c1 */, false /* c2 */, false /* c3 */); // clean up client 3 which still left - mService->removeResource(kTestPid2, getId(mTestClient3)); + mService->removeClient(kTestPid2, getId(mTestClient3)); } } @@ -518,10 +592,18 @@ TEST_F(ResourceManagerServiceTest, addResource) { addResource(); } +TEST_F(ResourceManagerServiceTest, combineResource) { + testCombineResource(); +} + TEST_F(ResourceManagerServiceTest, removeResource) { testRemoveResource(); } +TEST_F(ResourceManagerServiceTest, removeClient) { + testRemoveClient(); +} + TEST_F(ResourceManagerServiceTest, reclaimResource) { testReclaimResourceSecure(); testReclaimResourceNonSecure(); From 22c4350bd3122d4c0b22f1bb2e02533c449412ad Mon Sep 17 00:00:00 2001 From: Chong Zhang Date: Wed, 14 Aug 2019 13:15:32 -0700 Subject: [PATCH 06/34] Change video battery stats to better track actual use Reporting video on upon first queue/dequeue buffer activity, off after a period of inactivity (3 sec currently). bug: 138381810 test: Manual testing for now; unit tests will come after. With BatteryStatService instrumented to print out noteVideo*: Test case #1: Camera recording (using GCA): Camera creates encoder immediately when Video tab is entered, but recording won't start until button is pressed. After recording is stopped, a new encoder is again created to prepare for next session. So under old scheme, video is on as soon as we're in Video tab, and off/on even pairs are seen when video is stopped. Video stats is basically always On when we're in video tab. With this CL one should see: - There shouldn't be videoOn when Video tab is first entered - Start recording and there should be a videoOn. - Stop recording and there should be a videoOff, AND videoOn shouldn't immediately follow. - Kill camera app during active recording, video should be restored to Off. Test case #2: Playback of video clips in Photos with pause With old scheme pausing state will be videoOn because codec instance is still there. With new scheme, videoOff should come after video is paused for 3 sec, and video On should come again when video is resumed. Change-Id: Ifa8aa5d8c2c95fa25393fcb936e6e54578716783 --- media/libstagefright/MediaCodec.cpp | 87 ++++++++++++++++--- .../include/media/stagefright/MediaCodec.h | 15 +++- 2 files changed, 90 insertions(+), 12 deletions(-) diff --git a/media/libstagefright/MediaCodec.cpp b/media/libstagefright/MediaCodec.cpp index 4b52591aea..ae0fa3ca45 100644 --- a/media/libstagefright/MediaCodec.cpp +++ b/media/libstagefright/MediaCodec.cpp @@ -116,6 +116,7 @@ static bool isResourceError(status_t err) { static const int kMaxRetry = 2; static const int kMaxReclaimWaitTimeInUs = 500000; // 0.5s static const int kNumBuffersAlign = 16; +static const int kBatteryStatsTimeoutUs = 3000000ll; // 3 seconds //////////////////////////////////////////////////////////////////////////////// @@ -207,7 +208,17 @@ void MediaCodec::ResourceManagerServiceProxy::addResource( mService->addResource(mPid, mUid, clientId, client, resources); } -void MediaCodec::ResourceManagerServiceProxy::removeResource(int64_t clientId) { +void MediaCodec::ResourceManagerServiceProxy::removeResource( + int64_t clientId, + const Vector &resources) { + Mutex::Autolock _l(mLock); + if (mService == NULL) { + return; + } + mService->removeResource(mPid, clientId, resources); +} + +void MediaCodec::ResourceManagerServiceProxy::removeClient(int64_t clientId) { Mutex::Autolock _l(mLock); if (mService == NULL) { return; @@ -517,7 +528,6 @@ MediaCodec::MediaCodec(const sp &looper, pid_t pid, uid_t uid) mStickyError(OK), mSoftRenderer(NULL), mAnalyticsItem(NULL), - mBatteryStatNotified(false), mIsVideo(false), mVideoWidth(0), mVideoHeight(0), @@ -529,7 +539,10 @@ MediaCodec::MediaCodec(const sp &looper, pid_t pid, uid_t uid) mHaveInputSurface(false), mHavePendingInputBuffers(false), mCpuBoostRequested(false), - mLatencyUnknown(0) { + mLatencyUnknown(0), + mLastActivityTimeUs(-1ll), + mBatteryStatNotified(false), + mBatteryCheckerGeneration(0) { if (uid == kNoUid) { mUid = IPCThreadState::self()->getCallingUid(); } else { @@ -543,7 +556,7 @@ MediaCodec::MediaCodec(const sp &looper, pid_t pid, uid_t uid) MediaCodec::~MediaCodec() { CHECK_EQ(mState, UNINITIALIZED); - mResourceManagerService->removeResource(getId(mResourceManagerClient)); + mResourceManagerService->removeClient(getId(mResourceManagerClient)); flushAnalyticsItem(); } @@ -742,6 +755,8 @@ void MediaCodec::statsBufferSent(int64_t presentationUs) { return; } + scheduleBatteryCheckerIfNeeded(); + const int64_t nowNs = systemTime(SYSTEM_TIME_MONOTONIC); BufferFlightTiming_t startdata = { presentationUs, nowNs }; @@ -776,6 +791,8 @@ void MediaCodec::statsBufferReceived(int64_t presentationUs) { return; } + scheduleBatteryCheckerIfNeeded(); + BufferFlightTiming_t startdata; bool valid = false; while (mBuffersInFlight.size() > 0) { @@ -1221,6 +1238,13 @@ void MediaCodec::addResource( getId(mResourceManagerClient), mResourceManagerClient, resources); } +void MediaCodec::removeResource( + MediaResource::Type type, MediaResource::SubType subtype, uint64_t value) { + Vector resources; + resources.push_back(MediaResource(type, subtype, value)); + mResourceManagerService->removeResource(getId(mResourceManagerClient), resources); +} + status_t MediaCodec::start() { sp msg = new AMessage(kWhatStart, this); @@ -1682,6 +1706,45 @@ void MediaCodec::requestCpuBoostIfNeeded() { } } +void MediaCodec::scheduleBatteryCheckerIfNeeded() { + if (!mIsVideo || !isExecuting()) { + // ignore if not executing + return; + } + if (!mBatteryStatNotified) { + addResource(MediaResource::kBattery, MediaResource::kVideoCodec, 1); + mBatteryStatNotified = true; + sp msg = new AMessage(kWhatCheckBatteryStats, this); + msg->setInt32("generation", mBatteryCheckerGeneration); + + // post checker and clear last activity time + msg->post(kBatteryStatsTimeoutUs); + mLastActivityTimeUs = -1ll; + } else { + // update last activity time + mLastActivityTimeUs = ALooper::GetNowUs(); + } +} + +void MediaCodec::onBatteryChecker(const sp &msg) { + // ignore if this checker already expired because the client resource was removed + int32_t generation; + if (!msg->findInt32("generation", &generation) + || generation != mBatteryCheckerGeneration) { + return; + } + + if (mLastActivityTimeUs < 0ll) { + // timed out inactive, do not repost checker + removeResource(MediaResource::kBattery, MediaResource::kVideoCodec, 1); + mBatteryStatNotified = false; + } else { + // repost checker and clear last activity time + msg->post(kBatteryStatsTimeoutUs + mLastActivityTimeUs - ALooper::GetNowUs()); + mLastActivityTimeUs = -1ll; + } +} + //////////////////////////////////////////////////////////////////////////////// void MediaCodec::cancelPendingDequeueOperations() { @@ -1977,11 +2040,6 @@ void MediaCodec::onMessageReceived(const sp &msg) { if (mIsVideo) { // audio codec is currently ignored. addResource(resourceType, MediaResource::kVideoCodec, 1); - // TODO: track battery on/off by actual queueing/dequeueing - // For now, keep existing behavior and request battery on/off - // together with codec init/uninit. We'll improve the tracking - // later by adding/removing this based on queue/dequeue timing. - addResource(MediaResource::kBattery, MediaResource::kVideoCodec, 1); } (new AMessage)->postReply(mReplyID); @@ -2323,7 +2381,10 @@ void MediaCodec::onMessageReceived(const sp &msg) { mFlags &= ~kFlagIsComponentAllocated; - mResourceManagerService->removeResource(getId(mResourceManagerClient)); + // off since we're removing all resources including the battery on + mBatteryStatNotified = false; + mBatteryCheckerGeneration++; + mResourceManagerService->removeClient(getId(mResourceManagerClient)); (new AMessage)->postReply(mReplyID); break; @@ -3034,6 +3095,12 @@ void MediaCodec::onMessageReceived(const sp &msg) { break; } + case kWhatCheckBatteryStats: + { + onBatteryChecker(msg); + break; + } + default: TRESPASS(); } diff --git a/media/libstagefright/include/media/stagefright/MediaCodec.h b/media/libstagefright/include/media/stagefright/MediaCodec.h index 0218a88af7..462eb84e50 100644 --- a/media/libstagefright/include/media/stagefright/MediaCodec.h +++ b/media/libstagefright/include/media/stagefright/MediaCodec.h @@ -257,6 +257,7 @@ private: kWhatSetCallback = 'setC', kWhatSetNotification = 'setN', kWhatDrmReleaseCrypto = 'rDrm', + kWhatCheckBatteryStats = 'chkB', }; enum { @@ -296,7 +297,11 @@ private: const sp &client, const Vector &resources); - void removeResource(int64_t clientId); + void removeResource( + int64_t clientId, + const Vector &resources); + + void removeClient(int64_t clientId); bool reclaimResource(const Vector &resources); @@ -336,7 +341,6 @@ private: sp mResourceManagerClient; sp mResourceManagerService; - bool mBatteryStatNotified; bool mIsVideo; int32_t mVideoWidth; int32_t mVideoHeight; @@ -430,6 +434,7 @@ private: uint64_t getGraphicBufferSize(); void addResource(MediaResource::Type type, MediaResource::SubType subtype, uint64_t value); + void removeResource(MediaResource::Type type, MediaResource::SubType subtype, uint64_t value); void requestCpuBoostIfNeeded(); bool hasPendingBuffer(int portIndex); @@ -458,6 +463,12 @@ private: Mutex mLatencyLock; int64_t mLatencyUnknown; // buffers for which we couldn't calculate latency + int64_t mLastActivityTimeUs; + bool mBatteryStatNotified; + int32_t mBatteryCheckerGeneration; + void onBatteryChecker(const sp& msg); + void scheduleBatteryCheckerIfNeeded(); + void statsBufferSent(int64_t presentationUs); void statsBufferReceived(int64_t presentationUs); From 599a092e344e00032cfbc56a7395910fc0439453 Mon Sep 17 00:00:00 2001 From: Chong Zhang Date: Wed, 14 Aug 2019 13:15:32 -0700 Subject: [PATCH 07/34] Add ResourceManager unit tests to presubmit bug: 138381810 bug: 139440234 Change-Id: Id6a447ca070322b21d1b07457c16a6a5e2a8a1fe --- services/mediaresourcemanager/TEST_MAPPING | 10 ++++++++++ services/mediaresourcemanager/test/Android.bp | 2 ++ 2 files changed, 12 insertions(+) create mode 100644 services/mediaresourcemanager/TEST_MAPPING diff --git a/services/mediaresourcemanager/TEST_MAPPING b/services/mediaresourcemanager/TEST_MAPPING new file mode 100644 index 0000000000..418b159820 --- /dev/null +++ b/services/mediaresourcemanager/TEST_MAPPING @@ -0,0 +1,10 @@ +{ + "presubmit": [ + { + "name": "ResourceManagerService_test" + }, + { + "name": "ServiceLog_test" + } + ] +} diff --git a/services/mediaresourcemanager/test/Android.bp b/services/mediaresourcemanager/test/Android.bp index 70e883300a..543c87c544 100644 --- a/services/mediaresourcemanager/test/Android.bp +++ b/services/mediaresourcemanager/test/Android.bp @@ -2,6 +2,7 @@ cc_test { name: "ResourceManagerService_test", srcs: ["ResourceManagerService_test.cpp"], + test_suites: ["device-tests"], shared_libs: [ "libbinder", "liblog", @@ -23,6 +24,7 @@ cc_test { cc_test { name: "ServiceLog_test", srcs: ["ServiceLog_test.cpp"], + test_suites: ["device-tests"], shared_libs: [ "liblog", "libmedia", From b6d383dc8d7793e69629d189a7b018f15a0a7fc8 Mon Sep 17 00:00:00 2001 From: Vignesh Venkatasubramanian Date: Mon, 10 Jun 2019 15:11:58 -0700 Subject: [PATCH 08/34] codec2: libgav1 integration part 1 This CL contains stuff that are pretty much copied over from the libaom component. It makes it easier to review the series of CLs. All the libgav1 specific functions will follow in a subsequent CL. Test: all AV1 CTS tests still pass. Bug: 131989882 Bug: 130249450 Merged-In: Iaafbf5379577bd42c7de64c7ec96d5b0fbb45b18 Change-Id: Iaafbf5379577bd42c7de64c7ec96d5b0fbb45b18 --- media/codec2/components/gav1/Android.bp | 9 + .../codec2/components/gav1/C2SoftGav1Dec.cpp | 316 ++++++++++++++++++ media/codec2/components/gav1/C2SoftGav1Dec.h | 68 ++++ media/codec2/vndk/C2Store.cpp | 1 + services/mediacodec/registrant/Android.bp | 1 + 5 files changed, 395 insertions(+) create mode 100644 media/codec2/components/gav1/Android.bp create mode 100644 media/codec2/components/gav1/C2SoftGav1Dec.cpp create mode 100644 media/codec2/components/gav1/C2SoftGav1Dec.h diff --git a/media/codec2/components/gav1/Android.bp b/media/codec2/components/gav1/Android.bp new file mode 100644 index 0000000000..da76e9d023 --- /dev/null +++ b/media/codec2/components/gav1/Android.bp @@ -0,0 +1,9 @@ +cc_library_shared { + name: "libcodec2_soft_gav1dec", + defaults: [ + "libcodec2_soft-defaults", + "libcodec2_soft_sanitize_all-defaults", + ], + + srcs: ["C2SoftGav1Dec.cpp"], +} diff --git a/media/codec2/components/gav1/C2SoftGav1Dec.cpp b/media/codec2/components/gav1/C2SoftGav1Dec.cpp new file mode 100644 index 0000000000..f6dd14a5a9 --- /dev/null +++ b/media/codec2/components/gav1/C2SoftGav1Dec.cpp @@ -0,0 +1,316 @@ +/* + * Copyright (C) 2019 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. + */ + +//#define LOG_NDEBUG 0 +#define LOG_TAG "C2SoftGav1Dec" +#include "C2SoftGav1Dec.h" + +#include +#include +#include +#include +#include +#include + +namespace android { + +// TODO(vigneshv): This will be changed to c2.android.av1.decoder once this +// component is fully functional. +constexpr char COMPONENT_NAME[] = "c2.android.gav1.decoder"; + +class C2SoftGav1Dec::IntfImpl : public SimpleInterface::BaseParams { + public: + explicit IntfImpl(const std::shared_ptr &helper) + : SimpleInterface::BaseParams( + helper, COMPONENT_NAME, C2Component::KIND_DECODER, + C2Component::DOMAIN_VIDEO, MEDIA_MIMETYPE_VIDEO_AV1) { + noPrivateBuffers(); // TODO: account for our buffers here. + noInputReferences(); + noOutputReferences(); + noInputLatency(); + noTimeStretch(); + + addParameter(DefineParam(mAttrib, C2_PARAMKEY_COMPONENT_ATTRIBUTES) + .withConstValue(new C2ComponentAttributesSetting( + C2Component::ATTRIB_IS_TEMPORAL)) + .build()); + + addParameter( + DefineParam(mSize, C2_PARAMKEY_PICTURE_SIZE) + .withDefault(new C2StreamPictureSizeInfo::output(0u, 320, 240)) + .withFields({ + C2F(mSize, width).inRange(2, 2048, 2), + C2F(mSize, height).inRange(2, 2048, 2), + }) + .withSetter(SizeSetter) + .build()); + + addParameter(DefineParam(mProfileLevel, C2_PARAMKEY_PROFILE_LEVEL) + .withDefault(new C2StreamProfileLevelInfo::input( + 0u, C2Config::PROFILE_AV1_0, C2Config::LEVEL_AV1_2_1)) + .withFields({C2F(mProfileLevel, profile) + .oneOf({C2Config::PROFILE_AV1_0, + C2Config::PROFILE_AV1_1}), + C2F(mProfileLevel, level) + .oneOf({ + C2Config::LEVEL_AV1_2, + C2Config::LEVEL_AV1_2_1, + C2Config::LEVEL_AV1_2_2, + C2Config::LEVEL_AV1_3, + C2Config::LEVEL_AV1_3_1, + C2Config::LEVEL_AV1_3_2, + })}) + .withSetter(ProfileLevelSetter, mSize) + .build()); + + mHdr10PlusInfoInput = C2StreamHdr10PlusInfo::input::AllocShared(0); + addParameter( + DefineParam(mHdr10PlusInfoInput, C2_PARAMKEY_INPUT_HDR10_PLUS_INFO) + .withDefault(mHdr10PlusInfoInput) + .withFields({ + C2F(mHdr10PlusInfoInput, m.value).any(), + }) + .withSetter(Hdr10PlusInfoInputSetter) + .build()); + + mHdr10PlusInfoOutput = C2StreamHdr10PlusInfo::output::AllocShared(0); + addParameter( + DefineParam(mHdr10PlusInfoOutput, C2_PARAMKEY_OUTPUT_HDR10_PLUS_INFO) + .withDefault(mHdr10PlusInfoOutput) + .withFields({ + C2F(mHdr10PlusInfoOutput, m.value).any(), + }) + .withSetter(Hdr10PlusInfoOutputSetter) + .build()); + + addParameter( + DefineParam(mMaxSize, C2_PARAMKEY_MAX_PICTURE_SIZE) + .withDefault(new C2StreamMaxPictureSizeTuning::output(0u, 320, 240)) + .withFields({ + C2F(mSize, width).inRange(2, 2048, 2), + C2F(mSize, height).inRange(2, 2048, 2), + }) + .withSetter(MaxPictureSizeSetter, mSize) + .build()); + + addParameter(DefineParam(mMaxInputSize, C2_PARAMKEY_INPUT_MAX_BUFFER_SIZE) + .withDefault(new C2StreamMaxBufferSizeInfo::input( + 0u, 320 * 240 * 3 / 4)) + .withFields({ + C2F(mMaxInputSize, value).any(), + }) + .calculatedAs(MaxInputSizeSetter, mMaxSize) + .build()); + + C2ChromaOffsetStruct locations[1] = {C2ChromaOffsetStruct::ITU_YUV_420_0()}; + std::shared_ptr defaultColorInfo = + C2StreamColorInfo::output::AllocShared(1u, 0u, 8u /* bitDepth */, + C2Color::YUV_420); + memcpy(defaultColorInfo->m.locations, locations, sizeof(locations)); + + defaultColorInfo = C2StreamColorInfo::output::AllocShared( + {C2ChromaOffsetStruct::ITU_YUV_420_0()}, 0u, 8u /* bitDepth */, + C2Color::YUV_420); + helper->addStructDescriptors(); + + addParameter(DefineParam(mColorInfo, C2_PARAMKEY_CODED_COLOR_INFO) + .withConstValue(defaultColorInfo) + .build()); + + addParameter( + DefineParam(mDefaultColorAspects, C2_PARAMKEY_DEFAULT_COLOR_ASPECTS) + .withDefault(new C2StreamColorAspectsTuning::output( + 0u, C2Color::RANGE_UNSPECIFIED, C2Color::PRIMARIES_UNSPECIFIED, + C2Color::TRANSFER_UNSPECIFIED, C2Color::MATRIX_UNSPECIFIED)) + .withFields( + {C2F(mDefaultColorAspects, range) + .inRange(C2Color::RANGE_UNSPECIFIED, C2Color::RANGE_OTHER), + C2F(mDefaultColorAspects, primaries) + .inRange(C2Color::PRIMARIES_UNSPECIFIED, + C2Color::PRIMARIES_OTHER), + C2F(mDefaultColorAspects, transfer) + .inRange(C2Color::TRANSFER_UNSPECIFIED, + C2Color::TRANSFER_OTHER), + C2F(mDefaultColorAspects, matrix) + .inRange(C2Color::MATRIX_UNSPECIFIED, + C2Color::MATRIX_OTHER)}) + .withSetter(DefaultColorAspectsSetter) + .build()); + + // TODO: support more formats? + addParameter(DefineParam(mPixelFormat, C2_PARAMKEY_PIXEL_FORMAT) + .withConstValue(new C2StreamPixelFormatInfo::output( + 0u, HAL_PIXEL_FORMAT_YCBCR_420_888)) + .build()); + } + + static C2R SizeSetter(bool mayBlock, + const C2P &oldMe, + C2P &me) { + (void)mayBlock; + C2R res = C2R::Ok(); + if (!me.F(me.v.width).supportsAtAll(me.v.width)) { + res = res.plus(C2SettingResultBuilder::BadValue(me.F(me.v.width))); + me.set().width = oldMe.v.width; + } + if (!me.F(me.v.height).supportsAtAll(me.v.height)) { + res = res.plus(C2SettingResultBuilder::BadValue(me.F(me.v.height))); + me.set().height = oldMe.v.height; + } + return res; + } + + static C2R MaxPictureSizeSetter( + bool mayBlock, C2P &me, + const C2P &size) { + (void)mayBlock; + // TODO: get max width/height from the size's field helpers vs. + // hardcoding + me.set().width = c2_min(c2_max(me.v.width, size.v.width), 4096u); + me.set().height = c2_min(c2_max(me.v.height, size.v.height), 4096u); + return C2R::Ok(); + } + + static C2R MaxInputSizeSetter( + bool mayBlock, C2P &me, + const C2P &maxSize) { + (void)mayBlock; + // assume compression ratio of 2 + me.set().value = + (((maxSize.v.width + 63) / 64) * ((maxSize.v.height + 63) / 64) * 3072); + return C2R::Ok(); + } + + static C2R DefaultColorAspectsSetter( + bool mayBlock, C2P &me) { + (void)mayBlock; + if (me.v.range > C2Color::RANGE_OTHER) { + me.set().range = C2Color::RANGE_OTHER; + } + if (me.v.primaries > C2Color::PRIMARIES_OTHER) { + me.set().primaries = C2Color::PRIMARIES_OTHER; + } + if (me.v.transfer > C2Color::TRANSFER_OTHER) { + me.set().transfer = C2Color::TRANSFER_OTHER; + } + if (me.v.matrix > C2Color::MATRIX_OTHER) { + me.set().matrix = C2Color::MATRIX_OTHER; + } + return C2R::Ok(); + } + + static C2R ProfileLevelSetter( + bool mayBlock, C2P &me, + const C2P &size) { + (void)mayBlock; + (void)size; + (void)me; // TODO: validate + return C2R::Ok(); + } + + std::shared_ptr + getDefaultColorAspects_l() { + return mDefaultColorAspects; + } + + static C2R Hdr10PlusInfoInputSetter(bool mayBlock, + C2P &me) { + (void)mayBlock; + (void)me; // TODO: validate + return C2R::Ok(); + } + + static C2R Hdr10PlusInfoOutputSetter(bool mayBlock, + C2P &me) { + (void)mayBlock; + (void)me; // TODO: validate + return C2R::Ok(); + } + + private: + std::shared_ptr mProfileLevel; + std::shared_ptr mSize; + std::shared_ptr mMaxSize; + std::shared_ptr mMaxInputSize; + std::shared_ptr mColorInfo; + std::shared_ptr mPixelFormat; + std::shared_ptr mDefaultColorAspects; + std::shared_ptr mHdr10PlusInfoInput; + std::shared_ptr mHdr10PlusInfoOutput; +}; + +C2SoftGav1Dec::C2SoftGav1Dec(const char *name, c2_node_id_t id, + const std::shared_ptr &intfImpl) + : SimpleC2Component( + std::make_shared>(name, id, intfImpl)), + mIntf(intfImpl) {} + +c2_status_t C2SoftGav1Dec::onInit() { return C2_OK; } +c2_status_t C2SoftGav1Dec::onStop() { return C2_OK; } +void C2SoftGav1Dec::onReset() {} +void C2SoftGav1Dec::onRelease(){}; +c2_status_t C2SoftGav1Dec::onFlush_sm() { return C2_OK; } +void C2SoftGav1Dec::process(const std::unique_ptr & /*work*/, + const std::shared_ptr & /*pool*/) {} +c2_status_t C2SoftGav1Dec::drain( + uint32_t /*drainMode*/, const std::shared_ptr & /*pool*/) { + return C2_OK; +} + +class C2SoftGav1Factory : public C2ComponentFactory { + public: + C2SoftGav1Factory() + : mHelper(std::static_pointer_cast( + GetCodec2PlatformComponentStore()->getParamReflector())) {} + + virtual c2_status_t createComponent( + c2_node_id_t id, std::shared_ptr *const component, + std::function deleter) override { + *component = std::shared_ptr( + new C2SoftGav1Dec(COMPONENT_NAME, id, + std::make_shared(mHelper)), + deleter); + return C2_OK; + } + + virtual c2_status_t createInterface( + c2_node_id_t id, std::shared_ptr *const interface, + std::function deleter) override { + *interface = std::shared_ptr( + new SimpleInterface( + COMPONENT_NAME, id, + std::make_shared(mHelper)), + deleter); + return C2_OK; + } + + virtual ~C2SoftGav1Factory() override = default; + + private: + std::shared_ptr mHelper; +}; + +} // namespace android + +extern "C" ::C2ComponentFactory *CreateCodec2Factory() { + ALOGV("in %s", __func__); + return new ::android::C2SoftGav1Factory(); +} + +extern "C" void DestroyCodec2Factory(::C2ComponentFactory *factory) { + ALOGV("in %s", __func__); + delete factory; +} diff --git a/media/codec2/components/gav1/C2SoftGav1Dec.h b/media/codec2/components/gav1/C2SoftGav1Dec.h new file mode 100644 index 0000000000..40b217c30c --- /dev/null +++ b/media/codec2/components/gav1/C2SoftGav1Dec.h @@ -0,0 +1,68 @@ +/* + * Copyright (C) 2019 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. + */ + +#ifndef ANDROID_C2_SOFT_GAV1_DEC_H_ +#define ANDROID_C2_SOFT_GAV1_DEC_H_ + +#include + +namespace android { + +struct C2SoftGav1Dec : public SimpleC2Component { + class IntfImpl; + + C2SoftGav1Dec(const char* name, c2_node_id_t id, + const std::shared_ptr& intfImpl); + + // Begin SimpleC2Component overrides. + c2_status_t onInit() override; + c2_status_t onStop() override; + void onReset() override; + void onRelease() override; + c2_status_t onFlush_sm() override; + void process(const std::unique_ptr& work, + const std::shared_ptr& pool) override; + c2_status_t drain(uint32_t drainMode, + const std::shared_ptr& pool) override; + // End SimpleC2Component overrides. + + private: + std::shared_ptr mIntf; + + uint32_t mWidth; + uint32_t mHeight; + bool mSignalledOutputEos; + bool mSignalledError; + + struct timeval mTimeStart; // Time at the start of decode() + struct timeval mTimeEnd; // Time at the end of decode() + + bool initDecoder(); + void destroyDecoder(); + void finishWork(uint64_t index, const std::unique_ptr& work, + const std::shared_ptr& block); + bool outputBuffer(const std::shared_ptr& pool, + const std::unique_ptr& work); + c2_status_t drainInternal(uint32_t drainMode, + const std::shared_ptr& pool, + const std::unique_ptr& work); + + C2_DO_NOT_COPY(C2SoftGav1Dec); +}; + +} // namespace android + +#endif // ANDROID_C2_SOFT_GAV1_DEC_H_ diff --git a/media/codec2/vndk/C2Store.cpp b/media/codec2/vndk/C2Store.cpp index f8afa7c539..6b4ed35130 100644 --- a/media/codec2/vndk/C2Store.cpp +++ b/media/codec2/vndk/C2Store.cpp @@ -849,6 +849,7 @@ C2PlatformComponentStore::C2PlatformComponentStore() emplace("libcodec2_soft_amrwbdec.so"); emplace("libcodec2_soft_amrwbenc.so"); emplace("libcodec2_soft_av1dec.so"); + emplace("libcodec2_soft_gav1dec.so"); emplace("libcodec2_soft_avcdec.so"); emplace("libcodec2_soft_avcenc.so"); emplace("libcodec2_soft_flacdec.so"); diff --git a/services/mediacodec/registrant/Android.bp b/services/mediacodec/registrant/Android.bp index 17c2e0261b..e3893e5570 100644 --- a/services/mediacodec/registrant/Android.bp +++ b/services/mediacodec/registrant/Android.bp @@ -43,6 +43,7 @@ cc_library_shared { "libcodec2_soft_vp8dec", "libcodec2_soft_vp9dec", "libcodec2_soft_av1dec", + "libcodec2_soft_gav1dec", "libcodec2_soft_vp8enc", "libcodec2_soft_vp9enc", "libcodec2_soft_rawdec", From 0f3e7425b4908aed49a494b1e24ab1d5ea730d75 Mon Sep 17 00:00:00 2001 From: Vignesh Venkatasubramanian Date: Mon, 17 Jun 2019 16:21:36 -0700 Subject: [PATCH 09/34] codec2: Implement gav1 decoder component Note that this CL does not switch the default AV1 decoder to gav1 yet. That will be done in a follow-up. Test: CTS media tests related to AV1 still pass. Bug: 131989882 Bug: 130249450 Merged-In: Ifb88d0f51d85e59fa37d5cead8a5d4757067fedd Change-Id: Ifb88d0f51d85e59fa37d5cead8a5d4757067fedd --- media/codec2/components/gav1/Android.bp | 5 + .../codec2/components/gav1/C2SoftGav1Dec.cpp | 498 +++++++++++++++++- media/codec2/components/gav1/C2SoftGav1Dec.h | 9 + 3 files changed, 501 insertions(+), 11 deletions(-) diff --git a/media/codec2/components/gav1/Android.bp b/media/codec2/components/gav1/Android.bp index da76e9d023..0a0545d1fe 100644 --- a/media/codec2/components/gav1/Android.bp +++ b/media/codec2/components/gav1/Android.bp @@ -6,4 +6,9 @@ cc_library_shared { ], srcs: ["C2SoftGav1Dec.cpp"], + static_libs: ["libgav1"], + + include_dirs: [ + "external/libgav1/libgav1/", + ], } diff --git a/media/codec2/components/gav1/C2SoftGav1Dec.cpp b/media/codec2/components/gav1/C2SoftGav1Dec.cpp index f6dd14a5a9..3ba480a31e 100644 --- a/media/codec2/components/gav1/C2SoftGav1Dec.cpp +++ b/media/codec2/components/gav1/C2SoftGav1Dec.cpp @@ -256,20 +256,496 @@ C2SoftGav1Dec::C2SoftGav1Dec(const char *name, c2_node_id_t id, const std::shared_ptr &intfImpl) : SimpleC2Component( std::make_shared>(name, id, intfImpl)), - mIntf(intfImpl) {} - -c2_status_t C2SoftGav1Dec::onInit() { return C2_OK; } -c2_status_t C2SoftGav1Dec::onStop() { return C2_OK; } -void C2SoftGav1Dec::onReset() {} -void C2SoftGav1Dec::onRelease(){}; -c2_status_t C2SoftGav1Dec::onFlush_sm() { return C2_OK; } -void C2SoftGav1Dec::process(const std::unique_ptr & /*work*/, - const std::shared_ptr & /*pool*/) {} -c2_status_t C2SoftGav1Dec::drain( - uint32_t /*drainMode*/, const std::shared_ptr & /*pool*/) { + mIntf(intfImpl), + mCodecCtx(nullptr) { + gettimeofday(&mTimeStart, nullptr); + gettimeofday(&mTimeEnd, nullptr); +} + +C2SoftGav1Dec::~C2SoftGav1Dec() { onRelease(); } + +c2_status_t C2SoftGav1Dec::onInit() { + return initDecoder() ? C2_OK : C2_CORRUPTED; +} + +c2_status_t C2SoftGav1Dec::onStop() { + mSignalledError = false; + mSignalledOutputEos = false; + return C2_OK; +} + +void C2SoftGav1Dec::onReset() { + (void)onStop(); + c2_status_t err = onFlush_sm(); + if (err != C2_OK) { + ALOGW("Failed to flush the av1 decoder. Trying to hard reset."); + destroyDecoder(); + if (!initDecoder()) { + ALOGE("Hard reset failed."); + } + } +} + +void C2SoftGav1Dec::onRelease() { destroyDecoder(); } + +c2_status_t C2SoftGav1Dec::onFlush_sm() { + Libgav1StatusCode status = + mCodecCtx->EnqueueFrame(/*data=*/nullptr, /*size=*/0, + /*user_private_data=*/0); + if (status != kLibgav1StatusOk) { + ALOGE("Failed to flush av1 decoder. status: %d.", status); + return C2_CORRUPTED; + } + + // Dequeue frame (if any) that was enqueued previously. + const libgav1::DecoderBuffer *buffer; + status = mCodecCtx->DequeueFrame(&buffer); + if (status != kLibgav1StatusOk) { + ALOGE("Failed to dequeue frame after flushing the av1 decoder. status: %d", + status); + return C2_CORRUPTED; + } + + mSignalledError = false; + mSignalledOutputEos = false; + + return C2_OK; +} + +static int GetCPUCoreCount() { + int cpuCoreCount = 1; +#if defined(_SC_NPROCESSORS_ONLN) + cpuCoreCount = sysconf(_SC_NPROCESSORS_ONLN); +#else + // _SC_NPROC_ONLN must be defined... + cpuCoreCount = sysconf(_SC_NPROC_ONLN); +#endif + CHECK(cpuCoreCount >= 1); + ALOGV("Number of CPU cores: %d", cpuCoreCount); + return cpuCoreCount; +} + +bool C2SoftGav1Dec::initDecoder() { + mSignalledError = false; + mSignalledOutputEos = false; + mCodecCtx.reset(new libgav1::Decoder()); + + if (mCodecCtx == nullptr) { + ALOGE("mCodecCtx is null"); + return false; + } + + libgav1::DecoderSettings settings = {}; + settings.threads = GetCPUCoreCount(); + + Libgav1StatusCode status = mCodecCtx->Init(&settings); + if (status != kLibgav1StatusOk) { + ALOGE("av1 decoder failed to initialize. status: %d.", status); + return false; + } + + return true; +} + +void C2SoftGav1Dec::destroyDecoder() { mCodecCtx = nullptr; } + +void fillEmptyWork(const std::unique_ptr &work) { + uint32_t flags = 0; + if (work->input.flags & C2FrameData::FLAG_END_OF_STREAM) { + flags |= C2FrameData::FLAG_END_OF_STREAM; + ALOGV("signalling eos"); + } + work->worklets.front()->output.flags = (C2FrameData::flags_t)flags; + work->worklets.front()->output.buffers.clear(); + work->worklets.front()->output.ordinal = work->input.ordinal; + work->workletsProcessed = 1u; +} + +void C2SoftGav1Dec::finishWork(uint64_t index, + const std::unique_ptr &work, + const std::shared_ptr &block) { + std::shared_ptr buffer = + createGraphicBuffer(block, C2Rect(mWidth, mHeight)); + auto fillWork = [buffer, index](const std::unique_ptr &work) { + uint32_t flags = 0; + if ((work->input.flags & C2FrameData::FLAG_END_OF_STREAM) && + (c2_cntr64_t(index) == work->input.ordinal.frameIndex)) { + flags |= C2FrameData::FLAG_END_OF_STREAM; + ALOGV("signalling eos"); + } + work->worklets.front()->output.flags = (C2FrameData::flags_t)flags; + work->worklets.front()->output.buffers.clear(); + work->worklets.front()->output.buffers.push_back(buffer); + work->worklets.front()->output.ordinal = work->input.ordinal; + work->workletsProcessed = 1u; + }; + if (work && c2_cntr64_t(index) == work->input.ordinal.frameIndex) { + fillWork(work); + } else { + finish(index, fillWork); + } +} + +void C2SoftGav1Dec::process(const std::unique_ptr &work, + const std::shared_ptr &pool) { + work->result = C2_OK; + work->workletsProcessed = 0u; + work->worklets.front()->output.configUpdate.clear(); + work->worklets.front()->output.flags = work->input.flags; + if (mSignalledError || mSignalledOutputEos) { + work->result = C2_BAD_VALUE; + return; + } + + size_t inOffset = 0u; + size_t inSize = 0u; + C2ReadView rView = mDummyReadView; + if (!work->input.buffers.empty()) { + rView = work->input.buffers[0]->data().linearBlocks().front().map().get(); + inSize = rView.capacity(); + if (inSize && rView.error()) { + ALOGE("read view map failed %d", rView.error()); + work->result = C2_CORRUPTED; + return; + } + } + + bool codecConfig = + ((work->input.flags & C2FrameData::FLAG_CODEC_CONFIG) != 0); + bool eos = ((work->input.flags & C2FrameData::FLAG_END_OF_STREAM) != 0); + + ALOGV("in buffer attr. size %zu timestamp %d frameindex %d, flags %x", inSize, + (int)work->input.ordinal.timestamp.peeku(), + (int)work->input.ordinal.frameIndex.peeku(), work->input.flags); + + if (codecConfig) { + fillEmptyWork(work); + return; + } + + int64_t frameIndex = work->input.ordinal.frameIndex.peekll(); + if (inSize) { + uint8_t *bitstream = const_cast(rView.data() + inOffset); + int32_t decodeTime = 0; + int32_t delay = 0; + + GETTIME(&mTimeStart, nullptr); + TIME_DIFF(mTimeEnd, mTimeStart, delay); + + const Libgav1StatusCode status = + mCodecCtx->EnqueueFrame(bitstream, inSize, frameIndex); + + GETTIME(&mTimeEnd, nullptr); + TIME_DIFF(mTimeStart, mTimeEnd, decodeTime); + ALOGV("decodeTime=%4d delay=%4d\n", decodeTime, delay); + + if (status != kLibgav1StatusOk) { + ALOGE("av1 decoder failed to decode frame. status: %d.", status); + work->result = C2_CORRUPTED; + work->workletsProcessed = 1u; + mSignalledError = true; + return; + } + + } else { + const Libgav1StatusCode status = + mCodecCtx->EnqueueFrame(/*data=*/nullptr, /*size=*/0, + /*user_private_data=*/0); + if (status != kLibgav1StatusOk) { + ALOGE("Failed to flush av1 decoder. status: %d.", status); + work->result = C2_CORRUPTED; + work->workletsProcessed = 1u; + mSignalledError = true; + return; + } + } + + (void)outputBuffer(pool, work); + + if (eos) { + drainInternal(DRAIN_COMPONENT_WITH_EOS, pool, work); + mSignalledOutputEos = true; + } else if (!inSize) { + fillEmptyWork(work); + } +} + +static void copyOutputBufferToYV12Frame(uint8_t *dst, const uint8_t *srcY, + const uint8_t *srcU, + const uint8_t *srcV, size_t srcYStride, + size_t srcUStride, size_t srcVStride, + uint32_t width, uint32_t height) { + const size_t dstYStride = align(width, 16); + const size_t dstUVStride = align(dstYStride / 2, 16); + uint8_t *const dstStart = dst; + + for (size_t i = 0; i < height; ++i) { + memcpy(dst, srcY, width); + srcY += srcYStride; + dst += dstYStride; + } + + dst = dstStart + dstYStride * height; + for (size_t i = 0; i < height / 2; ++i) { + memcpy(dst, srcV, width / 2); + srcV += srcVStride; + dst += dstUVStride; + } + + dst = dstStart + (dstYStride * height) + (dstUVStride * height / 2); + for (size_t i = 0; i < height / 2; ++i) { + memcpy(dst, srcU, width / 2); + srcU += srcUStride; + dst += dstUVStride; + } +} + +static void convertYUV420Planar16ToY410(uint32_t *dst, const uint16_t *srcY, + const uint16_t *srcU, + const uint16_t *srcV, size_t srcYStride, + size_t srcUStride, size_t srcVStride, + size_t dstStride, size_t width, + size_t height) { + // Converting two lines at a time, slightly faster + for (size_t y = 0; y < height; y += 2) { + uint32_t *dstTop = (uint32_t *)dst; + uint32_t *dstBot = (uint32_t *)(dst + dstStride); + uint16_t *ySrcTop = (uint16_t *)srcY; + uint16_t *ySrcBot = (uint16_t *)(srcY + srcYStride); + uint16_t *uSrc = (uint16_t *)srcU; + uint16_t *vSrc = (uint16_t *)srcV; + + uint32_t u01, v01, y01, y23, y45, y67, uv0, uv1; + size_t x = 0; + for (; x < width - 3; x += 4) { + u01 = *((uint32_t *)uSrc); + uSrc += 2; + v01 = *((uint32_t *)vSrc); + vSrc += 2; + + y01 = *((uint32_t *)ySrcTop); + ySrcTop += 2; + y23 = *((uint32_t *)ySrcTop); + ySrcTop += 2; + y45 = *((uint32_t *)ySrcBot); + ySrcBot += 2; + y67 = *((uint32_t *)ySrcBot); + ySrcBot += 2; + + uv0 = (u01 & 0x3FF) | ((v01 & 0x3FF) << 20); + uv1 = (u01 >> 16) | ((v01 >> 16) << 20); + + *dstTop++ = 3 << 30 | ((y01 & 0x3FF) << 10) | uv0; + *dstTop++ = 3 << 30 | ((y01 >> 16) << 10) | uv0; + *dstTop++ = 3 << 30 | ((y23 & 0x3FF) << 10) | uv1; + *dstTop++ = 3 << 30 | ((y23 >> 16) << 10) | uv1; + + *dstBot++ = 3 << 30 | ((y45 & 0x3FF) << 10) | uv0; + *dstBot++ = 3 << 30 | ((y45 >> 16) << 10) | uv0; + *dstBot++ = 3 << 30 | ((y67 & 0x3FF) << 10) | uv1; + *dstBot++ = 3 << 30 | ((y67 >> 16) << 10) | uv1; + } + + // There should be at most 2 more pixels to process. Note that we don't + // need to consider odd case as the buffer is always aligned to even. + if (x < width) { + u01 = *uSrc; + v01 = *vSrc; + y01 = *((uint32_t *)ySrcTop); + y45 = *((uint32_t *)ySrcBot); + uv0 = (u01 & 0x3FF) | ((v01 & 0x3FF) << 20); + *dstTop++ = ((y01 & 0x3FF) << 10) | uv0; + *dstTop++ = ((y01 >> 16) << 10) | uv0; + *dstBot++ = ((y45 & 0x3FF) << 10) | uv0; + *dstBot++ = ((y45 >> 16) << 10) | uv0; + } + + srcY += srcYStride * 2; + srcU += srcUStride; + srcV += srcVStride; + dst += dstStride * 2; + } +} + +static void convertYUV420Planar16ToYUV420Planar( + uint8_t *dst, const uint16_t *srcY, const uint16_t *srcU, + const uint16_t *srcV, size_t srcYStride, size_t srcUStride, + size_t srcVStride, size_t dstStride, size_t width, size_t height) { + uint8_t *dstY = (uint8_t *)dst; + size_t dstYSize = dstStride * height; + size_t dstUVStride = align(dstStride / 2, 16); + size_t dstUVSize = dstUVStride * height / 2; + uint8_t *dstV = dstY + dstYSize; + uint8_t *dstU = dstV + dstUVSize; + + for (size_t y = 0; y < height; ++y) { + for (size_t x = 0; x < width; ++x) { + dstY[x] = (uint8_t)(srcY[x] >> 2); + } + + srcY += srcYStride; + dstY += dstStride; + } + + for (size_t y = 0; y < (height + 1) / 2; ++y) { + for (size_t x = 0; x < (width + 1) / 2; ++x) { + dstU[x] = (uint8_t)(srcU[x] >> 2); + dstV[x] = (uint8_t)(srcV[x] >> 2); + } + + srcU += srcUStride; + srcV += srcVStride; + dstU += dstUVStride; + dstV += dstUVStride; + } +} + +bool C2SoftGav1Dec::outputBuffer(const std::shared_ptr &pool, + const std::unique_ptr &work) { + if (!(work && pool)) return false; + + const libgav1::DecoderBuffer *buffer; + const Libgav1StatusCode status = mCodecCtx->DequeueFrame(&buffer); + + if (status != kLibgav1StatusOk) { + ALOGE("av1 decoder DequeueFrame failed. status: %d.", status); + return false; + } + + // |buffer| can be NULL if status was equal to kLibgav1StatusOk. This is not + // an error. This could mean one of two things: + // - The EnqueueFrame() call was either a flush (called with nullptr). + // - The enqueued frame did not have any displayable frames. + if (!buffer) { + return false; + } + + const int width = buffer->displayed_width[0]; + const int height = buffer->displayed_height[0]; + if (width != mWidth || height != mHeight) { + mWidth = width; + mHeight = height; + + C2StreamPictureSizeInfo::output size(0u, mWidth, mHeight); + std::vector> failures; + c2_status_t err = mIntf->config({&size}, C2_MAY_BLOCK, &failures); + if (err == C2_OK) { + work->worklets.front()->output.configUpdate.push_back( + C2Param::Copy(size)); + } else { + ALOGE("Config update size failed"); + mSignalledError = true; + work->result = C2_CORRUPTED; + work->workletsProcessed = 1u; + return false; + } + } + + // TODO(vigneshv): Add support for monochrome videos since AV1 supports it. + CHECK(buffer->image_format == libgav1::kImageFormatYuv420); + + std::shared_ptr block; + uint32_t format = HAL_PIXEL_FORMAT_YV12; + if (buffer->bitdepth == 10) { + IntfImpl::Lock lock = mIntf->lock(); + std::shared_ptr defaultColorAspects = + mIntf->getDefaultColorAspects_l(); + + if (defaultColorAspects->primaries == C2Color::PRIMARIES_BT2020 && + defaultColorAspects->matrix == C2Color::MATRIX_BT2020 && + defaultColorAspects->transfer == C2Color::TRANSFER_ST2084) { + format = HAL_PIXEL_FORMAT_RGBA_1010102; + } + } + C2MemoryUsage usage = {C2MemoryUsage::CPU_READ, C2MemoryUsage::CPU_WRITE}; + + c2_status_t err = pool->fetchGraphicBlock(align(mWidth, 16), mHeight, format, + usage, &block); + + if (err != C2_OK) { + ALOGE("fetchGraphicBlock for Output failed with status %d", err); + work->result = err; + return false; + } + + C2GraphicView wView = block->map().get(); + + if (wView.error()) { + ALOGE("graphic view map failed %d", wView.error()); + work->result = C2_CORRUPTED; + return false; + } + + ALOGV("provided (%dx%d) required (%dx%d), out frameindex %d", block->width(), + block->height(), mWidth, mHeight, (int)buffer->user_private_data); + + uint8_t *dst = const_cast(wView.data()[C2PlanarLayout::PLANE_Y]); + size_t srcYStride = buffer->stride[0]; + size_t srcUStride = buffer->stride[1]; + size_t srcVStride = buffer->stride[2]; + + if (buffer->bitdepth == 10) { + const uint16_t *srcY = (const uint16_t *)buffer->plane[0]; + const uint16_t *srcU = (const uint16_t *)buffer->plane[1]; + const uint16_t *srcV = (const uint16_t *)buffer->plane[2]; + + if (format == HAL_PIXEL_FORMAT_RGBA_1010102) { + convertYUV420Planar16ToY410( + (uint32_t *)dst, srcY, srcU, srcV, srcYStride / 2, srcUStride / 2, + srcVStride / 2, align(mWidth, 16), mWidth, mHeight); + } else { + convertYUV420Planar16ToYUV420Planar(dst, srcY, srcU, srcV, srcYStride / 2, + srcUStride / 2, srcVStride / 2, + align(mWidth, 16), mWidth, mHeight); + } + } else { + const uint8_t *srcY = (const uint8_t *)buffer->plane[0]; + const uint8_t *srcU = (const uint8_t *)buffer->plane[1]; + const uint8_t *srcV = (const uint8_t *)buffer->plane[2]; + copyOutputBufferToYV12Frame(dst, srcY, srcU, srcV, srcYStride, srcUStride, + srcVStride, mWidth, mHeight); + } + finishWork(buffer->user_private_data, work, std::move(block)); + block = nullptr; + return true; +} + +c2_status_t C2SoftGav1Dec::drainInternal( + uint32_t drainMode, const std::shared_ptr &pool, + const std::unique_ptr &work) { + if (drainMode == NO_DRAIN) { + ALOGW("drain with NO_DRAIN: no-op"); + return C2_OK; + } + if (drainMode == DRAIN_CHAIN) { + ALOGW("DRAIN_CHAIN not supported"); + return C2_OMITTED; + } + + Libgav1StatusCode status = + mCodecCtx->EnqueueFrame(/*data=*/nullptr, /*size=*/0, + /*user_private_data=*/0); + if (status != kLibgav1StatusOk) { + ALOGE("Failed to flush av1 decoder. status: %d.", status); + return C2_CORRUPTED; + } + + while (outputBuffer(pool, work)) { + } + + if (drainMode == DRAIN_COMPONENT_WITH_EOS && work && + work->workletsProcessed == 0u) { + fillEmptyWork(work); + } + return C2_OK; } +c2_status_t C2SoftGav1Dec::drain(uint32_t drainMode, + const std::shared_ptr &pool) { + return drainInternal(drainMode, pool, nullptr); +} + class C2SoftGav1Factory : public C2ComponentFactory { public: C2SoftGav1Factory() diff --git a/media/codec2/components/gav1/C2SoftGav1Dec.h b/media/codec2/components/gav1/C2SoftGav1Dec.h index 40b217c30c..a7c08bbc39 100644 --- a/media/codec2/components/gav1/C2SoftGav1Dec.h +++ b/media/codec2/components/gav1/C2SoftGav1Dec.h @@ -18,6 +18,13 @@ #define ANDROID_C2_SOFT_GAV1_DEC_H_ #include +#include "libgav1/src/decoder.h" +#include "libgav1/src/decoder_settings.h" + +#define GETTIME(a, b) gettimeofday(a, b); +#define TIME_DIFF(start, end, diff) \ + diff = (((end).tv_sec - (start).tv_sec) * 1000000) + \ + ((end).tv_usec - (start).tv_usec); namespace android { @@ -26,6 +33,7 @@ struct C2SoftGav1Dec : public SimpleC2Component { C2SoftGav1Dec(const char* name, c2_node_id_t id, const std::shared_ptr& intfImpl); + ~C2SoftGav1Dec(); // Begin SimpleC2Component overrides. c2_status_t onInit() override; @@ -41,6 +49,7 @@ struct C2SoftGav1Dec : public SimpleC2Component { private: std::shared_ptr mIntf; + std::unique_ptr mCodecCtx; uint32_t mWidth; uint32_t mHeight; From 61ba2cf252e63accb97d16b8c74448587fddcdf0 Mon Sep 17 00:00:00 2001 From: Vignesh Venkatasubramanian Date: Mon, 24 Jun 2019 10:04:00 -0700 Subject: [PATCH 10/34] codec2: Add log message for the AV1 decoder Add a log message about which AV1 decoder is being used. Also remove the TODO for renaming the gav1 component. Bug: 131989882 Bug: 130249450 Test: cts-tradefed run commandAndExit cts-dev --include-filter CtsMediaTestCases --module-arg "CtsMediaTestCases:include-filter:android.media.cts.DecoderTest#testAV1*" --module-arg "CtsMediaTestCases:include-filter:android.media.cts.AdaptivePlaybackTest#testAV1*" Merged-In: I3eec15cc74d8ab8f9e6d4539a25407cf93d4efeb Change-Id: I3eec15cc74d8ab8f9e6d4539a25407cf93d4efeb --- media/codec2/components/aom/C2SoftAomDec.cpp | 1 + media/codec2/components/gav1/C2SoftGav1Dec.cpp | 3 +-- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/media/codec2/components/aom/C2SoftAomDec.cpp b/media/codec2/components/aom/C2SoftAomDec.cpp index 769895cec9..0cf277ff88 100644 --- a/media/codec2/components/aom/C2SoftAomDec.cpp +++ b/media/codec2/components/aom/C2SoftAomDec.cpp @@ -340,6 +340,7 @@ status_t C2SoftAomDec::initDecoder() { aom_codec_flags_t flags; memset(&flags, 0, sizeof(aom_codec_flags_t)); + ALOGV("Using libaom AV1 software decoder."); aom_codec_err_t err; if ((err = aom_codec_dec_init(mCodecCtx, aom_codec_av1_dx(), &cfg, 0))) { ALOGE("av1 decoder failed to initialize. (%d)", err); diff --git a/media/codec2/components/gav1/C2SoftGav1Dec.cpp b/media/codec2/components/gav1/C2SoftGav1Dec.cpp index 3ba480a31e..f5321bacb9 100644 --- a/media/codec2/components/gav1/C2SoftGav1Dec.cpp +++ b/media/codec2/components/gav1/C2SoftGav1Dec.cpp @@ -27,8 +27,6 @@ namespace android { -// TODO(vigneshv): This will be changed to c2.android.av1.decoder once this -// component is fully functional. constexpr char COMPONENT_NAME[] = "c2.android.gav1.decoder"; class C2SoftGav1Dec::IntfImpl : public SimpleInterface::BaseParams { @@ -338,6 +336,7 @@ bool C2SoftGav1Dec::initDecoder() { libgav1::DecoderSettings settings = {}; settings.threads = GetCPUCoreCount(); + ALOGV("Using libgav1 AV1 software decoder."); Libgav1StatusCode status = mCodecCtx->Init(&settings); if (status != kLibgav1StatusOk) { ALOGE("av1 decoder failed to initialize. status: %d.", status); From 2f0fa55f83b151518ea0401e614a1cbf8d44a6bd Mon Sep 17 00:00:00 2001 From: Vignesh Venkatasubramanian Date: Wed, 26 Jun 2019 15:22:29 -0700 Subject: [PATCH 11/34] stagefright: Use libgav1 for AV1 decoding Use the c2.android.gav1.decoder component instead of c2.android.av1.decoder component. All the AV1 related CTS tests still pass. Bug: 131989882 Bug: 130249450 Test: cts-tradefed run commandAndExit cts-dev --include-filter CtsMediaTestCases --module-arg "CtsMediaTestCases:include-filter:android.media.cts.DecoderTest#testAV1*" --module-arg "CtsMediaTestCases:include-filter:android.media.cts.AdaptivePlaybackTest#testAV1*" Merged-In: I519040c7a7974058d1d2a4f210bb93f1c8f50ea1 Change-Id: I519040c7a7974058d1d2a4f210bb93f1c8f50ea1 --- media/libstagefright/data/media_codecs_google_c2_video.xml | 2 +- media/libstagefright/data/media_codecs_sw.xml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/media/libstagefright/data/media_codecs_google_c2_video.xml b/media/libstagefright/data/media_codecs_google_c2_video.xml index 04041ebfc1..a07eb8c5db 100644 --- a/media/libstagefright/data/media_codecs_google_c2_video.xml +++ b/media/libstagefright/data/media_codecs_google_c2_video.xml @@ -77,7 +77,7 @@ - + diff --git a/media/libstagefright/data/media_codecs_sw.xml b/media/libstagefright/data/media_codecs_sw.xml index 67d3f1af12..9532ba6b66 100644 --- a/media/libstagefright/data/media_codecs_sw.xml +++ b/media/libstagefright/data/media_codecs_sw.xml @@ -182,7 +182,7 @@ - + From f08d61d30e9ed041d7426c0168e2ee092eaf36ac Mon Sep 17 00:00:00 2001 From: Pawin Vongmasa Date: Fri, 9 Aug 2019 21:57:44 -0700 Subject: [PATCH 12/34] Codec2 VTS: Move device resource files to data/local/tmp Some devices make /sdcard a symbolic link to a non-constant target. The target changes between the setup and the execution, so files pushed to /sdcard during the setup cannot be found when the test runs. Test: vts-tradefed run vts -m VtsHalMediaC2V1_0Host Bug: 138388013 Change-Id: Ia27bd9eedfc6f97225abb553c4b8efbb8957966e --- .../hidl/1.0/vts/functional/common/README.md | 43 +++++++++++-------- .../common/media_c2_hidl_test_common.h | 2 +- 2 files changed, 25 insertions(+), 20 deletions(-) diff --git a/media/codec2/hidl/1.0/vts/functional/common/README.md b/media/codec2/hidl/1.0/vts/functional/common/README.md index 50e8356f9e..f2f579c98d 100644 --- a/media/codec2/hidl/1.0/vts/functional/common/README.md +++ b/media/codec2/hidl/1.0/vts/functional/common/README.md @@ -1,31 +1,36 @@ -## Codec2 VTS Hal @ 1.0 tests ## ---- -#### master : +# Codec2 VTS Hal @ 1.0 tests # + +## master : Functionality of master is to enumerate all the Codec2 components available in C2 media service. -usage: VtsHalMediaC2V1\_0TargetMasterTest -I default +usage: `VtsHalMediaC2V1_0TargetMasterTest -I default` -#### component : +## component : Functionality of component test is to validate common functionality across all the Codec2 components available in C2 media service. For a standard C2 component, these tests are expected to pass. -usage: VtsHalMediaC2V1\_0TargetComponentTest -I software -C -example: VtsHalMediaC2V1\_0TargetComponentTest -I software -C c2.android.vorbis.decoder +usage: `VtsHalMediaC2V1_0TargetComponentTest -I software -C ` + +example: `VtsHalMediaC2V1_0TargetComponentTest -I software -C c2.android.vorbis.decoder` + +## audio : +Functionality of audio test is to validate audio specific functionality of Codec2 components. The resource files for this test are taken from `frameworks/av/media/codec2/hidl/1.0/vts/functional/res`. The path to these files on the device can be specified with `-P`. (If the device path is omitted, `/data/local/tmp/media/` is the default value.) + +usage: `VtsHalMediaC2V1_0TargetAudioDecTest -I default -C -P ` + +usage: `VtsHalMediaC2V1_0TargetAudioEncTest -I software -C -P ` + +example: `VtsHalMediaC2V1_0TargetAudioDecTest -I software -C c2.android.flac.decoder -P /data/local/tmp/media/` -#### audio : -Functionality of audio test is to validate audio specific functionality Codec2 components. The resource files for this test are taken from media/codec2/hidl/1.0/vts/functional/res. The path to these files on the device is required to be given for bitstream tests. +example: `VtsHalMediaC2V1_0TargetAudioEncTest -I software -C c2.android.opus.encoder -P /data/local/tmp/media/` -usage: VtsHalMediaC2V1\_0TargetAudioDecTest -I default -C -P /sdcard/media/ -usage: VtsHalMediaC2V1\_0TargetAudioEncTest -I software -C -P /sdcard/media/ +## video : +Functionality of video test is to validate video specific functionality of Codec2 components. The resource files for this test are taken from `frameworks/av/media/codec2/hidl/1.0/vts/functional/res`. The path to these files on the device can be specified with `-P`. (If the device path is omitted, `/data/local/tmp/media/` is the default value.) -example: VtsHalMediaC2V1\_0TargetAudioDecTest -I software -C c2.android.flac.decoder -P /sdcard/media/ -example: VtsHalMediaC2V1\_0TargetAudioEncTest -I software -C c2.android.opus.encoder -P /sdcard/media/ +usage: `VtsHalMediaC2V1_0TargetVideoDecTest -I default -C -P ` -#### video : -Functionality of video test is to validate video specific functionality Codec2 components. The resource files for this test are taken from media/codec2/hidl/1.0/vts/functional/res. The path to these files on the device is required to be given for bitstream tests. +usage: `VtsHalMediaC2V1_0TargetVideoEncTest -I software -C -P ` -usage: VtsHalMediaC2V1\_0TargetVideoDecTest -I default -C -P /sdcard/media/ -usage: VtsHalMediaC2V1\_0TargetVideoEncTest -I software -C -P /sdcard/media/ +example: `VtsHalMediaC2V1_0TargetVideoDecTest -I software -C c2.android.avc.decoder -P /data/local/tmp/media/` -example: VtsHalMediaC2V1\_0TargetVideoDecTest -I software -C c2.android.avc.decoder -P /sdcard/media/ -example: VtsHalMediaC2V1\_0TargetVideoEncTest -I software -C c2.android.vp9.encoder -P /sdcard/media/ +example: `VtsHalMediaC2V1_0TargetVideoEncTest -I software -C c2.android.vp9.encoder -P /data/local/tmp/media/` diff --git a/media/codec2/hidl/1.0/vts/functional/common/media_c2_hidl_test_common.h b/media/codec2/hidl/1.0/vts/functional/common/media_c2_hidl_test_common.h index c577dac6d7..db59e547a8 100644 --- a/media/codec2/hidl/1.0/vts/functional/common/media_c2_hidl_test_common.h +++ b/media/codec2/hidl/1.0/vts/functional/common/media_c2_hidl_test_common.h @@ -118,7 +118,7 @@ class ComponentTestEnvironment : public ::testing::VtsHalHidlTargetTestEnvBase { registerTestService(); } - ComponentTestEnvironment() : res("/sdcard/media/") {} + ComponentTestEnvironment() : res("/data/local/tmp/media/") {} void setComponent(const char* _component) { component = _component; } From c2cc437a64593f0f49a040bd055a7e626304d578 Mon Sep 17 00:00:00 2001 From: Ray Essick Date: Wed, 21 Aug 2019 14:02:28 -0700 Subject: [PATCH 13/34] Switch av1 implementations from aom to gav1 Deprecate the aom implementation in favor of the faster/smaller gav1 implementation. Bug: 130249450 Bug: 131989882 Test: playback multiple av1 clips Change-Id: I04a3507376e32b1f7a037809cbb7848e65a404e5 --- media/codec2/components/aom/Android.bp | 8 +++++++- media/codec2/components/aom/C2SoftAomDec.cpp | 3 ++- media/codec2/components/gav1/Android.bp | 8 +++++++- media/codec2/components/gav1/C2SoftGav1Dec.cpp | 3 ++- media/codec2/vndk/C2Store.cpp | 4 ++-- .../libstagefright/data/media_codecs_google_c2_video.xml | 2 +- media/libstagefright/data/media_codecs_sw.xml | 2 +- services/mediacodec/registrant/Android.bp | 4 ++-- 8 files changed, 24 insertions(+), 10 deletions(-) diff --git a/media/codec2/components/aom/Android.bp b/media/codec2/components/aom/Android.bp index 0fabf5cf87..61dbd4ccc2 100644 --- a/media/codec2/components/aom/Android.bp +++ b/media/codec2/components/aom/Android.bp @@ -1,10 +1,16 @@ cc_library_shared { - name: "libcodec2_soft_av1dec", + name: "libcodec2_soft_av1dec_aom", defaults: [ "libcodec2_soft-defaults", "libcodec2_soft_sanitize_all-defaults", ], + // coordinated with frameworks/av/media/codec2/components/gav1/Android.bp + // so only 1 of them has the official c2.android.av1.decoder name + cflags: [ + "-DCODECNAME=\"c2.android.av1-aom.decoder\"", + ], + srcs: ["C2SoftAomDec.cpp"], static_libs: ["libaom"], diff --git a/media/codec2/components/aom/C2SoftAomDec.cpp b/media/codec2/components/aom/C2SoftAomDec.cpp index 0cf277ff88..36137e6089 100644 --- a/media/codec2/components/aom/C2SoftAomDec.cpp +++ b/media/codec2/components/aom/C2SoftAomDec.cpp @@ -29,7 +29,8 @@ namespace android { -constexpr char COMPONENT_NAME[] = "c2.android.av1.decoder"; +// codecname set and passed in as a compile flag from Android.bp +constexpr char COMPONENT_NAME[] = CODECNAME; class C2SoftAomDec::IntfImpl : public SimpleInterface::BaseParams { public: diff --git a/media/codec2/components/gav1/Android.bp b/media/codec2/components/gav1/Android.bp index 0a0545d1fe..5c4abb7e30 100644 --- a/media/codec2/components/gav1/Android.bp +++ b/media/codec2/components/gav1/Android.bp @@ -1,10 +1,16 @@ cc_library_shared { - name: "libcodec2_soft_gav1dec", + name: "libcodec2_soft_av1dec_gav1", defaults: [ "libcodec2_soft-defaults", "libcodec2_soft_sanitize_all-defaults", ], + // coordinated with frameworks/av/media/codec2/components/aom/Android.bp + // so only 1 of them has the official c2.android.av1.decoder name + cflags: [ + "-DCODECNAME=\"c2.android.av1.decoder\"", + ], + srcs: ["C2SoftGav1Dec.cpp"], static_libs: ["libgav1"], diff --git a/media/codec2/components/gav1/C2SoftGav1Dec.cpp b/media/codec2/components/gav1/C2SoftGav1Dec.cpp index f5321bacb9..ec5f549049 100644 --- a/media/codec2/components/gav1/C2SoftGav1Dec.cpp +++ b/media/codec2/components/gav1/C2SoftGav1Dec.cpp @@ -27,7 +27,8 @@ namespace android { -constexpr char COMPONENT_NAME[] = "c2.android.gav1.decoder"; +// codecname set and passed in as a compile flag from Android.bp +constexpr char COMPONENT_NAME[] = CODECNAME; class C2SoftGav1Dec::IntfImpl : public SimpleInterface::BaseParams { public: diff --git a/media/codec2/vndk/C2Store.cpp b/media/codec2/vndk/C2Store.cpp index 6b4ed35130..5b2bd7b62f 100644 --- a/media/codec2/vndk/C2Store.cpp +++ b/media/codec2/vndk/C2Store.cpp @@ -848,8 +848,8 @@ C2PlatformComponentStore::C2PlatformComponentStore() emplace("libcodec2_soft_amrnbenc.so"); emplace("libcodec2_soft_amrwbdec.so"); emplace("libcodec2_soft_amrwbenc.so"); - emplace("libcodec2_soft_av1dec.so"); - emplace("libcodec2_soft_gav1dec.so"); + //emplace("libcodec2_soft_av1dec_aom.so"); // deprecated for the gav1 implementation + emplace("libcodec2_soft_av1dec_gav1.so"); emplace("libcodec2_soft_avcdec.so"); emplace("libcodec2_soft_avcenc.so"); emplace("libcodec2_soft_flacdec.so"); diff --git a/media/libstagefright/data/media_codecs_google_c2_video.xml b/media/libstagefright/data/media_codecs_google_c2_video.xml index a07eb8c5db..04041ebfc1 100644 --- a/media/libstagefright/data/media_codecs_google_c2_video.xml +++ b/media/libstagefright/data/media_codecs_google_c2_video.xml @@ -77,7 +77,7 @@ - + diff --git a/media/libstagefright/data/media_codecs_sw.xml b/media/libstagefright/data/media_codecs_sw.xml index 9532ba6b66..67d3f1af12 100644 --- a/media/libstagefright/data/media_codecs_sw.xml +++ b/media/libstagefright/data/media_codecs_sw.xml @@ -182,7 +182,7 @@ - + diff --git a/services/mediacodec/registrant/Android.bp b/services/mediacodec/registrant/Android.bp index e3893e5570..fa5bc4a15a 100644 --- a/services/mediacodec/registrant/Android.bp +++ b/services/mediacodec/registrant/Android.bp @@ -42,8 +42,8 @@ cc_library_shared { "libcodec2_soft_opusenc", "libcodec2_soft_vp8dec", "libcodec2_soft_vp9dec", - "libcodec2_soft_av1dec", - "libcodec2_soft_gav1dec", + // "libcodec2_soft_av1dec_aom", // replaced by the gav1 implementation + "libcodec2_soft_av1dec_gav1", "libcodec2_soft_vp8enc", "libcodec2_soft_vp9enc", "libcodec2_soft_rawdec", From cf9526cffcf206d38b81de5651d1ecedf09921f6 Mon Sep 17 00:00:00 2001 From: Sundong Ahn Date: Thu, 22 Aug 2019 22:54:02 +0900 Subject: [PATCH 14/34] Add sub-elements for "Variant" type The Quirk, Attribute, Alias, Limit and Feature elements are added to "Variant" type for fixing VTS error. Bug: 139506623 Test: m -j40 vts && vts-tradefed run vts -m VtsValidateMediaCodecs Change-Id: I55066cc1de8b820c24faccdfb469377c897e9808 --- media/libstagefright/xmlparser/api/current.txt | 5 +++++ media/libstagefright/xmlparser/media_codecs.xsd | 7 +++++++ 2 files changed, 12 insertions(+) diff --git a/media/libstagefright/xmlparser/api/current.txt b/media/libstagefright/xmlparser/api/current.txt index 9d7c57dfc8..16c8af8c84 100644 --- a/media/libstagefright/xmlparser/api/current.txt +++ b/media/libstagefright/xmlparser/api/current.txt @@ -138,7 +138,12 @@ package media.codecs { public class Variant { ctor public Variant(); + method public java.util.List getAlias_optional(); + method public java.util.List getAttribute_optional(); + method public java.util.List getFeature_optional(); + method public java.util.List getLimit_optional(); method public String getName(); + method public java.util.List getQuirk_optional(); method public void setName(String); } diff --git a/media/libstagefright/xmlparser/media_codecs.xsd b/media/libstagefright/xmlparser/media_codecs.xsd index 63ec5d049a..3b5681f6fe 100644 --- a/media/libstagefright/xmlparser/media_codecs.xsd +++ b/media/libstagefright/xmlparser/media_codecs.xsd @@ -107,6 +107,13 @@ + + + + + + + From dd726805942fdd953509a2038ecb88a18dc63081 Mon Sep 17 00:00:00 2001 From: Chong Zhang Date: Wed, 21 Aug 2019 17:24:13 -0700 Subject: [PATCH 15/34] Add more testing for ResourceManagerService Refactor the calls to other system services to an interface to allow more testing. Add test for callbacks to BatteryStatsService and SchedulingPolicyService. bug: 138381810 bug: 139440234 test: presubmit Change-Id: I04408ad5a126e9f4083b5806b228449cb432acf6 --- .../ResourceManagerService.cpp | 42 ++++- .../ResourceManagerService.h | 13 +- .../test/ResourceManagerService_test.cpp | 151 +++++++++++++++++- 3 files changed, 197 insertions(+), 9 deletions(-) diff --git a/services/mediaresourcemanager/ResourceManagerService.cpp b/services/mediaresourcemanager/ResourceManagerService.cpp index 5a52b3d5dc..bdcd5e4381 100644 --- a/services/mediaresourcemanager/ResourceManagerService.cpp +++ b/services/mediaresourcemanager/ResourceManagerService.cpp @@ -200,16 +200,44 @@ status_t ResourceManagerService::dump(int fd, const Vector& /* args */ return OK; } +struct SystemCallbackImpl : + public ResourceManagerService::SystemCallbackInterface { + SystemCallbackImpl() {} + + virtual void noteStartVideo(int uid) override { + BatteryNotifier::getInstance().noteStartVideo(uid); + } + virtual void noteStopVideo(int uid) override { + BatteryNotifier::getInstance().noteStopVideo(uid); + } + virtual void noteResetVideo() override { + BatteryNotifier::getInstance().noteResetVideo(); + } + virtual bool requestCpusetBoost( + bool enable, const sp &client) override { + return android::requestCpusetBoost(enable, client); + } + +protected: + virtual ~SystemCallbackImpl() {} + +private: + DISALLOW_EVIL_CONSTRUCTORS(SystemCallbackImpl); +}; + ResourceManagerService::ResourceManagerService() - : ResourceManagerService(new ProcessInfo()) {} + : ResourceManagerService(new ProcessInfo(), new SystemCallbackImpl()) {} -ResourceManagerService::ResourceManagerService(sp processInfo) +ResourceManagerService::ResourceManagerService( + const sp &processInfo, + const sp &systemResource) : mProcessInfo(processInfo), + mSystemCB(systemResource), mServiceLog(new ServiceLog()), mSupportsMultipleSecureCodecs(true), mSupportsSecureWithNonSecureCodec(true), mCpuBoostCount(0) { - BatteryNotifier::getInstance().noteResetVideo(); + mSystemCB->noteResetVideo(); } ResourceManagerService::~ResourceManagerService() {} @@ -238,13 +266,13 @@ void ResourceManagerService::onFirstAdded( // Request it on every new instance of kCpuBoost, as the media.codec // could have died, if we only do it the first time subsequent instances // never gets the boost. - if (requestCpusetBoost(true, this) != OK) { + if (mSystemCB->requestCpusetBoost(true, this) != OK) { ALOGW("couldn't request cpuset boost"); } mCpuBoostCount++; } else if (resource.mType == MediaResource::kBattery && resource.mSubType == MediaResource::kVideoCodec) { - BatteryNotifier::getInstance().noteStartVideo(clientInfo.uid); + mSystemCB->noteStartVideo(clientInfo.uid); } } @@ -254,11 +282,11 @@ void ResourceManagerService::onLastRemoved( && resource.mSubType == MediaResource::kUnspecifiedSubType && mCpuBoostCount > 0) { if (--mCpuBoostCount == 0) { - requestCpusetBoost(false, this); + mSystemCB->requestCpusetBoost(false, this); } } else if (resource.mType == MediaResource::kBattery && resource.mSubType == MediaResource::kVideoCodec) { - BatteryNotifier::getInstance().noteStopVideo(clientInfo.uid); + mSystemCB->noteStopVideo(clientInfo.uid); } } diff --git a/services/mediaresourcemanager/ResourceManagerService.h b/services/mediaresourcemanager/ResourceManagerService.h index b9147ffbbb..f086dc3b22 100644 --- a/services/mediaresourcemanager/ResourceManagerService.h +++ b/services/mediaresourcemanager/ResourceManagerService.h @@ -51,12 +51,22 @@ class ResourceManagerService public BnResourceManagerService { public: + struct SystemCallbackInterface : public RefBase { + virtual void noteStartVideo(int uid) = 0; + virtual void noteStopVideo(int uid) = 0; + virtual void noteResetVideo() = 0; + virtual bool requestCpusetBoost( + bool enable, const sp &client) = 0; + }; + static char const *getServiceName() { return "media.resource_manager"; } virtual status_t dump(int fd, const Vector& args); ResourceManagerService(); - explicit ResourceManagerService(sp processInfo); + explicit ResourceManagerService( + const sp &processInfo, + const sp &systemResource); // IResourceManagerService interface virtual void config(const Vector &policies); @@ -118,6 +128,7 @@ private: mutable Mutex mLock; sp mProcessInfo; + sp mSystemCB; sp mServiceLog; PidResourceInfosMap mMap; bool mSupportsMultipleSecureCodecs; diff --git a/services/mediaresourcemanager/test/ResourceManagerService_test.cpp b/services/mediaresourcemanager/test/ResourceManagerService_test.cpp index be592f537a..ae97ec8324 100644 --- a/services/mediaresourcemanager/test/ResourceManagerService_test.cpp +++ b/services/mediaresourcemanager/test/ResourceManagerService_test.cpp @@ -52,6 +52,62 @@ private: DISALLOW_EVIL_CONSTRUCTORS(TestProcessInfo); }; +struct TestSystemCallback : + public ResourceManagerService::SystemCallbackInterface { + TestSystemCallback() : + mLastEvent({EventType::INVALID, 0}), mEventCount(0) {} + + enum EventType { + INVALID = -1, + VIDEO_ON = 0, + VIDEO_OFF = 1, + VIDEO_RESET = 2, + CPUSET_ENABLE = 3, + CPUSET_DISABLE = 4, + }; + + struct EventEntry { + EventType type; + int arg; + }; + + virtual void noteStartVideo(int uid) override { + mLastEvent = {EventType::VIDEO_ON, uid}; + mEventCount++; + } + + virtual void noteStopVideo(int uid) override { + mLastEvent = {EventType::VIDEO_OFF, uid}; + mEventCount++; + } + + virtual void noteResetVideo() override { + mLastEvent = {EventType::VIDEO_RESET, 0}; + mEventCount++; + } + + virtual bool requestCpusetBoost( + bool enable, const sp &/*client*/) override { + mLastEvent = {enable ? EventType::CPUSET_ENABLE : EventType::CPUSET_DISABLE, 0}; + mEventCount++; + return true; + } + + size_t eventCount() { return mEventCount; } + EventType lastEventType() { return mLastEvent.type; } + EventEntry lastEvent() { return mLastEvent; } + +protected: + virtual ~TestSystemCallback() {} + +private: + EventEntry mLastEvent; + size_t mEventCount; + + DISALLOW_EVIL_CONSTRUCTORS(TestSystemCallback); +}; + + struct TestClient : public BnResourceManagerClient { TestClient(int pid, sp service) : mReclaimed(false), mPid(pid), mService(service) {} @@ -95,10 +151,17 @@ static const int kLowPriorityPid = 40; static const int kMidPriorityPid = 25; static const int kHighPriorityPid = 10; +using EventType = TestSystemCallback::EventType; +using EventEntry = TestSystemCallback::EventEntry; +bool operator== (const EventEntry& lhs, const EventEntry& rhs) { + return lhs.type == rhs.type && lhs.arg == rhs.arg; +} + class ResourceManagerServiceTest : public ::testing::Test { public: ResourceManagerServiceTest() - : mService(new ResourceManagerService(new TestProcessInfo)), + : mSystemCB(new TestSystemCallback()), + mService(new ResourceManagerService(new TestProcessInfo, mSystemCB)), mTestClient1(new TestClient(kTestPid1, mService)), mTestClient2(new TestClient(kTestPid2, mService)), mTestClient3(new TestClient(kTestPid2, mService)) { @@ -578,6 +641,84 @@ protected: EXPECT_TRUE(mService->isCallingPriorityHigher_l(99, 100)); } + void testBatteryStats() { + // reset should always be called when ResourceManagerService is created (restarted) + EXPECT_EQ(1u, mSystemCB->eventCount()); + EXPECT_EQ(EventType::VIDEO_RESET, mSystemCB->lastEventType()); + + // new client request should cause VIDEO_ON + Vector resources1; + resources1.push_back(MediaResource(MediaResource::kBattery, MediaResource::kVideoCodec, 1)); + mService->addResource(kTestPid1, kTestUid1, getId(mTestClient1), mTestClient1, resources1); + EXPECT_EQ(2u, mSystemCB->eventCount()); + EXPECT_EQ(EventEntry({EventType::VIDEO_ON, kTestUid1}), mSystemCB->lastEvent()); + + // each client should only cause 1 VIDEO_ON + mService->addResource(kTestPid1, kTestUid1, getId(mTestClient1), mTestClient1, resources1); + EXPECT_EQ(2u, mSystemCB->eventCount()); + + // new client request should cause VIDEO_ON + Vector resources2; + resources2.push_back(MediaResource(MediaResource::kBattery, MediaResource::kVideoCodec, 2)); + mService->addResource(kTestPid2, kTestUid2, getId(mTestClient2), mTestClient2, resources2); + EXPECT_EQ(3u, mSystemCB->eventCount()); + EXPECT_EQ(EventEntry({EventType::VIDEO_ON, kTestUid2}), mSystemCB->lastEvent()); + + // partially remove mTestClient1's request, shouldn't be any VIDEO_OFF + mService->removeResource(kTestPid1, getId(mTestClient1), resources1); + EXPECT_EQ(3u, mSystemCB->eventCount()); + + // remove mTestClient1's request, should be VIDEO_OFF for kTestUid1 + // (use resource2 to test removing more instances than previously requested) + mService->removeResource(kTestPid1, getId(mTestClient1), resources2); + EXPECT_EQ(4u, mSystemCB->eventCount()); + EXPECT_EQ(EventEntry({EventType::VIDEO_OFF, kTestUid1}), mSystemCB->lastEvent()); + + // remove mTestClient2, should be VIDEO_OFF for kTestUid2 + mService->removeClient(kTestPid2, getId(mTestClient2)); + EXPECT_EQ(5u, mSystemCB->eventCount()); + EXPECT_EQ(EventEntry({EventType::VIDEO_OFF, kTestUid2}), mSystemCB->lastEvent()); + } + + void testCpusetBoost() { + // reset should always be called when ResourceManagerService is created (restarted) + EXPECT_EQ(1u, mSystemCB->eventCount()); + EXPECT_EQ(EventType::VIDEO_RESET, mSystemCB->lastEventType()); + + // new client request should cause CPUSET_ENABLE + Vector resources1; + resources1.push_back(MediaResource(MediaResource::kCpuBoost, 1)); + mService->addResource(kTestPid1, kTestUid1, getId(mTestClient1), mTestClient1, resources1); + EXPECT_EQ(2u, mSystemCB->eventCount()); + EXPECT_EQ(EventType::CPUSET_ENABLE, mSystemCB->lastEventType()); + + // each client should only cause 1 CPUSET_ENABLE + mService->addResource(kTestPid1, kTestUid1, getId(mTestClient1), mTestClient1, resources1); + EXPECT_EQ(2u, mSystemCB->eventCount()); + + // new client request should cause CPUSET_ENABLE + Vector resources2; + resources2.push_back(MediaResource(MediaResource::kCpuBoost, 2)); + mService->addResource(kTestPid2, kTestUid2, getId(mTestClient2), mTestClient2, resources2); + EXPECT_EQ(3u, mSystemCB->eventCount()); + EXPECT_EQ(EventType::CPUSET_ENABLE, mSystemCB->lastEventType()); + + // remove mTestClient2 should not cause CPUSET_DISABLE, mTestClient1 still active + mService->removeClient(kTestPid2, getId(mTestClient2)); + EXPECT_EQ(3u, mSystemCB->eventCount()); + + // remove 1 cpuboost from mTestClient1, should not be CPUSET_DISABLE (still 1 left) + mService->removeResource(kTestPid1, getId(mTestClient1), resources1); + EXPECT_EQ(3u, mSystemCB->eventCount()); + + // remove 2 cpuboost from mTestClient1, should be CPUSET_DISABLE + // (use resource2 to test removing more than previously requested) + mService->removeResource(kTestPid1, getId(mTestClient1), resources2); + EXPECT_EQ(4u, mSystemCB->eventCount()); + EXPECT_EQ(EventType::CPUSET_DISABLE, mSystemCB->lastEventType()); + } + + sp mSystemCB; sp mService; sp mTestClient1; sp mTestClient2; @@ -629,4 +770,12 @@ TEST_F(ResourceManagerServiceTest, isCallingPriorityHigher_l) { testIsCallingPriorityHigher(); } +TEST_F(ResourceManagerServiceTest, testBatteryStats) { + testBatteryStats(); +} + +TEST_F(ResourceManagerServiceTest, testCpusetBoost) { + testCpusetBoost(); +} + } // namespace android From ad2ceb8560232dd00d225d5e29e31fd1beda91f4 Mon Sep 17 00:00:00 2001 From: Chong Zhang Date: Thu, 22 Aug 2019 16:28:59 -0700 Subject: [PATCH 16/34] Add more tests for battery checker code There is no change on functionality, only refactoring to allow unit testing. bug: 138381810 bug: 139440234 Change-Id: Id927c091607d9ea5ea84b6e292d53154ef5171a6 --- media/libstagefright/MediaCodec.cpp | 69 +++-- media/libstagefright/TEST_MAPPING | 3 + .../media/stagefright/BatteryChecker.h | 47 ++++ .../include/media/stagefright/MediaCodec.h | 7 +- media/libstagefright/tests/Android.bp | 18 ++ .../tests/BatteryChecker_test.cpp | 242 ++++++++++++++++++ 6 files changed, 363 insertions(+), 23 deletions(-) create mode 100644 media/libstagefright/include/media/stagefright/BatteryChecker.h create mode 100644 media/libstagefright/tests/BatteryChecker_test.cpp diff --git a/media/libstagefright/MediaCodec.cpp b/media/libstagefright/MediaCodec.cpp index ae0fa3ca45..eceb84efb6 100644 --- a/media/libstagefright/MediaCodec.cpp +++ b/media/libstagefright/MediaCodec.cpp @@ -48,6 +48,7 @@ #include #include #include +#include #include #include #include @@ -116,7 +117,6 @@ static bool isResourceError(status_t err) { static const int kMaxRetry = 2; static const int kMaxReclaimWaitTimeInUs = 500000; // 0.5s static const int kNumBuffersAlign = 16; -static const int kBatteryStatsTimeoutUs = 3000000ll; // 3 seconds //////////////////////////////////////////////////////////////////////////////// @@ -539,10 +539,7 @@ MediaCodec::MediaCodec(const sp &looper, pid_t pid, uid_t uid) mHaveInputSurface(false), mHavePendingInputBuffers(false), mCpuBoostRequested(false), - mLatencyUnknown(0), - mLastActivityTimeUs(-1ll), - mBatteryStatNotified(false), - mBatteryCheckerGeneration(0) { + mLatencyUnknown(0) { if (uid == kNoUid) { mUid = IPCThreadState::self()->getCallingUid(); } else { @@ -755,7 +752,11 @@ void MediaCodec::statsBufferSent(int64_t presentationUs) { return; } - scheduleBatteryCheckerIfNeeded(); + if (mBatteryChecker != nullptr) { + mBatteryChecker->onCodecActivity([this] () { + addResource(MediaResource::kBattery, MediaResource::kVideoCodec, 1); + }); + } const int64_t nowNs = systemTime(SYSTEM_TIME_MONOTONIC); BufferFlightTiming_t startdata = { presentationUs, nowNs }; @@ -791,7 +792,11 @@ void MediaCodec::statsBufferReceived(int64_t presentationUs) { return; } - scheduleBatteryCheckerIfNeeded(); + if (mBatteryChecker != nullptr) { + mBatteryChecker->onCodecActivity([this] () { + addResource(MediaResource::kBattery, MediaResource::kVideoCodec, 1); + }); + } BufferFlightTiming_t startdata; bool valid = false; @@ -981,6 +986,10 @@ status_t MediaCodec::init(const AString &name) { mAnalyticsItem->setCString(kCodecMode, mIsVideo ? kCodecModeVideo : kCodecModeAudio); } + if (mIsVideo) { + mBatteryChecker = new BatteryChecker(new AMessage(kWhatCheckBatteryStats, this)); + } + status_t err; Vector resources; MediaResource::Type type = @@ -1706,19 +1715,27 @@ void MediaCodec::requestCpuBoostIfNeeded() { } } -void MediaCodec::scheduleBatteryCheckerIfNeeded() { - if (!mIsVideo || !isExecuting()) { +BatteryChecker::BatteryChecker(const sp &msg, int64_t timeoutUs) + : mTimeoutUs(timeoutUs) + , mLastActivityTimeUs(-1ll) + , mBatteryStatNotified(false) + , mBatteryCheckerGeneration(0) + , mIsExecuting(false) + , mBatteryCheckerMsg(msg) {} + +void BatteryChecker::onCodecActivity(std::function batteryOnCb) { + if (!isExecuting()) { // ignore if not executing return; } if (!mBatteryStatNotified) { - addResource(MediaResource::kBattery, MediaResource::kVideoCodec, 1); + batteryOnCb(); mBatteryStatNotified = true; - sp msg = new AMessage(kWhatCheckBatteryStats, this); + sp msg = mBatteryCheckerMsg->dup(); msg->setInt32("generation", mBatteryCheckerGeneration); // post checker and clear last activity time - msg->post(kBatteryStatsTimeoutUs); + msg->post(mTimeoutUs); mLastActivityTimeUs = -1ll; } else { // update last activity time @@ -1726,7 +1743,8 @@ void MediaCodec::scheduleBatteryCheckerIfNeeded() { } } -void MediaCodec::onBatteryChecker(const sp &msg) { +void BatteryChecker::onCheckBatteryTimer( + const sp &msg, std::function batteryOffCb) { // ignore if this checker already expired because the client resource was removed int32_t generation; if (!msg->findInt32("generation", &generation) @@ -1736,15 +1754,20 @@ void MediaCodec::onBatteryChecker(const sp &msg) { if (mLastActivityTimeUs < 0ll) { // timed out inactive, do not repost checker - removeResource(MediaResource::kBattery, MediaResource::kVideoCodec, 1); + batteryOffCb(); mBatteryStatNotified = false; } else { // repost checker and clear last activity time - msg->post(kBatteryStatsTimeoutUs + mLastActivityTimeUs - ALooper::GetNowUs()); + msg->post(mTimeoutUs + mLastActivityTimeUs - ALooper::GetNowUs()); mLastActivityTimeUs = -1ll; } } +void BatteryChecker::onClientRemoved() { + mBatteryStatNotified = false; + mBatteryCheckerGeneration++; +} + //////////////////////////////////////////////////////////////////////////////// void MediaCodec::cancelPendingDequeueOperations() { @@ -2382,8 +2405,10 @@ void MediaCodec::onMessageReceived(const sp &msg) { mFlags &= ~kFlagIsComponentAllocated; // off since we're removing all resources including the battery on - mBatteryStatNotified = false; - mBatteryCheckerGeneration++; + if (mBatteryChecker != nullptr) { + mBatteryChecker->onClientRemoved(); + } + mResourceManagerService->removeClient(getId(mResourceManagerClient)); (new AMessage)->postReply(mReplyID); @@ -3097,7 +3122,11 @@ void MediaCodec::onMessageReceived(const sp &msg) { case kWhatCheckBatteryStats: { - onBatteryChecker(msg); + if (mBatteryChecker != nullptr) { + mBatteryChecker->onCheckBatteryTimer(msg, [this] () { + removeResource(MediaResource::kBattery, MediaResource::kVideoCodec, 1); + }); + } break; } @@ -3197,6 +3226,10 @@ void MediaCodec::setState(State newState) { mState = newState; + if (mBatteryChecker != nullptr) { + mBatteryChecker->setExecuting(isExecuting()); + } + cancelPendingDequeueOperations(); } diff --git a/media/libstagefright/TEST_MAPPING b/media/libstagefright/TEST_MAPPING index 96818eb70d..c1b270cddf 100644 --- a/media/libstagefright/TEST_MAPPING +++ b/media/libstagefright/TEST_MAPPING @@ -7,6 +7,9 @@ "include-annotation": "android.platform.test.annotations.RequiresDevice" } ] + }, + { + "name": "BatteryChecker_test" } ] } diff --git a/media/libstagefright/include/media/stagefright/BatteryChecker.h b/media/libstagefright/include/media/stagefright/BatteryChecker.h new file mode 100644 index 0000000000..2ec4ac046a --- /dev/null +++ b/media/libstagefright/include/media/stagefright/BatteryChecker.h @@ -0,0 +1,47 @@ +/* + * Copyright (C) 2019 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. + */ + +#ifndef BATTERY_CHECKER_H_ +#define BATTERY_CHECKER_H_ + +#include + +namespace android { + +struct BatteryChecker : public RefBase { + BatteryChecker(const sp &msg, int64_t timeout = 3000000ll); + + void setExecuting(bool executing) { mIsExecuting = executing; } + void onCodecActivity(std::function batteryOnCb); + void onCheckBatteryTimer(const sp& msg, std::function batteryOffCb); + void onClientRemoved(); + +private: + const int64_t mTimeoutUs; + int64_t mLastActivityTimeUs; + bool mBatteryStatNotified; + int32_t mBatteryCheckerGeneration; + bool mIsExecuting; + sp mBatteryCheckerMsg; + + bool isExecuting() { return mIsExecuting; } + + DISALLOW_EVIL_CONSTRUCTORS(BatteryChecker); +}; + +} // namespace android + +#endif // BATTERY_CHECKER_H_ diff --git a/media/libstagefright/include/media/stagefright/MediaCodec.h b/media/libstagefright/include/media/stagefright/MediaCodec.h index 462eb84e50..cd303477cb 100644 --- a/media/libstagefright/include/media/stagefright/MediaCodec.h +++ b/media/libstagefright/include/media/stagefright/MediaCodec.h @@ -36,6 +36,7 @@ struct ABuffer; struct AMessage; struct AReplyToken; struct AString; +struct BatteryChecker; class BufferChannelBase; struct CodecBase; class IBatteryStats; @@ -463,11 +464,7 @@ private: Mutex mLatencyLock; int64_t mLatencyUnknown; // buffers for which we couldn't calculate latency - int64_t mLastActivityTimeUs; - bool mBatteryStatNotified; - int32_t mBatteryCheckerGeneration; - void onBatteryChecker(const sp& msg); - void scheduleBatteryCheckerIfNeeded(); + sp mBatteryChecker; void statsBufferSent(int64_t presentationUs); void statsBufferReceived(int64_t presentationUs); diff --git a/media/libstagefright/tests/Android.bp b/media/libstagefright/tests/Android.bp index be10fdc942..a7f94c1365 100644 --- a/media/libstagefright/tests/Android.bp +++ b/media/libstagefright/tests/Android.bp @@ -27,3 +27,21 @@ cc_test { "-Wall", ], } + +cc_test { + name: "BatteryChecker_test", + srcs: ["BatteryChecker_test.cpp"], + test_suites: ["device-tests"], + + shared_libs: [ + "libstagefright", + "libstagefright_foundation", + "libutils", + "liblog", + ], + + cflags: [ + "-Werror", + "-Wall", + ], +} \ No newline at end of file diff --git a/media/libstagefright/tests/BatteryChecker_test.cpp b/media/libstagefright/tests/BatteryChecker_test.cpp new file mode 100644 index 0000000000..0c5ee9b462 --- /dev/null +++ b/media/libstagefright/tests/BatteryChecker_test.cpp @@ -0,0 +1,242 @@ +/* + * Copyright 2019 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. + */ + +// #define LOG_NDEBUG 0 +#define LOG_TAG "BatteryChecker_test" +#include + +#include + +#include +#include +#include + +#include + +namespace android { + +static const int kBatteryTimeoutUs = 1000000ll; // 1 seconds +static const int kTestMarginUs = 50000ll; // 50ms +static const int kWaitStatusChangeUs = kBatteryTimeoutUs + kTestMarginUs; +static const int kSparseFrameIntervalUs = kBatteryTimeoutUs - kTestMarginUs; + +class BatteryCheckerTestHandler : public AHandler { + enum EventType { + // Events simulating MediaCodec + kWhatStart = 0, // codec entering executing state + kWhatStop, // codec exiting executing state + kWhatActivity, // codec queue input or dequeue output + kWhatReleased, // codec released + kWhatCheckpoint, // test checkpoing with expected values on On/Off + + // Message for battery checker monitor (not for testing through runTest()) + kWhatBatteryChecker, + }; + + struct Operation { + int32_t event; + int64_t delay = 0; + uint32_t repeatCount = 0; + int32_t expectedOnCounter = 0; + int32_t expectedOffCounter = 0; + }; + + std::vector mOps; + sp mBatteryChecker; + int32_t mOnCounter; + int32_t mOffCounter; + Condition mDone; + Mutex mLock; + + BatteryCheckerTestHandler() : mOnCounter(0), mOffCounter(0) {} + + void runTest(const std::vector &ops, int64_t timeoutUs) { + mOps = ops; + + mBatteryChecker = new BatteryChecker( + new AMessage(kWhatBatteryChecker, this), kBatteryTimeoutUs); + + (new AMessage(ops[0].event, this))->post(); + + // wait for done + AutoMutex lock(mLock); + EXPECT_NE(TIMED_OUT, mDone.waitRelative(mLock, timeoutUs * 1000ll)); + } + + virtual void onMessageReceived(const sp &msg); + + friend class BatteryCheckerTest; +}; + +class BatteryCheckerTest : public ::testing::Test { +public: + BatteryCheckerTest() + : mLooper(new ALooper) + , mHandler(new BatteryCheckerTestHandler()) { + mLooper->setName("BatterCheckerLooper"); + mLooper->start(false, false, ANDROID_PRIORITY_AUDIO); + mLooper->registerHandler(mHandler); + } + +protected: + using EventType = BatteryCheckerTestHandler::EventType; + using Operation = BatteryCheckerTestHandler::Operation; + + virtual ~BatteryCheckerTest() { + mLooper->stop(); + mLooper->unregisterHandler(mHandler->id()); + } + + void runTest(const std::vector &ops, int64_t timeoutUs) { + mHandler->runTest(ops, timeoutUs); + } + + sp mLooper; + sp mHandler; +}; + +void BatteryCheckerTestHandler::onMessageReceived(const sp &msg) { + switch(msg->what()) { + case kWhatStart: + mBatteryChecker->setExecuting(true); + break; + case kWhatStop: + mBatteryChecker->setExecuting(false); + break; + case kWhatActivity: + mBatteryChecker->onCodecActivity([this] () { mOnCounter++; }); + break; + case kWhatReleased: + mBatteryChecker->onClientRemoved(); + break; + case kWhatBatteryChecker: + mBatteryChecker->onCheckBatteryTimer(msg, [this] () { mOffCounter++; }); + break; + case kWhatCheckpoint: + // verify ON/OFF state and total events + EXPECT_EQ(mOnCounter, mOps[0].expectedOnCounter); + EXPECT_EQ(mOffCounter, mOps[0].expectedOffCounter); + break; + default: + TRESPASS(); + } + if (msg->what() != kWhatBatteryChecker) { + EXPECT_EQ(msg->what(), mOps[0].event); + // post next message + if (!mOps[0].repeatCount) { + mOps.erase(mOps.begin()); + } else { + mOps[0].repeatCount--; + } + int64_t duration = mOps[0].delay; + if (!mOps.empty()) { + (new AMessage(mOps[0].event, this))->post(duration); + } else { + AutoMutex lock(mLock); + mDone.signal(); + } + } +} + +TEST_F(BatteryCheckerTest, testNormalOperations) { + runTest({ + {EventType::kWhatStart, 0ll}, + {EventType::kWhatCheckpoint, 0ll, 0, 0, 0}, + {EventType::kWhatActivity, 33333ll}, + {EventType::kWhatCheckpoint, 0ll, 0, 1, 0}, // ON + {EventType::kWhatActivity, 33333ll, 2*kWaitStatusChangeUs/33333ll}, + {EventType::kWhatCheckpoint, 0ll, 0, 1, 0}, + {EventType::kWhatCheckpoint, kWaitStatusChangeUs, 0, 1, 1}, // OFF + }, 10000000ll); +} + +TEST_F(BatteryCheckerTest, testPauseResume) { + runTest({ + {EventType::kWhatStart, 0ll}, + {EventType::kWhatCheckpoint, 0ll, 0, 0, 0}, + {EventType::kWhatActivity, 33333ll}, + {EventType::kWhatCheckpoint, 0ll, 0, 1, 0}, // ON + {EventType::kWhatCheckpoint, kWaitStatusChangeUs, 0, 1, 1}, // OFF + {EventType::kWhatActivity, 33333ll}, + {EventType::kWhatCheckpoint, 0ll, 0, 2, 1}, // ON + {EventType::kWhatCheckpoint, kWaitStatusChangeUs, 0, 2, 2}, // OFF + }, 10000000ll); +} + +TEST_F(BatteryCheckerTest, testClientRemovedAndRestart) { + runTest({ + {EventType::kWhatStart, 0ll}, + {EventType::kWhatActivity, 33333ll, kWaitStatusChangeUs/33333ll}, + {EventType::kWhatCheckpoint, 0ll, 0, 1, 0}, + + // stop executing state itself shouldn't trigger any calls + {EventType::kWhatStop, 0ll}, + {EventType::kWhatCheckpoint, 0ll, 0, 1, 0}, + + // release shouldn't trigger any calls either, + // client resource will be removed entirely + {EventType::kWhatReleased, 0ll}, + {EventType::kWhatCheckpoint, 0ll, 0, 1, 0}, + {EventType::kWhatCheckpoint, kWaitStatusChangeUs, 0, 1, 0}, + + // start pushing buffers again, On should be received without any Off + {EventType::kWhatStart, 0ll}, + {EventType::kWhatActivity, 33333ll}, + {EventType::kWhatCheckpoint, 0ll, 0, 2, 0}, + + // double check that only new checker msg triggers OFF, + // left-over checker msg from stale generate discarded + {EventType::kWhatCheckpoint, kWaitStatusChangeUs, 0, 2, 1}, + }, 10000000ll); +} + +TEST_F(BatteryCheckerTest, testActivityWhileNotExecuting) { + runTest({ + // activity before start shouldn't trigger + {EventType::kWhatActivity, 0ll}, + {EventType::kWhatCheckpoint, 0ll, 0, 0, 0}, + + {EventType::kWhatStart, 0ll}, + {EventType::kWhatCheckpoint, 0ll, 0, 0, 0}, + + // activity after start before stop should trigger + {EventType::kWhatActivity, 33333ll}, + {EventType::kWhatCheckpoint, 0ll, 0, 1, 0}, + + // stop executing state itself shouldn't trigger any calls + {EventType::kWhatStop, 0ll}, + {EventType::kWhatCheckpoint, 0ll, 0, 1, 0}, + + // keep pushing another 3 seconds after stop, expected to OFF + {EventType::kWhatActivity, 33333ll, kWaitStatusChangeUs/33333ll}, + {EventType::kWhatCheckpoint, 0ll, 0, 1, 1}, + }, 10000000ll); +} + +TEST_F(BatteryCheckerTest, testSparseActivity) { + runTest({ + {EventType::kWhatStart, 0ll}, + {EventType::kWhatCheckpoint, 0ll, 0, 0, 0}, + + // activity arrives sparsely with interval only slightly small than timeout + // should only trigger 1 ON + {EventType::kWhatActivity, kSparseFrameIntervalUs, 2}, + {EventType::kWhatCheckpoint, 0ll, 0, 1, 0}, + {EventType::kWhatCheckpoint, kSparseFrameIntervalUs, 0, 1, 0}, + {EventType::kWhatCheckpoint, kTestMarginUs, 0, 1, 1}, // OFF + }, 10000000ll); +} +} // namespace android From 5dd7590c6c6fcedd77ac5ec2dd97c8badfb9eb36 Mon Sep 17 00:00:00 2001 From: Robert Shih Date: Mon, 26 Aug 2019 17:03:13 -0700 Subject: [PATCH 17/34] [RESTRICT AUTOMERGE] clearkey hidl CryptoPlugin: security fixes * reject native handle output * validate subsample sizes Bug: 137283376 Test: cryptopoc Change-Id: Ic4267fdc0e391bdecc1caab3b8fd4aa34ad76541 --- .../plugins/clearkey/hidl/CryptoPlugin.cpp | 46 +++++++++++-------- 1 file changed, 28 insertions(+), 18 deletions(-) diff --git a/drm/mediadrm/plugins/clearkey/hidl/CryptoPlugin.cpp b/drm/mediadrm/plugins/clearkey/hidl/CryptoPlugin.cpp index f33f94e711..198e0997d0 100644 --- a/drm/mediadrm/plugins/clearkey/hidl/CryptoPlugin.cpp +++ b/drm/mediadrm/plugins/clearkey/hidl/CryptoPlugin.cpp @@ -77,6 +77,10 @@ Return CryptoPlugin::decrypt( "destination decrypt buffer base not set"); return Void(); } + } else { + _hidl_cb(Status::ERROR_DRM_CANNOT_HANDLE, 0, + "destination type not supported"); + return Void(); } sp sourceBase = mSharedBufferMap[source.bufferId]; @@ -94,24 +98,19 @@ Return CryptoPlugin::decrypt( (static_cast(sourceBase->getPointer())); uint8_t* srcPtr = static_cast(base + source.offset + offset); void* destPtr = NULL; - if (destination.type == BufferType::SHARED_MEMORY) { - const SharedBuffer& destBuffer = destination.nonsecureMemory; - sp destBase = mSharedBufferMap[destBuffer.bufferId]; - if (destBase == nullptr) { - _hidl_cb(Status::ERROR_DRM_CANNOT_HANDLE, 0, "destination is a nullptr"); - return Void(); - } + // destination.type == BufferType::SHARED_MEMORY + const SharedBuffer& destBuffer = destination.nonsecureMemory; + sp destBase = mSharedBufferMap[destBuffer.bufferId]; + if (destBase == nullptr) { + _hidl_cb(Status::ERROR_DRM_CANNOT_HANDLE, 0, "destination is a nullptr"); + return Void(); + } - if (destBuffer.offset + destBuffer.size > destBase->getSize()) { - _hidl_cb(Status::ERROR_DRM_CANNOT_HANDLE, 0, "invalid buffer size"); - return Void(); - } - destPtr = static_cast(base + destination.nonsecureMemory.offset); - } else if (destination.type == BufferType::NATIVE_HANDLE) { - native_handle_t *handle = const_cast( - destination.secureMemory.getNativeHandle()); - destPtr = static_cast(handle); + if (destBuffer.offset + destBuffer.size > destBase->getSize()) { + _hidl_cb(Status::ERROR_DRM_CANNOT_HANDLE, 0, "invalid buffer size"); + return Void(); } + destPtr = static_cast(base + destination.nonsecureMemory.offset); // Calculate the output buffer size and determine if any subsamples are // encrypted. @@ -119,13 +118,24 @@ Return CryptoPlugin::decrypt( bool haveEncryptedSubsamples = false; for (size_t i = 0; i < subSamples.size(); i++) { const SubSample &subSample = subSamples[i]; - destSize += subSample.numBytesOfClearData; - destSize += subSample.numBytesOfEncryptedData; + if (__builtin_add_overflow(destSize, subSample.numBytesOfClearData, &destSize)) { + _hidl_cb(Status::BAD_VALUE, 0, "subsample clear size overflow"); + return Void(); + } + if (__builtin_add_overflow(destSize, subSample.numBytesOfEncryptedData, &destSize)) { + _hidl_cb(Status::BAD_VALUE, 0, "subsample encrypted size overflow"); + return Void(); + } if (subSample.numBytesOfEncryptedData > 0) { haveEncryptedSubsamples = true; } } + if (destSize > destBuffer.size) { + _hidl_cb(Status::BAD_VALUE, 0, "subsample sum too large"); + return Void(); + } + if (mode == Mode::UNENCRYPTED) { if (haveEncryptedSubsamples) { _hidl_cb(Status::ERROR_DRM_CANNOT_HANDLE, 0, From d23bd2a2a80f6bfc8c827bf21e53ad4500582d56 Mon Sep 17 00:00:00 2001 From: Robert Shih Date: Mon, 26 Aug 2019 17:03:13 -0700 Subject: [PATCH 18/34] clearkey hidl CryptoPlugin: misc & security fixes * propagate decrypt error message * reject native handle output * validate subsample sizes Bug: 137283376 Test: cryptopoc Change-Id: Ic4267fdc0e391bdecc1caab3b8fd4aa34ad76541 --- .../plugins/clearkey/hidl/CryptoPlugin.cpp | 53 +++++++++++-------- 1 file changed, 31 insertions(+), 22 deletions(-) diff --git a/drm/mediadrm/plugins/clearkey/hidl/CryptoPlugin.cpp b/drm/mediadrm/plugins/clearkey/hidl/CryptoPlugin.cpp index 23a35e5bdb..f164f2859b 100644 --- a/drm/mediadrm/plugins/clearkey/hidl/CryptoPlugin.cpp +++ b/drm/mediadrm/plugins/clearkey/hidl/CryptoPlugin.cpp @@ -62,10 +62,8 @@ Return CryptoPlugin::decrypt( secure, keyId, iv, mode, pattern, subSamples, source, offset, destination, [&](Status_V1_2 hStatus, uint32_t hBytesWritten, hidl_string hDetailedError) { status = toStatus_1_0(hStatus); - if (status == Status::OK) { - bytesWritten = hBytesWritten; - detailedError = hDetailedError; - } + bytesWritten = hBytesWritten; + detailedError = hDetailedError; } ); @@ -109,6 +107,10 @@ Return CryptoPlugin::decrypt_1_2( "destination decrypt buffer base not set"); return Void(); } + } else { + _hidl_cb(Status_V1_2::ERROR_DRM_CANNOT_HANDLE, 0, + "destination type not supported"); + return Void(); } sp sourceBase = mSharedBufferMap[source.bufferId]; @@ -126,24 +128,20 @@ Return CryptoPlugin::decrypt_1_2( (static_cast(sourceBase->getPointer())); uint8_t* srcPtr = static_cast(base + source.offset + offset); void* destPtr = NULL; - if (destination.type == BufferType::SHARED_MEMORY) { - const SharedBuffer& destBuffer = destination.nonsecureMemory; - sp destBase = mSharedBufferMap[destBuffer.bufferId]; - if (destBase == nullptr) { - _hidl_cb(Status_V1_2::ERROR_DRM_CANNOT_HANDLE, 0, "destination is a nullptr"); - return Void(); - } + // destination.type == BufferType::SHARED_MEMORY + const SharedBuffer& destBuffer = destination.nonsecureMemory; + sp destBase = mSharedBufferMap[destBuffer.bufferId]; + if (destBase == nullptr) { + _hidl_cb(Status_V1_2::ERROR_DRM_CANNOT_HANDLE, 0, "destination is a nullptr"); + return Void(); + } - if (destBuffer.offset + destBuffer.size > destBase->getSize()) { - _hidl_cb(Status_V1_2::ERROR_DRM_FRAME_TOO_LARGE, 0, "invalid buffer size"); - return Void(); - } - destPtr = static_cast(base + destination.nonsecureMemory.offset); - } else if (destination.type == BufferType::NATIVE_HANDLE) { - native_handle_t *handle = const_cast( - destination.secureMemory.getNativeHandle()); - destPtr = static_cast(handle); + if (destBuffer.offset + destBuffer.size > destBase->getSize()) { + _hidl_cb(Status_V1_2::ERROR_DRM_FRAME_TOO_LARGE, 0, "invalid buffer size"); + return Void(); } + destPtr = static_cast(base + destination.nonsecureMemory.offset); + // Calculate the output buffer size and determine if any subsamples are // encrypted. @@ -151,13 +149,24 @@ Return CryptoPlugin::decrypt_1_2( bool haveEncryptedSubsamples = false; for (size_t i = 0; i < subSamples.size(); i++) { const SubSample &subSample = subSamples[i]; - destSize += subSample.numBytesOfClearData; - destSize += subSample.numBytesOfEncryptedData; + if (__builtin_add_overflow(destSize, subSample.numBytesOfClearData, &destSize)) { + _hidl_cb(Status_V1_2::ERROR_DRM_FRAME_TOO_LARGE, 0, "subsample clear size overflow"); + return Void(); + } + if (__builtin_add_overflow(destSize, subSample.numBytesOfEncryptedData, &destSize)) { + _hidl_cb(Status_V1_2::ERROR_DRM_FRAME_TOO_LARGE, 0, "subsample encrypted size overflow"); + return Void(); + } if (subSample.numBytesOfEncryptedData > 0) { haveEncryptedSubsamples = true; } } + if (destSize > destBuffer.size) { + _hidl_cb(Status_V1_2::ERROR_DRM_FRAME_TOO_LARGE, 0, "subsample sum too large"); + return Void(); + } + if (mode == Mode::UNENCRYPTED) { if (haveEncryptedSubsamples) { _hidl_cb(Status_V1_2::ERROR_DRM_CANNOT_HANDLE, 0, From d7463d1d08f4cfde5a4fef2e4e555518ec6533b5 Mon Sep 17 00:00:00 2001 From: Sungtak Lee Date: Wed, 4 Sep 2019 16:01:00 -0700 Subject: [PATCH 19/34] Increase max dequeue buffer count only for non-secure decoders Since secure buffers are limited resources, use less buffers for secure output display. Bug: 138295327 Bug: 140087403 Change-Id: If763bed99cecc0acdd88d0111d30aa4ab98bce71 --- media/codec2/sfplugin/CCodecBufferChannel.cpp | 20 ++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/media/codec2/sfplugin/CCodecBufferChannel.cpp b/media/codec2/sfplugin/CCodecBufferChannel.cpp index 0cbf62bb1b..2efb9871e4 100644 --- a/media/codec2/sfplugin/CCodecBufferChannel.cpp +++ b/media/codec2/sfplugin/CCodecBufferChannel.cpp @@ -224,7 +224,7 @@ CCodecBufferChannel::CCodecBufferChannel( mFirstValidFrameIndex(0u), mMetaMode(MODE_NONE), mInputMetEos(false) { - mOutputSurface.lock()->maxDequeueBuffers = 2 * kSmoothnessFactor + kRenderingDepth; + mOutputSurface.lock()->maxDequeueBuffers = kSmoothnessFactor + kRenderingDepth; { Mutexed::Locked input(mInput); input->buffers.reset(new DummyInputBuffers("")); @@ -948,8 +948,11 @@ status_t CCodecBufferChannel::start( uint32_t outputGeneration; { Mutexed::Locked output(mOutputSurface); - output->maxDequeueBuffers = numOutputSlots + numInputSlots + + output->maxDequeueBuffers = numOutputSlots + reorderDepth.value + kRenderingDepth; + if (!secure) { + output->maxDequeueBuffers += numInputSlots; + } outputSurface = output->surface ? output->surface->getIGraphicBufferProducer() : nullptr; if (outputSurface) { @@ -1329,14 +1332,18 @@ bool CCodecBufferChannel::handleWork( case C2PortReorderBufferDepthTuning::CORE_INDEX: { C2PortReorderBufferDepthTuning::output reorderDepth; if (reorderDepth.updateFrom(*param)) { + bool secure = mComponent->getName().find(".secure") != std::string::npos; mReorderStash.lock()->setDepth(reorderDepth.value); ALOGV("[%s] onWorkDone: updated reorder depth to %u", mName, reorderDepth.value); size_t numOutputSlots = mOutput.lock()->numSlots; size_t numInputSlots = mInput.lock()->numSlots; Mutexed::Locked output(mOutputSurface); - output->maxDequeueBuffers = numOutputSlots + numInputSlots + + output->maxDequeueBuffers = numOutputSlots + reorderDepth.value + kRenderingDepth; + if (!secure) { + output->maxDequeueBuffers += numInputSlots; + } if (output->surface) { output->surface->setMaxDequeuedBufferCount(output->maxDequeueBuffers); } @@ -1380,6 +1387,7 @@ bool CCodecBufferChannel::handleWork( if (outputDelay.updateFrom(*param)) { ALOGV("[%s] onWorkDone: updating output delay %u", mName, outputDelay.value); + bool secure = mComponent->getName().find(".secure") != std::string::npos; (void)mPipelineWatcher.lock()->outputDelay(outputDelay.value); bool outputBuffersChanged = false; @@ -1409,8 +1417,10 @@ bool CCodecBufferChannel::handleWork( uint32_t depth = mReorderStash.lock()->depth(); Mutexed::Locked output(mOutputSurface); - output->maxDequeueBuffers = numOutputSlots + numInputSlots + - depth + kRenderingDepth; + output->maxDequeueBuffers = numOutputSlots + depth + kRenderingDepth; + if (!secure) { + output->maxDequeueBuffers += numInputSlots; + } if (output->surface) { output->surface->setMaxDequeuedBufferCount(output->maxDequeueBuffers); } From 97390c6bf23326a0d9a97b9d63e7f72eab396303 Mon Sep 17 00:00:00 2001 From: Chong Zhang Date: Thu, 25 Jul 2019 16:57:47 -0700 Subject: [PATCH 20/34] ACodec: query grid config from component even if grid is disabled bug: 138397508 test: cts HeifWriterTest on devices that can enable OMX HEIC encoder. Change-Id: I6e8437c0b8cc4901e943f2ed7cb23cb16ed096fe (cherry picked from commit 7b0ff1953ddeee2e83c8f8871cdf69db3ac32c85) --- media/libstagefright/ACodec.cpp | 33 ++++++++++++++++++++++++++++----- 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/media/libstagefright/ACodec.cpp b/media/libstagefright/ACodec.cpp index 3d67c911a2..d198d39bc6 100644 --- a/media/libstagefright/ACodec.cpp +++ b/media/libstagefright/ACodec.cpp @@ -4505,22 +4505,38 @@ status_t ACodec::setupAVCEncoderParameters(const sp &msg) { status_t ACodec::configureImageGrid( const sp &msg, sp &outputFormat) { int32_t tileWidth, tileHeight, gridRows, gridCols; - if (!msg->findInt32("tile-width", &tileWidth) || - !msg->findInt32("tile-height", &tileHeight) || - !msg->findInt32("grid-rows", &gridRows) || - !msg->findInt32("grid-cols", &gridCols)) { + OMX_BOOL useGrid = OMX_FALSE; + if (msg->findInt32("tile-width", &tileWidth) && + msg->findInt32("tile-height", &tileHeight) && + msg->findInt32("grid-rows", &gridRows) && + msg->findInt32("grid-cols", &gridCols)) { + useGrid = OMX_TRUE; + } else { + // when bEnabled is false, the tile info is not used, + // but clear out these too. + tileWidth = tileHeight = gridRows = gridCols = 0; + } + + if (!mIsImage && !useGrid) { return OK; } OMX_VIDEO_PARAM_ANDROID_IMAGEGRIDTYPE gridType; InitOMXParams(&gridType); gridType.nPortIndex = kPortIndexOutput; - gridType.bEnabled = OMX_TRUE; + gridType.bEnabled = useGrid; gridType.nTileWidth = tileWidth; gridType.nTileHeight = tileHeight; gridType.nGridRows = gridRows; gridType.nGridCols = gridCols; + ALOGV("sending image grid info to component: bEnabled %d, tile %dx%d, grid %dx%d", + gridType.bEnabled, + gridType.nTileWidth, + gridType.nTileHeight, + gridType.nGridRows, + gridType.nGridCols); + status_t err = mOMXNode->setParameter( (OMX_INDEXTYPE)OMX_IndexParamVideoAndroidImageGrid, &gridType, sizeof(gridType)); @@ -4541,6 +4557,13 @@ status_t ACodec::configureImageGrid( (OMX_INDEXTYPE)OMX_IndexParamVideoAndroidImageGrid, &gridType, sizeof(gridType)); + ALOGV("received image grid info from component: bEnabled %d, tile %dx%d, grid %dx%d", + gridType.bEnabled, + gridType.nTileWidth, + gridType.nTileHeight, + gridType.nGridRows, + gridType.nGridCols); + if (err == OK && gridType.bEnabled) { outputFormat->setInt32("tile-width", gridType.nTileWidth); outputFormat->setInt32("tile-height", gridType.nTileHeight); From 23c90c87fd0804bfc98da07e242d27c925a2cfc8 Mon Sep 17 00:00:00 2001 From: Pawin Vongmasa Date: Tue, 3 Sep 2019 00:44:42 -0700 Subject: [PATCH 21/34] Codec2: Retry interface creation until successful Test: atest CtsMediaTestCases -- \ --module-arg CtsMediaTestCases:size:small Bug: 139466364 Change-Id: I6e552cb90af517f2c0f8c14dd679d6d60abee6fc --- media/codec2/hidl/client/client.cpp | 108 +++++++++--------- .../hidl/client/include/codec2/hidl/client.h | 12 +- 2 files changed, 65 insertions(+), 55 deletions(-) diff --git a/media/codec2/hidl/client/client.cpp b/media/codec2/hidl/client/client.cpp index 5ed54f1a24..c620badfab 100644 --- a/media/codec2/hidl/client/client.cpp +++ b/media/codec2/hidl/client/client.cpp @@ -883,66 +883,72 @@ std::shared_ptr Codec2Client::CreateComponentByName( const char* componentName, const std::shared_ptr& listener, - std::shared_ptr* owner) { - std::shared_ptr component; - c2_status_t status = ForAllServices( - componentName, - [owner, &component, componentName, &listener]( - const std::shared_ptr &client) - -> c2_status_t { - c2_status_t status = client->createComponent(componentName, - listener, - &component); - if (status == C2_OK) { - if (owner) { - *owner = client; + std::shared_ptr* owner, + size_t numberOfAttempts) { + while (true) { + std::shared_ptr component; + c2_status_t status = ForAllServices( + componentName, + [owner, &component, componentName, &listener]( + const std::shared_ptr &client) + -> c2_status_t { + c2_status_t status = client->createComponent(componentName, + listener, + &component); + if (status == C2_OK) { + if (owner) { + *owner = client; + } + } else if (status != C2_NOT_FOUND) { + LOG(DEBUG) << "IComponentStore(" + << client->getServiceName() + << ")::createComponent(\"" << componentName + << "\") returned status = " + << status << "."; } - } else if (status != C2_NOT_FOUND) { - LOG(DEBUG) << "IComponentStore(" - << client->getServiceName() - << ")::createComponent(\"" << componentName - << "\") returned status = " - << status << "."; - } - return status; - }); - if (status != C2_OK) { - LOG(DEBUG) << "Could not create component \"" << componentName << "\". " - "Status = " << status << "."; + return status; + }); + if (numberOfAttempts > 0 && status == C2_TRANSACTION_FAILED) { + --numberOfAttempts; + continue; + } + return component; } - return component; } std::shared_ptr Codec2Client::CreateInterfaceByName( const char* interfaceName, - std::shared_ptr* owner) { - std::shared_ptr interface; - c2_status_t status = ForAllServices( - interfaceName, - [owner, &interface, interfaceName]( - const std::shared_ptr &client) - -> c2_status_t { - c2_status_t status = client->createInterface(interfaceName, - &interface); - if (status == C2_OK) { - if (owner) { - *owner = client; + std::shared_ptr* owner, + size_t numberOfAttempts) { + while (true) { + std::shared_ptr interface; + c2_status_t status = ForAllServices( + interfaceName, + [owner, &interface, interfaceName]( + const std::shared_ptr &client) + -> c2_status_t { + c2_status_t status = client->createInterface(interfaceName, + &interface); + if (status == C2_OK) { + if (owner) { + *owner = client; + } + } else if (status != C2_NOT_FOUND) { + LOG(DEBUG) << "IComponentStore(" + << client->getServiceName() + << ")::createInterface(\"" << interfaceName + << "\") returned status = " + << status << "."; } - } else if (status != C2_NOT_FOUND) { - LOG(DEBUG) << "IComponentStore(" - << client->getServiceName() - << ")::createInterface(\"" << interfaceName - << "\") returned status = " - << status << "."; - } - return status; - }); - if (status != C2_OK) { - LOG(DEBUG) << "Could not create interface \"" << interfaceName << "\". " - "Status = " << status << "."; + return status; + }); + if (numberOfAttempts > 0 && status == C2_TRANSACTION_FAILED) { + --numberOfAttempts; + continue; + } + return interface; } - return interface; } std::vector const& Codec2Client::ListComponents() { diff --git a/media/codec2/hidl/client/include/codec2/hidl/client.h b/media/codec2/hidl/client/include/codec2/hidl/client.h index b8a7fb5d5b..848901d07f 100644 --- a/media/codec2/hidl/client/include/codec2/hidl/client.h +++ b/media/codec2/hidl/client/include/codec2/hidl/client.h @@ -179,17 +179,21 @@ struct Codec2Client : public Codec2ConfigurableClient { static std::vector> CreateFromAllServices(); // Try to create a component with a given name from all known - // IComponentStore services. + // IComponentStore services. numberOfAttempts determines the number of times + // to retry the HIDL call if the transaction fails. static std::shared_ptr CreateComponentByName( char const* componentName, std::shared_ptr const& listener, - std::shared_ptr* owner = nullptr); + std::shared_ptr* owner = nullptr, + size_t numberOfAttempts = 10); // Try to create a component interface with a given name from all known - // IComponentStore services. + // IComponentStore services. numberOfAttempts determines the number of times + // to retry the HIDL call if the transaction fails. static std::shared_ptr CreateInterfaceByName( char const* interfaceName, - std::shared_ptr* owner = nullptr); + std::shared_ptr* owner = nullptr, + size_t numberOfAttempts = 10); // List traits from all known IComponentStore services. static std::vector const& ListComponents(); From 87f731b26ed82ee49889bd3a8c9199089841078b Mon Sep 17 00:00:00 2001 From: jovanak Date: Mon, 2 Sep 2019 11:54:39 -0700 Subject: [PATCH 22/34] Allow --user parameter for audio service commands. This allows testing on secondary users. Change similar to ag/7022195 and ag/6936694. Fixes: 139301368 Test: cts-tradefed run cts -m CtsMediaTestCases -t android.media.cts.AudioRecordTest#testRecordNoDataForIdleUids and ... $ adb install testcases/CtsMediaTestCases.apk Success $ adb shell cmd media.audio_policy set-uid-state android.media.cts active $ adb shell cmd media.audio_policy get-uid-state android.media.cts --user 0 active $ adb shell cmd media.audio_policy get-uid-state android.media.cts --user 10 idle $ adb shell cmd media.audio_policy set-uid-state android.media.cts --user 10 active $ adb shell cmd media.audio_policy get-uid-state android.media.cts --user 10 idle $ adb shell cmd media.audio_policy set-uid-state android.media.cts active --user 10 $ adb shell cmd media.audio_policy get-uid-state android.media.cts --user 10 active $ adb shell cmd media.audio_policy set-uid-state android.media.cts idle --user 0 $ adb shell cmd media.audio_policy get-uid-state android.media.cts --user 0 idle $ adb shell cmd media.audio_policy reset-uid-state android.media.cts --user 10 $ adb shell cmd media.audio_policy get-uid-state android.media.cts --user 10 idle Change-Id: I17774efa6aca7c9b9d0af9bf9c1e75474e90d990 (cherry picked from commit be066e1a9aa613f4e7e56fbd16b35d8ad377b68f) --- .../service/AudioPolicyService.cpp | 87 ++++++++++++++----- 1 file changed, 67 insertions(+), 20 deletions(-) diff --git a/services/audiopolicy/service/AudioPolicyService.cpp b/services/audiopolicy/service/AudioPolicyService.cpp index f8cecbf6fc..a6cda20798 100644 --- a/services/audiopolicy/service/AudioPolicyService.cpp +++ b/services/audiopolicy/service/AudioPolicyService.cpp @@ -726,11 +726,11 @@ status_t AudioPolicyService::shellCommand(int in, int out, int err, Vector= 3 && args[0] == String16("set-uid-state")) { return handleSetUidState(args, err); - } else if (args.size() == 2 && args[0] == String16("reset-uid-state")) { + } else if (args.size() >= 2 && args[0] == String16("reset-uid-state")) { return handleResetUidState(args, err); - } else if (args.size() == 2 && args[0] == String16("get-uid-state")) { + } else if (args.size() >= 2 && args[0] == String16("get-uid-state")) { return handleGetUidState(args, out, err); } else if (args.size() == 1 && args[0] == String16("help")) { printHelp(out); @@ -740,14 +740,32 @@ status_t AudioPolicyService::shellCommand(int in, int out, int err, Vector& args, int err) { +static status_t getUidForPackage(String16 packageName, int userId, /*inout*/uid_t& uid, int err) { + if (userId < 0) { + ALOGE("Invalid user: %d", userId); + dprintf(err, "Invalid user: %d\n", userId); + return BAD_VALUE; + } + PermissionController pc; - int uid = pc.getPackageUid(args[1], 0); + uid = pc.getPackageUid(packageName, 0); if (uid <= 0) { - ALOGE("Unknown package: '%s'", String8(args[1]).string()); - dprintf(err, "Unknown package: '%s'\n", String8(args[1]).string()); + ALOGE("Unknown package: '%s'", String8(packageName).string()); + dprintf(err, "Unknown package: '%s'\n", String8(packageName).string()); + return BAD_VALUE; + } + + uid = multiuser_get_uid(userId, uid); + return NO_ERROR; +} + +status_t AudioPolicyService::handleSetUidState(Vector& args, int err) { + // Valid arg.size() is 3 or 5, args.size() is 5 with --user option. + if (!(args.size() == 3 || args.size() == 5)) { + printHelp(err); return BAD_VALUE; } + bool active = false; if (args[2] == String16("active")) { active = true; @@ -755,30 +773,59 @@ status_t AudioPolicyService::handleSetUidState(Vector& args, int err) ALOGE("Expected active or idle but got: '%s'", String8(args[2]).string()); return BAD_VALUE; } + + int userId = 0; + if (args.size() >= 5 && args[3] == String16("--user")) { + userId = atoi(String8(args[4])); + } + + uid_t uid; + if (getUidForPackage(args[1], userId, uid, err) == BAD_VALUE) { + return BAD_VALUE; + } + mUidPolicy->addOverrideUid(uid, active); return NO_ERROR; } status_t AudioPolicyService::handleResetUidState(Vector& args, int err) { - PermissionController pc; - int uid = pc.getPackageUid(args[1], 0); - if (uid < 0) { - ALOGE("Unknown package: '%s'", String8(args[1]).string()); - dprintf(err, "Unknown package: '%s'\n", String8(args[1]).string()); + // Valid arg.size() is 2 or 4, args.size() is 4 with --user option. + if (!(args.size() == 2 || args.size() == 4)) { + printHelp(err); + return BAD_VALUE; + } + + int userId = 0; + if (args.size() >= 4 && args[2] == String16("--user")) { + userId = atoi(String8(args[3])); + } + + uid_t uid; + if (getUidForPackage(args[1], userId, uid, err) == BAD_VALUE) { return BAD_VALUE; } + mUidPolicy->removeOverrideUid(uid); return NO_ERROR; } status_t AudioPolicyService::handleGetUidState(Vector& args, int out, int err) { - PermissionController pc; - int uid = pc.getPackageUid(args[1], 0); - if (uid < 0) { - ALOGE("Unknown package: '%s'", String8(args[1]).string()); - dprintf(err, "Unknown package: '%s'\n", String8(args[1]).string()); + // Valid arg.size() is 2 or 4, args.size() is 4 with --user option. + if (!(args.size() == 2 || args.size() == 4)) { + printHelp(err); + return BAD_VALUE; + } + + int userId = 0; + if (args.size() >= 4 && args[2] == String16("--user")) { + userId = atoi(String8(args[3])); + } + + uid_t uid; + if (getUidForPackage(args[1], userId, uid, err) == BAD_VALUE) { return BAD_VALUE; } + if (mUidPolicy->isUidActive(uid)) { return dprintf(out, "active\n"); } else { @@ -788,9 +835,9 @@ status_t AudioPolicyService::handleGetUidState(Vector& args, int out, status_t AudioPolicyService::printHelp(int out) { return dprintf(out, "Audio policy service commands:\n" - " get-uid-state gets the uid state\n" - " set-uid-state overrides the uid state\n" - " reset-uid-state clears the uid state override\n" + " get-uid-state [--user USER_ID] gets the uid state\n" + " set-uid-state [--user USER_ID] overrides the uid state\n" + " reset-uid-state [--user USER_ID] clears the uid state override\n" " help print this message\n"); } From 329ac9c93b7de104f4b7e0a111195a772294e980 Mon Sep 17 00:00:00 2001 From: Pawin Vongmasa Date: Mon, 9 Sep 2019 21:28:05 -0700 Subject: [PATCH 23/34] Codec2: Retain compatibility with old HAL Make the vndk module "libstagefright_bufferpool@2.0" work with existing Codec2 HALs, and create a new vendor (non-vndk) module "libstagefright_bufferpool@2.0.1" that new Codec2 HALs can use. Test: atest CtsMediaTestCases -- \ --module-arg CtsMediaTestCases:size:small Bug: 138171841 Bug: 140471279 Change-Id: I2886432afef86ef66fbd48ee744e5fd8de2d1e21 --- media/bufferpool/2.0/Android.bp | 28 ++++++++++++++++++++------ media/bufferpool/2.0/ClientManager.cpp | 8 ++++++++ media/codec2/hidl/1.0/utils/Android.bp | 8 ++++---- media/codec2/hidl/client/Android.bp | 2 +- media/codec2/vndk/Android.bp | 2 +- 5 files changed, 36 insertions(+), 12 deletions(-) diff --git a/media/bufferpool/2.0/Android.bp b/media/bufferpool/2.0/Android.bp index c71ac17d32..e8a69c9ce1 100644 --- a/media/bufferpool/2.0/Android.bp +++ b/media/bufferpool/2.0/Android.bp @@ -1,9 +1,5 @@ -cc_library { - name: "libstagefright_bufferpool@2.0", - vendor_available: true, - vndk: { - enabled: true, - }, +cc_defaults { + name: "libstagefright_bufferpool@2.0-default", srcs: [ "Accessor.cpp", "AccessorImpl.cpp", @@ -31,3 +27,23 @@ cc_library { "android.hardware.media.bufferpool@2.0", ], } + +cc_library { + name: "libstagefright_bufferpool@2.0.1", + defaults: ["libstagefright_bufferpool@2.0-default"], + vendor_available: true, + cflags: [ + "-DBUFFERPOOL_CLONE_HANDLES", + ], +} + +// Deprecated. Do not use. Use libstagefright_bufferpool@2.0.1 instead. +cc_library { + name: "libstagefright_bufferpool@2.0", + defaults: ["libstagefright_bufferpool@2.0-default"], + vendor_available: true, + vndk: { + enabled: true, + }, +} + diff --git a/media/bufferpool/2.0/ClientManager.cpp b/media/bufferpool/2.0/ClientManager.cpp index 48c2da4be4..87ee4e848e 100644 --- a/media/bufferpool/2.0/ClientManager.cpp +++ b/media/bufferpool/2.0/ClientManager.cpp @@ -351,6 +351,7 @@ ResultStatus ClientManager::Impl::allocate( } client = it->second; } +#ifdef BUFFERPOOL_CLONE_HANDLES native_handle_t *origHandle; ResultStatus res = client->allocate(params, &origHandle, buffer); if (res != ResultStatus::OK) { @@ -362,6 +363,9 @@ ResultStatus ClientManager::Impl::allocate( return ResultStatus::NO_MEMORY; } return ResultStatus::OK; +#else + return client->allocate(params, handle, buffer); +#endif } ResultStatus ClientManager::Impl::receive( @@ -377,6 +381,7 @@ ResultStatus ClientManager::Impl::receive( } client = it->second; } +#ifdef BUFFERPOOL_CLONE_HANDLES native_handle_t *origHandle; ResultStatus res = client->receive( transactionId, bufferId, timestampUs, &origHandle, buffer); @@ -389,6 +394,9 @@ ResultStatus ClientManager::Impl::receive( return ResultStatus::NO_MEMORY; } return ResultStatus::OK; +#else + return client->receive(transactionId, bufferId, timestampUs, handle, buffer); +#endif } ResultStatus ClientManager::Impl::postSend( diff --git a/media/codec2/hidl/1.0/utils/Android.bp b/media/codec2/hidl/1.0/utils/Android.bp index 63fe36bfe5..f1f1536617 100644 --- a/media/codec2/hidl/1.0/utils/Android.bp +++ b/media/codec2/hidl/1.0/utils/Android.bp @@ -24,7 +24,7 @@ cc_library { "libgui", "libhidlbase", "liblog", - "libstagefright_bufferpool@2.0", + "libstagefright_bufferpool@2.0.1", "libui", "libutils", ], @@ -37,7 +37,7 @@ cc_library { "android.hardware.media.c2@1.0", "libcodec2", "libgui", - "libstagefright_bufferpool@2.0", + "libstagefright_bufferpool@2.0.1", "libui", ], } @@ -83,7 +83,7 @@ cc_library { "libhidltransport", "libhwbinder", "liblog", - "libstagefright_bufferpool@2.0", + "libstagefright_bufferpool@2.0.1", "libstagefright_bufferqueue_helper", "libui", "libutils", @@ -98,7 +98,7 @@ cc_library { "libcodec2", "libcodec2_vndk", "libhidlbase", - "libstagefright_bufferpool@2.0", + "libstagefright_bufferpool@2.0.1", "libui", ], } diff --git a/media/codec2/hidl/client/Android.bp b/media/codec2/hidl/client/Android.bp index 6038a4043f..e18422337b 100644 --- a/media/codec2/hidl/client/Android.bp +++ b/media/codec2/hidl/client/Android.bp @@ -19,7 +19,7 @@ cc_library { "libhidlbase", "libhidltransport", "liblog", - "libstagefright_bufferpool@2.0", + "libstagefright_bufferpool@2.0.1", "libui", "libutils", ], diff --git a/media/codec2/vndk/Android.bp b/media/codec2/vndk/Android.bp index b6ddfabf2d..52cc7adf23 100644 --- a/media/codec2/vndk/Android.bp +++ b/media/codec2/vndk/Android.bp @@ -66,7 +66,7 @@ cc_library_shared { "liblog", "libnativewindow", "libstagefright_foundation", - "libstagefright_bufferpool@2.0", + "libstagefright_bufferpool@2.0.1", "libui", "libutils", ], From 641cf39651b4e1a34bce760d53a7f460acf93fdf Mon Sep 17 00:00:00 2001 From: Robert Shih Date: Tue, 10 Sep 2019 18:00:42 -0700 Subject: [PATCH 24/34] NdkMediaDrm: properly check null listeners Bug: 140456613 Test: NativeMediaDrmClearkeyTest Change-Id: Id3bfaa3da4a73d1607f191d2ad44dce47ae7f4a0 --- media/ndk/NdkMediaDrm.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/media/ndk/NdkMediaDrm.cpp b/media/ndk/NdkMediaDrm.cpp index 85dbffe16d..cd5a23aa32 100644 --- a/media/ndk/NdkMediaDrm.cpp +++ b/media/ndk/NdkMediaDrm.cpp @@ -89,7 +89,7 @@ struct AMediaDrm { }; void DrmListener::notify(DrmPlugin::EventType eventType, int extra, const Parcel *obj) { - if (!mEventListener && !mExpirationUpdateListener && !mKeysChangeListener) { + if (!mEventListener || !mExpirationUpdateListener || !mKeysChangeListener) { ALOGE("No listeners are specified"); return; } From 343be8a876759a20bab67492951a95b9f05e723a Mon Sep 17 00:00:00 2001 From: Harish Mahendrakar Date: Thu, 1 Aug 2019 12:38:58 -0700 Subject: [PATCH 25/34] codec2: Add support for updating output delay in avc and hevc decoders OUTPUT_DELAY was set as 8 for avc and hevc c2 decoder plugins. It is now set based on the value present in SPS Bug: 138627015 Test: poc in bug Test: atest android.media.cts.DecoderTest Test: atest android.media.cts.AdaptivePlaybackTest Change-Id: I50f7b3e8bd08c9d3b19e84c3c25acaa4eb767226 --- media/codec2/components/avc/C2SoftAvcDec.cpp | 29 +++++++++++++++++-- media/codec2/components/avc/C2SoftAvcDec.h | 2 +- .../codec2/components/hevc/C2SoftHevcDec.cpp | 28 ++++++++++++++++-- media/codec2/components/hevc/C2SoftHevcDec.h | 2 +- 4 files changed, 54 insertions(+), 7 deletions(-) diff --git a/media/codec2/components/avc/C2SoftAvcDec.cpp b/media/codec2/components/avc/C2SoftAvcDec.cpp index 3f015b464c..fa98178187 100644 --- a/media/codec2/components/avc/C2SoftAvcDec.cpp +++ b/media/codec2/components/avc/C2SoftAvcDec.cpp @@ -33,7 +33,8 @@ namespace android { namespace { constexpr char COMPONENT_NAME[] = "c2.android.avc.decoder"; - +constexpr uint32_t kDefaultOutputDelay = 8; +constexpr uint32_t kMaxOutputDelay = 16; } // namespace class C2SoftAvcDec::IntfImpl : public SimpleInterface::BaseParams { @@ -54,7 +55,9 @@ public: // TODO: Proper support for reorder depth. addParameter( DefineParam(mActualOutputDelay, C2_PARAMKEY_OUTPUT_DELAY) - .withConstValue(new C2PortActualDelayTuning::output(8u)) + .withDefault(new C2PortActualDelayTuning::output(kDefaultOutputDelay)) + .withFields({C2F(mActualOutputDelay, value).inRange(0, kMaxOutputDelay)}) + .withSetter(Setter::StrictValueWithNoDeps) .build()); // TODO: output latency and reordering @@ -196,7 +199,6 @@ public: 0u, HAL_PIXEL_FORMAT_YCBCR_420_888)) .build()); } - static C2R SizeSetter(bool mayBlock, const C2P &oldMe, C2P &me) { (void)mayBlock; @@ -333,6 +335,7 @@ C2SoftAvcDec::C2SoftAvcDec( mDecHandle(nullptr), mOutBufferFlush(nullptr), mIvColorFormat(IV_YUV_420P), + mOutputDelay(kDefaultOutputDelay), mWidth(320), mHeight(240), mHeaderDecoded(false), @@ -882,6 +885,26 @@ void C2SoftAvcDec::process( work->result = C2_CORRUPTED; return; } + if (s_decode_op.i4_reorder_depth >= 0 && mOutputDelay != s_decode_op.i4_reorder_depth) { + mOutputDelay = s_decode_op.i4_reorder_depth; + ALOGV("New Output delay %d ", mOutputDelay); + + C2PortActualDelayTuning::output outputDelay(mOutputDelay); + std::vector> failures; + c2_status_t err = + mIntf->config({&outputDelay}, C2_MAY_BLOCK, &failures); + if (err == OK) { + work->worklets.front()->output.configUpdate.push_back( + C2Param::Copy(outputDelay)); + } else { + ALOGE("Cannot set output delay"); + mSignalledError = true; + work->workletsProcessed = 1u; + work->result = C2_CORRUPTED; + return; + } + continue; + } if (0 < s_decode_op.u4_pic_wd && 0 < s_decode_op.u4_pic_ht) { if (mHeaderDecoded == false) { mHeaderDecoded = true; diff --git a/media/codec2/components/avc/C2SoftAvcDec.h b/media/codec2/components/avc/C2SoftAvcDec.h index 72ee583a33..4414a26c96 100644 --- a/media/codec2/components/avc/C2SoftAvcDec.h +++ b/media/codec2/components/avc/C2SoftAvcDec.h @@ -157,7 +157,7 @@ private: size_t mNumCores; IV_COLOR_FORMAT_T mIvColorFormat; - + uint32_t mOutputDelay; uint32_t mWidth; uint32_t mHeight; uint32_t mStride; diff --git a/media/codec2/components/hevc/C2SoftHevcDec.cpp b/media/codec2/components/hevc/C2SoftHevcDec.cpp index 7232572789..df677c284e 100644 --- a/media/codec2/components/hevc/C2SoftHevcDec.cpp +++ b/media/codec2/components/hevc/C2SoftHevcDec.cpp @@ -33,7 +33,8 @@ namespace android { namespace { constexpr char COMPONENT_NAME[] = "c2.android.hevc.decoder"; - +constexpr uint32_t kDefaultOutputDelay = 8; +constexpr uint32_t kMaxOutputDelay = 16; } // namespace class C2SoftHevcDec::IntfImpl : public SimpleInterface::BaseParams { @@ -54,7 +55,9 @@ public: // TODO: Proper support for reorder depth. addParameter( DefineParam(mActualOutputDelay, C2_PARAMKEY_OUTPUT_DELAY) - .withConstValue(new C2PortActualDelayTuning::output(8u)) + .withDefault(new C2PortActualDelayTuning::output(kDefaultOutputDelay)) + .withFields({C2F(mActualOutputDelay, value).inRange(0, kMaxOutputDelay)}) + .withSetter(Setter::StrictValueWithNoDeps) .build()); addParameter( @@ -327,6 +330,7 @@ C2SoftHevcDec::C2SoftHevcDec( mDecHandle(nullptr), mOutBufferFlush(nullptr), mIvColorformat(IV_YUV_420P), + mOutputDelay(kDefaultOutputDelay), mWidth(320), mHeight(240), mHeaderDecoded(false), @@ -877,6 +881,26 @@ void C2SoftHevcDec::process( work->result = C2_CORRUPTED; return; } + if (s_decode_op.i4_reorder_depth >= 0 && mOutputDelay != s_decode_op.i4_reorder_depth) { + mOutputDelay = s_decode_op.i4_reorder_depth; + ALOGV("New Output delay %d ", mOutputDelay); + + C2PortActualDelayTuning::output outputDelay(mOutputDelay); + std::vector> failures; + c2_status_t err = + mIntf->config({&outputDelay}, C2_MAY_BLOCK, &failures); + if (err == OK) { + work->worklets.front()->output.configUpdate.push_back( + C2Param::Copy(outputDelay)); + } else { + ALOGE("Cannot set output delay"); + mSignalledError = true; + work->workletsProcessed = 1u; + work->result = C2_CORRUPTED; + return; + } + continue; + } if (0 < s_decode_op.u4_pic_wd && 0 < s_decode_op.u4_pic_ht) { if (mHeaderDecoded == false) { mHeaderDecoded = true; diff --git a/media/codec2/components/hevc/C2SoftHevcDec.h b/media/codec2/components/hevc/C2SoftHevcDec.h index b7664e65e5..ce63a6c4c3 100644 --- a/media/codec2/components/hevc/C2SoftHevcDec.h +++ b/media/codec2/components/hevc/C2SoftHevcDec.h @@ -115,7 +115,7 @@ struct C2SoftHevcDec : public SimpleC2Component { size_t mNumCores; IV_COLOR_FORMAT_T mIvColorformat; - + uint32_t mOutputDelay; uint32_t mWidth; uint32_t mHeight; uint32_t mStride; From 7712a4be1507698f66663ae6ce5f04806c16ce74 Mon Sep 17 00:00:00 2001 From: Ray Essick Date: Fri, 6 Sep 2019 17:06:29 -0700 Subject: [PATCH 26/34] Allow codec2/g711 codec to handle 6 channels OMX allowed 6, but codec2 was restricting to 1 channel. Bug: 140469404 Test: playback clip from bug (cherry picked from commit edba5005d9012e809cb08f8864e073c57f392421) Change-Id: I9d86c40741f1442ca8147e2802a9ce54df31bbd1 --- media/codec2/components/g711/C2SoftG711Dec.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/media/codec2/components/g711/C2SoftG711Dec.cpp b/media/codec2/components/g711/C2SoftG711Dec.cpp index 43b843ae88..b6cc32e7f4 100644 --- a/media/codec2/components/g711/C2SoftG711Dec.cpp +++ b/media/codec2/components/g711/C2SoftG711Dec.cpp @@ -73,7 +73,7 @@ public: addParameter( DefineParam(mChannelCount, C2_PARAMKEY_CHANNEL_COUNT) - .withDefault(new C2StreamChannelCountInfo::output(0u, 1)) + .withDefault(new C2StreamChannelCountInfo::output(0u, 6)) .withFields({C2F(mChannelCount, value).equalTo(1)}) .withSetter(Setter::StrictValueWithNoDeps) .build()); From 447eaf16ad3483a6e95b9a7be7890dbb92a70899 Mon Sep 17 00:00:00 2001 From: Wonsik Kim Date: Tue, 3 Sep 2019 14:10:37 -0700 Subject: [PATCH 27/34] MediaCodec: recognize KEY_CAPTURE_RATE Bug: 140363871 Test: manual Change-Id: Idc25d048f792ad214497baf60950a73821f14148 --- media/codec2/sfplugin/CCodec.cpp | 14 +++++++++++--- media/libstagefright/ACodec.cpp | 7 ++++++- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/media/codec2/sfplugin/CCodec.cpp b/media/codec2/sfplugin/CCodec.cpp index 8223273b40..4a31953a80 100644 --- a/media/codec2/sfplugin/CCodec.cpp +++ b/media/codec2/sfplugin/CCodec.cpp @@ -814,9 +814,17 @@ void CCodec::configure(const sp &msg) { } { - double value; - if (msg->findDouble("time-lapse-fps", &value)) { - config->mISConfig->mCaptureFps = value; + bool captureFpsFound = false; + double timeLapseFps; + float captureRate; + if (msg->findDouble("time-lapse-fps", &timeLapseFps)) { + config->mISConfig->mCaptureFps = timeLapseFps; + captureFpsFound = true; + } else if (msg->findAsFloat(KEY_CAPTURE_RATE, &captureRate)) { + config->mISConfig->mCaptureFps = captureRate; + captureFpsFound = true; + } + if (captureFpsFound) { (void)msg->findAsFloat(KEY_FRAME_RATE, &config->mISConfig->mCodedFps); } } diff --git a/media/libstagefright/ACodec.cpp b/media/libstagefright/ACodec.cpp index d198d39bc6..35492bd304 100644 --- a/media/libstagefright/ACodec.cpp +++ b/media/libstagefright/ACodec.cpp @@ -1844,7 +1844,12 @@ status_t ACodec::configureCodec( } if (!msg->findDouble("time-lapse-fps", &mCaptureFps)) { - mCaptureFps = -1.0; + float captureRate; + if (msg->findAsFloat(KEY_CAPTURE_RATE, &captureRate)) { + mCaptureFps = captureRate; + } else { + mCaptureFps = -1.0; + } } if (!msg->findInt32( From ae55cbf717757115f53b8a9cf71ba76e763599ce Mon Sep 17 00:00:00 2001 From: Ray Essick Date: Fri, 13 Sep 2019 09:20:20 -0700 Subject: [PATCH 28/34] force g711 test to match 1-channel sample data special case the g711 test to know that it is a 1-channel input. Test case itself can't infer the channel count from the g711 stream since there is no header/control info within the g711 stream. Bug: 140773833 Test: vts-tradefed run vts --module VtsHalMediaC2V1_0Host Change-Id: I9ac1f4d3ee0ccc42ffc8c199fcb88dc5c7122c0e (cherry picked from commit 72b55c5e31a3835a15349dffe24d8acd49adadc3) --- .../functional/audio/VtsHalMediaC2V1_0TargetAudioDecTest.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/media/codec2/hidl/1.0/vts/functional/audio/VtsHalMediaC2V1_0TargetAudioDecTest.cpp b/media/codec2/hidl/1.0/vts/functional/audio/VtsHalMediaC2V1_0TargetAudioDecTest.cpp index 6469735611..a8a552cfba 100644 --- a/media/codec2/hidl/1.0/vts/functional/audio/VtsHalMediaC2V1_0TargetAudioDecTest.cpp +++ b/media/codec2/hidl/1.0/vts/functional/audio/VtsHalMediaC2V1_0TargetAudioDecTest.cpp @@ -547,6 +547,10 @@ TEST_P(Codec2AudioDecDecodeTest, DecodeTest) { if (mCompName == raw) { bitStreamInfo[0] = 8000; bitStreamInfo[1] = 1; + } else if (mCompName == g711alaw || mCompName == g711mlaw) { + // g711 test data is all 1-channel and has no embedded config info. + bitStreamInfo[0] = 8000; + bitStreamInfo[1] = 1; } else { ASSERT_NO_FATAL_FAILURE( getInputChannelInfo(mComponent, mCompName, bitStreamInfo)); From 62cc92bb3801f1181403c78b4f942874050bb874 Mon Sep 17 00:00:00 2001 From: Jeffrey Carlyle Date: Tue, 17 Sep 2019 11:15:15 -0700 Subject: [PATCH 29/34] ServiceUtilities: don't do RECORD_AUDIO check for system server Only one system server seems to exist across multiple-users; however, when the primary user is backgrounded, system server loses its RECORD_AUDIO permission. Through a number of steps this prevents the secondary user from starting any services that use the SoundTrigger HAL; specifically, this was found because it prevents Oslo from functioning when a secondary user is active. Bug: 139839188 Test: Oslo use cases pass Change-Id: Ia0dd5fd3b6992cb18279b81146b35ed040771245 Signed-off-by: Jeffrey Carlyle --- media/utils/Android.bp | 1 + media/utils/ServiceUtilities.cpp | 5 ++++- media/utils/include/mediautils/ServiceUtilities.h | 6 ++++++ 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/media/utils/Android.bp b/media/utils/Android.bp index 3adb40f5a3..e2cd4e3483 100644 --- a/media/utils/Android.bp +++ b/media/utils/Android.bp @@ -27,6 +27,7 @@ cc_library { ], shared_libs: [ "libbinder", + "libcutils", "liblog", "libutils", "libmemunreachable", diff --git a/media/utils/ServiceUtilities.cpp b/media/utils/ServiceUtilities.cpp index b824212b51..bc8fff6ef2 100644 --- a/media/utils/ServiceUtilities.cpp +++ b/media/utils/ServiceUtilities.cpp @@ -63,7 +63,10 @@ static bool checkRecordingInternal(const String16& opPackageName, pid_t pid, uid_t uid, bool start) { // Okay to not track in app ops as audio server is us and if // device is rooted security model is considered compromised. - if (isAudioServerOrRootUid(uid)) return true; + // system_server loses its RECORD_AUDIO permission when a secondary + // user is active, but it is a core system service so let it through. + // TODO(b/141210120): UserManager.DISALLOW_RECORD_AUDIO should not affect system user 0 + if (isAudioServerOrSystemServerOrRootUid(uid)) return true; // We specify a pid and uid here as mediaserver (aka MediaRecorder or StageFrightRecorder) // may open a record track on behalf of a client. Note that pid may be a tid. diff --git a/media/utils/include/mediautils/ServiceUtilities.h b/media/utils/include/mediautils/ServiceUtilities.h index 2a6e60966f..e1089d5bbc 100644 --- a/media/utils/include/mediautils/ServiceUtilities.h +++ b/media/utils/include/mediautils/ServiceUtilities.h @@ -58,6 +58,12 @@ static inline bool isAudioServerOrSystemServerUid(uid_t uid) { return multiuser_get_app_id(uid) == AID_SYSTEM || uid == AID_AUDIOSERVER; } +// used for calls that should come from system_server or audio_server and +// include AID_ROOT for command-line tests. +static inline bool isAudioServerOrSystemServerOrRootUid(uid_t uid) { + return multiuser_get_app_id(uid) == AID_SYSTEM || uid == AID_AUDIOSERVER || uid == AID_ROOT; +} + // Mediaserver may forward the client PID and UID as part of a binder interface call; // otherwise the calling UID must be equal to the client UID. static inline bool isAudioServerOrMediaServerUid(uid_t uid) { From cf935409652e633096729bdb64029cf2383534e3 Mon Sep 17 00:00:00 2001 From: Jayant Chowdhary Date: Wed, 18 Sep 2019 20:14:02 -0700 Subject: [PATCH 30/34] Do not include hidden secure cameras in camera1: getNumberOfCameras Apps cannot connect to hidden secure cameras. Do not include them in the number of cameras reported by camera1 api. Bug: 141247926 Test: Without CL -> mark all cameras as hidden secure cameras; atest FastBasicsTest.java#testCamera1 fails With CL -> mark all cameras as hidden secure cameras; atest FastBasicsTest.java#testCamera1 passes Test: camera CTS Merged-In: I9d1721fd5e94fa7f692c3da52aa667ae9247d368 Change-Id: I229a336bed6b2695e16c1457cb8f74c26b07f7e8 Signed-off-by: Jayant Chowdhary --- .../common/CameraProviderManager.cpp | 19 ++++++++++++++++--- .../common/CameraProviderManager.h | 5 ++++- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/services/camera/libcameraservice/common/CameraProviderManager.cpp b/services/camera/libcameraservice/common/CameraProviderManager.cpp index c72029ff5a..7aa671400f 100644 --- a/services/camera/libcameraservice/common/CameraProviderManager.cpp +++ b/services/camera/libcameraservice/common/CameraProviderManager.cpp @@ -108,7 +108,13 @@ int CameraProviderManager::getCameraCount() const { std::lock_guard lock(mInterfaceMutex); int count = 0; for (auto& provider : mProviders) { - count += provider->mUniqueCameraIds.size(); + for (auto& id : provider->mUniqueCameraIds) { + // Hidden secure camera ids are not to be exposed to camera1 api. + if (isPublicallyHiddenSecureCameraLocked(id)) { + continue; + } + count++; + } } return count; } @@ -134,7 +140,11 @@ std::vector CameraProviderManager::getAPI1CompatibleCameraDeviceIds // for each camera facing, only take the first id advertised by HAL in // all [logical, physical1, physical2, ...] id combos, and filter out the rest. filterLogicalCameraIdsLocked(providerDeviceIds); - + // Hidden secure camera ids are not to be exposed to camera1 api. + providerDeviceIds.erase(std::remove_if(providerDeviceIds.begin(), providerDeviceIds.end(), + [this](const std::string& s) { + return this->isPublicallyHiddenSecureCameraLocked(s);}), + providerDeviceIds.end()); deviceIds.insert(deviceIds.end(), providerDeviceIds.begin(), providerDeviceIds.end()); } @@ -1046,9 +1056,12 @@ bool CameraProviderManager::isLogicalCamera(const std::string& id, return deviceInfo->mIsLogicalCamera; } -bool CameraProviderManager::isPublicallyHiddenSecureCamera(const std::string& id) { +bool CameraProviderManager::isPublicallyHiddenSecureCamera(const std::string& id) const { std::lock_guard lock(mInterfaceMutex); + return isPublicallyHiddenSecureCameraLocked(id); +} +bool CameraProviderManager::isPublicallyHiddenSecureCameraLocked(const std::string& id) const { auto deviceInfo = findDeviceInfoLocked(id); if (deviceInfo == nullptr) { return false; diff --git a/services/camera/libcameraservice/common/CameraProviderManager.h b/services/camera/libcameraservice/common/CameraProviderManager.h index 8cdfc24a9c..01eb56fe10 100644 --- a/services/camera/libcameraservice/common/CameraProviderManager.h +++ b/services/camera/libcameraservice/common/CameraProviderManager.h @@ -272,7 +272,7 @@ public: */ bool isLogicalCamera(const std::string& id, std::vector* physicalCameraIds); - bool isPublicallyHiddenSecureCamera(const std::string& id); + bool isPublicallyHiddenSecureCamera(const std::string& id) const; bool isHiddenPhysicalCamera(const std::string& cameraId); static const float kDepthARTolerance; @@ -594,6 +594,9 @@ private: status_t getCameraCharacteristicsLocked(const std::string &id, CameraMetadata* characteristics) const; + + bool isPublicallyHiddenSecureCameraLocked(const std::string& id) const; + void filterLogicalCameraIdsLocked(std::vector& deviceIds) const; }; From 24b44157d3a9736fce60479cbbbabadf53471df0 Mon Sep 17 00:00:00 2001 From: Shuzhen Wang Date: Fri, 20 Sep 2019 10:38:11 -0700 Subject: [PATCH 31/34] Camera: Delay pingCameraServiceProxy to end of onFirstRef The cameraservice is only registered to service manager after onFirstRef is returned. To reduce the probability of getBinderService() call returning null, move pingCameraServiceProxy to end of onFirstRef(). Also add debug messages in onFirstRef and main() for better diagnoses. Test: Camera CTS Bug: 140414594 Change-Id: I52defd1c138ade57ebc5181c42aff30921fbeaeb --- camera/cameraserver/main_cameraserver.cpp | 1 + services/camera/libcameraservice/CameraService.cpp | 7 +++++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/camera/cameraserver/main_cameraserver.cpp b/camera/cameraserver/main_cameraserver.cpp index 53b3d84894..cef8ef5456 100644 --- a/camera/cameraserver/main_cameraserver.cpp +++ b/camera/cameraserver/main_cameraserver.cpp @@ -34,6 +34,7 @@ int main(int argc __unused, char** argv __unused) sp sm = defaultServiceManager(); ALOGI("ServiceManager: %p", sm.get()); CameraService::instantiate(); + ALOGI("ServiceManager: %p done instantiate", sm.get()); ProcessState::self()->startThreadPool(); IPCThreadState::self()->joinThreadPool(); } diff --git a/services/camera/libcameraservice/CameraService.cpp b/services/camera/libcameraservice/CameraService.cpp index 048d0e6bce..2f889573be 100644 --- a/services/camera/libcameraservice/CameraService.cpp +++ b/services/camera/libcameraservice/CameraService.cpp @@ -153,8 +153,6 @@ void CameraService::onFirstRef() mInitialized = true; } - CameraService::pingCameraServiceProxy(); - mUidPolicy = new UidPolicy(this); mUidPolicy->registerSelf(); mSensorPrivacyPolicy = new SensorPrivacyPolicy(this); @@ -164,6 +162,11 @@ void CameraService::onFirstRef() ALOGE("%s: Failed to register default android.frameworks.cameraservice.service@1.0", __FUNCTION__); } + + // This needs to be last call in this function, so that it's as close to + // ServiceManager::addService() as possible. + CameraService::pingCameraServiceProxy(); + ALOGI("CameraService pinged cameraservice proxy"); } status_t CameraService::enumerateProviders() { From 55badc66542311fe695322fb85dbcfdf23993f20 Mon Sep 17 00:00:00 2001 From: Chong Zhang Date: Tue, 17 Sep 2019 13:20:17 -0700 Subject: [PATCH 32/34] ACodec: read max pts gap and max fps keys for image encoders These keys are needed to disable backward frame drops for image encoders. They should be read for both video and image types. bug: 141169323 Change-Id: I12bea9b25a9902a50ba0e7f7d3f8aa5a497c581c (cherry picked from commit 226cedd0ccd3c17c917c951952e278eacd26d7c8) --- media/libstagefright/ACodec.cpp | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/media/libstagefright/ACodec.cpp b/media/libstagefright/ACodec.cpp index 35492bd304..369e13f780 100644 --- a/media/libstagefright/ACodec.cpp +++ b/media/libstagefright/ACodec.cpp @@ -1826,6 +1826,23 @@ status_t ACodec::configureCodec( mRepeatFrameDelayUs = -1LL; } + if (!msg->findDouble("time-lapse-fps", &mCaptureFps)) { + float captureRate; + if (msg->findAsFloat(KEY_CAPTURE_RATE, &captureRate)) { + mCaptureFps = captureRate; + } else { + mCaptureFps = -1.0; + } + } + + if (!msg->findInt32( + KEY_CREATE_INPUT_SURFACE_SUSPENDED, + (int32_t*)&mCreateInputBuffersSuspended)) { + mCreateInputBuffersSuspended = false; + } + } + + if (encoder && (mIsVideo || mIsImage)) { // only allow 32-bit value, since we pass it as U32 to OMX. if (!msg->findInt64(KEY_MAX_PTS_GAP_TO_ENCODER, &mMaxPtsGapUs)) { mMaxPtsGapUs = 0LL; @@ -1842,21 +1859,6 @@ status_t ACodec::configureCodec( if (mMaxPtsGapUs < 0LL) { mMaxFps = -1; } - - if (!msg->findDouble("time-lapse-fps", &mCaptureFps)) { - float captureRate; - if (msg->findAsFloat(KEY_CAPTURE_RATE, &captureRate)) { - mCaptureFps = captureRate; - } else { - mCaptureFps = -1.0; - } - } - - if (!msg->findInt32( - KEY_CREATE_INPUT_SURFACE_SUSPENDED, - (int32_t*)&mCreateInputBuffersSuspended)) { - mCreateInputBuffersSuspended = false; - } } // NOTE: we only use native window for video decoders From 1fa2686026ce128fa16f47fb00f33ecc70ecdb4f Mon Sep 17 00:00:00 2001 From: Chong Zhang Date: Mon, 16 Sep 2019 16:15:00 -0700 Subject: [PATCH 33/34] Make a copy of output format in onOutputFormatChanged Make a copy of output format in NDK and camera's heic encoder. (MediaCodec.java JNI is already making a copy, so copy out of MediaCodec.cpp to avoid unnecessary dup) bug:141140020 Change-Id: I8afb1e48b73f0fb2fa584bdee2b93985b48a5e91 (cherry picked from commit 860eff192e00df5977be5ff8a6fbf3efa6f1f8d5) --- media/ndk/NdkMediaCodec.cpp | 8 +++++++- .../camera/libcameraservice/api2/HeicCompositeStream.cpp | 9 +++++++-- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/media/ndk/NdkMediaCodec.cpp b/media/ndk/NdkMediaCodec.cpp index c23f19b9d2..e041533586 100644 --- a/media/ndk/NdkMediaCodec.cpp +++ b/media/ndk/NdkMediaCodec.cpp @@ -221,7 +221,13 @@ void CodecHandler::onMessageReceived(const sp &msg) { break; } - AMediaFormat *aMediaFormat = AMediaFormat_fromMsg(&format); + // Here format is MediaCodec's internal copy of output format. + // Make a copy since the client might modify it. + sp copy; + if (format != nullptr) { + copy = format->dup(); + } + AMediaFormat *aMediaFormat = AMediaFormat_fromMsg(©); Mutex::Autolock _l(mCodec->mAsyncCallbackLock); if (mCodec->mAsyncCallbackUserData != NULL diff --git a/services/camera/libcameraservice/api2/HeicCompositeStream.cpp b/services/camera/libcameraservice/api2/HeicCompositeStream.cpp index 5a87134743..052112a821 100644 --- a/services/camera/libcameraservice/api2/HeicCompositeStream.cpp +++ b/services/camera/libcameraservice/api2/HeicCompositeStream.cpp @@ -1671,8 +1671,13 @@ void HeicCompositeStream::CodecCallbackHandler::onMessageReceived(const sponHeicFormatChanged(format); + // Here format is MediaCodec's internal copy of output format. + // Make a copy since onHeicFormatChanged() might modify it. + sp formatCopy; + if (format != nullptr) { + formatCopy = format->dup(); + } + parent->onHeicFormatChanged(formatCopy); break; } From b4e0c3b19f11e972c9b789550c4e321cf26fa1c8 Mon Sep 17 00:00:00 2001 From: Marco Nelissen Date: Fri, 27 Sep 2019 10:21:55 -0700 Subject: [PATCH 34/34] Remove libmediadrm symlinks Test: build Change-Id: I53d703b6d34226926437b0207a2b3d51dff6881a Merged-In: I53d703b6d34226926437b0207a2b3d51dff6881a --- cmds/screenrecord/Android.bp | 4 +++ cmds/screenrecord/screenrecord.cpp | 2 +- cmds/stagefright/Android.mk | 8 ++++-- cmds/stagefright/SimplePlayer.cpp | 2 +- cmds/stagefright/codec.cpp | 2 +- cmds/stagefright/mediafilter.cpp | 2 +- cmds/stagefright/stagefright.cpp | 1 - drm/libmediadrm/Android.bp | 26 +++++++++++++++++++ .../libmediadrm/include/mediadrm}/CryptoHal.h | 0 .../libmediadrm/include/mediadrm}/DrmHal.h | 0 .../include/mediadrm}/DrmMetrics.h | 0 .../include/mediadrm}/DrmPluginPath.h | 0 .../mediadrm}/DrmSessionClientInterface.h | 0 .../include/mediadrm}/DrmSessionManager.h | 0 .../libmediadrm/include/mediadrm}/IDrm.h | 0 .../include/mediadrm}/IDrmClient.h | 0 .../include/mediadrm}/IMediaDrmService.h | 0 .../include/mediadrm}/SharedLibrary.h | 0 .../libmediadrm/interface/mediadrm}/ICrypto.h | 0 drm/libmediadrm/tests/Android.bp | 1 + include/mediadrm/CryptoHal.h | 1 - include/mediadrm/DrmHal.h | 1 - include/mediadrm/DrmMetrics.h | 1 - include/mediadrm/DrmPluginPath.h | 1 - include/mediadrm/DrmSessionClientInterface.h | 1 - include/mediadrm/DrmSessionManager.h | 1 - include/mediadrm/ICrypto.h | 1 - include/mediadrm/IDrm.h | 1 - include/mediadrm/IDrmClient.h | 1 - include/mediadrm/IMediaDrmService.h | 1 - include/mediadrm/SharedLibrary.h | 1 - media/codec2/components/cmds/Android.bp | 4 +++ media/codec2/components/cmds/codec2.cpp | 2 +- media/codec2/sfplugin/Android.bp | 1 + media/codec2/sfplugin/CCodecBufferChannel.h | 1 - media/codec2/sfplugin/Codec2Buffer.h | 2 +- media/codec2/sfplugin/tests/Android.bp | 4 +++ .../sfplugin/tests/MediaCodec_sanity_test.cpp | 2 +- .../libmediaplayerservice/nuplayer/Android.bp | 1 + .../libmediaplayerservice/nuplayer/NuPlayer.h | 2 +- .../nuplayer/NuPlayerDecoder.cpp | 2 +- .../nuplayer/NuPlayerDecoderPassThrough.cpp | 2 +- .../nuplayer/NuPlayerDrm.h | 4 +-- .../nuplayer/NuPlayerSource.h | 2 +- media/libstagefright/Android.bp | 5 ++++ media/libstagefright/BufferImpl.cpp | 2 +- media/libstagefright/CodecBase.cpp | 2 +- media/libstagefright/FrameDecoder.cpp | 2 +- media/libstagefright/MediaCodec.cpp | 2 +- .../MediaCodecListOverrides.cpp | 2 +- media/libstagefright/MediaCodecSource.cpp | 2 +- media/libstagefright/SimpleDecodingSource.cpp | 2 +- media/libstagefright/filters/Android.bp | 4 +++ .../include/ACodecBufferChannel.h | 2 +- media/libstagefright/include/SecureBuffer.h | 2 +- media/ndk/Android.bp | 6 ++++- media/ndk/NdkMediaCrypto.cpp | 4 +-- media/ndk/NdkMediaCryptoPriv.h | 2 +- media/ndk/NdkMediaDrm.cpp | 6 ++--- services/camera/libcameraservice/Android.bp | 4 +++ .../api2/HeicCompositeStream.cpp | 2 +- services/mediadrm/Android.mk | 3 +++ 62 files changed, 97 insertions(+), 45 deletions(-) rename {media/libmedia/include/media => drm/libmediadrm/include/mediadrm}/CryptoHal.h (100%) rename {media/libmedia/include/media => drm/libmediadrm/include/mediadrm}/DrmHal.h (100%) rename {media/libmedia/include/media => drm/libmediadrm/include/mediadrm}/DrmMetrics.h (100%) rename {media/libmedia/include/media => drm/libmediadrm/include/mediadrm}/DrmPluginPath.h (100%) rename {media/libmedia/include/media => drm/libmediadrm/include/mediadrm}/DrmSessionClientInterface.h (100%) rename {media/libmedia/include/media => drm/libmediadrm/include/mediadrm}/DrmSessionManager.h (100%) rename {media/libmedia/include/media => drm/libmediadrm/include/mediadrm}/IDrm.h (100%) rename {media/libmedia/include/media => drm/libmediadrm/include/mediadrm}/IDrmClient.h (100%) rename {media/libmedia/include/media => drm/libmediadrm/include/mediadrm}/IMediaDrmService.h (100%) rename {media/libmedia/include/media => drm/libmediadrm/include/mediadrm}/SharedLibrary.h (100%) rename {media/libmedia/include/media => drm/libmediadrm/interface/mediadrm}/ICrypto.h (100%) delete mode 120000 include/mediadrm/CryptoHal.h delete mode 120000 include/mediadrm/DrmHal.h delete mode 120000 include/mediadrm/DrmMetrics.h delete mode 120000 include/mediadrm/DrmPluginPath.h delete mode 120000 include/mediadrm/DrmSessionClientInterface.h delete mode 120000 include/mediadrm/DrmSessionManager.h delete mode 120000 include/mediadrm/ICrypto.h delete mode 120000 include/mediadrm/IDrm.h delete mode 120000 include/mediadrm/IDrmClient.h delete mode 120000 include/mediadrm/IMediaDrmService.h delete mode 120000 include/mediadrm/SharedLibrary.h diff --git a/cmds/screenrecord/Android.bp b/cmds/screenrecord/Android.bp index 86476cd0ca..6bdbab178d 100644 --- a/cmds/screenrecord/Android.bp +++ b/cmds/screenrecord/Android.bp @@ -24,6 +24,10 @@ cc_binary { "Program.cpp", ], + header_libs: [ + "libmediadrm_headers", + ], + shared_libs: [ "libstagefright", "libmedia", diff --git a/cmds/screenrecord/screenrecord.cpp b/cmds/screenrecord/screenrecord.cpp index df2884228d..f2a71b3533 100644 --- a/cmds/screenrecord/screenrecord.cpp +++ b/cmds/screenrecord/screenrecord.cpp @@ -52,7 +52,7 @@ #include #include #include -#include +#include #include #include "screenrecord.h" diff --git a/cmds/stagefright/Android.mk b/cmds/stagefright/Android.mk index caf478d227..185307ed15 100644 --- a/cmds/stagefright/Android.mk +++ b/cmds/stagefright/Android.mk @@ -133,6 +133,9 @@ LOCAL_SRC_FILES:= \ codec.cpp \ SimplePlayer.cpp \ +LOCAL_HEADER_LIBRARIES := \ + libmediadrm_headers \ + LOCAL_SHARED_LIBRARIES := \ libstagefright liblog libutils libbinder libstagefright_foundation \ libmedia libmedia_omx libaudioclient libui libgui libcutils @@ -159,17 +162,18 @@ LOCAL_SRC_FILES:= \ filters/saturation.rscript \ mediafilter.cpp \ +LOCAL_HEADER_LIBRARIES := \ + libmediadrm_headers \ + LOCAL_SHARED_LIBRARIES := \ libstagefright \ liblog \ libutils \ libbinder \ libstagefright_foundation \ - libmedia \ libmedia_omx \ libui \ libgui \ - libcutils \ libRScpp \ LOCAL_C_INCLUDES:= \ diff --git a/cmds/stagefright/SimplePlayer.cpp b/cmds/stagefright/SimplePlayer.cpp index afb7db38b6..f4b8164970 100644 --- a/cmds/stagefright/SimplePlayer.cpp +++ b/cmds/stagefright/SimplePlayer.cpp @@ -23,7 +23,7 @@ #include #include -#include +#include #include #include #include diff --git a/cmds/stagefright/codec.cpp b/cmds/stagefright/codec.cpp index e5a43373c5..f2d1c29f24 100644 --- a/cmds/stagefright/codec.cpp +++ b/cmds/stagefright/codec.cpp @@ -23,7 +23,7 @@ #include #include -#include +#include #include #include #include diff --git a/cmds/stagefright/mediafilter.cpp b/cmds/stagefright/mediafilter.cpp index 2cf6955e63..66302b0488 100644 --- a/cmds/stagefright/mediafilter.cpp +++ b/cmds/stagefright/mediafilter.cpp @@ -24,9 +24,9 @@ #include #include #include -#include #include #include +#include #include #include #include diff --git a/cmds/stagefright/stagefright.cpp b/cmds/stagefright/stagefright.cpp index bf36be09e3..d55931cbcf 100644 --- a/cmds/stagefright/stagefright.cpp +++ b/cmds/stagefright/stagefright.cpp @@ -33,7 +33,6 @@ #include #include #include -#include #include #include #include diff --git a/drm/libmediadrm/Android.bp b/drm/libmediadrm/Android.bp index c29d00433c..39b048a964 100644 --- a/drm/libmediadrm/Android.bp +++ b/drm/libmediadrm/Android.bp @@ -2,6 +2,15 @@ // libmediadrm // +cc_library_headers { + name: "libmediadrm_headers", + + export_include_dirs: [ + "interface" + ], + +} + cc_library_shared { name: "libmediadrm", @@ -17,6 +26,15 @@ cc_library_shared { "CryptoHal.cpp", ], + local_include_dirs: [ + "include", + "interface" + ], + + export_include_dirs: [ + "include" + ], + shared_libs: [ "libbinder", "libcutils", @@ -49,6 +67,10 @@ cc_library_shared { "protos/metrics.proto", ], + local_include_dirs: [ + "include" + ], + proto: { export_proto_headers: true, type: "lite", @@ -80,6 +102,10 @@ cc_library_shared { "protos/metrics.proto", ], + local_include_dirs: [ + "include" + ], + proto: { export_proto_headers: true, type: "full", diff --git a/media/libmedia/include/media/CryptoHal.h b/drm/libmediadrm/include/mediadrm/CryptoHal.h similarity index 100% rename from media/libmedia/include/media/CryptoHal.h rename to drm/libmediadrm/include/mediadrm/CryptoHal.h diff --git a/media/libmedia/include/media/DrmHal.h b/drm/libmediadrm/include/mediadrm/DrmHal.h similarity index 100% rename from media/libmedia/include/media/DrmHal.h rename to drm/libmediadrm/include/mediadrm/DrmHal.h diff --git a/media/libmedia/include/media/DrmMetrics.h b/drm/libmediadrm/include/mediadrm/DrmMetrics.h similarity index 100% rename from media/libmedia/include/media/DrmMetrics.h rename to drm/libmediadrm/include/mediadrm/DrmMetrics.h diff --git a/media/libmedia/include/media/DrmPluginPath.h b/drm/libmediadrm/include/mediadrm/DrmPluginPath.h similarity index 100% rename from media/libmedia/include/media/DrmPluginPath.h rename to drm/libmediadrm/include/mediadrm/DrmPluginPath.h diff --git a/media/libmedia/include/media/DrmSessionClientInterface.h b/drm/libmediadrm/include/mediadrm/DrmSessionClientInterface.h similarity index 100% rename from media/libmedia/include/media/DrmSessionClientInterface.h rename to drm/libmediadrm/include/mediadrm/DrmSessionClientInterface.h diff --git a/media/libmedia/include/media/DrmSessionManager.h b/drm/libmediadrm/include/mediadrm/DrmSessionManager.h similarity index 100% rename from media/libmedia/include/media/DrmSessionManager.h rename to drm/libmediadrm/include/mediadrm/DrmSessionManager.h diff --git a/media/libmedia/include/media/IDrm.h b/drm/libmediadrm/include/mediadrm/IDrm.h similarity index 100% rename from media/libmedia/include/media/IDrm.h rename to drm/libmediadrm/include/mediadrm/IDrm.h diff --git a/media/libmedia/include/media/IDrmClient.h b/drm/libmediadrm/include/mediadrm/IDrmClient.h similarity index 100% rename from media/libmedia/include/media/IDrmClient.h rename to drm/libmediadrm/include/mediadrm/IDrmClient.h diff --git a/media/libmedia/include/media/IMediaDrmService.h b/drm/libmediadrm/include/mediadrm/IMediaDrmService.h similarity index 100% rename from media/libmedia/include/media/IMediaDrmService.h rename to drm/libmediadrm/include/mediadrm/IMediaDrmService.h diff --git a/media/libmedia/include/media/SharedLibrary.h b/drm/libmediadrm/include/mediadrm/SharedLibrary.h similarity index 100% rename from media/libmedia/include/media/SharedLibrary.h rename to drm/libmediadrm/include/mediadrm/SharedLibrary.h diff --git a/media/libmedia/include/media/ICrypto.h b/drm/libmediadrm/interface/mediadrm/ICrypto.h similarity index 100% rename from media/libmedia/include/media/ICrypto.h rename to drm/libmediadrm/interface/mediadrm/ICrypto.h diff --git a/drm/libmediadrm/tests/Android.bp b/drm/libmediadrm/tests/Android.bp index 9e0115e635..873083b162 100644 --- a/drm/libmediadrm/tests/Android.bp +++ b/drm/libmediadrm/tests/Android.bp @@ -28,6 +28,7 @@ cc_test { ], static_libs: ["libgmock"], include_dirs: [ + "frameworks/av/drm/libmediadrm/include", "frameworks/av/include/media", ], cflags: [ diff --git a/include/mediadrm/CryptoHal.h b/include/mediadrm/CryptoHal.h deleted file mode 120000 index 92f3137604..0000000000 --- a/include/mediadrm/CryptoHal.h +++ /dev/null @@ -1 +0,0 @@ -../../media/libmedia/include/media/CryptoHal.h \ No newline at end of file diff --git a/include/mediadrm/DrmHal.h b/include/mediadrm/DrmHal.h deleted file mode 120000 index 17bb667d08..0000000000 --- a/include/mediadrm/DrmHal.h +++ /dev/null @@ -1 +0,0 @@ -../../media/libmedia/include/media/DrmHal.h \ No newline at end of file diff --git a/include/mediadrm/DrmMetrics.h b/include/mediadrm/DrmMetrics.h deleted file mode 120000 index abc966b473..0000000000 --- a/include/mediadrm/DrmMetrics.h +++ /dev/null @@ -1 +0,0 @@ -../../media/libmedia/include/media/DrmMetrics.h \ No newline at end of file diff --git a/include/mediadrm/DrmPluginPath.h b/include/mediadrm/DrmPluginPath.h deleted file mode 120000 index 9e05194642..0000000000 --- a/include/mediadrm/DrmPluginPath.h +++ /dev/null @@ -1 +0,0 @@ -../../media/libmedia/include/media/DrmPluginPath.h \ No newline at end of file diff --git a/include/mediadrm/DrmSessionClientInterface.h b/include/mediadrm/DrmSessionClientInterface.h deleted file mode 120000 index f4e32111fc..0000000000 --- a/include/mediadrm/DrmSessionClientInterface.h +++ /dev/null @@ -1 +0,0 @@ -../../media/libmedia/include/media/DrmSessionClientInterface.h \ No newline at end of file diff --git a/include/mediadrm/DrmSessionManager.h b/include/mediadrm/DrmSessionManager.h deleted file mode 120000 index f0a47bfef8..0000000000 --- a/include/mediadrm/DrmSessionManager.h +++ /dev/null @@ -1 +0,0 @@ -../../media/libmedia/include/media/DrmSessionManager.h \ No newline at end of file diff --git a/include/mediadrm/ICrypto.h b/include/mediadrm/ICrypto.h deleted file mode 120000 index b250e07d80..0000000000 --- a/include/mediadrm/ICrypto.h +++ /dev/null @@ -1 +0,0 @@ -../../media/libmedia/include/media/ICrypto.h \ No newline at end of file diff --git a/include/mediadrm/IDrm.h b/include/mediadrm/IDrm.h deleted file mode 120000 index 841bb1b65e..0000000000 --- a/include/mediadrm/IDrm.h +++ /dev/null @@ -1 +0,0 @@ -../../media/libmedia/include/media/IDrm.h \ No newline at end of file diff --git a/include/mediadrm/IDrmClient.h b/include/mediadrm/IDrmClient.h deleted file mode 120000 index 10aa5c0d06..0000000000 --- a/include/mediadrm/IDrmClient.h +++ /dev/null @@ -1 +0,0 @@ -../../media/libmedia/include/media/IDrmClient.h \ No newline at end of file diff --git a/include/mediadrm/IMediaDrmService.h b/include/mediadrm/IMediaDrmService.h deleted file mode 120000 index f3c260f8f9..0000000000 --- a/include/mediadrm/IMediaDrmService.h +++ /dev/null @@ -1 +0,0 @@ -../../media/libmedia/include/media/IMediaDrmService.h \ No newline at end of file diff --git a/include/mediadrm/SharedLibrary.h b/include/mediadrm/SharedLibrary.h deleted file mode 120000 index 9f8f5a44f3..0000000000 --- a/include/mediadrm/SharedLibrary.h +++ /dev/null @@ -1 +0,0 @@ -../../media/libmedia/include/media/SharedLibrary.h \ No newline at end of file diff --git a/media/codec2/components/cmds/Android.bp b/media/codec2/components/cmds/Android.bp index 35f689e1bd..681a1716e4 100644 --- a/media/codec2/components/cmds/Android.bp +++ b/media/codec2/components/cmds/Android.bp @@ -9,6 +9,10 @@ cc_binary { include_dirs: [ ], + header_libs: [ + "libmediadrm_headers", + ], + shared_libs: [ "libbase", "libbinder", diff --git a/media/codec2/components/cmds/codec2.cpp b/media/codec2/components/cmds/codec2.cpp index f2cf545b82..479f064776 100644 --- a/media/codec2/components/cmds/codec2.cpp +++ b/media/codec2/components/cmds/codec2.cpp @@ -31,7 +31,7 @@ #include #include #include -#include +#include #include #include #include diff --git a/media/codec2/sfplugin/Android.bp b/media/codec2/sfplugin/Android.bp index 9c84c711e2..5112e80ddb 100644 --- a/media/codec2/sfplugin/Android.bp +++ b/media/codec2/sfplugin/Android.bp @@ -22,6 +22,7 @@ cc_library_shared { header_libs: [ "libcodec2_internal", + "libmediadrm_headers", ], shared_libs: [ diff --git a/media/codec2/sfplugin/CCodecBufferChannel.h b/media/codec2/sfplugin/CCodecBufferChannel.h index ee3455d567..c0fa138949 100644 --- a/media/codec2/sfplugin/CCodecBufferChannel.h +++ b/media/codec2/sfplugin/CCodecBufferChannel.h @@ -29,7 +29,6 @@ #include #include #include -#include #include "CCodecBuffers.h" #include "InputSurfaceWrapper.h" diff --git a/media/codec2/sfplugin/Codec2Buffer.h b/media/codec2/sfplugin/Codec2Buffer.h index 36dcab97dc..6f87101d52 100644 --- a/media/codec2/sfplugin/Codec2Buffer.h +++ b/media/codec2/sfplugin/Codec2Buffer.h @@ -25,7 +25,7 @@ #include #include #include -#include +#include namespace android { diff --git a/media/codec2/sfplugin/tests/Android.bp b/media/codec2/sfplugin/tests/Android.bp index be7f55cc8b..b6eb2b46ee 100644 --- a/media/codec2/sfplugin/tests/Android.bp +++ b/media/codec2/sfplugin/tests/Android.bp @@ -33,6 +33,10 @@ cc_test { "frameworks/av/media/codec2/sfplugin", ], + header_libs: [ + "libmediadrm_headers", + ], + shared_libs: [ "libbinder", "libcodec2", diff --git a/media/codec2/sfplugin/tests/MediaCodec_sanity_test.cpp b/media/codec2/sfplugin/tests/MediaCodec_sanity_test.cpp index ba3687bc13..6deede09f0 100644 --- a/media/codec2/sfplugin/tests/MediaCodec_sanity_test.cpp +++ b/media/codec2/sfplugin/tests/MediaCodec_sanity_test.cpp @@ -21,7 +21,7 @@ #include #include #include -#include +#include #include #include #include diff --git a/media/libmediaplayerservice/nuplayer/Android.bp b/media/libmediaplayerservice/nuplayer/Android.bp index 23a19e79cd..71d80947da 100644 --- a/media/libmediaplayerservice/nuplayer/Android.bp +++ b/media/libmediaplayerservice/nuplayer/Android.bp @@ -18,6 +18,7 @@ cc_library_static { ], header_libs: [ + "libmediadrm_headers", "media_plugin_headers", ], diff --git a/media/libmediaplayerservice/nuplayer/NuPlayer.h b/media/libmediaplayerservice/nuplayer/NuPlayer.h index 9f5be06d76..0e58ec2dbd 100644 --- a/media/libmediaplayerservice/nuplayer/NuPlayer.h +++ b/media/libmediaplayerservice/nuplayer/NuPlayer.h @@ -19,7 +19,7 @@ #define NU_PLAYER_H_ #include -#include +#include #include #include diff --git a/media/libmediaplayerservice/nuplayer/NuPlayerDecoder.cpp b/media/libmediaplayerservice/nuplayer/NuPlayerDecoder.cpp index 2f0da2d783..bd2b884c83 100644 --- a/media/libmediaplayerservice/nuplayer/NuPlayerDecoder.cpp +++ b/media/libmediaplayerservice/nuplayer/NuPlayerDecoder.cpp @@ -28,7 +28,7 @@ #include "NuPlayerSource.h" #include -#include +#include #include #include #include diff --git a/media/libmediaplayerservice/nuplayer/NuPlayerDecoderPassThrough.cpp b/media/libmediaplayerservice/nuplayer/NuPlayerDecoderPassThrough.cpp index 0997e7d438..793014eecf 100644 --- a/media/libmediaplayerservice/nuplayer/NuPlayerDecoderPassThrough.cpp +++ b/media/libmediaplayerservice/nuplayer/NuPlayerDecoderPassThrough.cpp @@ -24,7 +24,7 @@ #include "NuPlayerRenderer.h" #include "NuPlayerSource.h" -#include +#include #include #include #include diff --git a/media/libmediaplayerservice/nuplayer/NuPlayerDrm.h b/media/libmediaplayerservice/nuplayer/NuPlayerDrm.h index 50f69ff40e..4360656249 100644 --- a/media/libmediaplayerservice/nuplayer/NuPlayerDrm.h +++ b/media/libmediaplayerservice/nuplayer/NuPlayerDrm.h @@ -18,8 +18,8 @@ #define NUPLAYER_DRM_H_ #include -#include -#include +#include +#include #include // for CryptInfo diff --git a/media/libmediaplayerservice/nuplayer/NuPlayerSource.h b/media/libmediaplayerservice/nuplayer/NuPlayerSource.h index 9f5ef78690..f137c520c7 100644 --- a/media/libmediaplayerservice/nuplayer/NuPlayerSource.h +++ b/media/libmediaplayerservice/nuplayer/NuPlayerSource.h @@ -20,7 +20,7 @@ #include "NuPlayer.h" -#include +#include #include #include #include diff --git a/media/libstagefright/Android.bp b/media/libstagefright/Android.bp index bb7f2a5da1..9c37b49649 100644 --- a/media/libstagefright/Android.bp +++ b/media/libstagefright/Android.bp @@ -58,6 +58,10 @@ cc_library_shared { "-Wall", ], + header_libs: [ + "libmediadrm_headers", + ], + shared_libs: [ "libgui", "liblog", @@ -220,6 +224,7 @@ cc_library { ], header_libs:[ + "libmediadrm_headers", "libnativeloader-headers", "libstagefright_xmlparser_headers", "media_ndk_headers", diff --git a/media/libstagefright/BufferImpl.cpp b/media/libstagefright/BufferImpl.cpp index b760273455..f73b6251a9 100644 --- a/media/libstagefright/BufferImpl.cpp +++ b/media/libstagefright/BufferImpl.cpp @@ -21,7 +21,7 @@ #include #include #include -#include +#include #include #include "include/SecureBuffer.h" diff --git a/media/libstagefright/CodecBase.cpp b/media/libstagefright/CodecBase.cpp index d0610b28ec..97f38f8d3c 100644 --- a/media/libstagefright/CodecBase.cpp +++ b/media/libstagefright/CodecBase.cpp @@ -18,7 +18,7 @@ #define LOG_TAG "CodecBase" #include -#include +#include #include #include diff --git a/media/libstagefright/FrameDecoder.cpp b/media/libstagefright/FrameDecoder.cpp index 18a6bd8512..9e5a779678 100644 --- a/media/libstagefright/FrameDecoder.cpp +++ b/media/libstagefright/FrameDecoder.cpp @@ -22,7 +22,7 @@ #include #include #include -#include +#include #include #include #include diff --git a/media/libstagefright/MediaCodec.cpp b/media/libstagefright/MediaCodec.cpp index f579e9de6b..161c1789fd 100644 --- a/media/libstagefright/MediaCodec.cpp +++ b/media/libstagefright/MediaCodec.cpp @@ -35,7 +35,7 @@ #include #include #include -#include +#include #include #include #include diff --git a/media/libstagefright/MediaCodecListOverrides.cpp b/media/libstagefright/MediaCodecListOverrides.cpp index dd7c3e67eb..b027a9703c 100644 --- a/media/libstagefright/MediaCodecListOverrides.cpp +++ b/media/libstagefright/MediaCodecListOverrides.cpp @@ -22,7 +22,7 @@ #include #include -#include +#include #include #include #include diff --git a/media/libstagefright/MediaCodecSource.cpp b/media/libstagefright/MediaCodecSource.cpp index 50e454cb52..7243b82f71 100644 --- a/media/libstagefright/MediaCodecSource.cpp +++ b/media/libstagefright/MediaCodecSource.cpp @@ -22,7 +22,7 @@ #include #include -#include +#include #include #include #include diff --git a/media/libstagefright/SimpleDecodingSource.cpp b/media/libstagefright/SimpleDecodingSource.cpp index babdc7a669..b809848bbd 100644 --- a/media/libstagefright/SimpleDecodingSource.cpp +++ b/media/libstagefright/SimpleDecodingSource.cpp @@ -20,7 +20,7 @@ #include -#include +#include #include #include #include diff --git a/media/libstagefright/filters/Android.bp b/media/libstagefright/filters/Android.bp index b1f62c7c1f..88f30c47b3 100644 --- a/media/libstagefright/filters/Android.bp +++ b/media/libstagefright/filters/Android.bp @@ -23,6 +23,10 @@ cc_library_static { "-Wall", ], + header_libs: [ + "libmediadrm_headers", + ], + shared_libs: [ "libgui", "libmedia", diff --git a/media/libstagefright/include/ACodecBufferChannel.h b/media/libstagefright/include/ACodecBufferChannel.h index 7c01e450e6..3a087d18c0 100644 --- a/media/libstagefright/include/ACodecBufferChannel.h +++ b/media/libstagefright/include/ACodecBufferChannel.h @@ -25,7 +25,7 @@ #include #include -#include +#include #include namespace android { diff --git a/media/libstagefright/include/SecureBuffer.h b/media/libstagefright/include/SecureBuffer.h index cf7933aeee..c45e0e5754 100644 --- a/media/libstagefright/include/SecureBuffer.h +++ b/media/libstagefright/include/SecureBuffer.h @@ -18,7 +18,7 @@ #define SECURE_BUFFER_H_ -#include +#include #include namespace android { diff --git a/media/ndk/Android.bp b/media/ndk/Android.bp index afe3746673..0020ccce48 100644 --- a/media/ndk/Android.bp +++ b/media/ndk/Android.bp @@ -69,6 +69,10 @@ cc_library_shared { "libgrallocusage", ], + header_libs: [ + "libmediadrm_headers", + ], + shared_libs: [ "android.hardware.graphics.bufferqueue@1.0", "android.hidl.token@1.0-utils", @@ -76,9 +80,9 @@ cc_library_shared { "libbase", "libbinder", "libmedia", + "libmediadrm", "libmedia_omx", "libmedia_jni_utils", - "libmediadrm", "libstagefright", "libstagefright_foundation", "liblog", diff --git a/media/ndk/NdkMediaCrypto.cpp b/media/ndk/NdkMediaCrypto.cpp index ce2c660d11..792fc00d47 100644 --- a/media/ndk/NdkMediaCrypto.cpp +++ b/media/ndk/NdkMediaCrypto.cpp @@ -27,8 +27,8 @@ #include #include #include -#include -#include +#include +#include #include #include diff --git a/media/ndk/NdkMediaCryptoPriv.h b/media/ndk/NdkMediaCryptoPriv.h index 14ea928e04..8664d95de7 100644 --- a/media/ndk/NdkMediaCryptoPriv.h +++ b/media/ndk/NdkMediaCryptoPriv.h @@ -30,7 +30,7 @@ #include #include -#include +#include using namespace android; diff --git a/media/ndk/NdkMediaDrm.cpp b/media/ndk/NdkMediaDrm.cpp index 85dbffe16d..60f3e8e4dd 100644 --- a/media/ndk/NdkMediaDrm.cpp +++ b/media/ndk/NdkMediaDrm.cpp @@ -29,12 +29,12 @@ #include #include -#include -#include +#include +#include #include #include -#include #include +#include using namespace android; diff --git a/services/camera/libcameraservice/Android.bp b/services/camera/libcameraservice/Android.bp index b26398e117..87aed41fc2 100644 --- a/services/camera/libcameraservice/Android.bp +++ b/services/camera/libcameraservice/Android.bp @@ -69,6 +69,10 @@ cc_library_shared { "utils/LatencyHistogram.cpp", ], + header_libs: [ + "libmediadrm_headers" + ], + shared_libs: [ "libbase", "libdl", diff --git a/services/camera/libcameraservice/api2/HeicCompositeStream.cpp b/services/camera/libcameraservice/api2/HeicCompositeStream.cpp index 5a87134743..3d1235e304 100644 --- a/services/camera/libcameraservice/api2/HeicCompositeStream.cpp +++ b/services/camera/libcameraservice/api2/HeicCompositeStream.cpp @@ -28,7 +28,7 @@ #include #include -#include +#include #include #include #include diff --git a/services/mediadrm/Android.mk b/services/mediadrm/Android.mk index 3e9459652c..d4bb48a5c1 100644 --- a/services/mediadrm/Android.mk +++ b/services/mediadrm/Android.mk @@ -20,6 +20,9 @@ LOCAL_SRC_FILES:= \ MediaDrmService.cpp \ main_mediadrmserver.cpp +LOCAL_HEADER_LIBRARIES:= \ + libmediadrm_headers + LOCAL_SHARED_LIBRARIES:= \ libbinder \ liblog \