Skip to content

RDKEMW-13073: L1 test case fail for playerisobmff module#81

Open
ALSAMEEMA wants to merge 38 commits intofeature/dev_sprint_plifrom
feature/RDK-58324
Open

RDKEMW-13073: L1 test case fail for playerisobmff module#81
ALSAMEEMA wants to merge 38 commits intofeature/dev_sprint_plifrom
feature/RDK-58324

Conversation

@ALSAMEEMA
Copy link
Copy Markdown

No description provided.

rekhap2kandhavelan and others added 30 commits September 15, 2025 23:13
Refactor base64_Encode function to handle NULL and empty input cases properly.
Copy callback and session to local variables to avoid race conditions.
Added UpdateBufferData function for buffer management.
@ALSAMEEMA ALSAMEEMA requested a review from a team as a code owner February 23, 2026 05:44
Copilot AI review requested due to automatic review settings February 23, 2026 05:44
@github-actions
Copy link
Copy Markdown


Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just posting a Pull Request Comment same as the below format.


I have read the CLA Document and I hereby sign the CLA


1 out of 3 committers have signed the CLA.
✅ (ALSAMEEMA)[https://github.com/ALSAMEEMA]
@rekhap2kandhavelan
@joyal
joyal seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses L1 test case failures for the playerisobmff module by adding comprehensive unit test infrastructure and fixing validation issues in production code.

Changes:

  • Added 20+ new test suites covering DRM, subtitles, plugins, player components, and externals
  • Enhanced input validation in subtitle parsers (WebVtt, TextStyleAttributes)
  • Improved thread safety in HlsDrmSessionManager
  • Added null pointer checks in base64 encoding/decoding and DRM utilities
  • Refactored fake implementations for better test support

Reviewed changes

Copilot reviewed 74 out of 143 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
test/utests/tests/*/CMakeLists.txt Added CMake configurations for new test suites
test/utests/tests//.cpp Added test runner and test case files
subtec/subtecparser/*.cpp Added input validation for width/height parameters
drm/HlsDrmSessionManager.cpp Improved thread safety by copying callback locally
baseConversion/_base64.cpp Added null checks and empty input handling
drm/DrmUtils.cpp Added null pointer check for psshData
PlayerUtils.cpp Used std::move for efficiency, exposed internal function for testing
InterfacePlayerRDK.cpp Added null checks and improved error logging
GstHandlerControl.cpp Added validation for negative delay values
test/utests/fakes/*.cpp Updated fake implementations to support new tests

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +26 to +27
if(width <= 0 || height <= 0) {
throw std::invalid_argument("Width and height must be greater than zero");
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The input validation should use braces on separate lines for consistency with the rest of the codebase. The check validates that width and height are positive, which is good defensive programming.

Suggested change
if(width <= 0 || height <= 0) {
throw std::invalid_argument("Width and height must be greater than zero");
if (width <= 0 || height <= 0)
{
throw std::invalid_argument("Width and height must be greater than zero");

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants