Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Safari video playback, and fragment timing #2902

Merged
merged 2 commits into from Jul 20, 2020

Conversation

robwalch
Copy link
Collaborator

@robwalch robwalch commented Jul 18, 2020

This PR will...

  • Fix Safari playback by removing Safari workarounds in mp4-remuxer.
  • Fix remuxer (PTS based) time-offset nextAvcDts estimation to avoid false hole/overlap detection.
  • Fix forward fragment start overestimation by adding fragment.minEndPTS
  • Avoid removing overlapping media in the forward buffer when flushing future segments after setting nextLevel (prevents gaps when adapting fmp4 and the previous segment's media extends past the target segment's start in "HLS fMP4 by Bitmovin")
  • Use minimum segment PTS when calculating initPTS and TS AV time-offset in remuxer (since the first sample PTS is not always the fragment start)

These changes also help make fragment DTS and PTS updates more stable on adaptations, when reloading previously parsed fragments, and when simply forward buffering. The demo timeline was updated to help in this effort by outlining the current, and loading levels in different colors.

Why is this Pull Request needed?

Remuxed AVC video is not buffering at all in Safari without removing these workarounds. These workaround may have been for Safari 9 to avoid buffer gaps and decode errors, but I don't see any issues with most video in Safari 10.1 and up. Or not...

The PTS > DTS workaround did not work in Safari and was probably the reason for most if not all the Safari specific code. Safari has very little tolerance for composition time offsets that are not close to the video framerate (cts of 0 causes decoding glitches or dropped frames and frames with large cts are thrown out).

Estimating the start of the next fragment correctly is very important because we use frag.startPTS or frag.start as the time-offset passed to the remuxer which is used to establish "initPTS". It can determine where media is appended as well as if parsed timecode should be shifted. With these changes we get the next frag.start right most of the time, so fragment times are updated more accurately and don't change each time they are reloaded and parsed.

There's also a few other fixes:

  • Estimate

Resolves Issues:

#1801
#2122

Checklist

  • changes have been done against master branch, and PR does not conflict
  • new unit / functional tests have been added (whenever applicable)
  • API or design changes are documented in API.md

@robwalch robwalch added this to the 0.14.4 milestone Jul 18, 2020
@robwalch robwalch requested a review from itsjamie July 18, 2020 03:09
(Only this video buffers/playback back with issues https://playertest.longtailvideo.com/adaptive/captions/playlist.m3u8)
Safari 9 may have some issues with gaps and decode errors, but without removing this code video will not buffer at all
@robwalch robwalch force-pushed the bugfix/safari-avc-remux branch 9 times, most recently from 5aa4790 to ef4c86f Compare July 19, 2020 22:06
Improve PTS < DTS fix so that it works in Safari
Show current and loading level in timeline
@robwalch robwalch changed the title Fix Safari video playback Fix Safari video playback, and fragment timing Jul 20, 2020
@robwalch robwalch merged commit ef99bb3 into master Jul 20, 2020
@robwalch robwalch deleted the bugfix/safari-avc-remux branch July 20, 2020 03:37
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.

None yet

1 participant