Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix MediaSDK_C2 Coverity issues #88

Open
wants to merge 1 commit into
base: celadon/s/mr0/stable
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions c2_components/src/mfx_c2_decoder_component.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -633,6 +633,7 @@ MfxC2DecoderComponent::MfxC2DecoderComponent(const C2String name, const CreateCo
// By default prepare buffer to be displayed on any of the common surfaces
m_consumerUsage = kDefaultConsumerUsage;

MFX_ZERO_MEMORY(m_signalInfo);
//m_paramStorage.DumpParams();
}

Expand Down Expand Up @@ -797,17 +798,16 @@ c2_status_t MfxC2DecoderComponent::Release()
}
#else
sts = m_mfxSession.Close();
if (MFX_ERR_NONE != sts) res = MfxStatusToC2(sts);
#endif

if (MFX_ERR_NONE != sts) res = MfxStatusToC2(sts);

if (m_allocator) {
m_allocator = nullptr;
}

if (m_device) {
m_device->Close();
if (MFX_ERR_NONE != sts) res = MfxStatusToC2(sts);

m_device = nullptr;
}
Expand Down Expand Up @@ -1814,6 +1814,10 @@ c2_status_t MfxC2DecoderComponent::AllocateFrame(MfxC2FrameOut* frame_out)
std::unique_ptr<native_handle_t, decltype(hndl_deleter)> hndl(
android::UnwrapNativeCodec2GrallocHandle(out_block->handle()), hndl_deleter);

if(hndl == nullptr)
{
return C2_NO_MEMORY;
}
auto it = m_surfaces.end();
if (m_mfxVideoParams.IOPattern == MFX_IOPATTERN_OUT_VIDEO_MEMORY) {

Expand Down Expand Up @@ -1851,7 +1855,7 @@ c2_status_t MfxC2DecoderComponent::AllocateFrame(MfxC2FrameOut* frame_out)
m_surfacePool.push_back(frame_out->GetMfxFrameSurface());
} else {
auto it = m_surfacePool.begin();
for(auto mfx_frame: m_surfacePool) {
for(auto& mfx_frame: m_surfacePool) {
// Check if there is avaiable surface in the pool
if (!mfx_frame->Data.Locked) {
auto blk = m_blocks.begin();
Expand Down
19 changes: 6 additions & 13 deletions c2_components/src/mfx_c2_encoder_component.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -459,6 +459,8 @@ MfxC2EncoderComponent::MfxC2EncoderComponent(const C2String name, const CreateCo
m_codedColorAspects->matrix = C2Color::MATRIX_UNSPECIFIED;
m_codedColorAspects->primaries = C2Color::PRIMARIES_UNSPECIFIED;

MFX_ZERO_MEMORY(m_signalInfo);
MFX_ZERO_MEMORY(m_mfxInputInfo);
//m_paramStorage.DumpParams();
}

Expand Down Expand Up @@ -1252,6 +1254,7 @@ void MfxC2EncoderComponent::DoWork(std::unique_ptr<C2Work>&& work)

if (MFX_ERR_NONE != mfx_sts) {
res = MfxStatusToC2(mfx_sts);
MFX_ZERO_MEMORY(mfx_frame_in);
break;
}

Expand Down Expand Up @@ -1305,8 +1308,8 @@ void MfxC2EncoderComponent::DoWork(std::unique_ptr<C2Work>&& work)
break;
}

m_waitingQueue.Push( [ mfx_frame = std::move(mfx_frame_in), this ] () mutable {
RetainLockedFrame(std::move(mfx_frame));
m_waitingQueue.Push([&mfx_frame_in, this ] () mutable {
RetainLockedFrame(std::move(mfx_frame_in));
} );

m_pendingWorks.push(std::move(work));
Expand Down Expand Up @@ -1877,11 +1880,6 @@ void MfxC2EncoderComponent::DoUpdateMfxParam(const std::vector<C2Param*> &params
}
case kParamIndexColorAspects: {
if (C2StreamColorAspectsInfo::input::PARAM_TYPE == param->index()) {
m_colorAspects->range = m_colorAspects->range;
m_colorAspects->transfer = m_colorAspects->transfer;
m_colorAspects->matrix = m_colorAspects->matrix;
m_colorAspects->primaries = m_colorAspects->primaries;

// set video signal info
setColorAspects_l();

Expand All @@ -1890,11 +1888,6 @@ void MfxC2EncoderComponent::DoUpdateMfxParam(const std::vector<C2Param*> &params
MFX_DEBUG_TRACE_U32(m_colorAspects->transfer);
MFX_DEBUG_TRACE_U32(m_colorAspects->matrix);
} else {
m_codedColorAspects->range = m_codedColorAspects->range;
m_codedColorAspects->transfer = m_codedColorAspects->transfer;
m_codedColorAspects->matrix = m_codedColorAspects->matrix;
m_codedColorAspects->primaries = m_codedColorAspects->primaries;

MFX_DEBUG_TRACE_U32(m_codedColorAspects->range);
MFX_DEBUG_TRACE_U32(m_codedColorAspects->primaries);
MFX_DEBUG_TRACE_U32(m_codedColorAspects->transfer);
Expand Down Expand Up @@ -2102,7 +2095,7 @@ uint32_t MfxC2EncoderComponent::getSyncFramePeriod_l(int32_t sync_frame_period)
return 0;
}

double frame_rate = m_mfxVideoParamsConfig.mfx.FrameInfo.FrameRateExtN / m_mfxVideoParamsConfig.mfx.FrameInfo.FrameRateExtD;
double frame_rate = m_mfxVideoParamsConfig.mfx.FrameInfo.FrameRateExtN / m_mfxVideoParamsConfig.mfx.FrameInfo.FrameRateExtD * 1.0;
double period = sync_frame_period / 1e6 * frame_rate;

MFX_DEBUG_TRACE_F64(frame_rate);
Expand Down
1 change: 1 addition & 0 deletions c2_components/src/mfx_c2_vpp_wrapp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ MfxC2VppWrapp::MfxC2VppWrapp(void):
MFX_ZERO_MEMORY(m_vppParam);
MFX_ZERO_MEMORY(m_allocator);
MFX_ZERO_MEMORY(m_responses);
MFX_ZERO_MEMORY(m_vppSrf);
}

MfxC2VppWrapp::~MfxC2VppWrapp(void)
Expand Down
9 changes: 8 additions & 1 deletion c2_store/src/mfx_c2_service.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,14 @@ int main(int /* argc */, char** /* argv */) {
ALOGD("[email protected] starting...");

signal(SIGPIPE, SIG_IGN);
android::SetUpMinijail(kBaseSeccompPolicyPath, kExtSeccompPolicyPath);
try
{
android::SetUpMinijail(kBaseSeccompPolicyPath, kExtSeccompPolicyPath);
}
catch(const std::exception& e)
{
ALOGE("SetUpMinijail function execution error");
}

// vndbinder is needed by BufferQueue.
android::ProcessState::initWithDriver("/dev/vndbinder");
Expand Down
1 change: 1 addition & 0 deletions c2_utils/include/mfx_cmd_queue.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

#include <C2Work.h>

#include <utils/Log.h>
#include <functional>
#include <queue>
#include <thread>
Expand Down
21 changes: 18 additions & 3 deletions c2_utils/include/mfx_pool.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

#pragma once

#include <utils/Log.h>
#include <mutex>
#include <future>
#include <memory>
Expand All @@ -46,9 +47,16 @@ class MfxPool
// Correct method to free pool instance.
// This waiting for destroyed is needed as module with pool could be unloaded from
// memory while some resource could try to add itself back to the pool.
std::future<void> destroyed = m_poolImpl->Destroyed();
try
{
std::future<void> destroyed = m_poolImpl->Destroyed();
destroyed.wait();
}
catch(const std::exception& e)
{
// ALOGE("Destroyed function execution error");
}
m_poolImpl.reset();
destroyed.wait();
}

MFX_CLASS_NO_COPY(MfxPool<T>)
Expand Down Expand Up @@ -80,7 +88,14 @@ class MfxPool
public:
~MfxPoolImpl()
{
m_destroyed.set_value();
try
{
m_destroyed.set_value();
}
catch(const std::exception& e)
{
// ALOGE("set_value function execution error");
}
}

void Append(std::unique_ptr<T>&& free_item)
Expand Down
5 changes: 4 additions & 1 deletion c2_utils/src/mfx_c2_utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -669,7 +669,10 @@ YUVWriter::YUVWriter(const std::string& dir,

if (!dir_exists) {
MFX_DEBUG_TRACE_STREAM(NAMED(full_name.str()));
mkdir(full_name.str().c_str(), S_IRWXU | S_IRWXG | S_IROTH | S_IXOTH);
if(mkdir(full_name.str().c_str(), S_IRWXU | S_IRWXG | S_IROTH | S_IXOTH)) {
MFX_DEBUG_TRACE_MSG("cannot create the path");
return;
}
}

full_name << "/";
Expand Down
9 changes: 8 additions & 1 deletion c2_utils/src/mfx_cmd_queue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,14 @@
MfxCmdQueue::~MfxCmdQueue()
{
MFX_DEBUG_TRACE(MFX_PTR_NAME(this));
Abort();
try
{
Abort();
}
catch(const std::exception& e)
{
// ALOGE("Abort function execution error");
}
}

void MfxCmdQueue::Start()
Expand Down
37 changes: 29 additions & 8 deletions c2_utils/src/mfx_va_allocator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -350,11 +350,22 @@ mfxStatus MfxVaFrameAllocator::UnlockFrame(mfxMemId mid, mfxFrameData *frame_dat
if (va_mid && va_mid->surface_) {

if (MFX_FOURCC_P8 == va_mid->fourcc_) { // bitstream processing
vaUnmapBuffer(m_dpy, *(va_mid->surface_));
}
VAStatus va_res = vaUnmapBuffer(m_dpy, *(va_mid->surface_));
if (VA_STATUS_SUCCESS != va_res) {
mfx_res = va_to_mfx_status(va_res);
return mfx_res;
}
else { // Image processing
vaUnmapBuffer(m_dpy, va_mid->image_.buf);
vaDestroyImage(m_dpy, va_mid->image_.image_id);
va_res = vaUnmapBuffer(m_dpy, va_mid->image_.buf);
if (VA_STATUS_SUCCESS != va_res) {
mfx_res = va_to_mfx_status(va_res);
return mfx_res;
}
va_res = vaDestroyImage(m_dpy, va_mid->image_.image_id);
if (VA_STATUS_SUCCESS != va_res) {
mfx_res = va_to_mfx_status(va_res);
return mfx_res;
}

if (nullptr != frame_data) {
frame_data->Pitch = 0;
Expand All @@ -366,6 +377,7 @@ mfxStatus MfxVaFrameAllocator::UnlockFrame(mfxMemId mid, mfxFrameData *frame_dat
}
} else {
mfx_res = MFX_ERR_INVALID_HANDLE;
}
}

MFX_DEBUG_TRACE__mfxStatus(mfx_res);
Expand Down Expand Up @@ -628,6 +640,7 @@ mfxStatus MfxVaFrameAllocator::TouchSurface(VASurfaceID surface)
VAImage image;
unsigned char* buffer;
VAStatus va_res;
mfxStatus mfx_res = MFX_ERR_NONE;

if (VA_INVALID_ID == surface) return MFX_ERR_UNKNOWN;

Expand All @@ -638,10 +651,18 @@ mfxStatus MfxVaFrameAllocator::TouchSurface(VASurfaceID surface)
if (VA_STATUS_SUCCESS == va_res)
{
*buffer = 0x0; // can have any value
vaUnmapBuffer(m_dpy, image.buf);
va_res = vaUnmapBuffer(m_dpy, image.buf);
if (VA_STATUS_SUCCESS != va_res) {
mfx_res = va_to_mfx_status(va_res);
return mfx_res;
}
}
vaDestroyImage(m_dpy, image.image_id);
}
return MFX_ERR_NONE;
va_res = vaDestroyImage(m_dpy, image.image_id);
if (VA_STATUS_SUCCESS != va_res) {
mfx_res = va_to_mfx_status(va_res);
return mfx_res;
}
}
return mfx_res;
}
#endif // #if defined(LIBVA_SUPPORT)
10 changes: 5 additions & 5 deletions c2_utils/src/mfx_va_frame_pool_allocator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -134,11 +134,11 @@ mfxStatus MfxVaFramePoolAllocator::AllocFrames(mfxFrameAllocRequest *request,
native_handle_t *hndl = android::UnwrapNativeCodec2GrallocHandle(new_block->handle());
m_grallocAllocator->GetBackingStore(hndl, &id);
m_cachedBufferId.emplace(id, i);
if (C2_OK != res) {
native_handle_delete(hndl);
mfx_res = MFX_ERR_MEMORY_ALLOC;
break;
}
// if (C2_OK != res) { //TODO dead code not need.
Kexiaox marked this conversation as resolved.
Show resolved Hide resolved
// native_handle_delete(hndl);
// mfx_res = MFX_ERR_MEMORY_ALLOC;
// break;
// }

// deep copy to have unique_ptr as m_pool required unique_ptr
std::unique_ptr<C2GraphicBlock> unique_block = std::make_unique<C2GraphicBlock>(*new_block);
Expand Down