Live stream jump piers

[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. Further more I used a test file as mentioned in #1517 and backported the changes to omega #2005 but with fixes in place as per review comments.

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

[oliv3r]

marking as draft, just to see the pipeline work, as i've also added tests.

[kodiai]

Kodiai Review Summary

What Changed

This PR adds support for jumping to live edge and start position in live streams by introducing a virtual second chapter, along with a new live_offset configuration parameter.

Reviewed: core logic, tests

Strengths

• ✅ Well-structured test coverage for the new live_offset and live_delay features with clear test cases
• ✅ Consistent parameter validation for live_offset (checks for unsigned number type before parsing)

Observations

Impact

[MAJOR] src/Session.cpp (1273): Unit mismatch in live offset conversion
m_liveOffset is stored in seconds (as documented and parsed in CompKodiProps.cpp), but GetChapterPos returns it directly without converting to milliseconds. The return type is int64_t representing milliseconds (consistent with line 1261 which divides m_totalTime by 1000). This causes the chapter position to be off by a factor of 1000 when live_offset is set.

[MAJOR] src/Session.cpp (1330): Potential seek to position zero when no streams enabled
If no streams are enabled in the SeekChapter method when jumping to live edge, maxTime remains 0, causing SeekTime(0, ...) to be called. This would incorrectly seek to the beginning instead of the live edge. Consider adding a check to verify at least one enabled stream exists before attempting the seek, or falling back to using m_totalTime as an alternative.

[MEDIUM] src/Session.cpp (1261): Missing validation for m_totalTime before division
When returning the virtual live chapter position, m_totalTime / 1000 is computed without verifying that m_totalTime is non-zero. While uncommon, if a live stream hasn't populated m_totalTime yet, this could return 0 for the live chapter position, making it identical to the start chapter position.

Preference

[MINOR] src/Session.cpp (1271-1273): Optional: Simplify conditional by extracting common checks
The condition number == 0 && m_adaptiveTree->IsLive() && m_adaptiveTree->m_periods.size() == 1 && m_adaptiveTree->m_liveOffset > 0 spans multiple concerns. Consider extracting the check into a helper method like ShouldApplyLiveOffset(int chapterIndex) to improve readability.

Suggestions

• Optional: Consider adding a test case that verifies the behavior when SeekChapter is called for the virtual live chapter with no enabled streams
• Future consideration: The TODO comment at line 1263-1267 mentions thread safety concerns with accessing m_periods asynchronously. The new virtual chapter logic also accesses m_periods.size() in multiple places without synchronization, which could be addressed in a follow-up PR

Verdict

🔴 Address before merging -- 2 blocking issues found

Review Details

• Files reviewed: 6
• Lines changed: +77 -9
• Profile: strict (auto, lines changed: 86)
• Author: first-time (adapted tone)
• Findings: 0 critical, 2 major, 1 medium, 1 minor (includes 4 from summary observations)
• Review completed: 2026-03-04T18:11:14.045Z
• Keyword parsing:
  • focus hints: [MANIFESTCONFIG], [SESSION], [TEST]
  • breaking change in body

[github-actions]

Review

Must Fix (1) Medium (2)

---

Must Fix - GetChapterName regression (src/Session.cpp:1250)

The refactor changed the non-debug fallback from nullptr to CHAPTER_NAME_UNKNOWN ("[Unknown]"). Previously callers (Kodi core) received nullptr for non-debug mode on all streams; now they receive "[Unknown]", which will surface as a visible chapter label for every non-live stream. The CHAPTER_NAME_UNKNOWN return was previously guarded inside the IsDebugVerbose() block intentionally.

Fix: restore return nullptr at the bottom and only return CHAPTER_NAME_UNKNOWN inside the IsDebugVerbose() guard.

---

Medium - Virtual live chapter position is 0 when m_totalTime == 0 (src/Session.cpp:1261)

MPEG-DASH live streams frequently omit mediaPresentationDuration (optional for live profiles), so m_totalTime stays 0. GetChapterPos for the virtual live chapter then returns 0, making it indistinguishable from the stream start. Meanwhile SeekChapter for the same chapter correctly uses getMaxTimeMs() to find the real live edge - the two are inconsistent. If Kodi core queries GetChapterPos before calling SeekChapter (e.g. for progress bar position), it will report 0 instead of the live edge.

---

Medium - Silent fall-through to SeekTime(0) in live-edge seek (src/Session.cpp:1330)

If no streams are enabled when seeking to the virtual live chapter, maxTime stays 0 and SeekTime(0.0, 0, false) seeks to the very beginning rather than failing gracefully. At minimum an early-return with a log error should guard this path.

[github-actions]

Review

Medium Code duplication (Session.cpp)
The loop to find the live-edge max time is copy-pasted verbatim in both GetChapterPos (lines 1264–1274) and SeekChapter (lines 1334–1344), including the same maxTime > 0 ? maxTime : m_totalTime fallback. Any future fix to one will need to be applied to the other. Extract to a GetLiveEdgeMs() private helper.

Low Missing unit in m_liveOffset comment (AdaptiveTree.h:73, CompKodiProps.h)
m_liveDelay is documented as "in seconds" but m_liveOffset (same unit, same usage pattern) is not. The field is used directly as seconds in GetChapterPos so the missing annotation could mislead future readers.

Low SeekChapter same-period else if silently changes VOD behavior (Session.cpp:1367)
Before this PR, requesting a chapter that maps to the current period returned false. The new else if branch now seeks instead, affecting all content types, not just live. For VOD multi-period streams this changes what Kodi considers a failed seek into a successful one. Confirm this side-effect is intentional, or guard it with m_adaptiveTree->IsLive().

[CastagnaIT]

now that i have had some time to read and understand it better
i will stop you right there before you continue wasting time
what you're doing is a hack, chapters must always match periods, and not generate fake/virtual chapters to use beyond their meaning

your proposal introduces complexities that lead to further complications in the maintenance of the add-on regards periods already difficult to maintain

managing periods is already difficult due to the multitude of different manifests that can not works well (e.g. ADS), and on HLS manifests the periods are not yet well managed

to note that already exists a property to allow start playback from start of TSB or his end
which unfortunately is not fully functional for all types of manifests due to the problems mentioned

we have more serious problems to solve in ISA before considering introducing strange things
so i cant accept this

IMO it would be better to have a shortcut key on Kodi to seek to the beginning or end of the stream, regardless of the chapters it would be a cleaner solution

[oliv3r]

now that i have had some time to read and understand it better i will stop you right there before you continue wasting time what you're doing is a hack, chapters must always match periods, and not generate fake/virtual chapters to use beyond their meaning

your proposal introduces complexities that lead to further complications in the maintenance of the add-on regards periods already difficult to maintain

managing periods is already difficult due to the multitude of different manifests that can not works well (e.g. ADS), and on HLS manifests the periods are not yet well managed

to note that already exists a property to allow start playback from start of TSB or his end which unfortunately is not fully functional for all types of manifests due to the problems mentioned

we have more serious problems to solve in ISA before considering introducing strange things so i cant accept this

that's sad of course, as the feature is quite useful from a UX point of view. I'll keep it for now in my local builds; but will look at:

to note that already exists a property to allow start playback from start of TSB or his end which unfortunately is not fully functional for all types of manifests due to the problems mentioned

to see if I could/should have used that instead and see if that can be fixed.

Having said that, there's 2 or 3 commits here worth looking at like:

• [AdaptiveStream] Make getMaxTimeMs() const
• [Test] Add tests for live_delay and live stream total time

IMO it would be better to have a shortcut key on Kodi to seek to the beginning or end of the stream, regardless of the chapters it would be a cleaner solution

I can see how that could also work, though i'd do it slightly differently. |< and >| are only available in the player if there is a playlist or a chapter. I think this could be extended to 'or if there's a livestream which is seekable'. that would stay clean of ISA; and would only be a kodi change, if they are willing to accept it of course.

[CastagnaIT]

Having said that, there's 2 or 3 commits here worth looking at like:

[AdaptiveStream] Make getMaxTimeMs() const

this cleanup makes sense but is not strictly needed, can be an idea to be done with other code cleanups

[Test] Add tests for live_delay and live stream total time

• about additional "live delay" test, yes can be an idea, but i don't think it's worth adding code that will probably be reworked, the underlying problem is that there is much broader problem that has to do with HLS implementation, where live delay is not set, there are much more problems such as TSB thats generated only with the child manifest download, that lead to problem in managing play_timeshift_buffer property and others side effects. All is related/connected and this requires many code changes that i will not discuss here, so adding this test before solving all these problems is not very useful
• about "total time" test, is more complex that this, mediaPresentationDuration should be present but is not so on all dash manifest, as for example specs says that can period duration can be alternative value, and we currently fallback to calculated timeline to have more precision. if you add new test case should cover at least the most common manifest cases. to note that this variable is also used by other types of manifests, if the purpose is to verify the functioning of m_totalTime, it would be a half-measure since are missing the other cases. Is it worth knowing that specific manifest you are testing returns this value and not the other ones? Because, as Dash is implemented, if you add all the use cases supported are much broader, that depends on how its generated or provided the segments timeline...so i don't know if it's worth it at this point

I can see how that could also work, though i'd do it slightly differently. |< and >| are only available in the player if there is a playlist or a chapter. I think this could be extended to 'or if there's a livestream which is seekable'. that would stay clean of ISA; and would only be a kodi change, if they are willing to accept it of course.

I think you should first open a feature request
https://forum.kodi.tv/forumdisplay.php?fid=9
and also ask for better alternatives to achieve what you want
doing what you ask on Kodi core will cover all playback uses, such as local files, PVR, InputStream, etc.... , not just ISA that is just a single InputStream addon