Live stream jump

[oliv3r]

Description

To be able to jump forward (to the edge, e.g. the live moment) and jump backward (e.g. the start of the stream) on a live stream (when properly exposed/handled by the content provider/API, we need to cherry-pick commit b2da233 (Cleanups to chapter methods) from Piers which fixes the fact that we do not properly report the chapter count. Then add a commit to add a 'virtual' second chapter matching 'the end' which then gives us 2 chapters; ch1 'the start' and ch2 'now'. A third commit is to help with the start offset, like live_delay, we can add a bit of time to the start, where content providers love to add a conservative padding, to ensure they didn't miss the start of the program, or to make sure we get anything that was shown before the program (like an ad).

Motivation and context

Being able to 'jump to start' (using |< for example) or 'jump to live/now' using >| which relies on proper chapters to trigger.

How has this been tested?

Using retrospect and NLZIET we have livestreams from dutch television programs, moving this functionality to the |< and >| buttons allowed us to jump back/forward to the start/edge of a live stream reliably, with offset (where progress bar was showing a 'dot' of the offsetted jump point.

Types of change

• Bug fix (non-breaking change which fixes an issue)
• Clean up (non-breaking change which removes non-working, unmaintained functionality)
• Improvement (non-breaking change which improves existing functionality)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that will cause existing functionality to change)
• Cosmetic change (non-breaking change that doesn't touch code)
• None of the above (please explain below)

Checklist:

• I have read the Contributing document
• My code follows the Code Guidelines of this project
• My change requires a change to the Wiki documentation
• I have updated the documentation accordingly

[kodiai]

Kodiai Review Summary

What Changed

This PR adds live stream jump functionality with virtual chapters, allowing users to jump to the start or live edge of streams using chapter navigation (|< and >|).

Reviewed: core logic

Strengths

• ✅ Well-documented PR description explaining the motivation and implementation approach for live stream jumping
• ✅ Adds proper null-safety check in GetChapterCount() before accessing m_adaptiveTree
• ✅ Uses std::numeric_limits::max() for seeking to live edge, which is a clear and idiomatic approach

Observations

Impact

[MAJOR] src/Session.cpp (1563): GetChapterPos called with inconsistent chapter index causes incorrect seek position
When seeking to the start of the current period (line 1563), the code calls GetChapterPos(ch + 1) but ch has already been decremented at line 1532. This means if the user is on chapter 1 (which becomes ch=0 after decrement) and seeks to it, GetChapterPos receives 1, which would calculate the position of chapter 2 instead of chapter 1. This causes seeking to the wrong timestamp.

[MAJOR] src/Session.cpp (1478-1484): Missing null-check for m_adaptiveTree before dereferencing
The GetChapterPos method checks m_adaptiveTree at line 1478 for the virtual chapter case and at line 1483 for live offset, but doesn't validate m_adaptiveTree is non-null before the loop at line 1486 accesses m_adaptiveTree->m_periods[ch - 1]. If m_adaptiveTree is null and ch > 0, this causes a null pointer dereference.

[MEDIUM] src/Session.cpp (1534-1538): SeekTime with max double may cause undefined behavior on systems with different floating-point handling
While using std::numeric_limits<double>::max() is clear intent-wise, passing this extreme value to SeekTime (which handles it at line 1180 by checking seekTime < 0) may cause overflow or precision issues during intermediate calculations before the boundary check. Consider using a sentinel value like -1.0 or a dedicated parameter to indicate "seek to end."

Preference

[MINOR] src/main.cpp (503-512): Redundant check could be simplified
Optional: The GetChapterCount logic checks if count > 1 before returning. Since the method already handles the case where m_session is null by returning 0, the inner check could be simplified to directly return the count and let Kodi core handle single-chapter streams consistently.

Suggestions

• Optional: Add unit tests for the new virtual chapter logic, particularly edge cases like seeking when m_adaptiveTree is null or when there's only one period
• Future consideration: Document the chapter numbering convention (1-based vs 0-based) more clearly in comments, as the code decrements ch at the start of several methods which can be confusing

Verdict

🔴 Address before merging -- 2 blocking issue(s) found

Review Details

• Files reviewed: 5
• Lines changed: +65 -17
• Profile: strict (auto, lines changed: 82)
• Author: first-time (adapted tone)
• Findings: 0 critical, 2 major, 1 medium, 1 minor (includes 4 from summary observations)
• Review completed: 2026-03-03T21:37:24.572Z
• Keyword parsing:
  • focus hints: [SESSION], [KODIPROPS]
  • breaking change in body

[oliv3r]

Since i don't have Piers, I couldn't run that, but the patches are trivial and should be very easy to port, i can push to a branch if needed/open a second MR if needed.

[github-actions]

Review

Medium (Undefined Behavior Risk)

SeekTime(DBL_MAX) for live edge seek (src/Session.cpp:1537)

SeekChapter guards the call with m_adaptiveTree->IsLive(), but SeekTime only applies the live clamp inside a separate if (m_adaptiveTree->IsLive()) block. If the stream transitions from live to VOD between these two checks (during a manifest update), seekTime stays at DBL_MAX and static_cast<uint64_t>(seekTime * STREAM_TIME_BASE) is undefined behavior (out of range). Consider using the actual max available time rather than DBL_MAX as sentinel.

---

Low

live_offset applied to chapter 1 of all multi-period live streams (src/Session.cpp:1483)

GetChapterPos(1) returns GetLiveOffset() for any live stream. For multi-period live streams this offset applies to each period's chapter-1 position, which is incorrect - the offset only makes sense relative to the very first period. Seeking to chapter 1 of period 2 would jump to liveOffset seconds from the absolute stream start instead of period 2's start.

---

Minor

Hardcoded "Live" chapter name (src/Session.cpp:1467): Not i18n-aware. Minor if this surfaces in Kodi UI, though consistent with period IDs being returned as raw strings.

live_offset is uint64_t (unsigned) (src/CompKodiProps.h:167): No support for negative offsets (seeking slightly before nominal program start). The analogous live_delay is also uint64_t so this appears intentional, just noting the constraint.

[github-actions]

Using DBL_MAX as a sentinel for "seek to live edge" is fragile. SeekTime clamps this via the if (m_adaptiveTree->IsLive()) block, but if IsLive() returns false inside SeekTime (e.g. live→VOD transition during a manifest update), seekTime stays at DBL_MAX and static_cast<uint64_t>(seekTime * STREAM_TIME_BASE) is undefined behavior (overflow).

Suggested change

	SeekTime(std::numeric_limits<double>::max(), 0, false);
	// Seek to the maximum buffered position (live edge)
	uint64_t maxTime{0};
	for (auto& stream : m_streams)
	{
	if (stream->m_isEnabled)
	{
	uint64_t curTime = stream->m_adStream.getMaxTimeMs();
	if (curTime > maxTime)
	maxTime = curTime;
	}
	}
	SeekTime(static_cast<double>(maxTime) / 1000, 0, false);

[github-actions]

This returns liveOffset as the position for any live stream's chapter 1, including multi-period live streams. For period 2+, this is wrong — GetChapterPos(1) when the user is in period 2 should return 0 (start of that period), not the absolute stream-start offset. Consider guarding with a check that the stream has only one period, or only applying the offset when ch == 0 and m_adaptiveTree->m_periods.size() == 1.

[CastagnaIT]

First and foremost, as per docs/wiki always develops for the master branch and, if allowed in older branches. If for x reason you cannot do so, then do not even propose the PR on old branches (unless we are talking about backports, but each case is evaluated individually)

the new property live_offset its a clear workaround and i will not accept it

I won't even analyze the other changes because in the old branch i will only accept critical fixes that do not involve too many changes or dont require deeper testing that could potentially break other use cases

EDIT: In addition, given the complexity of live streams, you should provide free stream to test the use case. If this is not possible, at least attach one or more manifests to understand the dynamics of the stream

[oliv3r]

First and foremost, as per docs/wiki always develops for the master branch and, if allowed in older branches. If for x reason you cannot do so, then do not even propose the PR on old branches (unless we are talking about backports, but each case is evaluated individually)

the new property live_offset its a clear workaround and i will not accept it

I won't even analyze the other changes because in the old branch i will only accept critical fixes that do not involve too many changes or dont require deeper testing that could potentially break other use cases

harsh; but sure fair. i saw that Piers was the default branch, and as I only had omega available i wanted to at least start the discussion. So i'll re-do things on master no problem. But do explain how live_offset is just a hack? what would you propose? how is it different from live_delay?

@CastagnaIT much appreciated for your feedback so far, Thank you!

[CastagnaIT]

Internal methods of the player should not be exposed to the user as setting, unless it is a problem that cannot be solved at the moment

You have not attached any sample stream to reproduce the problem, nor manifests to inspect, exists many different manifest structures of live streams and i have no idea of your use case

If you want to discuss about your playback problem i invite you to open an Issue thread and provide the necessary info

[oliv3r]

For googlers finding this later; I've pushed a branch (doesn't show up here) to the same origin backporting it from piers.