From 1fafbb6280c6c9bf43d596ab4be30f92f1a05dea Mon Sep 17 00:00:00 2001 From: Chen_Perry Date: Fri, 8 Sep 2023 10:34:21 +0800 Subject: [PATCH] [Encode] Fix encode coverity issues Refactor to (1) fix mem leak issue; (2) fix dead code issue; (3) revert some unnecessary check. --- .../common/codec/hal/codechal_vdenc_avc.cpp | 9 +++-- .../codec/hal/codechal_vdenc_vp9_g12.cpp | 34 ++++++++++++------- .../features/encode_preenc_basic_feature.cpp | 5 --- .../encode_scalability_multipipe.cpp | 2 +- 4 files changed, 27 insertions(+), 23 deletions(-) diff --git a/media_driver/agnostic/common/codec/hal/codechal_vdenc_avc.cpp b/media_driver/agnostic/common/codec/hal/codechal_vdenc_avc.cpp index c43f6f7cfb3..a6e553f77f2 100644 --- a/media_driver/agnostic/common/codec/hal/codechal_vdenc_avc.cpp +++ b/media_driver/agnostic/common/codec/hal/codechal_vdenc_avc.cpp @@ -4606,9 +4606,9 @@ MOS_STATUS CodechalVdencAvcState::SFDKernel() imageStateParams->pVDEncHmeMvCost = m_vdencHmeMvCostTbl; imageStateParams->pVDEncMvCost = m_vdencMvCostTbl; - CODECHAL_ENCODE_CHK_STATUS_RETURN(m_hwInterface->AddVdencSfdImgBuffer( - &m_resVdencSfdImageStateReadBuffer, imageStateParams)); + eStatus = m_hwInterface->AddVdencSfdImgBuffer(&m_resVdencSfdImageStateReadBuffer, imageStateParams); MOS_Delete(imageStateParams); + CODECHAL_ENCODE_CHK_STATUS_RETURN(eStatus); CODECHAL_DEBUG_TOOL( CODECHAL_ENCODE_CHK_STATUS_RETURN(PopulateEncParam( @@ -5295,10 +5295,9 @@ MOS_STATUS CodechalVdencAvcState::HuCBrcUpdate() } } - CODECHAL_ENCODE_CHK_STATUS_RETURN(AddVdencBrcImgBuffer( - &m_resVdencBrcImageStatesReadBuffer[m_currRecycledBufIdx], - imageStateParams)); + eStatus = AddVdencBrcImgBuffer(&m_resVdencBrcImageStatesReadBuffer[m_currRecycledBufIdx],imageStateParams); MOS_Delete(imageStateParams); + CODECHAL_ENCODE_CHK_STATUS_RETURN(eStatus); CODECHAL_DEBUG_TOOL( CODECHAL_ENCODE_CHK_STATUS_RETURN(PopulatePakParam( diff --git a/media_driver/agnostic/gen12/codec/hal/codechal_vdenc_vp9_g12.cpp b/media_driver/agnostic/gen12/codec/hal/codechal_vdenc_vp9_g12.cpp index cce4f8d491d..d70c3d5d492 100644 --- a/media_driver/agnostic/gen12/codec/hal/codechal_vdenc_vp9_g12.cpp +++ b/media_driver/agnostic/gen12/codec/hal/codechal_vdenc_vp9_g12.cpp @@ -4027,24 +4027,30 @@ MOS_STATUS CodechalVdencVp9StateG12::ExecutePictureLevel() pipeModeSelectParams = m_vdencInterface->CreateMhwVdboxPipeModeSelectParams(); CODECHAL_ENCODE_CHK_NULL_RETURN(pipeModeSelectParams); + auto release_func = [&]() + { + m_vdencInterface->ReleaseMhwVdboxPipeModeSelectParams(pipeModeSelectParams); + pipeModeSelectParams = nullptr; + }; + SetHcpPipeModeSelectParams(*pipeModeSelectParams); - CODECHAL_ENCODE_CHK_STATUS_RETURN(m_hcpInterface->AddHcpPipeModeSelectCmd(&cmdBuffer, pipeModeSelectParams)); + CODECHAL_ENCODE_CHK_STATUS_WITH_DESTROY_RETURN(m_hcpInterface->AddHcpPipeModeSelectCmd(&cmdBuffer, pipeModeSelectParams), release_func); - CODECHAL_ENCODE_CHK_STATUS_RETURN(m_miInterface->AddMfxWaitCmd(&cmdBuffer, nullptr, false)); + CODECHAL_ENCODE_CHK_STATUS_WITH_DESTROY_RETURN(m_miInterface->AddMfxWaitCmd(&cmdBuffer, nullptr, false), release_func); // Decoded picture #ifdef _MMC_SUPPORTED - CODECHAL_ENCODE_CHK_NULL_RETURN(m_mmcState); - CODECHAL_ENCODE_CHK_STATUS_RETURN(m_mmcState->SetSurfaceState(&surfaceParams[CODECHAL_HCP_DECODED_SURFACE_ID])); + CODECHAL_ENCODE_CHK_NULL_WITH_DESTROY_RETURN(m_mmcState, release_func); + CODECHAL_ENCODE_CHK_STATUS_WITH_DESTROY_RETURN(m_mmcState->SetSurfaceState(&surfaceParams[CODECHAL_HCP_DECODED_SURFACE_ID]), release_func); #endif - CODECHAL_ENCODE_CHK_STATUS_RETURN(m_hcpInterface->AddHcpSurfaceCmd(&cmdBuffer, &surfaceParams[CODECHAL_HCP_DECODED_SURFACE_ID])); + CODECHAL_ENCODE_CHK_STATUS_WITH_DESTROY_RETURN(m_hcpInterface->AddHcpSurfaceCmd(&cmdBuffer, &surfaceParams[CODECHAL_HCP_DECODED_SURFACE_ID]), release_func); // Source input #ifdef _MMC_SUPPORTED - CODECHAL_ENCODE_CHK_NULL_RETURN(m_mmcState); - CODECHAL_ENCODE_CHK_STATUS_RETURN(m_mmcState->SetSurfaceState(&surfaceParams[CODECHAL_HCP_SRC_SURFACE_ID])); + CODECHAL_ENCODE_CHK_NULL_WITH_DESTROY_RETURN(m_mmcState, release_func); + CODECHAL_ENCODE_CHK_STATUS_WITH_DESTROY_RETURN(m_mmcState->SetSurfaceState(&surfaceParams[CODECHAL_HCP_SRC_SURFACE_ID]), release_func); #endif - CODECHAL_ENCODE_CHK_STATUS_RETURN(m_hcpInterface->AddHcpSurfaceCmd(&cmdBuffer, &surfaceParams[CODECHAL_HCP_SRC_SURFACE_ID])); + CODECHAL_ENCODE_CHK_STATUS_WITH_DESTROY_RETURN(m_hcpInterface->AddHcpSurfaceCmd(&cmdBuffer, &surfaceParams[CODECHAL_HCP_SRC_SURFACE_ID]), release_func); if (MEDIA_IS_WA(m_waTable, Wa_Vp9UnalignedHeight)) { @@ -4064,8 +4070,8 @@ MOS_STATUS CodechalVdencVp9StateG12::ExecutePictureLevel() uint8_t skipMask = 0xf8; for (uint8_t i = CODECHAL_HCP_LAST_SURFACE_ID; i <= CODECHAL_HCP_ALTREF_SURFACE_ID; i++) { - CODECHAL_ENCODE_CHK_NULL_RETURN(m_mmcState); - CODECHAL_ENCODE_CHK_STATUS_RETURN(m_mmcState->SetSurfaceState(&surfaceParams[i])); + CODECHAL_ENCODE_CHK_NULL_WITH_DESTROY_RETURN(m_mmcState, release_func); + CODECHAL_ENCODE_CHK_STATUS_WITH_DESTROY_RETURN(m_mmcState->SetSurfaceState(&surfaceParams[i]), release_func); if (surfaceParams[i].mmcState == MOS_MEMCOMP_DISABLED) { skipMask |= (1 << (i - 2)); @@ -4083,17 +4089,21 @@ MOS_STATUS CodechalVdencVp9StateG12::ExecutePictureLevel() #endif for (uint8_t i = CODECHAL_HCP_LAST_SURFACE_ID; i <= CODECHAL_HCP_ALTREF_SURFACE_ID; i++) { - CODECHAL_ENCODE_CHK_STATUS_RETURN(m_hcpInterface->AddHcpSurfaceCmd(&cmdBuffer, &surfaceParams[i])); + CODECHAL_ENCODE_CHK_STATUS_WITH_DESTROY_RETURN(m_hcpInterface->AddHcpSurfaceCmd(&cmdBuffer, &surfaceParams[i]), release_func); } } - // set HCP_PIPE_BUF_ADDR_STATE values PMHW_VDBOX_PIPE_BUF_ADDR_PARAMS pipeBufAddrParams = nullptr; pipeBufAddrParams = CreateHcpPipeBufAddrParams(pipeBufAddrParams); auto delete_func = [&]() { + if (pipeModeSelectParams) + { + m_vdencInterface->ReleaseMhwVdboxPipeModeSelectParams(pipeModeSelectParams); + pipeModeSelectParams = nullptr; + } if (pipeBufAddrParams) { MOS_Delete(pipeBufAddrParams); diff --git a/media_softlet/agnostic/common/codec/hal/enc/shared/features/encode_preenc_basic_feature.cpp b/media_softlet/agnostic/common/codec/hal/enc/shared/features/encode_preenc_basic_feature.cpp index 1c8e96fb3dd..4a4ad61f765 100644 --- a/media_softlet/agnostic/common/codec/hal/enc/shared/features/encode_preenc_basic_feature.cpp +++ b/media_softlet/agnostic/common/codec/hal/enc/shared/features/encode_preenc_basic_feature.cpp @@ -848,11 +848,6 @@ MOS_STATUS PreEncBasicFeature::ValidateLowDelayBFrame() // forward for (int refIdx = 0; (refIdx < 1) && m_lowDelay; refIdx++) { - if (refIdx >= CODEC_MAX_NUM_REF_FRAME_HEVC) - { - break; - } - CODEC_PICTURE refPic = m_preEncConfig.RefPicList[0][refIdx]; if (!CodecHal_PictureIsInvalid(refPic) && m_preEncConfig.RefFramePOCList[refPic.FrameIdx] > m_preEncConfig.CurrPicOrderCnt) { diff --git a/media_softlet/agnostic/common/codec/hal/enc/shared/scalability/encode_scalability_multipipe.cpp b/media_softlet/agnostic/common/codec/hal/enc/shared/scalability/encode_scalability_multipipe.cpp index b1be8b8a9a0..684201fc755 100644 --- a/media_softlet/agnostic/common/codec/hal/enc/shared/scalability/encode_scalability_multipipe.cpp +++ b/media_softlet/agnostic/common/codec/hal/enc/shared/scalability/encode_scalability_multipipe.cpp @@ -328,7 +328,7 @@ MOS_STATUS EncodeScalabilityMultiPipe::VerifySpaceAvailable(uint32_t requestedSi { SCALABILITY_CHK_STATUS_RETURN(MediaScalability::VerifySpaceAvailable( requestedSize, requestedPatchListSize, bothPatchListAndCmdBufChkSuccess)); - if (bothPatchListAndCmdBufChkSuccess = true) + if (bothPatchListAndCmdBufChkSuccess == true) { singleTaskPhaseSupportedInPak = m_singleTaskPhaseSupported; return eStatus;