Skip to content

Rialto Server profiling#441

Open
smudri85 wants to merge 21 commits intomasterfrom
feature/RDKEMW-11764
Open

Rialto Server profiling#441
smudri85 wants to merge 21 commits intomasterfrom
feature/RDKEMW-11764

Conversation

@smudri85
Copy link
Copy Markdown

@smudri85 smudri85 commented Feb 6, 2026

Summary: Add profiling to Rialto Server.
Type: Feature
Test Plan: UT/CT, Fullstack
Jira: RDKEMW-11764

Copilot AI review requested due to automatic review settings February 6, 2026 14:37
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Feb 6, 2026

Pull request must be merged with a description containing the required fields,

Summary:
Type: Feature/Fix/Cleanup
Test Plan:
Jira:

If there is no jira releated to this change, please put 'Jira: NO-JIRA'.

Description can be changed by editing the top comment on your pull request and making a new commit.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Feb 6, 2026

common/source/Profiler.cpp:98:0: style: Consider using std::find_if algorithm instead of a raw loop. [useStlAlgorithm]
if (record.stage == stage)
^
common/source/Profiler.cpp:111:0: style: Consider using std::find_if algorithm instead of a raw loop. [useStlAlgorithm]
if (record.stage == stage && record.info == info)
^
common/source/Profiler.cpp:184:0: style: Consider using std::find_if algorithm instead of a raw loop. [useStlAlgorithm]
if (record.id == id)
^
nofile:0:0: information: Active checkers: 161/592 (use --checkers-report= to see details) [checkersReport]

@rdkcmf-jenkins
Copy link
Copy Markdown
Contributor

b'## Blackduck scan failure details

Summary: 0 violations, 0 files pending approval, 1 file pending identification.

  • Protex Server Path: /home/blackduck/github/rialto/441/rdkcentral/rialto

  • Commit: 282cf54

Report detail: gist'

Copy link
Copy Markdown

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 pull request adds profiling functionality to the Rialto Server to track performance metrics and pipeline events. The implementation includes a generic profiling interface in the common module and a GStreamer-specific profiler that integrates with the media player pipeline. The profiler records timestamped events at various stages of media playback and can be enabled via environment variable.

Changes:

  • Added core profiling infrastructure with IProfiler interface and Profiler implementation
  • Integrated GstProfiler into the media player pipeline to track key events
  • Added unit tests and mocks for the profiling functionality

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 25 comments.

Show a summary per file
File Description
common/interface/IProfiler.h Defines the profiler interface with methods for recording, finding, logging, and dumping profiling records
common/include/Profiler.h Declares the concrete Profiler class implementation with internal Record structure
common/source/Profiler.cpp Implements the profiler with thread-safe record storage, environment-based enabling, and file dumping
common/CMakeLists.txt Adds Profiler.cpp to the build configuration
media/server/gstplayer/include/GstProfiler.h Defines GStreamer-specific profiler wrapper for pipeline profiling
media/server/gstplayer/include/GenericPlayerContext.h Adds GstProfiler instance to player context
media/server/gstplayer/source/GstProfiler.cpp Implements GStreamer profiler with pad probe callbacks and element filtering
media/server/gstplayer/source/GstGenericPlayer.cpp Integrates profiler into pipeline lifecycle (creation, termination, segment handling)
media/server/gstplayer/source/tasks/generic/HandleBusMessage.cpp Records pipeline state changes
media/server/gstplayer/source/tasks/generic/FinishSetupSource.cpp Records when all sources are attached
media/server/gstplayer/source/tasks/generic/AttachSource.cpp Records AppSrc element creation
media/server/gstplayer/source/tasks/generic/DeepElementAdded.cpp Schedules profiling for dynamically added GStreamer elements
media/server/gstplayer/CMakeLists.txt Adds GstProfiler.cpp to build and includes common headers
tests/unittests/common/unittests/ProfilerTests.cpp Unit tests for core profiler functionality
tests/unittests/common/unittests/CMakeLists.txt Adds ProfilerTests.cpp to test build
tests/unittests/common/mocks/ProfilerMock.h Mock implementation of IProfiler for testing
tests/unittests/common/mocks/ProfilerFactoryMock.h Mock implementation of IProfilerFactory for testing

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

Comment thread media/server/gstplayer/source/tasks/generic/AttachSource.cpp Outdated
Comment thread common/interface/IProfiler.h Outdated
Comment thread common/interface/IProfiler.h Outdated
Comment thread media/server/gstplayer/source/GstProfiler.cpp Outdated
Comment thread tests/unittests/common/unittests/ProfilerTests.cpp
Comment thread common/source/Profiler.cpp Outdated
Comment thread media/server/gstplayer/source/GstGenericPlayer.cpp Outdated
Comment thread media/server/gstplayer/source/GstProfiler.cpp Outdated
Comment thread tests/unittests/common/mocks/ProfilerFactoryMock.h
Comment thread media/server/gstplayer/include/GstProfiler.h
@rdkcmf-jenkins
Copy link
Copy Markdown
Contributor

rdkcmf-jenkins commented Feb 6, 2026

Coverity Issue - Uncaught exception

An exception of type "std::bad_optional_access" is thrown but the exception specification "/implicit/noexcept" doesn't allow it to be thrown. This will result in a call to terminate().

Medium Impact, CWE-248
UNCAUGHT_EXCEPT

Issue location

This issue was discovered outside the diff for this Pull Request. You can find it at:
media/server/gstplayer/source/GstGenericPlayer.cpp:210

Comment thread media/server/gstplayer/source/GstProfiler.cpp Outdated
Comment thread media/server/gstplayer/source/GstProfiler.cpp Outdated
Comment thread media/server/gstplayer/source/GstProfiler.cpp Outdated
Comment thread media/server/gstplayer/source/GstProfiler.cpp Outdated
@mhughesacn
Copy link
Copy Markdown

Hi @smudri85 : The blackduck issue is not a problem but all new code files should have proper headers please.

  • MartinH, RDK CMF Compliance Team

@rdkcmf-jenkins
Copy link
Copy Markdown
Contributor

b'## WARNING: A Blackduck scan failure has been waived

A prior failure has been upvoted

  • Upvote reason: ok

  • Commit: 282cf54
    '

Copilot AI review requested due to automatic review settings February 11, 2026 12:19
@smudri85 smudri85 force-pushed the feature/RDKEMW-11764 branch from 95cc046 to 43eb771 Compare February 11, 2026 12:19
Copy link
Copy Markdown

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

Copilot reviewed 19 out of 19 changed files in this pull request and generated 10 comments.


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

Comment thread media/server/gstplayer/source/GstProfiler.cpp Outdated
Comment thread media/server/gstplayer/source/GstProfiler.cpp Outdated
Comment thread media/server/gstplayer/source/GstProfiler.cpp
Comment thread common/source/Profiler.cpp
Comment thread media/server/gstplayer/source/tasks/generic/AttachSource.cpp Outdated
Comment thread media/server/gstplayer/source/GstProfiler.cpp
Comment thread media/server/gstplayer/include/GenericPlayerContext.h Outdated
Comment thread media/server/gstplayer/CMakeLists.txt Outdated
Comment thread tests/unittests/common/unittests/ProfilerTests.cpp
Comment thread tests/unittests/common/unittests/ProfilerTests.cpp Outdated
@smudri85 smudri85 force-pushed the feature/RDKEMW-11764 branch from 43eb771 to e476ba7 Compare February 11, 2026 13:45
Copilot AI review requested due to automatic review settings February 11, 2026 14:06
@smudri85 smudri85 force-pushed the feature/RDKEMW-11764 branch from e476ba7 to 5862d6c Compare February 11, 2026 14:06
Copy link
Copy Markdown

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

Copilot reviewed 19 out of 19 changed files in this pull request and generated 8 comments.


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

Comment thread tests/unittests/common/unittests/ProfilerTests.cpp
Comment thread media/server/gstplayer/source/tasks/generic/FinishSetupSource.cpp
Comment thread media/server/gstplayer/source/tasks/generic/DeepElementAdded.cpp
Comment thread media/server/gstplayer/source/tasks/generic/HandleBusMessage.cpp Outdated
Comment thread media/server/gstplayer/source/tasks/generic/AttachSource.cpp Outdated
Comment thread media/server/gstplayer/source/GstProfiler.cpp
Comment thread media/server/gstplayer/include/GenericPlayerContext.h
Comment thread tests/unittests/common/unittests/ProfilerTests.cpp
@smudri85 smudri85 force-pushed the feature/RDKEMW-11764 branch 2 times, most recently from 3934f05 to 761b259 Compare February 12, 2026 09:18
Copilot AI review requested due to automatic review settings February 12, 2026 09:18
Copy link
Copy Markdown

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

Copilot reviewed 19 out of 19 changed files in this pull request and generated 7 comments.


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

Comment thread media/server/gstplayer/source/GstProfiler.cpp Outdated
Comment thread media/server/gstplayer/include/GenericPlayerContext.h
Comment thread media/server/gstplayer/source/tasks/generic/AttachSource.cpp Outdated
Comment thread common/source/Profiler.cpp
Comment thread common/source/Profiler.cpp Outdated
Comment thread tests/unittests/common/unittests/ProfilerTests.cpp
Comment thread media/server/gstplayer/include/GstProfiler.h Outdated
@smudri85 smudri85 force-pushed the feature/RDKEMW-11764 branch from 761b259 to 3d607f2 Compare February 12, 2026 13:28
@github-actions
Copy link
Copy Markdown

Coverage statistics of your commit:
WARNING: Lines coverage decreased from: 85.2% to 84.6%
WARNING: Functions coverage decreased from: 92.5% to 91.9%

Copilot AI review requested due to automatic review settings April 3, 2026 19:42
@smudri85 smudri85 force-pushed the feature/RDKEMW-11764 branch from 3d607f2 to 024c46d Compare April 3, 2026 19:42
@rdkcmf-jenkins
Copy link
Copy Markdown
Contributor

b'## Blackduck scan failure details

Summary: 0 violations, 0 files pending approval, 1 file pending identification.

  • Protex Server Path: /home/blackduck/github/rialto/441/rdkcentral/rialto

  • Commit: 024c46d

Report detail: gist'

Copilot AI review requested due to automatic review settings April 22, 2026 14:08
@smudri85 smudri85 force-pushed the feature/RDKEMW-11764 branch from 7f24ab6 to 289c905 Compare April 22, 2026 14:08
Copy link
Copy Markdown

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

Copilot reviewed 37 out of 37 changed files in this pull request and generated 2 comments.


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

Comment on lines +204 to +207
gchar *rawName = m_gstWrapper->gstElementGetName(element);
elementInfo = deriveElementInfoFromName(rawName ? rawName : "<null>");
if (rawName)
m_glibWrapper->gFree(rawName);
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

This is false negative. From the method description at IGstWrapper.h:
... Caller should g_free the return value after usage ...

}
if (appSrc)
{
auto recordId = m_context.gstProfiler->createRecord("Created AppSrc Element", std::move(profilerInfo));
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Resolved.

@smudri85 smudri85 force-pushed the feature/RDKEMW-11764 branch from 289c905 to 44b708d Compare April 23, 2026 09:21
@github-actions
Copy link
Copy Markdown

Coverage statistics of your commit:
WARNING: Lines coverage decreased from: 84.5% to 84.3%
Functions coverage stays unchanged and is: 92.6%

Sasa Mudri added 21 commits April 23, 2026 12:05
Summary: Add profiling to Rialto Server.
Type: Feature
Test Plan: UT/CT, Fullstack
Jira: RDKEMW-11764
Summary: Add profiling to Rialto Server.
Type: Feature
Test Plan: UT/CT, Fullstack
Jira: RDKEMW-117641
Summary: Add profiling to Rialto Server.
Type: Feature
Test Plan: UT/CT, Fullstack
Jira: RDKEMW-11764
Summary: Add profiling to Rialto Server.
Type: Feature
Test Plan: UT/CT, Fullstack
Jira: RDKEMW-11764
Summary: Add profiling to Rialto Server.
Type: Feature
Test Plan: UT/CT, Fullstack
Jira: RDKEMW-11764
Summary: Add profiling to Rialto Server.
Type: Feature
Test Plan: UT/CT, Fullstack
Jira: RDKEMW-11764
Summary: Add profiling to Rialto Server.
Type: Feature
Test Plan: UT/CT, Fullstack
Jira: RDKEMW-11764
Summary: Add profiling to Rialto Server.
Type: Feature
Test Plan: UT/CT, Fullstack
Jira: RDKEMW-11764
@smudri85 smudri85 force-pushed the feature/RDKEMW-11764 branch from 44b708d to 71ea315 Compare April 23, 2026 10:06
@github-actions
Copy link
Copy Markdown

Coverage statistics of your commit:
WARNING: Lines coverage decreased from: 84.5% to 84.3%
Functions coverage stays unchanged and is: 92.6%

if (!m_enabled || !m_profiler)
return;

(void)m_profiler->dumpToFile();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A warning if dumpToFile failed :)

std::string info;
};

GstPadProbeReturn probeCb(GstPad *pad, GstPadProbeInfo *info, gpointer userData)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Well, that's better, but not exactly what I meant :)
That's what you should do if we want to have similar implementation everywhere and no new/delete:

  1. Move ProbeCtx contents to GstDecryptor
  2. Create GstPadProbeReturn GstDecryptor::handleProbeCb(GstPad *pad, GstPadProbeInfo *info) function and move the content of the function below there
  3. Implementation of this function should be:
    auto *self = static_cast<GstDecryptor *>(userData); return self->handleProbeCb(pad, info);
  4. remove probe in GstDecryptor destructor
  5. Probe registration will be:
    m_gstWrapper->gstPadAddProbe(pad, GST_PAD_PROBE_TYPE_BUFFER, &probeCb, this, nullptr);
    As you can see, no new/delete and destroy method is needed


for key in suites:
executeCmd = []
cmdEnv = os.environ.copy()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

So why those 2 lines are needed?

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.

6 participants