SRGSSR / srgmediaplayer-apple

An advanced media player library, simple and reliable
MIT License
158 stars 33 forks source link

Incorrect seeks in DVR streams #54

Closed defagos closed 5 years ago

defagos commented 5 years ago

When seeking forwards or backwards, e.g. with the built-in SRGMediaPlayerViewController, seeks are erratic.

Issue type

Incorrect behavior

Description of the problem

Jumping forward or backward in a DVR stream does not work. The jumps are either too large or in the wrong direction.

Environment information

Reproducibility

Always reproducible

Steps to reproduce

  1. Open the SRG Media Player demo and play the Timeshift audio stream with the SRG media player.
  2. Seek to the past and wait a minute. Then use the +15 / -15 buttons to skip. The behavior is erratic.
defagos commented 5 years ago

The reason was that the start of a DVR time range was not always zero because of chunks being added. The time range normalization introduced in version 2.5 implementation was therefore incorrectly normalizing times.

To solve the issue, there was two possibilities:

  1. Fix the normalization.
  2. Fix time ranges so that their origin is always 0.

    Since the start position is ultimately not interesting, and for consistency with on-demand streams (which have always 0 as start position), I fixed the issue by ensuring that time ranges always start at 0.

defagos commented 5 years ago

The fix is ready for review on the feature/dvr-seek-issues branch.

defagos commented 5 years ago

After some testing this approach worked but was misleading. For a DVR stream, and as chunks are retrieved, the time range origin moves in time. Always fixing time ranges to start at 0 means that the current time must be fixed accordingly, so that it moves back in time. To understand this behavior, imagine that the player is paused. As new chunks are pumped in, the playback position must namely go back in time.

In my opinion this is misleading because if you associate a date with the current position (which AVPlayer supports), the date remains fixed. This is why I recommend letting the current position and time range as they are, which in addition provides consistent times with those which can be retrieved from the underlying AVPlayer, available as a readonly property. This way, when the player is paused, the current time does not change, but the range slides forward, eventually catching up with the current position.

To solve the problem, we must therefore be especially careful when building a seek position from the current position. In such cases, since all seek positions in SRGMediaPlayerController are relative to the time range, we must subtract to the targeted CMTime the time range start to obtain a relative position to which to seek to.

I enhanced the documentation accordingly and I fixed SRGMediaPlayerViewController skip implementation, but I suggest we stop there. This is the probably the easiest and most consistent approach to deal with such issues.

defagos commented 5 years ago

One last improvement: When seeking into a segment, positions must be relative. When seeking outside any segment, positions must not be relative to the content time range (whose origin might shift in time in the case of DVR streams). This avoids having to introduce a "relative vs. absolute" time concept, and is intuitive to understand.