Feature/vplay 12344 by Reshma-JO07 · Pull Request #951 · rdkcentral/aamp

Reshma-JO07 · 2026-02-02T07:58:33Z

No description provided.

Copilot

Pull request overview

This PR introduces a static GStreamer pipeline implementation for the AAMP player as an alternative to the dynamic playbin approach, controlled by a new configuration flag useStaticPipeline. The static pipeline provides explicit control over the media processing chain including demuxing, decryption, and rendering elements.

Changes:

Adds a new configuration option useStaticPipeline to enable static pipeline construction
Implements static pipeline setup for audio, video, and subtitle streams with explicit element creation and linking
Introduces DRM system UUID-based decryptor selection via GetDecryptorPluginName function
Changes default subtitle format from FORMAT_SUBTITLE_MP4 to FORMAT_ISO_BMFF for qtdemux compatibility
Adds property existence checks before setting volume/mute properties on GStreamer elements
Changes default value of useRialtoSink configuration to true

Reviewed changes

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

Show a summary per file

File	Description
test/utests/fakes/FakeStreamAbstractionAamp.cpp	Minor formatting fix - adds missing newline before closing brace
test/utests/fakes/FakeIsoBmffBuffer.cpp	Minor formatting fix - adds missing newline before closing brace
test/gstTestHarness/gst-utils.h	Adds missing `<cstdio>` include for standard I/O functions
priv_aamp.h	Removes inline comment from TIMEOUT_FOR_LLD macro definition
priv_aamp.cpp	Adds debug marker "RRP -->> " to tune log message
middleware/InterfacePlayerRDK.h	Adds `useStaticPipeline` boolean field to Configs struct
middleware/InterfacePlayerRDK.cpp	Major implementation: adds static pipeline creation, DRM decryptor selection, property validation, and new pad-added callback for static pipeline
fragmentcollector_mpd.cpp	Changes subtitle output format from FORMAT_SUBTITLE_MP4 to FORMAT_ISO_BMFF
aampgstplayer.cpp	Initializes useStaticPipeline config parameter from AAMP config
AampConfig.h	Adds eAAMPConfig_UseStaticPipeline enum value for new configuration option
AampConfig.cpp	Adds useStaticPipeline config entry and changes useRialtoSink default to true

Copilot · 2026-02-02T08:03:55Z

middleware/InterfacePlayerRDK.cpp

+						// Add all elements to bin
+						gst_bin_add_many(GST_BIN(stream->sinkbin), stream->source, subtecmp4transform, vipertransform, textsink, NULL);
+
+						// Link: appsrc → subtecmp4transform → vipertransform → fakesink


The comment on line 2561 states "Link: appsrc → subtecmp4transform → vipertransform → fakesink" but the actual code links to textsink (rialtomsesubtitlesink), not fakesink. The comment should be updated to accurately reflect the actual pipeline being created.

Copilot · 2026-02-02T08:03:56Z

middleware/InterfacePlayerRDK.cpp

+					auto demux = gst_element_factory_make("qtdemux", NULL);
+					if (!demux)
+					{
+						MW_LOG_ERR("DEBUG: Failed to create qtdemux element for audio!");
+						return -1;
+					}
+					MW_LOG_WARN("DEBUG: qtdemux created successfully for audio");
+
+					// Create rialto audio sink and add to bin
+					auto audioSink = gst_element_factory_make("rialtomseaudiosink", NULL);


The auto keyword is used for local variable declarations (demux, audioSink) within functions that manipulate GStreamer elements. While the types are clear from the factory calls, the coding guidelines emphasize const correctness and explicit types for clarity. Consider using explicit GstElement* types for better readability and maintainability, especially in complex GStreamer pipeline setup code where clarity about element types is important for debugging.

Copilot · 2026-02-02T08:03:56Z

middleware/InterfacePlayerRDK.cpp

+static const char* GetDecryptorPluginName(const char* drmSystemId)
+{
+	if (!drmSystemId)
+	{
+		return GstPluginNameWV; // Default to Widevine for backward compatibility
+	}
+
+	if (strcmp(drmSystemId, WIDEVINE_UUID) == 0)
+	{
+		return GstPluginNameWV;
+	}
+	else if (strcmp(drmSystemId, PLAYREADY_UUID) == 0)
+	{
+		return GstPluginNamePR;
+	}
+	else if (strcmp(drmSystemId, CLEARKEY_UUID) == 0)
+	{
+		return GstPluginNameCK;
+	}
+	else if (strcmp(drmSystemId, VERIMATRIX_UUID) == 0)
+	{
+		return GstPluginNameVMX;
+	}
+	else
+	{
+		MW_LOG_WARN("Unknown DRM system ID: %s, defaulting to Widevine", drmSystemId);
+		return GstPluginNameWV; // Default fallback
+	}
+}


The new function GetDecryptorPluginName performs multiple string comparisons using strcmp. While the function has proper documentation and follows good practices, it would benefit from const correctness by declaring the return type and local string constants as const char* const where appropriate. Additionally, the function could leverage a map or switch-case pattern for better maintainability if more DRM systems are added in the future.

Copilot · 2026-02-02T08:03:56Z

middleware/InterfacePlayerRDK.cpp

+	if (linkRet != GST_PAD_LINK_OK)
+	{
+		MW_LOG_ERR("Failed to link demux pad to queue sink pad");
+		gst_element_set_state(queue, GST_STATE_NULL);
+		gst_bin_remove(GST_BIN(stream->sinkbin), queue);
+		return;
+	}
+
+	MW_LOG_WARN("Linked demux pad to queue sink pad");
+
+	// Get appropriate decryptor based on actual content protection-system
+	// Use content protection-system if available, otherwise fall back to mDrmSystem
+	const char* drmSystemToUse = contentProtectionSystem ? contentProtectionSystem : pInterfacePlayerRDK->mDrmSystem;
+	const char* decryptorPluginName = GetDecryptorPluginName(drmSystemToUse);
+
+	if (contentProtectionSystem && pInterfacePlayerRDK->mDrmSystem)
+	{
+		// Log if there's a mismatch between preferred and actual DRM
+		if (strcmp(contentProtectionSystem, pInterfacePlayerRDK->mDrmSystem) != 0)
+		{
+			MW_LOG_WARN("DRM mismatch - Preferred: %s, Content: %s. Using content protection-system.", 
+			            pInterfacePlayerRDK->mDrmSystem, contentProtectionSystem);
+		}
+	}
+
+	if (drmSystemToUse)
+	{
+		MW_LOG_MIL("Creating decryptor for DRM system: %s, plugin: %s", 
+		           drmSystemToUse, decryptorPluginName);
+	}
+	else
+	{
+		MW_LOG_WARN("DRM system not set, using default decryptor: %s", decryptorPluginName);
+	}
+
+	GstElement* decryptor = gst_element_factory_make(decryptorPluginName, nullptr);
+	if (!decryptor)
+	{
+		MW_LOG_ERR("Failed to create %s decryptor for demux:%s. Plugin may not be available on this platform.", 
+		           decryptorPluginName, GST_ELEMENT_NAME(demux));
+		return;
+	}
+
+	MW_LOG_WARN("Created %s decryptor for demux:%s", decryptorPluginName, GST_ELEMENT_NAME(demux));
+	gst_bin_add(GST_BIN(stream->sinkbin), decryptor);
+	// Elements stay in NULL state for static pipeline linking
+
+	MW_LOG_MIL("Linking elements in static pipeline for %s", gstGetMediaTypeName(mediaType));
+
+	// Manual pad linking to bypass custom element checks while respecting caps
+	// gst_element_link() fails on Xi One UK widevinedecryptor despite compatible template caps
+	GstPad* queueSrcPad = gst_element_get_static_pad(queue, "src");
+	GstPad* decryptorSinkPad = gst_element_get_static_pad(decryptor, "sink");
+
+	if (!queueSrcPad || !decryptorSinkPad)
+	{
+		MW_LOG_ERR("Failed to get pads for manual linking");
+		if (queueSrcPad) gst_object_unref(queueSrcPad);
+		if (decryptorSinkPad) gst_object_unref(decryptorSinkPad);
+		gst_element_set_state(decryptor, GST_STATE_NULL);
+		gst_bin_remove(GST_BIN(stream->sinkbin), decryptor);
+		gst_element_set_state(queue, GST_STATE_NULL);
+		gst_bin_remove(GST_BIN(stream->sinkbin), queue);
+		return;
+	}
+
+	// Use pad linking with NO checks - Xi One UK widevinedecryptor blocks all link checks
+	// Caps negotiation will happen during NULL->READY->PAUSED state transitions
+	// This is standard for DRM decryptor elements that require runtime initialization
+	GstPadLinkReturn decryptorLinkRet = gst_pad_link_full(queueSrcPad, decryptorSinkPad, 
+	                                                        GST_PAD_LINK_CHECK_NOTHING);
+	gst_object_unref(queueSrcPad);
+	gst_object_unref(decryptorSinkPad);
+
+	if (decryptorLinkRet != GST_PAD_LINK_OK)
+	{
+		MW_LOG_ERR("Failed to link queue to decryptor pads - error: %d", decryptorLinkRet);
+		gst_element_set_state(decryptor, GST_STATE_NULL);
+		gst_bin_remove(GST_BIN(stream->sinkbin), decryptor);
+		gst_element_set_state(queue, GST_STATE_NULL);
+		gst_bin_remove(GST_BIN(stream->sinkbin), queue);
+		return;
+	}
+
+	MW_LOG_WARN("Successfully linked queue to decryptor via manual pad linking");
+
+	// Get the pre-created rialto sink (created in SetupStream)
+	GstElement* realSink = nullptr;
+	if (mediaType == eGST_MEDIATYPE_VIDEO)
+	{
+		realSink = privatePlayer->gstPrivateContext->video_sink;
+		MW_LOG_WARN("Using pre-created rialtomsevideosink: %p", realSink);
+	}
+	else if (mediaType == eGST_MEDIATYPE_AUDIO)
+	{
+		realSink = privatePlayer->gstPrivateContext->audio_sink;
+		MW_LOG_WARN("Using pre-created rialtomseaudiosink: %p", realSink);
+	}
+
+	if (!realSink)
+	{
+		MW_LOG_ERR("Pre-created rialto sink not found for demux:%s", GST_ELEMENT_NAME(demux));
+		return;
+	}
+
+	// Sink already added to bin in SetupStream, no need to add again
+
+	// Link decryptor to rialto sink (both in NULL state)
+	if (!gst_element_link(decryptor, realSink))
+	{
+		MW_LOG_ERR("Failed to link decryptor to rialto sink in static pipeline");
+		gst_element_set_state(realSink, GST_STATE_NULL);
+		gst_bin_remove(GST_BIN(stream->sinkbin), realSink);
+		gst_element_set_state(decryptor, GST_STATE_NULL);
+		gst_bin_remove(GST_BIN(stream->sinkbin), decryptor);
+		gst_element_set_state(queue, GST_STATE_NULL);
+		gst_bin_remove(GST_BIN(stream->sinkbin), queue);
+		return;


There is error handling code that attempts to clean up GStreamer elements, but the cleanup may not be complete. When link failures occur (lines 2048-2053, 2122-2129, 2158-2165), the code sets elements to GST_STATE_NULL and removes them from the bin. However, in these error paths, previously created and linked elements (e.g., queue in the first error path, queue and decryptor in subsequent paths) may remain in an inconsistent state. Consider implementing a comprehensive cleanup function that ensures all created elements are properly destroyed in all error paths to prevent resource leaks.

Copilot · 2026-02-02T08:03:56Z

middleware/InterfacePlayerRDK.cpp

+					if (!audioSink)
+					{
+						MW_LOG_ERR("Failed to create rialtomseaudiosink");
+						return -1;
+					}
+					MW_LOG_WARN("DEBUG: rialtomseaudiosink created successfully: %p", audioSink);
+					interfacePlayerPriv->gstPrivateContext->audio_sink = audioSink;
+
+					// Initialize appsrc after sink is created
+					gstInitializeSource(pInterfacePlayerRDK, G_OBJECT(stream->source), streamId);
+
+					g_signal_connect(demux, "pad-added", G_CALLBACK(GstPlayer_OnDemuxPadAddedCb_Static), pInterfacePlayerRDK);
+					gst_bin_add_many(GST_BIN(stream->sinkbin), stream->source, demux, audioSink, NULL);
+					gst_element_link_many(stream->source, demux, NULL);
+					gst_bin_add_many(GST_BIN(interfacePlayerPriv->gstPrivateContext->pipeline), stream->sinkbin, NULL);
+					// Sync state of elements in the bin
+					gst_element_sync_state_with_parent(stream->source);
+					gst_element_sync_state_with_parent(demux);
+					gst_element_sync_state_with_parent(stream->sinkbin);
+					return 0;
+				}
+			}
+			else if (eGST_MEDIATYPE_VIDEO == streamId)
+			{
+				MW_LOG_INFO("video using rialtomsevideosink in static pipeline format=%d", stream->format);
+
+				// Only use static pipeline for ISO_BMFF (MP4 container) format
+				if (stream->format != GST_FORMAT_ISO_BMFF)
+				{
+					MW_LOG_WARN("Non-ISO_BMFF video format (%d), falling back to playbin", stream->format);
+					// Fall through to use playbin below
+				}
+				else
+				{
+					MW_LOG_WARN("DEBUG: Entering ISO_BMFF video static pipeline setup");
+					stream->sinkbin = gst_bin_new("video_bin");
+					if (!stream->sinkbin)
+					{
+						MW_LOG_WARN("Cannot set up video_bin");
+						return -1;
+					}
+					MW_LOG_WARN("DEBUG: video_bin created successfully");
+					stream->sinkbin = GST_ELEMENT(gst_object_ref_sink(stream->sinkbin));
+					stream->source = GST_ELEMENT(gst_object_ref_sink(InterfacePlayerRDK_GetAppSrc(pInterfacePlayerRDK, (GstMediaType)streamId, false)));
+					MW_LOG_WARN("DEBUG: appsrc retrieved successfully");
+
+					MW_LOG_INFO("Using qtdemux for ISO_BMFF video stream");
+					auto demux = gst_element_factory_make("qtdemux", NULL);
+					if (!demux)
+					{
+						MW_LOG_ERR("DEBUG: Failed to create qtdemux element!");
+						return -1;
+					}
+					MW_LOG_WARN("DEBUG: qtdemux created successfully");
+
+					// Create rialto video sink and add to bin
+					auto videoSink = gst_element_factory_make("rialtomsevideosink", NULL);
+					if (!videoSink)
+					{
+						MW_LOG_ERR("Failed to create rialtomsevideosink");
+						return -1;


When sink creation fails (lines 2697-2700 for audio, 2754-2757 for video), the error path returns -1 without cleaning up the previously allocated elements: stream->source, stream->sinkbin, and demux. These elements need to be properly unreferenced to prevent memory leaks.

Copilot · 2026-02-02T08:03:58Z

middleware/InterfacePlayerRDK.cpp

+					{
+						MW_LOG_ERR("DEBUG: Failed to create qtdemux element for audio!");
+						return -1;
+					}
+					MW_LOG_WARN("DEBUG: qtdemux created successfully for audio");
+
+					// Create rialto audio sink and add to bin
+					auto audioSink = gst_element_factory_make("rialtomseaudiosink", NULL);
+					if (!audioSink)
+					{
+						MW_LOG_ERR("Failed to create rialtomseaudiosink");
+						return -1;
+					}
+					MW_LOG_WARN("DEBUG: rialtomseaudiosink created successfully: %p", audioSink);
+					interfacePlayerPriv->gstPrivateContext->audio_sink = audioSink;
+
+					// Initialize appsrc after sink is created
+					gstInitializeSource(pInterfacePlayerRDK, G_OBJECT(stream->source), streamId);
+
+					g_signal_connect(demux, "pad-added", G_CALLBACK(GstPlayer_OnDemuxPadAddedCb_Static), pInterfacePlayerRDK);
+					gst_bin_add_many(GST_BIN(stream->sinkbin), stream->source, demux, audioSink, NULL);
+					gst_element_link_many(stream->source, demux, NULL);
+					gst_bin_add_many(GST_BIN(interfacePlayerPriv->gstPrivateContext->pipeline), stream->sinkbin, NULL);
+					// Sync state of elements in the bin
+					gst_element_sync_state_with_parent(stream->source);
+					gst_element_sync_state_with_parent(demux);
+					gst_element_sync_state_with_parent(stream->sinkbin);
+					return 0;
+				}
+			}
+			else if (eGST_MEDIATYPE_VIDEO == streamId)
+			{
+				MW_LOG_INFO("video using rialtomsevideosink in static pipeline format=%d", stream->format);
+
+				// Only use static pipeline for ISO_BMFF (MP4 container) format
+				if (stream->format != GST_FORMAT_ISO_BMFF)
+				{
+					MW_LOG_WARN("Non-ISO_BMFF video format (%d), falling back to playbin", stream->format);
+					// Fall through to use playbin below
+				}
+				else
+				{
+					MW_LOG_WARN("DEBUG: Entering ISO_BMFF video static pipeline setup");
+					stream->sinkbin = gst_bin_new("video_bin");
+					if (!stream->sinkbin)
+					{
+						MW_LOG_WARN("Cannot set up video_bin");
+						return -1;
+					}
+					MW_LOG_WARN("DEBUG: video_bin created successfully");
+					stream->sinkbin = GST_ELEMENT(gst_object_ref_sink(stream->sinkbin));
+					stream->source = GST_ELEMENT(gst_object_ref_sink(InterfacePlayerRDK_GetAppSrc(pInterfacePlayerRDK, (GstMediaType)streamId, false)));
+					MW_LOG_WARN("DEBUG: appsrc retrieved successfully");
+
+					MW_LOG_INFO("Using qtdemux for ISO_BMFF video stream");
+					auto demux = gst_element_factory_make("qtdemux", NULL);
+					if (!demux)
+					{
+						MW_LOG_ERR("DEBUG: Failed to create qtdemux element!");
+						return -1;


Similar to the subtitle pipeline, in the audio and video static pipeline setup error paths, when qtdemux creation fails (lines 2689-2691 for audio, 2745-2748 for video), the code returns -1 without cleaning up the already-created stream->source (appsrc) and stream->sinkbin. These elements were ref_sunk earlier and should be properly unreferenced in the error path to prevent memory leaks.

Copilot · 2026-02-02T08:03:59Z

middleware/InterfacePlayerRDK.cpp

+						// Add elements to bin (without subtecmp4transform)
+						gst_bin_add_many(GST_BIN(stream->sinkbin), stream->source, vipertransform, textsink, NULL);
+
+						// Link: appsrc → vipertransform → fakesink


The comment on line 2577 states "Link: appsrc → vipertransform → fakesink" but the actual code links to textsink (rialtomsesubtitlesink), not fakesink. The comment should be updated to accurately reflect the actual pipeline being created.

Copilot · 2026-02-02T08:03:59Z

middleware/InterfacePlayerRDK.cpp

+					auto demux = gst_element_factory_make("qtdemux", NULL);
+					if (!demux)
+					{
+						MW_LOG_ERR("DEBUG: Failed to create qtdemux element!");
+						return -1;
+					}
+					MW_LOG_WARN("DEBUG: qtdemux created successfully");
+
+					// Create rialto video sink and add to bin
+					auto videoSink = gst_element_factory_make("rialtomsevideosink", NULL);


The auto keyword is used for local variable declarations (demux, videoSink) within functions that manipulate GStreamer elements. While the types are clear from the factory calls, the coding guidelines emphasize const correctness and explicit types for clarity. Consider using explicit GstElement* types for better readability and maintainability, especially in complex GStreamer pipeline setup code where clarity about element types is important for debugging.

Copilot · 2026-02-02T08:03:59Z

AampConfig.cpp

 	{false, "earlyProcessing", eAAMPConfig_EarlyID3Processing, false},
 	{false, "seamlessAudioSwitch", eAAMPConfig_SeamlessAudioSwitch, true},
-	{false, "useRialtoSink", eAAMPConfig_useRialtoSink, false},
+	{true, "useRialtoSink", eAAMPConfig_useRialtoSink, false},


The configuration value useRialtoSink has its default changed from false to true (first field of the config entry). This is a breaking change that affects the default behavior of the player. If this is intentional for this feature, it should be explicitly documented in the PR description or commit message as it changes the default playback behavior for all users. If not intentional, this should be reverted to maintain backward compatibility.

Suggested change

{true, "useRialtoSink", eAAMPConfig_useRialtoSink, false},

{false, "useRialtoSink", eAAMPConfig_useRialtoSink, false},

Copilot · 2026-02-02T08:04:00Z

fragmentcollector_mpd.cpp

+				// Mp4Demux is skipped for subtitles - use ISO BMFF format for qtdemux
+				subtitleOutputFormat = FORMAT_ISO_BMFF;


The comment states "Mp4Demux is skipped for subtitles - use ISO BMFF format for qtdemux". However, the change modifies the subtitle output format from FORMAT_SUBTITLE_MP4 to FORMAT_ISO_BMFF. This appears to be a significant behavioral change for subtitle handling, but there's insufficient documentation about why this change is necessary and what impact it has on subtitle processing. The comment should explain the relationship between this format change and the static pipeline implementation, and whether this affects subtitle rendering or compatibility.

Copilot

Pull request overview

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

Copilot · 2026-02-02T08:16:31Z

middleware/InterfacePlayerRDK.cpp

+					auto subtitlebin = gst_bin_new("subtitlebin");
+					auto vipertransform = gst_element_factory_make("vipertransform", NULL);
+					gst_bin_add_many(GST_BIN(subtitlebin),vipertransform,textsink,NULL);


Inconsistent use of auto keyword. At lines 2618-2620, the code uses auto for local variables (subtitlebin, vipertransform), but in the static pipeline subtitle setup at lines 2527-2529, explicit types are used (GstElement*). According to modern C++ guidelines (CodingGuidelineID: 1000003), auto should be used consistently where it improves readability. The codebase should follow a consistent pattern.

Copilot · 2026-02-02T08:16:31Z

middleware/InterfacePlayerRDK.cpp

+						// Add all elements to bin
+						gst_bin_add_many(GST_BIN(stream->sinkbin), stream->source, subtecmp4transform, vipertransform, textsink, NULL);
+
+						// Link: appsrc → subtecmp4transform → vipertransform → fakesink


Comment refers to "fakesink" but the actual element is "rialtomsesubtitlesink". At line 2561, the comment says "Link: appsrc → subtecmp4transform → vipertransform → fakesink" but the actual element being linked is textsink which is a rialtomsesubtitlesink. This misleading comment should be corrected to reflect the actual element type.

Copilot · 2026-02-02T08:16:31Z

middleware/InterfacePlayerRDK.cpp

+static const char* GetDecryptorPluginName(const char* drmSystemId)
+{
+	if (!drmSystemId)
+	{


The use of strcmp with potentially null drmSystemId is unsafe. While there is a null check at the start, if drmSystemId is null, the function returns early with GstPluginNameWV. However, the fallback to "Widevine for backward compatibility" comment suggests this might not always be the intended behavior. Consider whether returning a default plugin when DRM system ID is not specified is the correct behavior in all cases, or if an error should be logged more prominently.

Suggested change

{

{

MW_LOG_WARN("GetDecryptorPluginName called with null DRM system ID, defaulting to Widevine for backward compatibility");

Copilot · 2026-02-02T08:16:32Z

middleware/InterfacePlayerRDK.cpp

+	if (!decryptor)
+	{
+		MW_LOG_ERR("Failed to create %s decryptor for demux:%s. Plugin may not be available on this platform.", 
+		           decryptorPluginName, GST_ELEMENT_NAME(demux));


Incomplete error handling after element creation failure. If decryptor creation fails at line 2083-2089, the function logs an error and returns, but the queue element that was created and added to the bin at lines 2024-2032 is not cleaned up. This leaves an orphaned queue element in the bin. The error path should remove the queue from the bin and unref it before returning.

Suggested change

decryptorPluginName, GST_ELEMENT_NAME(demux));

decryptorPluginName, GST_ELEMENT_NAME(demux));

// Cleanup queue element added earlier to avoid leaving an orphan in the bin

if (queue != nullptr)

{

gst_bin_remove(GST_BIN(stream->sinkbin), queue);

queue = nullptr;

}

Copilot · 2026-02-02T08:16:32Z

middleware/InterfacePlayerRDK.cpp

+
+	if (!realSink)
+	{
+		MW_LOG_ERR("Pre-created rialto sink not found for demux:%s", GST_ELEMENT_NAME(demux));


Resource leak when rialto sink is not found. At lines 2147-2151, if realSink is null, the function returns without cleaning up the queue and decryptor elements that were previously created and added to the bin. These elements should be removed from the bin and their states set to NULL before returning.

Suggested change

MW_LOG_ERR("Pre-created rialto sink not found for demux:%s", GST_ELEMENT_NAME(demux));

MW_LOG_ERR("Pre-created rialto sink not found for demux:%s", GST_ELEMENT_NAME(demux));

gst_element_set_state(decryptor, GST_STATE_NULL);

gst_bin_remove(GST_BIN(stream->sinkbin), decryptor);

gst_element_set_state(queue, GST_STATE_NULL);

gst_bin_remove(GST_BIN(stream->sinkbin), queue);

Copilot · 2026-02-02T08:16:35Z

middleware/InterfacePlayerRDK.cpp

+		else
+		{
+			MW_LOG_WARN("Invalid streamId for rialto sink static pipeline");
+			return -1;
+		}


Missing braces for single-statement else block. At lines 2776-2780, the else block contains only one statement but the surrounding context uses braces for single-statement blocks. This is inconsistent with the coding guidelines (CodingGuidelineID: 1000003) which emphasize consistency in the codebase.

Copilot · 2026-02-02T08:16:35Z

priv_aamp.h

 #define AAMP_HIGH_BUFFER_BEFORE_RAMPUP_FOR_LLD	 4	/**< 4sec buffer before rampup for lld */
-#define TIMEOUT_FOR_LLD	3				/**< 3sec network timeout for lld */
+
+#define TIMEOUT_FOR_LLD	3


Comment removed from macro definition. The macro TIMEOUT_FOR_LLD previously had a comment /**< 3sec network timeout for lld */ which has been removed. According to the coding guidelines (CodingGuidelineID: 1000003), constants should be documented. Removing documentation makes the code less maintainable. The comment should be restored or replaced with proper documentation explaining the purpose and units of this timeout value.

Copilot · 2026-02-02T08:16:35Z

middleware/InterfacePlayerRDK.cpp

+static void GstPlayer_OnDemuxPadAddedCb_Static(GstElement* demux, GstPad* newPad, void* _this)
+{


Missing Doxygen documentation for the new static callback function. According to codebase conventions and custom coding guidelines (CodingGuidelineID: 1000003), all functions should have proper Doxygen-style documentation. The function GstPlayer_OnDemuxPadAddedCb_Static is a significant new addition handling the static pipeline pad-added callback and should be documented with proper parameter descriptions and purpose.

Copilot · 2026-02-02T08:16:36Z

middleware/InterfacePlayerRDK.cpp

+	GstCaps *caps = gst_pad_get_current_caps(newPad);
+	if (!caps)
+	{
+		MW_LOG_ERR("Failed to get caps from demux pad");
+		return;
+	}
+
+	gchar *capsStr = gst_caps_to_string(caps);
+	MW_LOG_WARN("Demux pad caps: %s", capsStr);
+	g_free(capsStr);
+
+	// Figure out if the caps is video or audio
+	GstStructure *s = gst_caps_get_structure(caps, 0);
+	const gchar *name = gst_structure_get_name(s);
+	const gchar *contentProtectionSystem = nullptr;
+
+	if (g_strcmp0(name, "application/x-cenc") == 0)
+	{
+		// Encrypted content - check original-media-type field
+		const gchar *originalMediaType = gst_structure_get_string(s, "original-media-type");
+		if (originalMediaType)
+		{
+			if (g_str_has_prefix(originalMediaType, "video/"))
+			{
+				mediaType = eGST_MEDIATYPE_VIDEO;
+			}
+			else if (g_str_has_prefix(originalMediaType, "audio/"))
+			{
+				mediaType = eGST_MEDIATYPE_AUDIO;
+			}
+		}
+
+		// Extract the actual protection-system UUID from content caps
+		contentProtectionSystem = gst_structure_get_string(s, "protection-system");
+		if (contentProtectionSystem)
+		{
+			MW_LOG_MIL("Content protection-system from caps: %s", contentProtectionSystem);
+		}
+	}
+	else
+	{
+		MW_LOG_WARN("Demux pad media type caps name: %s", name);
+	}
+
+	gst_caps_unref(caps);


Potential memory leak for GstCaps. At line 2012, gst_caps_unref(caps) is called unconditionally after using the caps. However, if an early return occurs between lines 1968-2011 (e.g., at lines 1972, 2018), the caps object obtained at line 1968 would not be unreferenced, causing a memory leak. Consider using a RAII wrapper or ensuring caps is unreferenced in all code paths.

Copilot · 2026-02-02T08:16:36Z

middleware/InterfacePlayerRDK.cpp

+	}
+
+	gchar *capsStr = gst_caps_to_string(caps);
+	MW_LOG_WARN("Demux pad caps: %s", capsStr);


Missing null check before using capsStr. At line 1976, gst_caps_to_string(caps) could potentially return NULL (though unlikely with valid caps), but the code logs it directly without checking. While GLib's g_free handles NULL gracefully at line 1977, the logging at 1976 could crash if capsStr is NULL. Consider adding a null check or using a safe logging pattern.

Suggested change

MW_LOG_WARN("Demux pad caps: %s", capsStr);

if (capsStr)

{

MW_LOG_WARN("Demux pad caps: %s", capsStr);

}

else

{

MW_LOG_WARN("Demux pad caps: <null>");

}

Vinish100 · 2026-02-02T11:11:24Z

middleware/InterfacePlayerRDK.cpp

+	MW_LOG_WARN("Successfully linked queue to decryptor via manual pad linking");
+
+	// Get the pre-created rialto sink (created in SetupStream)
+	GstElement* realSink = nullptr;


These can be a helper class to identify the sink

This statement is not clear. You mean need to create a helper class to identify the sink after sink gets created in SetupStream function?

Vinish100 · 2026-02-02T11:12:18Z

middleware/InterfacePlayerRDK.cpp

+
+	if (!queueSrcPad || !decryptorSinkPad)
+	{
+		MW_LOG_ERR("Failed to get pads for manual linking");


See, if some of these codes could be brought into a generic function for linking 2 elements. We can avoid code duplication

Vinish100 · 2026-02-02T11:16:49Z

middleware/InterfacePlayerRDK.cpp

 	}
 	return rtn;
 }
+static void GstPlayer_OnDemuxPadAddedCb_Static(GstElement* demux, GstPad* newPad, void* _this)


As a optimization, would recommend moving this function implementation into a helper function in InterfacePlayerRDK, so that we can add some unit tests for same

Copilot

Pull request overview

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

Copilot · 2026-02-09T06:59:44Z

middleware/InterfacePlayerRDK.cpp

+	}
+	else
+	{
+		MW_LOG_WARN("Demux pad media type caps name: %s", name);


In the non-encrypted caps case, mediaType is never set (it remains eGST_MEDIATYPE_DEFAULT) and the function returns, so clear (non-CENC) streams won’t be linked in the static pipeline. Determine mediaType from the caps name (e.g., video/, audio/) when name is not application/x-cenc.

Suggested change

MW_LOG_WARN("Demux pad media type caps name: %s", name);

// Non-encrypted (non-CENC) caps: derive mediaType from caps name, e.g. video/* or audio/*

if (name != nullptr)

{

if (g_str_has_prefix(name, "video/"))

{

mediaType = eGST_MEDIATYPE_VIDEO;

}

else if (g_str_has_prefix(name, "audio/"))

{

mediaType = eGST_MEDIATYPE_AUDIO;

}

}

MW_LOG_WARN("Demux pad media type caps name: %s", name ? name : "unknown");

Copilot · 2026-02-09T06:59:44Z

middleware/InterfacePlayerRDK.cpp

+	switch (mediaType)
+	{
+	case eGST_MEDIATYPE_VIDEO:
+		return "video_sink"; // or the actual name used in your pipeline
+	case eGST_MEDIATYPE_AUDIO:
+		return "audio_sink"; // or the actual name used in your pipeline
+	default:
+		return nullptr;
+	}


These hardcoded sink names ("video_sink"/"audio_sink") need to match the actual GstElement names in the bin. Currently the sinks are created with gst_element_factory_make(..., NULL), so they won’t have these names and name lookup will fail. Either create the sinks with deterministic names or avoid name lookup and use stored pointers.

Suggested change

switch (mediaType)

{

case eGST_MEDIATYPE_VIDEO:

return "video_sink"; // or the actual name used in your pipeline

case eGST_MEDIATYPE_AUDIO:

return "audio_sink"; // or the actual name used in your pipeline

default:

return nullptr;

}

// NOTE:

// The sink elements are currently created with gst_element_factory_make(..., NULL)

// and therefore do not have deterministic names. To avoid returning names that do

// not actually exist in the bin, this helper returns nullptr for all media types.

// Callers must use stored GstElement* pointers instead of name-based lookup.

(void)mediaType;

return nullptr;

Copilot · 2026-02-09T06:59:45Z

middleware/InterfacePlayerRDK.cpp

+		else
+		{
+			MW_LOG_WARN("Invalid streamId for rialto sink static pipeline");
+			return -1;
+		}


The else block here is currently paired with if (usingRialtoSink && useStaticPipeline) (because there’s no closing brace before it). That makes SetupStream return -1 when static pipeline is disabled (or usingRialtoSink is false), preventing the intended playbin fallback for normal playback. Add the missing brace and make this else apply only to invalid streamId values (i.e., the audio/video branch).

Copilot · 2026-02-09T06:59:45Z

priv_aamp.cpp

 			snprintf(tuneStrPrefix, sizeof(tuneStrPrefix), "%s PLAYER[%d]", (mbPlayEnabled?STRFGPLAYER:STRBGPLAYER), mPlayerId);
 		}
-		AAMPLOG_MIL("%s aamp_tune: attempt: %d format: %s URL: %s", tuneStrPrefix, mTuneAttempts, mMediaFormatName[mMediaFormat], mManifestUrl.c_str());
+		AAMPLOG_MIL("%s RR--->> aamp_tune: attempt: %d format: %s URL: %s", tuneStrPrefix, mTuneAttempts, mMediaFormatName[mMediaFormat], mManifestUrl.c_str());


This log line adds a non-descriptive "RR--->>" marker, which makes tune logs noisy and harder to grep/parse. Please remove the marker or replace it with a meaningful, consistent tag (or rely on log level/category instead).

Suggested change

AAMPLOG_MIL("%s RR--->> aamp_tune: attempt: %d format: %s URL: %s", tuneStrPrefix, mTuneAttempts, mMediaFormatName[mMediaFormat], mManifestUrl.c_str());

AAMPLOG_MIL("%s aamp_tune: attempt: %d format: %s URL: %s", tuneStrPrefix, mTuneAttempts, mMediaFormatName[mMediaFormat], mManifestUrl.c_str());

Copilot · 2026-02-09T06:59:45Z

AampConfig.cpp

 	{false, "seamlessAudioSwitch", eAAMPConfig_SeamlessAudioSwitch, true},
-	{false, "useRialtoSink", eAAMPConfig_useRialtoSink, false},
+	{true, "useRialtoSink", eAAMPConfig_useRialtoSink, false},
 	{false, "localTSBEnabled", eAAMPConfig_LocalTSBEnabled, true},


Changing the default of useRialtoSink from false to true is a major behavioral change (affects pipeline selection by default). Please confirm this is intentional for all platforms/builds; if it’s feature-gated, consider keeping the default false and enabling it via config/platform condition to avoid unexpected regressions.

Copilot · 2026-02-09T06:59:45Z

middleware/InterfacePlayerRDK.h

 * If not stated otherwise in this file or this component's license file the
 * following copyright and licenses apply:
- *
+	void HandleDemuxPadAddedStatic(GstElement* demux, GstPad* newPad, void* context);


There is a stray function declaration inside the Apache license comment block. Since it’s inside /* ... */ it doesn’t actually declare anything, and it also corrupts the standard copyright header text. Please remove it (or move it below the comment if an actual declaration is needed).

Suggested change

void HandleDemuxPadAddedStatic(GstElement* demux, GstPad* newPad, void* context);

Copilot · 2026-02-09T06:59:46Z

middleware/InterfacePlayerRDK.h

+	static GstElement* GetSinkByName(GstElement* bin, const char* sinkName)
+	{
+		if (!bin || !sinkName) return nullptr;
+		return gst_bin_get_by_name(GST_BIN(bin), sinkName);
+	}


SinkIdentifier::GetSinkByName returns a referenced GstElement (gst_bin_get_by_name adds a ref). Call sites must gst_object_unref() the returned element when done, otherwise this will leak. Consider documenting the ownership contract or returning a scoped/RAII wrapper instead of a raw pointer.

Copilot · 2026-02-09T06:59:46Z

middleware/InterfacePlayerRDK.cpp

+		MW_LOG_WARN("Demux pad media type caps name: %s", name);
+	}
+
+	gst_caps_unref(caps);
+


gst_structure_get_string() returns a pointer owned by the caps/structure. Since caps is unref’d here, any previously captured pointers from s (e.g., protection-system) can become invalid but are used later in the function. Copy any needed strings before gst_caps_unref(caps) or keep caps alive until you’re done using them.

Copilot · 2026-02-09T06:59:46Z

middleware/InterfacePlayerRDK.cpp

+	const char* sinkName = GetSinkNameForMediaType(mediaType);
+	if (sinkName)
+	{
+		realSink = SinkIdentifier::GetSinkByName(stream->sinkbin, sinkName);
+		MW_LOG_WARN("Using pre-created sink '%s': %p", sinkName, realSink);


realSink is looked up by name from stream->sinkbin, but the sink elements are created without explicit names in SetupStream, so this is likely to return nullptr and abort static pipeline linking. Fix by naming the sink elements at creation time (second arg to gst_element_factory_make) or by using gstPrivateContext->audio_sink/video_sink directly; also remember gst_bin_get_by_name returns a ref that must be unref’d when done.

Copilot · 2026-02-09T06:59:46Z

middleware/InterfacePlayerRDK.cpp

+		gst_element_set_state(realSink, GST_STATE_NULL);
+		gst_bin_remove(GST_BIN(stream->sinkbin), realSink);


On link failure you remove realSink from the bin, but the pre-created sink pointer is also stored in gstPrivateContext (audio_sink/video_sink). That can leave the stored pointer out of sync with the pipeline contents. If you remove the sink, also clear/update the stored pointer (or avoid removing the pre-created sink here).

Suggested change

gst_element_set_state(realSink, GST_STATE_NULL);

gst_bin_remove(GST_BIN(stream->sinkbin), realSink);

// Do not remove the pre-created sink from the bin here, as its pointer

// is also stored in gstPrivateContext (audio_sink/video_sink). Removing

// it locally would leave that stored pointer out of sync with the

// actual pipeline contents. Higher-level teardown logic is responsible

// for managing the lifetime of the pre-created sink element.

Reason for change: To avoid video looping with mid chunk failures, we are implementing an early abort logic if the first chunk was received considerably late. On abort, AAMP will rampdown to a lower profile. If the download still times out, AAMP will skip to next fragment Test Procedure: Throttle network during LLD playback and verify no video looping is observed Risks: Low Signed-off-by: Vinish100 <[email protected]>

Copilot

Pull request overview

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

Copilot · 2026-02-10T08:28:15Z

middleware/InterfacePlayerRDK.h

 		void SetStreamCaps(GstMediaType type, MediaCodecInfo&& codecInfo);
+		GstElement* GetPreCreatedRialtoSink(GstMediaType mediaType);



GetPreCreatedRialtoSink(GstMediaType) is declared here but there is no definition in the repository, which will cause a linker error if it’s referenced. Either provide an implementation (and add it to the build) or remove the declaration if it’s not needed.

Copilot · 2026-02-10T08:28:15Z

middleware/InterfacePlayerRDK.cpp

+static void HandleDemuxPadAddedStatic(GstElement* demux, GstPad* newPad, void* _this)
+{
+	MW_LOG_WARN("RR New-->> Entering static pipeline functionality");
+//	InterfacePlayerRDK* pInterfacePlayerRDK = (InterfacePlayerRDK *)_this;


The new static demux pad-added path logs with "RR..." prefixes. These look like temporary debug markers and will add noise to production logs; please remove them or gate them behind an existing debug/trace logging flag.

Copilot · 2026-02-10T08:28:15Z

middleware/InterfacePlayerRDK.cpp

+			gst_object_unref(queue);
+			queue = nullptr;
+		}
+		if (decryptor)
+		{
+			gst_element_set_state(decryptor, GST_STATE_NULL);
+			gst_bin_remove(GST_BIN(stream->sinkbin), decryptor);
+			gst_object_unref(decryptor);


Potential double-unref: after gst_bin_remove(...) the element is already unreffed by the bin. The additional gst_object_unref(queue/decryptor) can lead to use-after-free/crashes. Remove the extra unref calls (or only unref if you have explicitly taken an extra reference).

Suggested change

gst_object_unref(queue);

queue = nullptr;

}

if (decryptor)

{

gst_element_set_state(decryptor, GST_STATE_NULL);

gst_bin_remove(GST_BIN(stream->sinkbin), decryptor);

gst_object_unref(decryptor);

queue = nullptr;

}

if (decryptor)

{

gst_element_set_state(decryptor, GST_STATE_NULL);

gst_bin_remove(GST_BIN(stream->sinkbin), decryptor);

Copilot · 2026-02-10T08:28:16Z

middleware/InterfacePlayerRDK.cpp

 int InterfacePlayerRDK::SetupStream(int streamId,  void *playerInstance, std::string manifest)
+/**
+ * @brief Set up the GStreamer pipeline for a given stream type.
+ *
+ * For subtitle streams using the Rialto static pipeline, this function sets the appsrc caps to application/mp4.
+ * This is safe because the subtitle demuxing and parsing logic, as well as downstream elements (such as
+ * subtecmp4transform and rialtomsesubtitlesink), are designed to expect and handle MP4-based subtitle data.
+ * This explicit caps setting replaces the need for typefind and ensures correct pipeline negotiation.
+ * If support for additional subtitle formats is added in the future, a format check should be introduced here
+ * before setting caps to application/mp4.
+ *
+ * @param[in] streamId        The media stream type (e.g., video, audio, subtitle).
+ * @param[in] playerInstance  Pointer to the player instance.
+ * @param[in] manifest        The manifest URL or data.
+ * @return 0 on success, negative value on failure.
+ */
 {


The Doxygen-style function comment is placed after the function signature rather than before it, which can prevent documentation tools from associating it with SetupStream. Move the comment block above the function definition.

Copilot · 2026-02-10T08:28:16Z

middleware/InterfacePlayerRDK.cpp

+					MW_LOG_INFO("subs using rialto subtitle sink in static pipeline");
+
+					// Create appsrc
+					stream->source = GST_ELEMENT(gst_object_ref_sink(InterfacePlayerRDK_GetAppSrc(pInterfacePlayerRDK, eGST_MEDIATYPE_SUBTITLE)));


In the static subtitle-pipeline path, InterfacePlayerRDK_GetAppSrc(...) is immediately passed into gst_object_ref_sink(...) without checking for NULL. If appsrc creation fails this will crash; add a NULL check before ref-sinking and proceeding.

Suggested change

stream->source = GST_ELEMENT(gst_object_ref_sink(InterfacePlayerRDK_GetAppSrc(pInterfacePlayerRDK, eGST_MEDIATYPE_SUBTITLE)));

GstElement* appSrc = InterfacePlayerRDK_GetAppSrc(pInterfacePlayerRDK, eGST_MEDIATYPE_SUBTITLE);

if (!appSrc)

{

MW_LOG_ERROR("Failed to create subtitle appsrc for static pipeline");

return;

}

stream->source = GST_ELEMENT(gst_object_ref_sink(appSrc));

Copilot · 2026-02-10T08:28:16Z

priv_aamp.cpp

 			snprintf(tuneStrPrefix, sizeof(tuneStrPrefix), "%s PLAYER[%d]", (mbPlayEnabled?STRFGPLAYER:STRBGPLAYER), mPlayerId);
 		}
-		AAMPLOG_MIL("%s aamp_tune: attempt: %d format: %s URL: %s", tuneStrPrefix, mTuneAttempts, mMediaFormatName[mMediaFormat], mManifestUrl.c_str());
+		AAMPLOG_MIL("%s RR--->> aamp_tune: attempt: %d format: %s URL: %s", tuneStrPrefix, mTuneAttempts, mMediaFormatName[mMediaFormat], mManifestUrl.c_str());


The tune log message includes "RR--->>" which appears to be a temporary debug marker. Please remove it (or make it conditional) to keep logs stable and searchable.

Copilot · 2026-02-10T08:28:17Z

middleware/InterfacePlayerRDK.h

 /*
 * If not stated otherwise in this file or this component's license file the
 * following copyright and licenses apply:
- *
+	void HandleDemuxPadAddedStatic(GstElement* demux, GstPad* newPad, void* context);
 * Copyright 2024 RDK Management


A function declaration appears to have been accidentally inserted into the file’s license header comment. This should be removed (and, if a forward declaration is needed, placed outside the block comment in an appropriate location).

Copilot · 2026-02-10T08:28:17Z

middleware/InterfacePlayerRDK.cpp

+const char* GetSinkNameForMediaType(GstMediaType mediaType)
+{
+	switch (mediaType)
+	{
+	case eGST_MEDIATYPE_VIDEO:
+		return "video_sink"; // or the actual name used in your pipeline
+	case eGST_MEDIATYPE_AUDIO:
+		return "audio_sink"; // or the actual name used in your pipeline


GetSinkNameForMediaType() is defined at global scope without static/anonymous namespace, which unnecessarily exports the symbol from this TU. Make it static (or wrap in an anonymous namespace) to avoid potential symbol collisions.

Copilot · 2026-02-10T08:28:17Z

middleware/InterfacePlayerRDK.cpp

+		if (interfacePlayerPriv->gstPrivateContext->usingRialtoSink &&
+			m_gstConfigParam->useStaticPipeline)
+		{
+			if (eGST_MEDIATYPE_AUDIO == streamId)
+			{
+				MW_LOG_INFO("audio using rialtomseaudiosink in static pipeline format=%d", stream->format);
+


New static-pipeline setup and DRM plugin-selection logic is significant behavior change but no unit tests were added/updated to cover it (e.g., static ISO_BMFF audio/video setup, pad-added linking path, decryptor selection by UUID, cleanup paths). Please add tests under middleware/test/utests/tests to exercise these new branches.

Reason for change: Use static GST pipeline when using RialtoSinks for faster pipeline creation time Test Procedure: All Linear/VOD playback should work fine Risks: Medium Signed-off-by: Reshma-JO07 <[email protected]>

Reshma-JO07 requested a review from a team as a code owner February 2, 2026 07:58

Copilot AI review requested due to automatic review settings February 2, 2026 07:58

Copilot started reviewing on behalf of Reshma-JO07 February 2, 2026 07:58 View session

Copilot AI reviewed Feb 2, 2026

View reviewed changes

Reshma-JO07 force-pushed the feature/VPLAY-12344 branch from 648cd05 to dce4b86 Compare February 2, 2026 08:07

Copilot AI review requested due to automatic review settings February 2, 2026 08:09

Reshma-JO07 force-pushed the feature/VPLAY-12344 branch from dce4b86 to 50a170e Compare February 2, 2026 08:09

Copilot started reviewing on behalf of Reshma-JO07 February 2, 2026 08:09 View session

Copilot AI reviewed Feb 2, 2026

View reviewed changes

Vinish100 reviewed Feb 2, 2026

View reviewed changes

Reshma-JO07 force-pushed the feature/VPLAY-12344 branch from 50a170e to 8c46ca7 Compare February 9, 2026 06:14

Copilot AI review requested due to automatic review settings February 9, 2026 06:51

Reshma-JO07 force-pushed the feature/VPLAY-12344 branch from 8c46ca7 to 238b14c Compare February 9, 2026 06:51

Copilot started reviewing on behalf of Reshma-JO07 February 9, 2026 06:51 View session

Copilot AI reviewed Feb 9, 2026

View reviewed changes

Reshma-JO07 force-pushed the feature/VPLAY-12344 branch from 238b14c to 080cd60 Compare February 9, 2026 12:02

Copilot AI review requested due to automatic review settings February 10, 2026 08:20

Reshma-JO07 force-pushed the feature/VPLAY-12344 branch from 080cd60 to cf5860e Compare February 10, 2026 08:20

Copilot started reviewing on behalf of Reshma-JO07 February 10, 2026 08:21 View session

Copilot AI reviewed Feb 10, 2026

View reviewed changes

VPLAY-12344: Use static GST pipeline for RialtoSinks

9b314e6

Reason for change: Use static GST pipeline when using RialtoSinks for faster pipeline creation time Test Procedure: All Linear/VOD playback should work fine Risks: Medium Signed-off-by: Reshma-JO07 <[email protected]>

Reshma-JO07 force-pushed the feature/VPLAY-12344 branch from cf5860e to 9b314e6 Compare February 17, 2026 09:48

	{true, "useRialtoSink", eAAMPConfig_useRialtoSink, false},
	{false, "useRialtoSink", eAAMPConfig_useRialtoSink, false},

		// Mp4Demux is skipped for subtitles - use ISO BMFF format for qtdemux
		subtitleOutputFormat = FORMAT_ISO_BMFF;

	{
	{
	MW_LOG_WARN("GetDecryptorPluginName called with null DRM system ID, defaulting to Widevine for backward compatibility");

		static void GstPlayer_OnDemuxPadAddedCb_Static(GstElement* demux, GstPad* newPad, void* _this)
		{

-		MW_LOG_WARN("Demux pad media type caps name: %s", name);
+		// Non-encrypted (non-CENC) caps: derive mediaType from caps name, e.g. video/* or audio/*
+		if (name != nullptr)
+		{
+			if (g_str_has_prefix(name, "video/"))
+			{
+				mediaType = eGST_MEDIATYPE_VIDEO;
+			}
+			else if (g_str_has_prefix(name, "audio/"))
+			{
+				mediaType = eGST_MEDIATYPE_AUDIO;
+			}
+		}
+		MW_LOG_WARN("Demux pad media type caps name: %s", name ? name : "unknown");

	AAMPLOG_MIL("%s RR--->> aamp_tune: attempt: %d format: %s URL: %s", tuneStrPrefix, mTuneAttempts, mMediaFormatName[mMediaFormat], mManifestUrl.c_str());
	AAMPLOG_MIL("%s aamp_tune: attempt: %d format: %s URL: %s", tuneStrPrefix, mTuneAttempts, mMediaFormatName[mMediaFormat], mManifestUrl.c_str());

		gst_element_set_state(realSink, GST_STATE_NULL);
		gst_bin_remove(GST_BIN(stream->sinkbin), realSink);

-		gst_element_set_state(realSink, GST_STATE_NULL);
-		gst_bin_remove(GST_BIN(stream->sinkbin), realSink);
+		// Do not remove the pre-created sink from the bin here, as its pointer
+		// is also stored in gstPrivateContext (audio_sink/video_sink). Removing
+		// it locally would leave that stored pointer out of sync with the
+		// actual pipeline contents. Higher-level teardown logic is responsible
+		// for managing the lifetime of the pre-created sink element.

		void SetStreamCaps(GstMediaType type, MediaCodecInfo&& codecInfo);
		GstElement* GetPreCreatedRialtoSink(GstMediaType mediaType);

-					stream->source = GST_ELEMENT(gst_object_ref_sink(InterfacePlayerRDK_GetAppSrc(pInterfacePlayerRDK, eGST_MEDIATYPE_SUBTITLE)));
+					GstElement* appSrc = InterfacePlayerRDK_GetAppSrc(pInterfacePlayerRDK, eGST_MEDIATYPE_SUBTITLE);
+					if (!appSrc)
+					{
+						MW_LOG_ERROR("Failed to create subtitle appsrc for static pipeline");
+						return;
+					}
+					stream->source = GST_ELEMENT(gst_object_ref_sink(appSrc));

Conversation

Reshma-JO07 commented Feb 2, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 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 Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Vinish100 Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Reshma-JO07 Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

Reshma-JO07 Feb 5, 2026

Choose a reason for hiding this comment