RDKEMW-18010: Player Interface:Feature Additions, Metrics, Test Cases, and Quality Improvements#173
Open
dp0000 wants to merge 56 commits into
Open
RDKEMW-18010: Player Interface:Feature Additions, Metrics, Test Cases, and Quality Improvements#173dp0000 wants to merge 56 commits into
dp0000 wants to merge 56 commits into
Conversation
Signed-off-by: nrames759 <Naren_Ramesh@comcast.com>
Signed-off-by: nrames759 <Naren_Ramesh@comcast.com>
…rash & LLAMA freeze issue Reason for change : Addressing the above changes in middleware-player-interface Test Steps : E2E testing to check regression . Signed-off by: Deepikasri Natarajan
RDKEMW-18010 Middleware-player-interface feature development and inalign with dev_sprint
…and inalign with dev_sprint"
…_pli Revert "RDKEMW-18010 Middleware-player-interface feature development and inalign with dev_sprint"
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
This PR is a broad refactor/cleanup across the middleware player interface, focused on consolidating/reshaping unit tests (including DRM/OCDM coverage and CMake/coverage wiring), simplifying some vendor SoC abstraction code paths (notably video PTS handling), and removing or relocating several legacy test suites and helpers.
Changes:
- Simplifies/adjusts SoC vendor interfaces (e.g.,
GetVideoPts) and related fake implementations. - Restructures unit tests: removes multiple legacy utest suites/runners, adds a new
DrmOcdmutest suite, and updates many utest CMake/coverage/link settings. - Refactors/adjusts DRM and plugin code paths (GStreamer decryptors, session manager behavior) and moves
MediaSampledefinition intoDemuxDataTypes.h.
Reviewed changes
Copilot reviewed 84 out of 184 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
| vendor/SocInterface.h | Removes video-pts property probing helpers and updates GetVideoPts API comment. |
| vendor/SocInterface.cpp | Simplifies GetVideoPts implementation to directly query video-pts. |
| vendor/realtek/RealtekSocInterface.h | Removes Realtek overrides related to property discovery; updates GetVideoPts comment. |
| vendor/realtek/RealtekSocInterface.cpp | Changes Realtek GetVideoPts implementation to direct g_object_get. |
| test/utests/tests/WebVttSubtecParserTests/WebVttSubtecParserPlayer.cpp | Removes standalone utest runner main for this suite. |
| test/utests/tests/WebVttSubtecParserTests/CMakeLists.txt | Removes CMake target for this suite. |
| test/utests/tests/WebvttSubtecDevInterfaceTests/WebvttSubtecDevInterfacePlayer.cpp | Removes standalone utest runner main for this suite. |
| test/utests/tests/WebvttSubtecDevInterfaceTests/CMakeLists.txt | Removes CMake target for this suite. |
| test/utests/tests/VendorTests/PlayerVendorRun.cpp | Removes standalone utest runner main for this suite. |
| test/utests/tests/VendorTests/CMakeLists.txt | Removes CMake target for vendor unit tests suite. |
| test/utests/tests/TtmlSubtecParser/TtmlSubtecParserPlayer.cpp | Removes standalone utest runner main for this suite. |
| test/utests/tests/TtmlSubtecParser/CMakeLists.txt | Removes CMake target for this suite. |
| test/utests/tests/TextStyleAttributes/CMakeLists.txt | Updates include/source/link/coverage wiring for this utest target. |
| test/utests/tests/SubtitleTests/SubtitleRun.cpp | Removes standalone utest runner main for this suite. |
| test/utests/tests/SubtitleTests/CMakeLists.txt | Removes CMake target for this suite. |
| test/utests/tests/SubtecTests/PlayerSubtecTests.cpp | Removes standalone utest runner main for this suite. |
| test/utests/tests/SubtecTests/CMakeLists.txt | Removes CMake target for this suite. |
| test/utests/tests/PluginsTests/CMakeLists.txt | Refactors plugin utest build to compile gst decryptor sources directly and adjusts linking/coverage. |
| test/utests/tests/PlayerUtilsTests/CMakeLists.txt | Adjusts include paths (including casing changes) and keeps utest target wiring. |
| test/utests/tests/PlayerLogManagerTests/PlayerLogManagerRun.cpp | Removes standalone utest runner main for this suite. |
| test/utests/tests/PlayerLogManagerTests/CMakeLists.txt | Removes CMake target for this suite. |
| test/utests/tests/PlayerJsonObjectTests/PlayerJsonObjectRun.cpp | Removes standalone utest runner main for this suite. |
| test/utests/tests/PlayerJsonObjectTests/CMakeLists.txt | Removes CMake target for this suite. |
| test/utests/tests/PlayerIsoBmffTests/PlayerIsoBmffTestRunner.cpp | Removes standalone utest runner main for this suite. |
| test/utests/tests/PlayerIsoBmffTests/CMakeLists.txt | Removes CMake target for this suite. |
| test/utests/tests/PlayerExternalsTests/PlayerRFCFun.cpp | Removes RFC externals unit tests source. |
| test/utests/tests/PlayerExternalsTests/PlayerMacros.h | Removes custom EXPECT_NO_THROW macro header. |
| test/utests/tests/PlayerExternalsTests/PlayerExternalUtilsFun.cpp | Removes externals utils unit tests source. |
| test/utests/tests/PlayerExternalsTests/PlayerExternalsRun.cpp | Removes standalone utest runner main for this suite. |
| test/utests/tests/PlayerExternalsTests/CMakeLists.txt | Removes CMake target for this suite. |
| test/utests/tests/PlayerCommonTests/PlayerCommonRun.cpp | Removes standalone utest runner main for this suite. |
| test/utests/tests/PlayerCommonTests/gstPlayerTaskPoolFun.cpp | Removes task-pool unit test source. |
| test/utests/tests/PlayerCommonTests/CMakeLists.txt | Removes CMake target for this suite. |
| test/utests/tests/OCDMSessionAdapter/CMakeLists.txt | Updates coverage wiring and target link libraries. |
| test/utests/tests/OcdmBasicSessionAdapterTests/FunctionalTests.cpp | Fixes mock setup ordering and minor matcher/expectation issues. |
| test/utests/tests/OcdmBasicSessionAdapterTests/CMakeLists.txt | Simplifies/normalizes include paths and updates coverage wiring. |
| test/utests/tests/GstPluginsTests/GstwidevinedecryptorFun.cpp | Removes gst plugin L1 test source. |
| test/utests/tests/GstPluginsTests/GstverimatrixdecryptorFun.cpp | Removes gst plugin L1 test source. |
| test/utests/tests/GstPluginsTests/GstPlugingRun.cpp | Removes standalone utest runner main for this suite. |
| test/utests/tests/GstPluginsTests/GstplayreadydecryptorFun.cpp | Removes gst plugin L1 test source. |
| test/utests/tests/GstPluginsTests/GstclearkeydecryptorFun.cpp | Removes gst plugin L1 test source. |
| test/utests/tests/GstPluginsTests/GstcdmidecryptorFun.cpp | Removes gst plugin L1 test source. |
| test/utests/tests/GstPluginsTests/CMakeLists.txt | Removes CMake target for this suite. |
| test/utests/tests/GstPlugingSubtecTests/GstvipertransformFun.cpp | Removes gst subtec plugin L1 test source. |
| test/utests/tests/GstPlugingSubtecTests/GstsubtecsinkFun.cpp | Removes gst subtec plugin L1 test source. |
| test/utests/tests/GstPlugingSubtecTests/Gstsubtecmp4transformFun.cpp | Removes gst subtec plugin L1 test source. |
| test/utests/tests/GstPlugingSubtecTests/GstsubtecbinFun.cpp | Removes gst subtec plugin L1 test source. |
| test/utests/tests/GstPlugingSubtecTests/GstPlugingSubtecRun.cpp | Removes standalone utest runner main for this suite. |
| test/utests/tests/GstPlugingSubtecTests/CMakeLists.txt | Removes CMake target for this suite. |
| test/utests/tests/GstPlayer/PauseOnPlaybackTests.cpp | Updates tests to match new ConfigurePipeline signature; adjusts frame-step property test setup. |
| test/utests/tests/GstPlayer/FunctionalTests.cpp | Updates tests to match new ConfigurePipeline signature. |
| test/utests/tests/GstPlayer/CMakeLists.txt | Refactors GstPlayer utest target build; updates coverage/linking and sources list. |
| test/utests/tests/GstHandlerControlTests/CMakeLists.txt | Removes unused includes and updates coverage wiring. |
| test/utests/tests/FireBoltTests/FireBoltRun.cpp | Removes standalone utest runner main for this suite. |
| test/utests/tests/FireBoltTests/CMakeLists.txt | Removes CMake target for this suite. |
| test/utests/tests/DrmUrlTests/CMakeLists.txt | Updates include roots and coverage/link wiring. |
| test/utests/tests/DrmTests/DrmTestsRun.cpp | Removes standalone utest runner main for this suite. |
| test/utests/tests/DrmTests/DrmMemorySystemTests.cpp | Removes DRM memory system L1 tests source. |
| test/utests/tests/DrmTests/CMakeLists.txt | Removes CMake target for this suite. |
| test/utests/tests/DrmSessionManagerTests/CMakeLists.txt | Updates include path casing and retains target wiring. |
| test/utests/tests/DrmSecureClient/CMakeLists.txt | Updates include path casing and simplifies link options/coverage wiring. |
| test/utests/tests/DrmOcdmTests/DrmOcdmTestsRun.cpp | Removes standalone utest runner main for this suite. |
| test/utests/tests/DrmOcdmTests/CMakeLists.txt | Removes CMake target for this suite. |
| test/utests/tests/DrmOcdm/DrmUtilsTests.cpp | Adds new DrmOcdm unit tests for URL resolution behavior. |
| test/utests/tests/DrmOcdm/DrmTestsRun.cpp | Adds new DrmOcdm utest runner main. |
| test/utests/tests/DrmOcdm/DrmHLSTests.cpp | Adds new DrmOcdm tests for HLS content protection parsing behavior. |
| test/utests/tests/DrmOcdm/CMakeLists.txt | Renames/repurposes suite target to DrmOcdm; adjusts sources, defs, and linking. |
| test/utests/tests/DrmHelperTests/DrmHelperTestsRun.cpp | Removes standalone utest runner main for this suite. |
| test/utests/tests/DrmHelperTests/CMakeLists.txt | Removes CMake target for this suite. |
| test/utests/tests/ContentsecuritymanagerTests/PlayerContentSecurityRun.cpp | Removes standalone utest runner main for this suite. |
| test/utests/tests/ContentsecuritymanagerTests/CMakeLists.txt | Removes CMake target for this suite. |
| test/utests/tests/CMakeLists.txt | Updates utests directory list: adds DrmOcdm and removes many retired suites. |
| test/utests/tests/ClosedCaptionsTests/PlayerCCTrackInfoTests.cpp | Removes closed captions L1 test source. |
| test/utests/tests/ClosedCaptionsTests/PlayerCCManagerTests.cpp | Removes standalone utest runner main for this suite. |
| test/utests/tests/ClosedCaptionsTests/CMakeLists.txt | Removes CMake target for this suite. |
| test/utests/tests/Base64PLAYER/CMakeLists.txt | Updates coverage wiring and link line normalization. |
| test/utests/run.sh | Adjusts lcov invocation (removes --ignore-errors mismatch). |
| test/utests/mocks/MockPacketSender.h | Removes mock packet sender header. |
| test/utests/mocks/MockHlsDrmSession.h | Removes mock HLS DRM session header. |
| test/utests/mocks/MockDrmSession.h | Adjusts mock DRM session API surface (constructor, methods). |
| test/utests/mocks/MockDrmCallbacks.h | Removes mock DRM callbacks header. |
| test/utests/fakes/FakeSubtecConnecter.cpp | Removes fake Subtec connector implementation. |
| test/utests/fakes/FakeSocUtils.cpp | Removes isGstSubtecEnabled() fake implementation. |
| test/utests/fakes/FakeSocInterface.cpp | Updates fake SoC interface PTS/master logic and string prefix helper behavior. |
| test/utests/fakes/FakeGStreamer.cpp | Removes several gst get_type stubs for subtec/verimatrix helpers. |
| test/utests/fakes/FakeGstHandlerControl.cpp | Simplifies fake waitForDone behavior. |
| test/utests/fakes/FakeCrypto.cpp | Removes OpenSSL fake shim. |
| test/utests/fakes/FakeContentProtectionFireBolt.cpp | Removes Firebolt content protection fake implementation. |
| test/utests/CMakeLists.txt | Removes player_utest_add_cov() helper function. |
| playerLogManager/PlayerLogManager.cpp | Adjusts Ethan logger formatting behavior around newline stripping. |
| playerisobmff/playerisobmffbuffer.h | Removes multiple buffer accessors and getChunkedBox() API; keeps only getChunkedfBox(). |
| playerisobmff/playerisobmffbuffer.cpp | Removes several accessors; changes getChunkedfBox() implementation. |
| playerisobmff/playerisobmffbox.cpp | Refactors SencIsoBmffBox::truncate() implementation. |
| MediaSample.h | Removes standalone MediaSample transport header. |
| InterfacePlayerRDK.h | Removes MediaSample.h include, removes live-latency flag from ConfigurePipeline, and removes GetVideoPosition(). |
| GstHandlerControl.cpp | Removes negative-delay early return/logging. |
| gst-plugins/drm/gst/gstwidevinedecryptor.cpp | Removes AC4 caps entries from Widevine decryptor templates. |
| gst-plugins/drm/gst/gstverimatrixdecryptor.cpp | Removes AC4 caps entries from Verimatrix decryptor templates. |
| gst-plugins/drm/gst/gstplayreadydecryptor.cpp | Removes AC4 caps entries from PlayReady decryptor templates. |
| gst-plugins/drm/gst/gstclearkeydecryptor.cpp | Removes AC4 caps entries from ClearKey decryptor templates. |
| gst-plugins/drm/gst/gstcdmidecryptor.cpp | Adjusts initialization locking and sink-event resource management behavior. |
| externals/rdk/PlayerExternalsRdkInterface.cpp | Simplifies exception handling/logging in device settings interactions. |
| externals/PlayerExternalsInterfaceBase.h | Changes base constructor initialization list. |
| externals/PlayerExternalsInterface.h | Removes many inline getters/setters from the fake externals interface. |
| externals/IFirebolt/FireboltInterface.cpp | Removes explicit initialization of connection fields in FIREBOLT_ENDPOINT error path. |
| externals/contentsecuritymanager/SecManagerThunder.cpp | Refactors license decode allocation/log/free ordering. |
| externals/contentsecuritymanager/PlayerMemoryUtils.cpp | Changes shared memory key generation approach. |
| externals/contentsecuritymanager/IFirebolt/ContentProtectionFirebolt.cpp | Fixes license request variable usage and refactors decode/free ordering. |
| externals/contentsecuritymanager/ContentSecurityManagerSession.h | Refactors locking in copy ctor/assignment. |
| drm/ocdm/OcdmGstSessionAdapter.cpp | Adjusts SEI extraction buffer mapping logic. |
| drm/HlsOcdmBridge.h | Inlines GetState() implementation. |
| drm/HlsOcdmBridge.cpp | Removes mutex-protected GetState() implementation from .cpp. |
| drm/HlsDrmSessionManager.h | Removes in-class nullptr initialization of mDrmSession. |
| drm/helper/VerimatrixHelper.cpp | Removes null guard and changes setData() argument casting style. |
| drm/DrmSessionManager.cpp | Refactors failed-session cleanup logic and lock usage. |
| drm/aes/Aes.cpp | Refactors logging/state access patterns and constructor init ordering. |
| DemuxDataTypes.h | Adds MediaSample definition (vector-backed) and includes <cstring>. |
| CMakeLists.txt | Removes global COVERAGE_ENABLED compiler/linker flag injection; removes MediaSample install entries. |
| baseConversion/_base64.cpp | Removes input guards in base64 encode/decode. |
| aamp_sync_cid.txt | Updates sync CID value. |
| .gitignore | Removes sync artifacts ignore rules. |
| .github/workflows/L1-tests.yml | Adjusts install script invocation arguments for native build step. |
Comments suppressed due to low confidence (1)
gst-plugins/drm/gst/gstcdmidecryptor.cpp:779
sinkpadwas changed fromg_autoptr(GstPad)to a raw pointer, but there are multiple earlybreak;paths in this case block (e.g., whensessionManageris NULL orgst_buffer_mapfails). These paths now leaksinkpad(and potentially other resources) becausegst_object_unref(sinkpad)is only reached at the end.
GstPad * sinkpad = gst_element_get_static_pad(
reinterpret_cast<GstElement*>(cdmidecryptor), "sink");
if(cdmidecryptor->sessionManager == NULL)
{
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+53
to
+54
| set(FAKE_SOURCES ${UTESTS_ROOT}/fakes/FakeGStreamer.cpp | ||
| ${UTESTS_ROOT}fakes/FakeGLib.cpp) |
Comment on lines
+58
to
+59
| set(FAKE_SOURCE ${UTEST_ROOT}/fakes/FakeDRMSessionManager.cpp) | ||
|
|
Comment on lines
80
to
84
| add_executable(${EXEC_NAME} | ||
| ${TEST_SOURCES} | ||
| ${MOCK_SOURCES} | ||
| ${FAKE_SOURCES} | ||
| ${PLAYER_SOURCES}) |
Comment on lines
36
to
40
| char *base64_Encode(const unsigned char *src, size_t len) | ||
| { | ||
| if( !src ) | ||
| { | ||
| return NULL; | ||
| } | ||
| if( !len ) | ||
| { | ||
| return strdup(""); | ||
| } | ||
| const unsigned char *fin = &src[len]; | ||
| const static char *encode = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/"; // base64 | ||
| char *rc = (char *)malloc(((len+2)/3)*4+1); |
Comment on lines
65
to
69
| unsigned char *base64_Decode(const char *src, size_t *outLen, size_t srcLen) | ||
| { | ||
| if( !src || !outLen ) | ||
| { | ||
| return NULL; | ||
| } | ||
| static const signed char decode[256] = | ||
| { | ||
| -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, |
Comment on lines
+1415
to
+1419
| auto newEnd{sampleCountLoc + sizeof(uint32_t) + firstSampleSize}; | ||
| auto oldSize{getSize()}; | ||
| auto newSize{static_cast<uint32_t>(newEnd - getBase())}; | ||
|
|
||
| if ((oldSize - newSize) >= PLAYER_SIZEOF_SIZE_AND_TAG) |
Comment on lines
+100
to
+104
| ContentSecurityManagerSession& operator=(const ContentSecurityManagerSession& other) | ||
| { | ||
| if (this == &other) | ||
| return *this; | ||
| std::unique_lock<std::mutex> thisLock(sessionIdMutex, std::defer_lock); | ||
| std::unique_lock<std::mutex> otherLock(other.sessionIdMutex, std::defer_lock); | ||
| std::lock(thisLock, otherLock); | ||
| mpSessionManager = other.mpSessionManager; | ||
| std::lock(sessionIdMutex, other.sessionIdMutex); | ||
| std::lock_guard<std::mutex> thisLock(sessionIdMutex, std::adopt_lock); | ||
| std::lock_guard<std::mutex> otherLock(other.sessionIdMutex, std::adopt_lock); |
Comment on lines
193
to
197
| if (buffer) | ||
| { | ||
| if (!gst_buffer_map( buffer, &info, (GstMapFlags)(GST_MAP_READ) )) | ||
| { | ||
| MW_LOG_ERR("Failed to map GstBuffer for SEI extraction"); | ||
| return; | ||
| } | ||
| gst_buffer_map( buffer, &info, (GstMapFlags)(GST_MAP_READ) ); | ||
| ptr = info.data; | ||
| len = info.size; |
| g_mutex_unlock(&cdmidecryptor->mutex); | ||
| GST_DEBUG_OBJECT(cdmidecryptor, "\n releasing ...................... mutex\n"); | ||
|
|
||
| gst_object_unref(sinkpad); |
| public: | ||
|
|
||
| PlayerExternalsInterfaceBase():m_isHDCPEnabled(false),m_displayWidth(0),m_displayHeight(0),m_sourceWidth(0),m_sourceHeight(0),m_gstElement(nullptr){} | ||
| PlayerExternalsInterfaceBase():m_sourceWidth(0),m_sourceHeight(0),m_gstElement(nullptr){} |
…lopment and inalign with dev_sprint""
…/dev_sprint_pli Revert "Revert "RDKEMW-18010 Middleware-player-interface feature development and inalign with dev_sprint""
RDKEMW-19919 : No WebVTT subtitles shown on some channels when PTS restamping enabled
RDKEMW-19686 [Develop]Bringing RKDEMW-19683 , RDKEMW-19684, RDKEMW-19685 to middleware-player-interface
Comment on lines
+100
to
+104
| * Used by the HLS PTS-restamp path to align subtitle display time | ||
| * with the restamped video PTS without rewriting MPEGTS in the | ||
| * VTT header. Default no-op; subtec-based parsers override. | ||
| */ | ||
| virtual void setPtsOffset(double ptsOffsetSec) {} |
Comment on lines
+100
to
+103
| * Used by the HLS PTS-restamp path to align subtitle display time | ||
| * with the restamped video PTS without rewriting MPEGTS in the | ||
| * VTT header. Default no-op; subtec-based parsers override. | ||
| */ |
Comment on lines
+79
to
+83
| void WebVTTSubtecParser::setPtsOffset(double ptsOffsetSec) | ||
| { | ||
| // Subtec's display_time = media_PTS - time_offset_ms (subtraction | ||
| // convention shared with Rialto SetSubtitlePtsOffset). The HLS | ||
| // restamped video PTS is media_PTS + ptsOffsetSec, so we negate |
…_CONDITION, MISSING_MOVE_ASSIGNMENT Reason for change: Added move implementation and used proper waitcondition Test procedure: As in ticket Risks: Medium
Comment on lines
+524
to
+529
| // buffering_timeout will handle the PLAYING transition, so seekPausedState must not block it. | ||
| if (interfacePlayerPriv->gstPrivateContext->seekPausedState) | ||
| { | ||
| MW_LOG_WARN("ConfigurePipeline: clearing seekPausedState — buffering will drive PLAYING transition"); | ||
| interfacePlayerPriv->gstPrivateContext->seekPausedState = false; | ||
| } |
Comment on lines
+4842
to
+4852
| if (privatePlayer->gstPrivateContext->seekPausedState) | ||
| { | ||
| MW_LOG_WARN("buffering_timeout: skipping PLAYING — seekPausedState active (cnt %u, frames %d)", privatePlayer->gstPrivateContext->buffering_timeout_cnt, frames); | ||
| if (privatePlayer->gstPrivateContext->buffering_timeout_cnt == 0) | ||
| { | ||
| MW_LOG_ERR("buffering_timeout: seekPausedState still active after timeout exhausted — clearing to unblock"); | ||
| privatePlayer->gstPrivateContext->seekPausedState = false; | ||
| } | ||
| return privatePlayer->gstPrivateContext->buffering_in_progress; | ||
| } | ||
|
|
Comment on lines
+30
to
+35
| static std::shared_ptr<SocInterface> GetSocInterface() | ||
| { | ||
| static std::shared_ptr<SocInterface> socInterface = SocInterface::CreateSocInterface(); | ||
| return socInterface; | ||
| } | ||
|
|
Comment on lines
+48
to
+52
| PlayerHlsDrmSessionInterface::~PlayerHlsDrmSessionInterface() | ||
| { | ||
| #ifndef USE_OPENCDM_ADAPTER | ||
| if(m_pHlsDrmSessionManager != nullptr) | ||
| { |
Comment on lines
33
to
+37
| do { \ | ||
| std::printf("[MIDDLEWARE] %s:%d %s: " fmt, __FILE__, __LINE__, \ | ||
| __func__ , ##__VA_ARGS__); \ | ||
| std::fflush(stdout); \ | ||
| char _log_buf[512]; \ | ||
| int _len = std::snprintf(_log_buf, sizeof(_log_buf), \ | ||
| "[PLAYER_IF] %s:%d %s: " fmt, \ | ||
| __FILE__, __LINE__, __func__, \ |
Comment on lines
3086
to
3090
| mInterfaceGstPlayer->SetStreamCaps(eGST_MEDIATYPE_AUDIO, std::move(codecInfo)); | ||
|
|
||
| delete g_mockGstUtils; | ||
| } | ||
|
|
RDKEMW-17597-3: Fix medium priority Coverity issues-BAD_CHECK_OF_WAIT_CONDITION
Reason for change: Modified to fix CHECKED_RETURN and FORWARD_NULL issues Test procedure: As in ticket Risks: Medium
Comment on lines
+524
to
+529
| // buffering_timeout will handle the PLAYING transition, so seekPausedState must not block it. | ||
| if (interfacePlayerPriv->gstPrivateContext->seekPausedState) | ||
| { | ||
| MW_LOG_WARN("ConfigurePipeline: clearing seekPausedState — buffering will drive PLAYING transition"); | ||
| interfacePlayerPriv->gstPrivateContext->seekPausedState = false; | ||
| } |
Comment on lines
4837
to
+4851
| else if (frames == -1 || frames >= pInterfacePlayerRDK->m_gstConfigParam->framesToQueue || (privatePlayer->gstPrivateContext->buffering_timeout_cnt > 0 && --privatePlayer->gstPrivateContext->buffering_timeout_cnt == 0)) | ||
| { | ||
| /* Do not set PLAYING if a seek-with-keepPaused is in progress. | ||
| * The buffering_timeout timer may fire after ConfigurePipeline restarts buffering | ||
| * but BEFORE the Pause(1) from keepPaused logic arrives — causing a race. */ | ||
| if (privatePlayer->gstPrivateContext->seekPausedState) | ||
| { | ||
| MW_LOG_WARN("buffering_timeout: skipping PLAYING — seekPausedState active (cnt %u, frames %d)", privatePlayer->gstPrivateContext->buffering_timeout_cnt, frames); | ||
| if (privatePlayer->gstPrivateContext->buffering_timeout_cnt == 0) | ||
| { | ||
| MW_LOG_ERR("buffering_timeout: seekPausedState still active after timeout exhausted — clearing to unblock"); | ||
| privatePlayer->gstPrivateContext->seekPausedState = false; | ||
| } | ||
| return privatePlayer->gstPrivateContext->buffering_in_progress; | ||
| } |
Comment on lines
+48
to
+52
| PlayerHlsDrmSessionInterface::~PlayerHlsDrmSessionInterface() | ||
| { | ||
| #ifndef USE_OPENCDM_ADAPTER | ||
| if(m_pHlsDrmSessionManager != nullptr) | ||
| { |
Comment on lines
3088
to
3090
| delete g_mockGstUtils; | ||
| } | ||
|
|
RDKEMW-17597-0: Fix Coverity issues
Reason for change: Added fix for COPY_INSTEAD_OF_MOVE Test procedure: As in ticket Risks: Medium
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 27 out of 30 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
test/utests/tests/InterfacePlayerTests/InterfacePlayerFunctionTests.cpp:3091
- Several unit tests covering GetVideoPosition()/GetVideoPTS() edge cases (null pipeline/sinkbin, non-playing state, query failures, and PTS fallback behavior) were removed from this file, but the corresponding production APIs still exist (InterfacePlayerRDK::GetVideoPosition is still defined/used). This reduces coverage for timing/PTS logic and makes regressions harder to catch. Consider re-adding these tests here or moving them to a more appropriate test suite instead of deleting them.
mInterfaceGstPlayer->SetStreamCaps(eGST_MEDIATYPE_AUDIO, std::move(codecInfo));
delete g_mockGstUtils;
}
| - RDKEMW-14946 : Fix coverity workflow scan in middleware-player-interface repo [`#91`](https://github.com/rdkcentral/middleware-player-interface/pull/91) | ||
| - main sync [`#86`](https://github.com/rdkcentral/middleware-player-interface/pull/86) | ||
| - changelog update [`0e0120d`](https://github.com/rdkcentral/middleware-player-interface/commit/0e0120d2a7c68ec0b031ad34dc2536779ce67b83) | ||
| - Integerate LLM teamgiven test cases into dev_pli branch [`8695df1`](https://github.com/rdkcentral/middleware-player-interface/commit/8695df1648e8e7fb5d2f9be24be0fac5f77df6d8) |
RDKEMW-17597-1: Fix medium priority Coverity issues
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 40 out of 43 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
test/utests/tests/InterfacePlayerTests/InterfacePlayerFunctionTests.cpp:3091
- A large block of unit tests for
GetVideoPosition()/GetVideoPTS()was removed and no equivalent tests exist elsewhere in the test tree. This significantly reduces coverage for position/PTS behavior (including error paths like null pipeline, state checks, and position-query fallback). Consider restoring these tests or moving them to a more appropriate test file if the intent was reorganization.
Comment on lines
+524
to
+529
| // buffering_timeout will handle the PLAYING transition, so seekPausedState must not block it. | ||
| if (interfacePlayerPriv->gstPrivateContext->seekPausedState) | ||
| { | ||
| MW_LOG_WARN("ConfigurePipeline: clearing seekPausedState — buffering will drive PLAYING transition"); | ||
| interfacePlayerPriv->gstPrivateContext->seekPausedState = false; | ||
| } |
Comment on lines
+38
to
+40
| #define private public | ||
| #include "Aes.h" | ||
| #undef private |
Comment on lines
+48
to
+56
| PlayerHlsDrmSessionInterface::~PlayerHlsDrmSessionInterface() | ||
| { | ||
| #ifndef USE_OPENCDM_ADAPTER | ||
| if(m_pHlsDrmSessionManager != nullptr) | ||
| { | ||
| delete m_pHlsDrmSessionManager; | ||
| m_pHlsDrmSessionManager = nullptr; | ||
| } | ||
| #endif |
Reason for change: Fixed BUFFER_SIZE, PASS_BY_VALUE, PW.NARROWING_CONVERSION, PW.USELESS_TYPE_QUALIFIER_ON_RETURN_TYPE Test procedure: As mentioned in ticket Risks: Low
RDKEMW-17599: Fix Low priority Coverity issues
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 45 out of 48 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
test/utests/tests/InterfacePlayerTests/InterfacePlayerFunctionTests.cpp:3091
- A large set of unit tests for
GetVideoPosition()/GetVideoPTS()was removed, and there are no remaining occurrences of these tests in the repo. Since the production methods still exist (InterfacePlayerRDK::GetVideoPosition), this is a net loss in coverage for important playback timing logic. Consider restoring these tests or moving them to a dedicated test file so the behavior remains covered.
| CallbackData(gpointer _instance, gulong _id, std::string _name) : instance(_instance), id(_id), name(_name) {}; | ||
| CallbackData(gpointer _instance, gulong _id, std::string _name) : instance(_instance), id(_id), name(std::move(_name)) {}; | ||
| CallbackData(const CallbackData &original) : instance(original.instance), id(original.id), name(original.name) {}; | ||
| CallbackData(CallbackData &&original) : instance(original.instance), id(original.id), name(original.name) {}; |
Comment on lines
41
to
44
| set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-multichar -DSUBTEC_PACKET_DEBUG") | ||
|
|
||
| set(GSTSUBTEC_DEPENDENCIES ${GSTREAMERBASE_LIBRARIES} ${GSTREAMER_LIBRARIES}) | ||
| set(GSTSUBTEC_DEPENDENCIES ${GSTREAMERBASE_LINK_LIBRARIES} ${GSTREAMER_LINK_LIBRARIES}) | ||
|
|
Comment on lines
+48
to
+56
| PlayerHlsDrmSessionInterface::~PlayerHlsDrmSessionInterface() | ||
| { | ||
| #ifndef USE_OPENCDM_ADAPTER | ||
| if(m_pHlsDrmSessionManager != nullptr) | ||
| { | ||
| delete m_pHlsDrmSessionManager; | ||
| m_pHlsDrmSessionManager = nullptr; | ||
| } | ||
| #endif |
…(FF/RW/Seek) leads to blank screen, device hang, followed by automatic reboot. Multiple critical processes (gmem, WPENetworkProce, WorkerPoolType: etc.) crash were observed.
RDKEMW-21922: [RDKEMW] [BCM Rogers Monarch IUI V2] Linear trick play (FF/RW/Seek) leads to blank screen, device hang, followed by automatic reboot. Multiple critical processes (gmem, WPENetworkProce, WorkerPoolType: etc.) crash were observed
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 45 out of 48 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (3)
InterfacePlayerPriv.h:237
CallbackDatahas a move constructor but it currently copiesname(name(original.name)), which defeats move semantics and can cause unnecessary allocations. Update the move ctor to moveoriginal.name(and ideally mark itnoexcept).
CallbackData(gpointer _instance, gulong _id, std::string _name) : instance(_instance), id(_id), name(std::move(_name)) {};
CallbackData(const CallbackData &original) : instance(original.instance), id(original.id), name(original.name) {};
CallbackData(CallbackData &&original) : instance(original.instance), id(original.id), name(original.name) {};
externals/rdk/PlayerThunderAccess.cpp:845
txtDatais passed by value, so thepush_backcalls here only modify a local copy and the caller never receives the discovered tracks (despite the API docs stating the vector will be filled). Change the API to takestd::vector<PlayerTextData>& txtData(or return the vector) consistently across the interface/implementations.
test/utests/tests/InterfacePlayerTests/InterfacePlayerFunctionTests.cpp:3089- A large block of unit tests covering
GetVideoPosition()and theGetVideoPTS()fallback behavior was removed. These code paths still exist in InterfacePlayerRDK (and are non-trivial), so removing the tests reduces coverage for null-pipeline/sinkbin handling, state gating, query failure handling, and PTS fallback conversion.
Comment on lines
759
to
763
| PlayerAudioData temp(audioData["language"].String(), audioData["contentType"].String(), audioData["name"].String(), audioData["type"].String(), (int)audioData["pk"].Number(), audioData["mixType"].String()); | ||
|
|
||
| audData.push_back(temp); | ||
| audData.push_back(std::move(temp)); | ||
|
|
||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This ticket tracks all feature development, integration, bug fixes, and quality improvements associated with the feature/dev_sprint_pli branch of the middleware-player-interface repository. Key activities include:
Addition of new player interface features and public APIs
Implementation of metrics collection for API usage
Integration and migration of new L1 test cases
Resolution of native build-related issues and other reported bugs
Coverity scan fixes and automated code quality improvements
All changes targeting this branch aim to enhance the robustness, test coverage, and maintainability of the player interface.