VPLAY-13208 [VIPA] [Linear]- Progress Bar Point jumping to the End Wh… by anjali-syna · Pull Request #1363 · rdkcentral/aamp

anjali-syna · 2026-04-27T05:18:24Z

…ile performing Trick Play/Seek

Reason for change: Progress Bar Point jumping to the end and playing
from live play position while performing Trick Play/Seek in Linear channels.
This issue is seen only in one platform. In the problematic scenario,
aamp is receiving many underflows callbacks - GstPlayer_OnGstBufferUnderflowCb,
from rialtomsevideosink0, due to which EOS flow is triggered from
VideoDecoderPtsCheckerForEOS, as 'video-pts' property is not supported in
RialtoMSEVideoSink.

Changes:

Modifying aamp's underflow EOS implementation to fallback to gst_element_query_position, if 'video-pts' property is not supported.Both VideoDecoderPtsCheckerForEOS and GstPlayer_OnGstBufferUnderflowCb compare real playback positions in the absence of video-pts property, so a mid-underflow position change prevents NotifyEOS() from firing. Also, modified the code to allow pts check for EOS only during normal play, and not for forward trick play.

Test Procedure: Refer JIRA ticket

Risks: low

…ile performing Trick Play/Seek Reason for change: Progress Bar Point jumping to the end and playing from live play position while performing Trick Play/Seek in Linear channels. This issue is seen only in one platform. In the problematic scenario, aamp is receiving many underflows callbacks - GstPlayer_OnGstBufferUnderflowCb, from rialtomsevideosink0, due to which EOS flow is triggered from VideoDecoderPtsCheckerForEOS, as 'video-pts' property is not supported in RialtoMSEVideoSink. Changes: * Modifying aamp's underflow EOS implementation to fallback to gst_element_query_position, if 'video-pts' property is not supported. This stops the trick-play false-EOS, both VideoDecoderPtsCheckerForEOS and GstPlayer_OnGstBufferUnderflowCb compare real playback positions in the absence of video-pts property, so a mid-underflow position change prevents NotifyEOS() from firing. Test Procedure: Refer JIRA ticket Risks: low

Signed-off-by: anjali-syna <206662904+anjali-syna@users.noreply.github.com>

DomSyna · 2026-04-30T10:10:04Z

 *
 * @return Video PTS in nanoseconds, or -1 on error.
 */
 long long SocInterface::GetVideoPts(GstElement *video_sink, GstElement *video_dec, bool isWesteros)


Did you consider std::optional for the method instead or returning -1 ? it would avoid the need of the magic -1

Might be too bigger change in the various call sites

DomSyna · 2026-04-30T10:14:51Z

+	 * Once probed against a non-NULL video sink/decoder, the result is
+	 * reused for the lifetime of this SocInterface instance to avoid
+	 * repeated g_object_class_find_property() calls on hot paths. */
+	bool mVideoPtsPropertyChecked = false;


Do these need to be protected for thread safety ? Is it possible that the GST signal handling thread, and the AAMP player thread could access the method at same time ?

maybe:
std::once_flag mVideoPtsPropertyChecked
std::atomic mVideoPtsPropertySupported

DomSyna · 2026-04-30T10:18:36Z

+ */
+bool SocInterface::IsVideoPtsPropertySupported(GstElement *element)
+{
+	if(!mVideoPtsPropertyChecked)


with std::once_flag you could remove this check, and inside the scope of element
std::call_once(mVideoPtsPropertyChecked, lambda fn to find the property, set the supported flag, and log

DomSyna · 2026-04-30T10:24:41Z

 	gint64 currentPTS = 0;
 	if(video_dec)
 	{
+		if(!IsVideoPtsPropertySupported(video_dec))


This sequence is duplicated in both cpp files
If(!IsVideo..
g_object_get()

Is it worth moving into a protected helper in SicInterface.h
std::optional ReadVideoPts(GstElement *element); // null when no PTS

DomSyna · 2026-04-30T10:27:06Z

+		 * Fall back to gst_element_query_position and convert ms to 90 kHz
+		 * ticks (1 ms = 90 ticks) so that callers relying on PTS units
+		 * (90 kHz) keep working. */
+		MW_LOG_INFO("InterfacePlayerRDK - 'video-pts' property is not supported on this platform; Fall back to gst_element_query_position");


Could this flood the logs on a platform where video-pts isn't supported ?

Modified it as TRACE.

DomSyna · 2026-04-30T10:29:00Z

+		MW_LOG_INFO("InterfacePlayerRDK - 'video-pts' property is not supported on this platform; Fall back to gst_element_query_position");
+		currentPTS = MPEG_PTS_TICKS_PER_MS * GetVideoPosition();
+	}
 	return (long long)currentPTS;


cpp not C
static_cast

existing code; modified now.

DomSyna · 2026-04-30T10:30:28Z

+	}
+
+	MW_LOG_INFO("InterfacePlayerRDK: position(ms) = %" G_GINT64_FORMAT , GST_TIME_AS_MSECONDS(position));
+	return (long long)(GST_TIME_AS_MSECONDS(position));


as above cpp not C

DomSyna · 2026-04-30T10:33:22Z

+ *        Used to convert a millisecond playback position into 90 kHz ticks
+ *        when the 'video-pts' property is not exposed by the platform.
+ */
+static constexpr long long MPEG_PTS_TICKS_PER_MS = 90LL;


I'd be surprised if there isn't already a define for this somewhere ?
Would
inline constexpr
be better ?

anshephe · 2026-04-30T08:33:50Z

+			/* The 'video-pts' property is not exposed on this platform.
+			 * Signal to the caller so it can fall back to
+			 * gst_element_query_position. */
+			return -1;


Can we avoid the early return, same with SocInterface

anshephe · 2026-04-30T08:37:14Z

+		if(!IsVideoPtsPropertySupported(video_dec))
+		{
+			/* The 'video-pts' property is not exposed on this platform.
+			 * Signal to the caller so it can fall back to


I'd remove this bit of the comment, as it assumes the caller's intention

DomSyna · 2026-04-30T10:37:30Z

+	 *
+	 * @return true if 'video-pts' is exposed on the element, false otherwise.
+	 */
+	virtual bool IsVideoPtsPropertySupported(GstElement *element);


Should this alsobe marked as nodiscard ?

jfagunde · 2026-04-30T10:59:24Z

 }
+bool SocInterface::IsVideoPtsPropertySupported(GstElement *element)
+{
+	if(!mVideoPtsPropertyChecked)


Do we need all this logic in the fake? The same comment applies to SocInterface::GetVideoPts() fake. Ideally the fakes would contain no logic at all, although there is already logic in the existing ones :(

anjali-syna added 11 commits April 27, 2026 10:46

Merge branch 'dev_sprint_25_2' into feature/VPLAY-13208

5f7fb82

Adding more logs

8dc9542

Adding more logs

e8a8089

Merge branch 'dev_sprint_25_2' into feature/VPLAY-13208

bb2aefd

Updated few files

3268c98

Logs updated

effc7c8

Merge branch 'dev_sprint_25_2' into feature/VPLAY-13208

f5afa26

Allow PTS check in underflow only for NORMAL_PLAY_RATE

a846b2d

Signed-off-by: anjali-syna <206662904+anjali-syna@users.noreply.github.com>

Merge branch 'dev_sprint_25_2' into feature/VPLAY-13208

39ac4a1

anjali-syna marked this pull request as ready for review April 30, 2026 04:32

anjali-syna requested a review from a team as a code owner April 30, 2026 04:32

anjali-syna requested review from anshephe, dnfaulkner and jfagunde April 30, 2026 04:33

DomSyna self-requested a review April 30, 2026 09:32

DomSyna reviewed Apr 30, 2026

View reviewed changes

anshephe reviewed Apr 30, 2026

View reviewed changes

DomSyna reviewed Apr 30, 2026

View reviewed changes

jfagunde reviewed Apr 30, 2026

View reviewed changes

Conversation

anjali-syna commented Apr 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DomSyna Apr 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

anjali-syna commented Apr 27, 2026 •

edited

Loading

DomSyna Apr 30, 2026 •

edited

Loading