RDKPerf introduced in Rialto Server by balasaraswathy-n · Pull Request #488 · rdkcentral/rialto

balasaraswathy-n · 2026-04-24T13:54:49Z

Summary: RDKPerf introduced in Rialto Server
Type: Feature
Test Plan: UT/CT, Fullstack
Jira: RDKEMW-17139

github-actions · 2026-04-24T13:55:00Z

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.

Copilot

Pull request overview

Integrates rdk_perf instrumentation into the Rialto server startup path, and attempts to add the required perf libraries to the build so the server can be linked with the new dependency.

Changes:

Add rdk_perf.h include and instantiate an RDKPerf RAII object in RialtoServer’s main().
Add rdkperf/perftool link dependencies to the RialtoCommon CMake target.

Reviewed changes

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

File	Description
media/server/service/source/main.cpp	Adds `RDKPerf` startup instrumentation in `main()`.
common/CMakeLists.txt	Adds perf libraries to `RialtoCommon` link interface.

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

Copilot · 2026-04-24T13:58:13Z

    const char kSrcRev[] = SRCREV;
    const char kTags[] = TAGS;

+    RDKPerf perf(__FUNCTION__);


RDKPerf perf(__FUNCTION__); is never referenced. With repo-wide -Wall -Werror enabled, this is likely to fail the build due to -Wunused-variable. If the object is meant to exist only for RAII timing, mark it [[maybe_unused]] (or otherwise explicitly suppress the unused-variable warning) so the build remains warning-free.

Suggested change

RDKPerf perf(__FUNCTION__);

[[maybe_unused]] RDKPerf perf(__FUNCTION__);

Copilot · 2026-04-24T13:58:14Z

+        PUBLIC
+        rdkperf perftool


Linking rdkperf/perftool as PUBLIC dependencies of RialtoCommon makes every consumer (client, serverManager, tests, etc.) transitively require these external libraries, even though the new usage is only in the server's main.cpp. This can break builds/environments that don't ship these libs and also increases coupling; instead, link these dependencies only in the specific target(s) that use them (e.g., the server executable or a dedicated perf target), and add proper find_package/find_library/imported-target discovery so CMake fails early with a clear error when the dependency isn't available.

Suggested change

PUBLIC

rdkperf perftool

Copilot

Pull request overview

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

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

Copilot · 2026-04-27T08:19:10Z

    // GstAppSrc does not replace segment, if it's the same as previous one.
    // It causes problems with position reporing in amlogic devices, so we need to push
    // two segments with different reset time value.
+    RDKPerf perf(__FUNCTION__);


perf is only created for RAII side-effects and is otherwise unused; with -Wall -Werror this is likely to trigger -Wunused-variable in some build configurations. Please mark it as intentionally unused (e.g. [[maybe_unused]] or a project macro) to avoid build failures.

Suggested change

RDKPerf perf(__FUNCTION__);

[[maybe_unused]] RDKPerf perf(__FUNCTION__);

Copilot · 2026-04-27T08:19:10Z

        STATIC
-
+        
        source/EventThread.cpp


Whitespace-only blank line introduced here (line contains spaces). Please remove trailing whitespace to avoid noisy diffs / potential lint failures.

Copilot · 2026-04-27T08:19:11Z

        PUBLIC
+
        interface


Whitespace-only blank line introduced here (line contains spaces). Please remove trailing whitespace to avoid noisy diffs / potential lint failures.

Copilot · 2026-04-27T08:19:11Z

+
+    appSessionServer->startService();
+


There are whitespace-only lines introduced here (blank lines containing spaces / trailing whitespace). Please remove the trailing whitespace to avoid formatting/lint issues and noisy diffs.

Suggested change

appSessionServer->startService();

appSessionServer->startService();

Copilot · 2026-04-27T08:19:11Z


 void GstGenericPlayer::pushSampleIfRequired(GstElement *source, const std::string &typeStr)
 {
+


Whitespace-only blank line introduced at the start of the function body. Please remove trailing whitespace to avoid noisy diffs / potential lint failures.

Suggested change

Copilot · 2026-04-27T08:19:11Z

+#include "rdk_perf.h"



This adds a dependency on the external rdk_perf.h header, but the repo doesn't vendor it and the native dependency install script doesn't install rdkperf. To keep NATIVE_BUILD/CI builds working, please either (a) add proper dependency discovery + installation (and include dirs) for rdkperf, or (b) guard this include/instrumentation behind a build option and provide a stub implementation when rdkperf isn't available. Also consider using angle brackets for third-party headers (#include <rdk_perf.h>) to match other external includes in this file.

Suggested change

#include "rdk_perf.h"

#if defined(__has_include)

#if __has_include(<rdk_perf.h>)

#include <rdk_perf.h>

#define RIALTO_HAS_RDK_PERF 1

#endif

#endif

#ifndef RIALTO_HAS_RDK_PERF

class RDKPerf

{

public:

explicit RDKPerf(const char *) {}

};

#endif

Copilot · 2026-04-27T08:19:11Z

+#include "rdk_perf.h"



This introduces a dependency on external rdk_perf.h in the server gstplayer component, but the repo doesn't provide it and there’s no CMake discovery/guard for environments where rdkperf isn't present (e.g. native builds). Please add a build option / availability check (with a stub) or wire rdkperf in via find_package/pkg-config and include dirs. Also consider switching to #include <rdk_perf.h> to keep external header style consistent with other third-party includes (e.g. <gst/gst.h>).

Suggested change

#include "rdk_perf.h"

#if defined(__has_include)

#if __has_include(<rdk_perf.h>)

#include <rdk_perf.h>

#define RIALTO_HAS_RDK_PERF 1

#else

#define RIALTO_HAS_RDK_PERF 0

#endif

#else

#include <rdk_perf.h>

#define RIALTO_HAS_RDK_PERF 1

#endif

#if !RIALTO_HAS_RDK_PERF

#ifndef RDK_PERF_EVENT_ON

#define RDK_PERF_EVENT_ON(...) ((void)0)

#endif

#ifndef RDK_PERF_EVENT_OFF

#define RDK_PERF_EVENT_OFF(...) ((void)0)

#endif

#ifndef RDK_PERF_START

#define RDK_PERF_START(...) ((void)0)

#endif

#ifndef RDK_PERF_STOP

#define RDK_PERF_STOP(...) ((void)0)

#endif

#ifndef RDK_PERF_START_TS

#define RDK_PERF_START_TS(...) ((void)0)

#endif

#ifndef RDK_PERF_STOP_TS

#define RDK_PERF_STOP_TS(...) ((void)0)

#endif

#endif

Copilot

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 5 comments.

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

Copilot · 2026-04-28T12:02:32Z

    // GstAppSrc does not replace segment, if it's the same as previous one.
    // It causes problems with position reporing in amlogic devices, so we need to push
    // two segments with different reset time value.
+    auto perf = m_rdkPerfWrapperFactory->createRdkPerfWrapper(__FUNCTION__);


perf is never used, and the repo builds with -Werror, so this will fail compilation due to -Wunused-variable. If the intent is RAII timing, mark it [[maybe_unused]] or explicitly cast to void while keeping it in scope.

Suggested change

auto perf = m_rdkPerfWrapperFactory->createRdkPerfWrapper(__FUNCTION__);

auto perf = m_rdkPerfWrapperFactory->createRdkPerfWrapper(__FUNCTION__);

static_cast<void>(perf);

Copilot · 2026-04-28T12:02:32Z

        ${WPEFRAMEWORK_COM_INCLUDE_DIRS}
+        ${rdkperf_INCLUDE_DIRS}
    )


${rdkperf_INCLUDE_DIRS} is referenced here, but this CMakeLists doesn’t define it via pkg_check_modules(...)/find_package(...). This can leave the include path empty and make the build depend on rdk_perf headers being in a global include directory. Add proper rdkperf discovery and set the include dirs from that result.

Copilot · 2026-04-28T12:02:32Z

+        source/ThunderWrapper.cpp
+        source/RdkPerfWrapper.cpp
+        source/RdkPerfWrapperFactoryAccessor.cpp)


source/RdkPerfWrapperFactoryAccessor.cpp is added under WRAPPER_SOURCES, which is excluded for UnitTests/ComponentTests. This can leave IRdkPerfWrapperFactory::createFactory() undefined at link time in those builds. Move the factory accessor file into the unconditional add_library(RialtoWrappers ...) source list (keep only the concrete impl gated by WRAPPERS_ENABLED).

Copilot · 2026-04-28T12:02:33Z

 void ReadShmDataAndAttachSamples::execute() const
 {
    RIALTO_SERVER_LOG_DEBUG("Executing ReadShmDataAndAttachSamples");
+    auto perf = m_rdkPerfWrapperFactory->createRdkPerfWrapper(__FUNCTION__);


perf is never used, and the build uses -Werror, so this will fail compilation with -Wunused-variable. If this is RAII-only instrumentation, mark it [[maybe_unused]] or cast to void while keeping it alive for the intended scope.

Suggested change

auto perf = m_rdkPerfWrapperFactory->createRdkPerfWrapper(__FUNCTION__);

[[maybe_unused]] auto perf = m_rdkPerfWrapperFactory->createRdkPerfWrapper(__FUNCTION__);

Copilot · 2026-04-28T12:02:33Z

 #include "TypeConverters.h"
 #include "Utils.h"
 #include "WorkerThread.h"
+#include "rdk_perf.h"


#include "rdk_perf.h" introduces a direct dependency on the external rdk_perf headers for the gstplayer target. This file doesn’t use any rdk_perf types/symbols (it uses the wrapper factory), so the include should be removed to avoid build failures when rdk_perf headers aren’t on this target’s include path (e.g., native/unit/component builds).

Suggested change

#include "rdk_perf.h"

github-actions · 2026-04-28T13:13:48Z

Coverage statistics of your commit:
Lines coverage stays unchanged and is: 84.5%
Functions coverage stays unchanged and is: 92.6%

github-actions · 2026-04-29T06:39:25Z

stubs/rdk_perf/rdk_perf.h:25:5: style: Class 'RDKPerf' has a constructor with 1 argument that is not explicit. [noExplicitConstructor]
RDKPerf(const char *szName);
^
nofile:0:0: information: Active checkers: 161/592 (use --checkers-report= to see details) [checkersReport]

Copilot

Pull request overview

Copilot reviewed 33 out of 33 changed files in this pull request and generated 9 comments.

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

            context.currentPosition[GST_ELEMENT(&audioSrc)] =
                SegmentData{kPosition, kResetTime, kAppliedRate, kStopPosition};
        });
+    EXPECT_CALL(*m_rdkPerfWrapperFactoryMock, createRdkPerfWrapper(_)).WillOnce(Return(nullptr));


    set(WRAPPER_INCLUDES
        ${OCdm_INCLUDE_DIRS}
        ${WPEFRAMEWORK_CORE_INCLUDE_DIRS}
        ${WPEFRAMEWORK_COM_INCLUDE_DIRS}
+        ${rdkperf_INCLUDE_DIRS}
    )

    set(WRAPPER_LIBS
        rdkgstreamerutils
        ocdm::ocdm
        WPEFrameworkCore::WPEFrameworkCore
        WPEFrameworkCOM::WPEFrameworkCOM
+        rdkperf


+)
+
+install (
+        TARGETS rdkperf LIBRARY



+void GenericTasksTestsBase::shouldCreatePerfWrapper()
+{
+    EXPECT_CALL(*testContext->m_rdkPerfWrapperFactory, createRdkPerfWrapper(_)).WillOnce(Return(nullptr));


                SegmentData{kPosition, kResetTime, kAppliedRate, kStopPosition});
            context.streamInfo[firebolt::rialto::MediaSourceType::VIDEO].appSrc = GST_ELEMENT(&videoSrc);
        });
+    EXPECT_CALL(*m_rdkPerfWrapperFactoryMock, createRdkPerfWrapper(_)).WillOnce(Return(nullptr));


                SegmentData{kPosition, kDoNotResetTime, kAppliedRate, kStopPosition});
            context.streamInfo[firebolt::rialto::MediaSourceType::VIDEO].appSrc = GST_ELEMENT(&videoSrc);
        });
+    EXPECT_CALL(*m_rdkPerfWrapperFactoryMock, createRdkPerfWrapper(_)).WillOnce(Return(nullptr));


                SegmentData{kPosition, kDoNotResetTime, kAppliedRate, kUndefinedPosition});
            context.streamInfo[firebolt::rialto::MediaSourceType::VIDEO].appSrc = GST_ELEMENT(&videoSrc);
        });
+    EXPECT_CALL(*m_rdkPerfWrapperFactoryMock, createRdkPerfWrapper(_)).WillOnce(Return(nullptr));


                SegmentData{kPosition, kResetTime, kAppliedRate, kStopPosition});
            context.streamInfo[firebolt::rialto::MediaSourceType::AUDIO].appSrc = GST_ELEMENT(&audioSrc);
        });
+    EXPECT_CALL(*m_rdkPerfWrapperFactoryMock, createRdkPerfWrapper(_)).WillOnce(Return(nullptr));


    const char kSrcRev[] = SRCREV;
    const char kTags[] = TAGS;

+    auto perf = firebolt::rialto::wrappers::IRdkPerfWrapperFactory::createFactory()->createRdkPerfWrapper(__FUNCTION__);


github-actions · 2026-04-29T06:51:13Z

Coverage statistics of your commit:
Lines coverage stays unchanged and is: 84.5%
Functions coverage stays unchanged and is: 92.6%

balasaraswathy-n added 4 commits April 16, 2026 12:46

Update main.cpp

fb75e4e

Update main.cpp

4974a1b

Update CMakeLists.txt

8b0ec4e

Update main.cpp

9dfc574

Copilot AI review requested due to automatic review settings April 24, 2026 13:54

Copilot started reviewing on behalf of balasaraswathy-n April 24, 2026 13:55 View session

Copilot AI reviewed Apr 24, 2026

View reviewed changes

balasaraswathy-n added 2 commits April 24, 2026 19:29

Update CMakeLists.txt

66d18c8

Update GstGenericPlayer.cpp

98c8280

Copilot AI review requested due to automatic review settings April 27, 2026 08:13

Copilot started reviewing on behalf of balasaraswathy-n April 27, 2026 08:14 View session

Update AttachSamples.cpp

858f6fc

Copilot AI reviewed Apr 27, 2026

View reviewed changes

skywojciechowskim added 2 commits April 28, 2026 10:27

Merge branch 'master' into feature/rdkemw-17139

5f8ed77

wrapper impl

eeb74da

Copilot AI review requested due to automatic review settings April 28, 2026 11:56

Copilot started reviewing on behalf of skywojciechowskim April 28, 2026 11:57 View session

Copilot AI reviewed Apr 28, 2026

View reviewed changes

skywojciechowskim changed the title ~~Feature/rdkemw 17139~~ RDKPerf introduced in Rialto Server Apr 28, 2026

ut/ct fix

1789e36

native build fix

6cf5551

Copilot AI review requested due to automatic review settings April 29, 2026 06:38

Copilot started reviewing on behalf of skywojciechowskim April 29, 2026 06:39 View session

native build fix 2

2bad508

Copilot AI reviewed Apr 29, 2026

View reviewed changes

	RDKPerf perf(__FUNCTION__);
	[[maybe_unused]] RDKPerf perf(__FUNCTION__);


	appSessionServer->startService();

	appSessionServer->startService();


		void GstGenericPlayer::pushSampleIfRequired(GstElement *source, const std::string &typeStr)
		{

-#include "rdk_perf.h"
+#if defined(__has_include)
+#if __has_include(<rdk_perf.h>)
+#include <rdk_perf.h>
+#define RIALTO_HAS_RDK_PERF 1
+#endif
+#endif
+#ifndef RIALTO_HAS_RDK_PERF
+class RDKPerf
+{
+public:
+    explicit RDKPerf(const char *) {}
+};
+#endif

-#include "rdk_perf.h"
+#if defined(__has_include)
+#if __has_include(<rdk_perf.h>)
+#include <rdk_perf.h>
+#define RIALTO_HAS_RDK_PERF 1
+#else
+#define RIALTO_HAS_RDK_PERF 0
+#endif
+#else
+#include <rdk_perf.h>
+#define RIALTO_HAS_RDK_PERF 1
+#endif
+#if !RIALTO_HAS_RDK_PERF
+#ifndef RDK_PERF_EVENT_ON
+#define RDK_PERF_EVENT_ON(...) ((void)0)
+#endif
+#ifndef RDK_PERF_EVENT_OFF
+#define RDK_PERF_EVENT_OFF(...) ((void)0)
+#endif
+#ifndef RDK_PERF_START
+#define RDK_PERF_START(...) ((void)0)
+#endif
+#ifndef RDK_PERF_STOP
+#define RDK_PERF_STOP(...) ((void)0)
+#endif
+#ifndef RDK_PERF_START_TS
+#define RDK_PERF_START_TS(...) ((void)0)
+#endif
+#ifndef RDK_PERF_STOP_TS
+#define RDK_PERF_STOP_TS(...) ((void)0)
+#endif
+#endif

	auto perf = m_rdkPerfWrapperFactory->createRdkPerfWrapper(__FUNCTION__);
	auto perf = m_rdkPerfWrapperFactory->createRdkPerfWrapper(__FUNCTION__);
	static_cast<void>(perf);

	auto perf = m_rdkPerfWrapperFactory->createRdkPerfWrapper(__FUNCTION__);
	[[maybe_unused]] auto perf = m_rdkPerfWrapperFactory->createRdkPerfWrapper(__FUNCTION__);

Conversation

balasaraswathy-n commented Apr 24, 2026 • edited by skywojciechowskim Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Apr 24, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Apr 28, 2026

Uh oh!

github-actions Bot commented Apr 29, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

github-actions Bot commented Apr 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

balasaraswathy-n commented Apr 24, 2026 •

edited by skywojciechowskim

Loading