Skip to content

Comments

Narrow down startAt.fromLivePosition option to only apply on live stream#1683

Open
KunXi-Fox wants to merge 3 commits intocanalplus:devfrom
steve-taylor:bugfix/narrow-down-from-live-position-option
Open

Narrow down startAt.fromLivePosition option to only apply on live stream#1683
KunXi-Fox wants to merge 3 commits intocanalplus:devfrom
steve-taylor:bugfix/narrow-down-from-live-position-option

Conversation

@KunXi-Fox
Copy link
Contributor

startAt.fromLivePosition makes users think of the options only suitable for live stream, but from reading the code, it's actually not, this PR is to make this option only works on live stream.

@peaBerberian
Copy link
Collaborator

It's difficult to know which behavior would an application expect when setting fromLivePosition on a VoD content.

So here, it is basically ignored, whereas previously, we interpreted it as "from the end of the content".

@Florent-Bouisset what do you think? I'm not sure.

@peaBerberian peaBerberian force-pushed the dev branch 2 times, most recently from 00fc806 to b7216b4 Compare April 15, 2025 18:14
@steve-taylor
Copy link
Contributor

It's difficult to know which behavior would an application expect when setting fromLivePosition on a VoD content.

So here, it is basically ignored, whereas previously, we interpreted it as "from the end of the content".

I'd expect fromLivePosition to be ignored for VOD content. It's not live, so there's no live position. It doesn't make sense.

One could argue that the documentation acknowledges that fromLivePosition has some effect when not live:

If the live edge is unknown or if it does not make sense for the current content (for example, it won't make sense for a VoD content), that setting repeats the same behavior than fromLastPosition.

and therefore this means the change in this PR should be considered breaking, requiring a major version bump. However, when I look at the actual code, this seems like a side effect of the implementation of fromLivePosition, specifically that getLivePosition(manifest) is nullish for non-live content.

Based on all the above, I support this change being merged and releasing it in the next patch release. Although it's technically breaking, I don't think any sane code would have been relying on the side effect of fromLivePosition in non-live content.

However, this PR should also include relevant changes to Loading_a_Content.md.

@peaBerberian
Copy link
Collaborator

peaBerberian commented Jun 2, 2025

Based on all the above, I support this change being merged and releasing it in the next patch release. Although it's technically breaking, I don't think any sane code would have been relying on the side effect of fromLivePosition in non-live content.

Though I do agree that some theoretical breaking changes are actually OK in very blurry areas like this one where an application would kind of be relying on implementation details in an edge scenario, I imagine that it's theoretically possible that an application may want to play a content with a delta from its end without e.g. knowing in advance if that live content just ended or not (and if it did, play the last seconds of that just-ended live stream).

Though there is also the more explicit fromLastPosition option available which may often be more adapted in that scenario (the differences between fromLivePosition and fromLastPosition are VERY specific but it exists, especially for TV channels with ad-insertion where the "last" might be further than the "live" when playing ad breaks).


Yet the reverse is also true, an application may want to always set that one (live and VOD) and based on the name only, they assume that it will only apply for live.

Which I assume is the case you were in?

I also understand this case. Even if an API documentation is technically more of a reference on behavior, I think that in this case, user expectations regarding it have as much weight.


This is why I'm legitimately hesitating on this one.
It depends on if it breaks things, or complexify things for applications, I guess.

Else the "last position for non-live contents" has the merit of being a little more powerful, at least as I see it.

@peaBerberian peaBerberian force-pushed the dev branch 4 times, most recently from 0142e34 to 1fd9df3 Compare January 27, 2026 11:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants