Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Valgrind on native #303

Open
wants to merge 21 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
af6d8f4
some changes
aczs Jul 26, 2023
13f8f8f
Merge this branch with latest changes and make it compile
skyproudmanp Apr 15, 2024
b4cfdbe
Valgrind specific improvements to the shutdown process optionally cal…
skyproudmanp Apr 18, 2024
125c252
Merge branch 'master' into valgrindOnNative
skyproudmanp Apr 18, 2024
32a967c
Use a script to run RialtoServer with valgrind instead of changing Se…
skyproudmanp Apr 26, 2024
1ddc7d6
Interim checkin of work
skyproudmanp May 10, 2024
f6cf1fd
Merge branch 'master' into valgrindOnNative
skyproudmanp May 10, 2024
4052b1a
Improvements to the coverage build and optionally call gstDeinit on s…
skyproudmanp May 20, 2024
cc43618
Merge branch 'master' into valgrindOnNative
skyproudmanp May 20, 2024
f5722b3
header file correction
skyproudmanp May 21, 2024
33d1709
Merge branch 'master' into valgrindOnNative
skyproudmanp May 21, 2024
79ad7ed
Restore commented out code
skyproudmanp May 22, 2024
4e3b191
Fix a problem with CI coverage tests
skyproudmanp May 22, 2024
8e409bf
Fix a problem with CI coverage tests
skyproudmanp May 22, 2024
f250346
Fix a problem with CI coverage tests
skyproudmanp May 22, 2024
a0b31ef
Native build should not turn off optimization
skyproudmanp May 22, 2024
d73ad69
Missing loglevel in ServerManagerSim
skyproudmanp May 23, 2024
aa246a8
Remove redundant environment variable before gst_deinit
skyproudmanp May 23, 2024
a38bfe8
Merge latest Rialto changes
skyproudmanp Jun 12, 2024
6cd9d65
improve coverage of unit tests
skyproudmanp Jun 12, 2024
d710613
improve coverage of unit tests
skyproudmanp Jun 12, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 14 additions & 11 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ add_compile_definitions(PROJECT_VER_PATCH="${PROJECT_VERSION_PATCH}")
set(CMAKE_CXX_STANDARD 17)

if ( COVERAGE_ENABLED )
add_compile_options(-coverage -fprofile-update=atomic)
add_compile_options(--coverage -fprofile-update=atomic)
add_link_options(--coverage)
endif()

add_compile_options(-Wall -Werror)
Expand All @@ -51,7 +52,7 @@ else()
option(RIALTO_LOG_DEBUG_ENABLED "Enable debug logging for RialtoServer" OFF)
endif()

if (NOT RIALTO_LOG_FATAL_ENABLED OR
if (NOT RIALTO_LOG_FATAL_ENABLED OR
NOT RIALTO_LOG_ERROR_ENABLED OR
NOT RIALTO_LOG_WARN_ENABLED OR
NOT RIALTO_LOG_MIL_ENABLED OR
Expand All @@ -62,7 +63,7 @@ if (NOT RIALTO_LOG_FATAL_ENABLED OR
add_compile_options(-Wno-error=unused-function -Wno-error=unused-variable)

endif()

if ( RIALTO_LOG_FATAL_ENABLED )
message("RIALTO_LOG_FATAL IS ENABLED")
add_compile_definitions( RIALTO_LOG_FATAL_ENABLED )
Expand Down Expand Up @@ -97,13 +98,13 @@ if ( RIALTO_ENABLE_DECRYPT_BUFFER )
add_compile_definitions( RIALTO_ENABLE_DECRYPT_BUFFER )
endif()

# Retrieve the commit ID
# Retrieve the commit ID
execute_process(
COMMAND git rev-parse HEAD
WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}
RESULT_VARIABLE RESULT
OUTPUT_VARIABLE SRCREV
OUTPUT_STRIP_TRAILING_WHITESPACE
WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}
RESULT_VARIABLE RESULT
OUTPUT_VARIABLE SRCREV
OUTPUT_STRIP_TRAILING_WHITESPACE
)

if(RESULT)
Expand All @@ -113,15 +114,15 @@ endif()
# Retrieve release tag
execute_process(
COMMAND bash -c "git tag --list --contains ${SRCREV} | grep -E '^v[0-9]+\.[0-9]+\.[0-9]+$'"
WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}
WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}
OUTPUT_VARIABLE TAGS
OUTPUT_STRIP_TRAILING_WHITESPACE
OUTPUT_STRIP_TRAILING_WHITESPACE
)
string(REPLACE "\n" ", " TAGS "${TAGS}")

if(NOT TAGS STREQUAL "")
set(TAGS ${TAGS})
endif()
endif()

# Preprocesser Variable
add_compile_definitions(SRCREV="${SRCREV}")
Expand Down Expand Up @@ -206,6 +207,8 @@ endif()
if( NATIVE_BUILD )
add_subdirectory( stubs/rdk_gstreamer_utils )
add_subdirectory( stubs/opencdm )
add_compile_options(-DFREE_MEM_BEFORE_EXIT)
add_compile_options(-O0 -ggdb)
Copy link
Contributor

Choose a reason for hiding this comment

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

please add an additional flag for debugging with GDB


include( GNUInstallDirs )
configure_file( pkg-config/RialtoClient.in.pc ${CMAKE_BINARY_DIR}/RialtoClient.pc @ONLY )
Expand Down
5 changes: 3 additions & 2 deletions build_ct.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
# Entry script for running rialto componenttests

import argparse
import os
from scripts.gtest.build_and_run_tests import getGenericArguments, buildAndRunGTests
from scripts.gtest.utils import getSuitesToRun, getOutputFile
from scripts.gtest.generate_coverage import generateCoverageReport, generateSpecificCoverageStats
Expand Down Expand Up @@ -50,8 +51,8 @@

# Generate coverage
if args['coverage'] == True:
generateCoverageReport(args['output'], outputFile, suitesToRun)
generateCoverageReport(os.getcwd(), args['output'], outputFile)

# Also generate coverage stats for public interfaces source
files = ["*/main/source/*"]
generateSpecificCoverageStats(args['output'], outputFile, files, "coverage_statistics_public_apis")
generateSpecificCoverageStats(os.getcwd(), args['output'], outputFile, files, "coverage_statistics_public_apis")
3 changes: 2 additions & 1 deletion build_ut.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
# Entry script for running rialto unittests

import argparse
import os
from scripts.gtest.build_and_run_tests import getGenericArguments, buildAndRunGTests
from scripts.gtest.utils import getSuitesToRun, getOutputFile
from scripts.gtest.generate_coverage import generateCoverageReport
Expand Down Expand Up @@ -58,4 +59,4 @@
buildAndRunGTests(args, outputFile, buildDefines, suitesToRun)

if args['coverage'] == True:
generateCoverageReport(args['output'], outputFile, suitesToRun)
generateCoverageReport(os.getcwd(), args['output'], outputFile)
8 changes: 0 additions & 8 deletions common/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,3 @@ target_link_libraries (
PRIVATE
RialtoLogging
)

if ( COVERAGE_ENABLED )
target_link_libraries(
RialtoCommon
PRIVATE
gcov
)
endif()
2 changes: 1 addition & 1 deletion media/client/ipc/source/MediaPipelineIpc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@
* limitations under the License.
*/
#include "MediaPipelineIpc.h"
#include "IMediaPipeline.h"
#include "RialtoClientLogging.h"
#include "RialtoCommonIpc.h"
#include "mediapipelinemodule.pb.h"
#include <IMediaPipeline.h>

namespace firebolt::rialto::client
{
Expand Down
9 changes: 0 additions & 9 deletions media/client/main/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -88,15 +88,6 @@ target_link_libraries(
Threads::Threads

)
if ( COVERAGE_ENABLED )
target_link_libraries(
RialtoClient

PRIVATE
gcov
)
endif()


include( GNUInstallDirs )

Expand Down
7 changes: 4 additions & 3 deletions media/client/main/source/MediaPipeline.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,13 @@
* limitations under the License.
*/

#include "MediaPipeline.h"
#include "KeyIdMap.h"
#include "RialtoClientLogging.h"
#include <inttypes.h>
#include <stdint.h>

#include "KeyIdMap.h"
#include "MediaPipeline.h"
#include "RialtoClientLogging.h"

namespace
{
const char *toString(const firebolt::rialto::client::MediaPipeline::State &state)
Expand Down
1 change: 1 addition & 0 deletions media/server/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ target_link_libraries(
RialtoServerIpc
RialtoServerMain
RialtoServerService
protobuf::libprotobuf
)

if( NATIVE_BUILD )
Expand Down
2 changes: 1 addition & 1 deletion media/server/gstplayer/include/GstSrc.h
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ class GstSrc : public IGstSrc

void initSrc() override;

void setupAndAddAppArc(IDecryptionService *decryptionService, GstElement *source, StreamInfo &streamInfo,
void setupAndAddAppSrc(IDecryptionService *decryptionService, GstElement *source, StreamInfo &streamInfo,
GstAppSrcCallbacks *callbacks, gpointer userData,
firebolt::rialto::MediaSourceType type) override;

Expand Down
2 changes: 1 addition & 1 deletion media/server/gstplayer/include/IGstSrc.h
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ class IGstSrc
* @param[in] userData : Data to be passed to the callbacks.
* @param[in] type : The media type of the source.
*/
virtual void setupAndAddAppArc(IDecryptionService *decryptionService, GstElement *source, StreamInfo &streamInfo,
virtual void setupAndAddAppSrc(IDecryptionService *decryptionService, GstElement *source, StreamInfo &streamInfo,
GstAppSrcCallbacks *callbacks, gpointer userData,
firebolt::rialto::MediaSourceType type) = 0;

Expand Down
7 changes: 7 additions & 0 deletions media/server/gstplayer/interface/GstInit.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,13 @@ namespace firebolt::rialto::server
* @param[in] argv : Vector of C strings each containing a command line argument.
*/
bool gstInitalise(int argc, char **argv);

/**
* @brief Deinitalise gstreamer.
*
*/
bool gstDeinitalise();

}; // namespace firebolt::rialto::server

#endif // FIREBOLT_RIALTO_SERVER_GST_INIT_H_
2 changes: 1 addition & 1 deletion media/server/gstplayer/source/GstGenericPlayer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
*/

#include <chrono>
#include <cinttypes>

#include "GstDispatcherThread.h"
#include "GstGenericPlayer.h"
Expand All @@ -27,7 +28,6 @@
#include "RialtoServerLogging.h"
#include "WorkerThread.h"
#include "tasks/generic/GenericPlayerTaskFactory.h"
#include <cinttypes>

namespace
{
Expand Down
18 changes: 18 additions & 0 deletions media/server/gstplayer/source/GstInit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,22 @@ bool gstInitalise(int argc, char **argv)

return true;
}

bool gstDeinitalise()
{
using firebolt::rialto::wrappers::IGstWrapper;
using firebolt::rialto::wrappers::IGstWrapperFactory;
std::shared_ptr<IGstWrapperFactory> factory = IGstWrapperFactory::getFactory();
std::shared_ptr<IGstWrapper> gstWrapper = factory->getGstWrapper();

if (!gstWrapper)
{
RIALTO_SERVER_LOG_ERROR("Failed to create the gst wrapper");
return false;
}

gstWrapper->gstDeinit();

return true;
}
}; // namespace firebolt::rialto::server
2 changes: 1 addition & 1 deletion media/server/gstplayer/source/GstSrc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,7 @@ void GstSrc::setDefaultStreamFormatIfNeeded(GstElement *appSrc)
m_gstWrapper->gstCapsUnref(currentCaps);
}

void GstSrc::setupAndAddAppArc(IDecryptionService *decryptionService, GstElement *source, StreamInfo &streamInfo,
void GstSrc::setupAndAddAppSrc(IDecryptionService *decryptionService, GstElement *source, StreamInfo &streamInfo,
GstAppSrcCallbacks *callbacks, gpointer userData, firebolt::rialto::MediaSourceType type)
{
// Configure and add appsrc
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,15 +102,15 @@ void FinishSetupSource::execute() const
auto elem = m_context.streamInfo.find(firebolt::rialto::MediaSourceType::AUDIO);
if (elem != m_context.streamInfo.end())
{
m_context.gstSrc->setupAndAddAppArc(m_context.decryptionService, m_context.source, elem->second, &callbacks,
m_context.gstSrc->setupAndAddAppSrc(m_context.decryptionService, m_context.source, elem->second, &callbacks,
&m_player, firebolt::rialto::MediaSourceType::AUDIO);
m_player.notifyNeedMediaData(true, false);
}

elem = m_context.streamInfo.find(firebolt::rialto::MediaSourceType::VIDEO);
if (elem != m_context.streamInfo.end())
{
m_context.gstSrc->setupAndAddAppArc(m_context.decryptionService, m_context.source, elem->second, &callbacks,
m_context.gstSrc->setupAndAddAppSrc(m_context.decryptionService, m_context.source, elem->second, &callbacks,
&m_player, firebolt::rialto::MediaSourceType::VIDEO);
m_player.notifyNeedMediaData(false, true);
}
Expand Down
2 changes: 1 addition & 1 deletion media/server/ipc/source/ApplicationManagementServer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@
*/

#include "ApplicationManagementServer.h"
#include "IIpcServerFactory.h"
#include "IServerManagerModuleServiceFactory.h"
#include "RialtoServerLogging.h"
#include <IIpcServerFactory.h>

namespace
{
Expand Down
35 changes: 29 additions & 6 deletions media/server/service/source/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,14 @@
* limitations under the License.
*/

#include <cstring>
#include <thread>

#include <google/protobuf/service.h>

#include "GstInit.h"
#include "IApplicationSessionServer.h"
#include "RialtoServerLogging.h"
#include <cstring>

// NOLINT(build/filename_format)

Expand All @@ -47,13 +51,32 @@ int main(int argc, char *argv[])

firebolt::rialto::server::gstInitalise(argc, argv);

auto appSessionServer =
firebolt::rialto::server::IApplicationSessionServerFactory::getFactory()->createApplicationSessionServer();
{
// Creation of this variable in a local scope ensures Rialto
// destructors are called before potentially calling gst_deinit()
// which would cause free memory reads
auto appSessionServer =
firebolt::rialto::server::IApplicationSessionServerFactory::getFactory()->createApplicationSessionServer();

if (!appSessionServer->init(argc, argv))
{
firebolt::rialto::server::gstDeinitalise();
return EXIT_FAILURE;
}

appSessionServer->startService();
}

if (!appSessionServer->init(argc, argv))
#ifdef FREE_MEM_BEFORE_EXIT
if (getenv("RIALTO_FREE_MEM_BEFORE_EXIT"))
skyproudmanp marked this conversation as resolved.
Show resolved Hide resolved
{
return EXIT_FAILURE;
RIALTO_SERVER_LOG_INFO("Calling gst_deinit");
firebolt::rialto::server::gstDeinitalise();

RIALTO_SERVER_LOG_INFO("Calling ShutdownProtobufLibrary");
google::protobuf::ShutdownProtobufLibrary();
}
appSessionServer->startService();
#endif

return EXIT_SUCCESS;
}
Loading
Loading