Merge "MediaMetrics: Report last statsd atoms pulled" into sc-dev
diff --git a/media/codec2/components/avc/C2SoftAvcDec.cpp b/media/codec2/components/avc/C2SoftAvcDec.cpp
index f4a6e17..e8287f9 100644
--- a/media/codec2/components/avc/C2SoftAvcDec.cpp
+++ b/media/codec2/components/avc/C2SoftAvcDec.cpp
@@ -22,7 +22,6 @@
#include <C2Debug.h>
#include <C2PlatformSupport.h>
-#include <Codec2BufferUtils.h>
#include <Codec2Mapper.h>
#include <SimpleC2Interface.h>
@@ -332,14 +331,6 @@
free(mem);
}
-static IV_COLOR_FORMAT_T GetIvColorFormat() {
- static IV_COLOR_FORMAT_T sColorFormat =
- (GetYuv420FlexibleLayout() == FLEX_LAYOUT_SEMIPLANAR_UV) ? IV_YUV_420SP_UV :
- (GetYuv420FlexibleLayout() == FLEX_LAYOUT_SEMIPLANAR_VU) ? IV_YUV_420SP_VU :
- IV_YUV_420P;
- return sColorFormat;
-}
-
C2SoftAvcDec::C2SoftAvcDec(
const char *name,
c2_node_id_t id,
@@ -348,6 +339,7 @@
mIntf(intfImpl),
mDecHandle(nullptr),
mOutBufferFlush(nullptr),
+ mIvColorFormat(IV_YUV_420P),
mOutputDelay(kDefaultOutputDelay),
mWidth(320),
mHeight(240),
@@ -426,13 +418,7 @@
s_create_ip.s_ivd_create_ip_t.u4_size = sizeof(ivdext_create_ip_t);
s_create_ip.s_ivd_create_ip_t.e_cmd = IVD_CMD_CREATE;
s_create_ip.s_ivd_create_ip_t.u4_share_disp_buf = 0;
- s_create_ip.s_ivd_create_ip_t.e_output_format = GetIvColorFormat();
- switch (s_create_ip.s_ivd_create_ip_t.e_output_format) {
- case IV_YUV_420P: ALOGD("Flex Planar"); break;
- case IV_YUV_420SP_UV: ALOGD("Flex Semi-planar UV"); break;
- case IV_YUV_420SP_VU: ALOGD("Flex Semi-planar VU"); break;
- default: ALOGD("Unknown"); break;
- }
+ s_create_ip.s_ivd_create_ip_t.e_output_format = mIvColorFormat;
s_create_ip.s_ivd_create_ip_t.pf_aligned_alloc = ivd_aligned_malloc;
s_create_ip.s_ivd_create_ip_t.pf_aligned_free = ivd_aligned_free;
s_create_ip.s_ivd_create_ip_t.pv_mem_ctxt = nullptr;
@@ -569,12 +555,8 @@
ps_decode_ip->u4_num_Bytes = 0;
}
ps_decode_ip->s_out_buffer.u4_min_out_buf_size[0] = lumaSize;
- if (GetIvColorFormat() == IV_YUV_420P) {
- ps_decode_ip->s_out_buffer.u4_min_out_buf_size[1] = chromaSize;
- ps_decode_ip->s_out_buffer.u4_min_out_buf_size[2] = chromaSize;
- } else {
- ps_decode_ip->s_out_buffer.u4_min_out_buf_size[1] = chromaSize * 2;
- }
+ ps_decode_ip->s_out_buffer.u4_min_out_buf_size[1] = chromaSize;
+ ps_decode_ip->s_out_buffer.u4_min_out_buf_size[2] = chromaSize;
if (outBuffer) {
if (outBuffer->height() < displayHeight) {
ALOGE("Output buffer too small: provided (%dx%d) required (%ux%u)",
@@ -583,23 +565,13 @@
}
ps_decode_ip->s_out_buffer.pu1_bufs[0] = outBuffer->data()[C2PlanarLayout::PLANE_Y];
ps_decode_ip->s_out_buffer.pu1_bufs[1] = outBuffer->data()[C2PlanarLayout::PLANE_U];
- if (GetIvColorFormat() == IV_YUV_420P) {
- ps_decode_ip->s_out_buffer.pu1_bufs[2] = outBuffer->data()[C2PlanarLayout::PLANE_V];
- } else if (GetIvColorFormat() == IV_YUV_420SP_VU) {
- ps_decode_ip->s_out_buffer.pu1_bufs[1] = outBuffer->data()[C2PlanarLayout::PLANE_V];
- }
+ ps_decode_ip->s_out_buffer.pu1_bufs[2] = outBuffer->data()[C2PlanarLayout::PLANE_V];
} else {
ps_decode_ip->s_out_buffer.pu1_bufs[0] = mOutBufferFlush;
ps_decode_ip->s_out_buffer.pu1_bufs[1] = mOutBufferFlush + lumaSize;
- if (GetIvColorFormat() == IV_YUV_420P) {
- ps_decode_ip->s_out_buffer.pu1_bufs[2] = mOutBufferFlush + lumaSize + chromaSize;
- }
+ ps_decode_ip->s_out_buffer.pu1_bufs[2] = mOutBufferFlush + lumaSize + chromaSize;
}
- if (GetIvColorFormat() == IV_YUV_420P) {
- ps_decode_ip->s_out_buffer.u4_num_bufs = 3;
- } else {
- ps_decode_ip->s_out_buffer.u4_num_bufs = 2;
- }
+ ps_decode_ip->s_out_buffer.u4_num_bufs = 3;
ps_decode_op->u4_size = sizeof(ih264d_video_decode_op_t);
return true;
@@ -809,7 +781,7 @@
mOutBlock.reset();
}
if (!mOutBlock) {
- uint32_t format = HAL_PIXEL_FORMAT_YCBCR_420_888;
+ uint32_t format = HAL_PIXEL_FORMAT_YV12;
C2MemoryUsage usage = { C2MemoryUsage::CPU_READ, C2MemoryUsage::CPU_WRITE };
c2_status_t err =
pool->fetchGraphicBlock(ALIGN32(mWidth), mHeight, format, usage, &mOutBlock);
@@ -825,6 +797,8 @@
}
// TODO: can overall error checking be improved?
+// TODO: allow configuration of color format and usage for graphic buffers instead
+// of hard coding them to HAL_PIXEL_FORMAT_YV12
// TODO: pass coloraspects information to surface
// TODO: test support for dynamic change in resolution
// TODO: verify if the decoder sent back all frames
diff --git a/media/codec2/components/avc/C2SoftAvcDec.h b/media/codec2/components/avc/C2SoftAvcDec.h
index ed99ad1..5c07d29 100644
--- a/media/codec2/components/avc/C2SoftAvcDec.h
+++ b/media/codec2/components/avc/C2SoftAvcDec.h
@@ -155,6 +155,7 @@
uint8_t *mOutBufferFlush;
size_t mNumCores;
+ IV_COLOR_FORMAT_T mIvColorFormat;
uint32_t mOutputDelay;
uint32_t mWidth;
uint32_t mHeight;
diff --git a/media/libmediatranscoding/TEST_MAPPING b/media/libmediatranscoding/TEST_MAPPING
index f8a9db9..40f7b21 100644
--- a/media/libmediatranscoding/TEST_MAPPING
+++ b/media/libmediatranscoding/TEST_MAPPING
@@ -26,6 +26,9 @@
},
{
"name": "VideoTrackTranscoderTests"
+ },
+ {
+ "name": "CtsMediaTranscodingTestCases"
}
]
}
diff --git a/media/libstagefright/TEST_MAPPING b/media/libstagefright/TEST_MAPPING
index dff7b22..7ce2968 100644
--- a/media/libstagefright/TEST_MAPPING
+++ b/media/libstagefright/TEST_MAPPING
@@ -34,6 +34,9 @@
"presubmit": [
{
"name": "mediacodecTest"
+ },
+ {
+ "name": "CtsMediaTranscodingTestCases"
}
],
"postsubmit": [
diff --git a/services/camera/libcameraservice/common/CameraProviderManager.cpp b/services/camera/libcameraservice/common/CameraProviderManager.cpp
index 8942d05..6dffc5d 100644
--- a/services/camera/libcameraservice/common/CameraProviderManager.cpp
+++ b/services/camera/libcameraservice/common/CameraProviderManager.cpp
@@ -474,12 +474,12 @@
hardware::Return<void> CameraProviderManager::onRegistration(
const hardware::hidl_string& /*fqName*/,
const hardware::hidl_string& name,
- bool /*preexisting*/) {
+ bool preexisting) {
std::lock_guard<std::mutex> providerLock(mProviderLifecycleLock);
{
std::lock_guard<std::mutex> lock(mInterfaceMutex);
- addProviderLocked(name);
+ addProviderLocked(name, preexisting);
}
sp<StatusListener> listener = getStatusListener();
@@ -1230,33 +1230,53 @@
return falseRet;
}
-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__,
- newProvider.c_str());
- return ALREADY_EXISTS;
- }
- }
-
+status_t CameraProviderManager::tryToInitializeProviderLocked(
+ const std::string& providerName, const sp<ProviderInfo>& providerInfo) {
sp<provider::V2_4::ICameraProvider> interface;
- interface = mServiceProxy->tryGetService(newProvider);
+ interface = mServiceProxy->tryGetService(providerName);
if (interface == nullptr) {
// The interface may not be started yet. In that case, this is not a
// fatal error.
ALOGW("%s: Camera provider HAL '%s' is not actually available", __FUNCTION__,
- newProvider.c_str());
+ providerName.c_str());
return BAD_VALUE;
}
- sp<ProviderInfo> providerInfo = new ProviderInfo(newProvider, this);
- status_t res = providerInfo->initialize(interface, mDeviceState);
- if (res != OK) {
- return res;
+ return providerInfo->initialize(interface, mDeviceState);
+}
+
+status_t CameraProviderManager::addProviderLocked(const std::string& newProvider,
+ bool preexisting) {
+ // Several camera provider instances can be temporarily present.
+ // Defer initialization of a new instance until the older instance is properly removed.
+ auto providerInstance = newProvider + "-" + std::to_string(mProviderInstanceId);
+ bool providerPresent = false;
+ for (const auto& providerInfo : mProviders) {
+ if (providerInfo->mProviderName == newProvider) {
+ ALOGW("%s: Camera provider HAL with name '%s' already registered",
+ __FUNCTION__, newProvider.c_str());
+ if (preexisting) {
+ return ALREADY_EXISTS;
+ } else{
+ ALOGW("%s: The new provider instance will get initialized immediately after the"
+ " currently present instance is removed!", __FUNCTION__);
+ providerPresent = true;
+ break;
+ }
+ }
+ }
+
+ sp<ProviderInfo> providerInfo = new ProviderInfo(newProvider, providerInstance, this);
+ if (!providerPresent) {
+ status_t res = tryToInitializeProviderLocked(newProvider, providerInfo);
+ if (res != OK) {
+ return res;
+ }
}
mProviders.push_back(providerInfo);
+ mProviderInstanceId++;
return OK;
}
@@ -1266,12 +1286,14 @@
std::unique_lock<std::mutex> lock(mInterfaceMutex);
std::vector<String8> removedDeviceIds;
status_t res = NAME_NOT_FOUND;
+ std::string removedProviderName;
for (auto it = mProviders.begin(); it != mProviders.end(); it++) {
- if ((*it)->mProviderName == provider) {
+ if ((*it)->mProviderInstance == provider) {
removedDeviceIds.reserve((*it)->mDevices.size());
for (auto& deviceInfo : (*it)->mDevices) {
removedDeviceIds.push_back(String8(deviceInfo->mId.c_str()));
}
+ removedProviderName = (*it)->mProviderName;
mProviders.erase(it);
res = OK;
break;
@@ -1281,6 +1303,14 @@
ALOGW("%s: Camera provider HAL with name '%s' is not registered", __FUNCTION__,
provider.c_str());
} else {
+ // Check if there are any newer camera instances from the same provider and try to
+ // initialize.
+ for (const auto& providerInfo : mProviders) {
+ if (providerInfo->mProviderName == removedProviderName) {
+ return tryToInitializeProviderLocked(removedProviderName, providerInfo);
+ }
+ }
+
// Inform camera service of loss of presence for all the devices from this provider,
// without lock held for reentrancy
sp<StatusListener> listener = getStatusListener();
@@ -1289,7 +1319,9 @@
for (auto& id : removedDeviceIds) {
listener->onDeviceStatusChanged(id, CameraDeviceStatus::NOT_PRESENT);
}
+ lock.lock();
}
+
}
return res;
}
@@ -1303,8 +1335,10 @@
CameraProviderManager::ProviderInfo::ProviderInfo(
const std::string &providerName,
+ const std::string &providerInstance,
CameraProviderManager *manager) :
mProviderName(providerName),
+ mProviderInstance(providerInstance),
mProviderTagid(generateVendorTagId(providerName)),
mUniqueDeviceCount(0),
mManager(manager) {
@@ -1628,7 +1662,7 @@
status_t CameraProviderManager::ProviderInfo::dump(int fd, const Vector<String16>&) const {
dprintf(fd, "== Camera Provider HAL %s (v2.%d, %s) static info: %zu devices: ==\n",
- mProviderName.c_str(),
+ mProviderInstance.c_str(),
mMinorVersion,
mIsRemote ? "remote" : "passthrough",
mDevices.size());
@@ -1944,12 +1978,12 @@
void CameraProviderManager::ProviderInfo::serviceDied(uint64_t cookie,
const wp<hidl::base::V1_0::IBase>& who) {
(void) who;
- ALOGI("Camera provider '%s' has died; removing it", mProviderName.c_str());
+ ALOGI("Camera provider '%s' has died; removing it", mProviderInstance.c_str());
if (cookie != mId) {
ALOGW("%s: Unexpected serviceDied cookie %" PRIu64 ", expected %" PRIu32,
__FUNCTION__, cookie, mId);
}
- mManager->removeProvider(mProviderName);
+ mManager->removeProvider(mProviderInstance);
}
status_t CameraProviderManager::ProviderInfo::setUpVendorTags() {
diff --git a/services/camera/libcameraservice/common/CameraProviderManager.h b/services/camera/libcameraservice/common/CameraProviderManager.h
index 12bda9b..5531dd7 100644
--- a/services/camera/libcameraservice/common/CameraProviderManager.h
+++ b/services/camera/libcameraservice/common/CameraProviderManager.h
@@ -365,6 +365,7 @@
virtual public hardware::hidl_death_recipient
{
const std::string mProviderName;
+ const std::string mProviderInstance;
const metadata_vendor_id_t mProviderTagid;
int mMinorVersion;
sp<VendorTagDescriptor> mVendorTagDescriptor;
@@ -379,7 +380,7 @@
sp<hardware::camera::provider::V2_4::ICameraProvider> mSavedInterface;
- ProviderInfo(const std::string &providerName,
+ ProviderInfo(const std::string &providerName, const std::string &providerInstance,
CameraProviderManager *manager);
~ProviderInfo();
@@ -657,7 +658,10 @@
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);
+ status_t addProviderLocked(const std::string& newProvider, bool preexisting = false);
+
+ status_t tryToInitializeProviderLocked(const std::string& providerName,
+ const sp<ProviderInfo>& providerInfo);
bool isLogicalCameraLocked(const std::string& id, std::vector<std::string>* physicalCameraIds);
@@ -666,6 +670,7 @@
bool isValidDeviceLocked(const std::string &id, uint16_t majorVersion) const;
+ size_t mProviderInstanceId = 0;
std::vector<sp<ProviderInfo>> mProviders;
void addProviderToMap(
diff --git a/services/camera/libcameraservice/tests/CameraProviderManagerTest.cpp b/services/camera/libcameraservice/tests/CameraProviderManagerTest.cpp
index 855b5ab..a74fd9d 100644
--- a/services/camera/libcameraservice/tests/CameraProviderManagerTest.cpp
+++ b/services/camera/libcameraservice/tests/CameraProviderManagerTest.cpp
@@ -23,7 +23,9 @@
#include <android/hardware/camera/device/3.2/ICameraDeviceCallback.h>
#include <android/hardware/camera/device/3.2/ICameraDeviceSession.h>
#include <camera_metadata_hidden.h>
+#include <hidl/HidlBinderSupport.h>
#include <gtest/gtest.h>
+#include <utility>
using namespace android;
using namespace android::hardware::camera;
@@ -173,6 +175,25 @@
return hardware::Void();
}
+ virtual ::android::hardware::Return<bool> linkToDeath(
+ const ::android::sp<::android::hardware::hidl_death_recipient>& recipient,
+ uint64_t cookie) {
+ if (mInitialDeathRecipient.get() == nullptr) {
+ mInitialDeathRecipient =
+ std::make_unique<::android::hardware::hidl_binder_death_recipient>(recipient,
+ cookie, this);
+ }
+ return true;
+ }
+
+ void signalInitialBinderDeathRecipient() {
+ if (mInitialDeathRecipient.get() != nullptr) {
+ mInitialDeathRecipient->binderDied(nullptr /*who*/);
+ }
+ }
+
+ std::unique_ptr<::android::hardware::hidl_binder_death_recipient> mInitialDeathRecipient;
+
enum MethodNames {
SET_CALLBACK,
GET_VENDOR_TAGS,
@@ -567,3 +588,47 @@
ASSERT_EQ(serviceProxy.mLastRequestedServiceNames.back(), testProviderInstanceName) <<
"Incorrect instance requested from service manager";
}
+
+// Test that CameraProviderManager can handle races between provider death notifications and
+// provider registration callbacks
+TEST(CameraProviderManagerTest, BinderDeathRegistrationRaceTest) {
+
+ std::vector<hardware::hidl_string> deviceNames;
+ deviceNames.push_back("device@3.2/test/0");
+ deviceNames.push_back("device@3.2/test/1");
+ hardware::hidl_vec<common::V1_0::VendorTagSection> vendorSection;
+ status_t res;
+
+ sp<CameraProviderManager> providerManager = new CameraProviderManager();
+ sp<TestStatusListener> statusListener = new TestStatusListener();
+ TestInteractionProxy serviceProxy;
+ sp<TestICameraProvider> provider = new TestICameraProvider(deviceNames,
+ vendorSection);
+
+ // Not setting up provider in the service proxy yet, to test cases where a
+ // HAL isn't starting right
+ res = providerManager->initialize(statusListener, &serviceProxy);
+ ASSERT_EQ(res, OK) << "Unable to initialize provider manager";
+
+ // Now set up provider and trigger a registration
+ serviceProxy.setProvider(provider);
+
+ hardware::hidl_string testProviderFqInterfaceName =
+ "android.hardware.camera.provider@2.4::ICameraProvider";
+ hardware::hidl_string testProviderInstanceName = "test/0";
+ serviceProxy.mManagerNotificationInterface->onRegistration(
+ testProviderFqInterfaceName,
+ testProviderInstanceName, false);
+
+ // Simulate artificial delay of the registration callback which arrives before the
+ // death notification
+ serviceProxy.mManagerNotificationInterface->onRegistration(
+ testProviderFqInterfaceName,
+ testProviderInstanceName, false);
+
+ provider->signalInitialBinderDeathRecipient();
+
+ auto deviceCount = static_cast<unsigned> (providerManager->getCameraCount().second);
+ ASSERT_EQ(deviceCount, deviceNames.size()) <<
+ "Unexpected amount of camera devices";
+}