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

BADGERS-177 Use UTCTiming for wall-clock time #299

Merged
merged 25 commits into from
Jun 19, 2023

Conversation

eirikbjornr
Copy link
Contributor

📺 What

Get wall-clock time from the <UTCTiming> element present in dynamic DASH manifests.

Ticket: BADGERS-177

If serverDate is passed into bigscreen-player on init that timing will be used to derive times from the first manifest only. If a new manifest is fetched, or the current manifest is refreshed, then wall-clock time will be derived from <UTCTiming>.

Note:

  • This is a PR to merge into branch badgers-177, not master. I've chunked the ticket into two PRs to make reviews easier.
  • Therefore, this ticket does not fix:
    • timeCorrectionSeconds not being updated on failover/refresh (closure bug).
    • MPD time being passed as availability time on a simulcast failover by using the POSIX time anchor, causing restart point to be at least 1 minute off.

🛠 How

Fetches and parses wall-clock time from the timing resource defined by the <UTCTiming> element on the manifest.

Along the way:

  • Promisified mediasources to fix tests. Fixed a bug that caused the manifest to be loaded and parsed twice if it failed over when loadManifest was called.
  • Fixed the DebugTool so it formats errors correctly
  • Added warn method to DebugTool to log a deprecation notice.

@eirikbjornr eirikbjornr added bug fix Fix a bug semver patch This PR is a semver patch release labels Jun 16, 2023
@eirikbjornr eirikbjornr requested a review from a team as a code owner June 16, 2023 16:15
@eirikbjornr eirikbjornr self-assigned this Jun 16, 2023
Copy link
Contributor

@ShiningTrapez ShiningTrapez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Just a comment on documentation, also can't help but note that the Promises in src/manifest/manifestloader.js are very similar, could these be collapsed into a function to be more DRY?

docs/tutorials/00-getting-started.md Show resolved Hide resolved
@eirikbjornr
Copy link
Contributor Author

[...] can't help but note that the Promises in src/manifest/manifestloader.js are very similar, could these be collapsed into a function to be more DRY?

I did think about that but was unsure whether to port our LoadUrlAsync wrapper to another place. But tbf I can introduce a simple wrapper in the manifestloader. TV module network rewrite when 😭

@eirikbjornr
Copy link
Contributor Author

eirikbjornr commented Jun 19, 2023

☝️@ShiningTrapez On this I looked over again and not worth collapsing IMO. A wrapper wouldn't help in terms of performance or readability because each function interacts with LoadUrl slightly differently. That variation would end up in the promise chain instead.

@eirikbjornr eirikbjornr merged commit 5d1203e into badgers-177 Jun 19, 2023
@eirikbjornr eirikbjornr deleted the badgers-177-failover branch June 19, 2023 14:14
@eirikbjornr eirikbjornr mentioned this pull request Jul 6, 2023
eirikbjornr added a commit that referenced this pull request Jul 6, 2023
* BADGERS-177 Use `UTCTiming` for wall-clock time (#299)

* wip: commit to test upgrading jest

* chore: upgrade jest to v29

* test: missing timing resource

* fix: hls tests

* feat: promisify manifest loader

* chore: remove redundant placeholders

* test: make MediaSources tests handle promisified manifest loader

* feat: return promise from loadManifest

* fix: triple load bug

* fix: don't provide wallclock time on failover or refresh

* feat: log error in dev client

* feat: let DebugTool take error objects

* feat: log manifest load failure

* chore: remove unused vars

* docs: reference source

* fix: handle load failures with no cause given

* feat: add warn method and fix error method

* chore: add deprecation note to server date

* doc: deprecate server date

* fix: move jsdom to dev dependencies

* refactor: async/await tests

* docs: expand documentation

* fix: boolean logic

* fix: export mutable

* fix: use absolute time to seek sliding window in a failover

* chore: remove comment

* BADGERS-177 Failover with MPD times in MSE strategy (#302)

* style: make magic numbers in tests variables

* refactor: optimise manifest regex

* test: url with query param
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug fix Fix a bug semver patch This PR is a semver patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants