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

feat(live-status-gw): Add segment and part timing (SOFIE-2793) #1053

Merged
merged 16 commits into from
Nov 30, 2023

Conversation

ianshade
Copy link
Contributor

@ianshade ianshade commented Oct 17, 2023

This PR is being opened on behalf of TV 2 Norge.

  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
    A feature: new properties exposed by the Live Status Gateway in the segments and activePlaylist topics

  • What is the current behavior? (You can also link to an open issue here)
    No part or segment durations are available to the subscribers of the Live Status Gateway.

  • What is the new behavior (if this is a feature change)?
    New properties are added to segments in the segments topic:

    • timing.expectedDurationMs, calculated as a sum of expectedDurationWithPreroll of its parts (when available on at least one part)
    • timing.budgetDurationMs, calculated as a sum of budgetDuration of its parts (when available on at least one part)

    New properties added to currentPart in the activePlaylist topic:

    • timing.startTime - a timestamp of when the part is expected to start or has started (once its actual start time is reported by the Playout Gateway)
    • timing.expectedDurationMs - the expected duration
    • timing.expectedEndTime - a timestamp of when the part is expected to end, taking the expected duration and Display Duration Groups into consideration; no minimum-duration is currently applied

    New currentSegment in the activePlaylist topic with the following properties:

    • timing.expectedDurationMs - same as in the segments topic
    • timing.budgetDurationMs - same as in the segments topic
    • timing.expectedEndTime - a timestamp of when the segment is expected to end considering its budget (if available), and expected duration
  • Other information:

    • A simple sample client implementation is added that presents how to consume timing.
    • Also stabilizes the activePlaylist output by throttling (20ms) and preventing emitting incomplete data

Status

  • Code documentation for the relevant parts in the code have been added/updated by the PR author
  • The functionality has been tested by the PR author
  • Automated tests to cover the new functionality and/or guard against regressions have been added
  • The functionality has been tested by NRK

@codecov
Copy link

codecov bot commented Oct 17, 2023

Codecov Report

Attention: 62 lines in your changes are missing coverage. Please review.

Comparison is base (f79a6a5) 58.10% compared to head (178b6e3) 58.18%.
Report is 70 commits behind head on release50.

Files Patch % Lines
meteor/server/migration/1_50_0.ts 56.94% 31 Missing ⚠️
meteor/server/api/client.ts 64.51% 11 Missing ⚠️
meteor/lib/clientUserAction.ts 0.00% 4 Missing ⚠️
meteor/server/api/ExternalMessageQueue.ts 0.00% 4 Missing ⚠️
meteor/server/cronjobs.ts 96.52% 4 Missing ⚠️
...kages/server-core-integration/src/lib/ddpClient.ts 69.23% 4 Missing ⚠️
meteor/client/lib/clientAPI.ts 0.00% 2 Missing ⚠️
packages/server-core-integration/src/lib/ping.ts 66.66% 2 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##           release50    #1053      +/-   ##
=============================================
+ Coverage      58.10%   58.18%   +0.08%     
=============================================
  Files            479      479              
  Lines          78647    78730      +83     
  Branches        4120     4150      +30     
=============================================
+ Hits           45696    45812     +116     
+ Misses         32892    32860      -32     
+ Partials          59       58       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ianshade ianshade changed the title feat(live-status-gw): Add segment duration to segments topic feat(live-status-gw): Add segment and part timing Oct 30, 2023
plannedStartedPlayback as fallback
@ianshade ianshade marked this pull request as ready for review October 30, 2023 12:27
@Julusian Julusian requested a review from a team November 6, 2023 11:30
@nytamin nytamin requested a review from jstarpl November 20, 2023 10:19
@jstarpl
Copy link
Member

jstarpl commented Nov 20, 2023

Hello!

Thank you for contributing to the Sofie Project!

If you haven’t already, please give our contribution guidelines a read. Can you write me an e-mail at [email protected] so that we can schedule a workshop to discuss this change?

@jstarpl jstarpl changed the title feat(live-status-gw): Add segment and part timing feat(live-status-gw): Add segment and part timing (SOFIE-2793) Nov 20, 2023
@jstarpl
Copy link
Member

jstarpl commented Nov 21, 2023

An online workshop to discuss this PR has been scheduled for 2023/11/24 14:00 CET. If you'd like to participate in the workshop, please send an e-mail to [email protected] so that I can arrange that.

I will publish notes from the workshop in this thread afterwards.

@jesperstarkar jesperstarkar added the Contribution from TV 2 Norge Contributions sponsored by TV 2 Norge (tv2.no) label Nov 23, 2023
@jstarpl
Copy link
Member

jstarpl commented Nov 24, 2023

The meeting was held as planned; Mint de Wit and Jan Starzak from the NRK Sofie team and Krzysztof Zegzula representing TV 2 Norge participated.

  • The use-case is to build a custom UI to show countdowns to end-of-Part and end-of-Segment, matching what is displayed in the Sofie GUI
  • How should incomplete data be handled?
    • One option is to do the same simulation that Sofie Core GUI does (if a timestamp hasn’t been set yet, use current time).
    • Provide a “special” token (like null) to say that that computation can’t be done yet.
    • We agreed that having a “single” algorithm of fallback for all consumers means consistency at the price of potential greater accuracy, if additional fallback mechanisms are implemented. We agreed that consistency is more valuable.
  • Display Duration group handling has a TODO
    • There’s a difference when the 0-expectedDuration Part is before the non-zero expectedDuration Part inside
    • Core counts in a cascading fashion, top-to-bottom, while this implementation in Live Status Gateway does a calculation for currentPart only, so it counts all played and unplayed parts the same, no matter their positional relationship to the current Part. This may result in a difference in calculation
  • Other code comments from the PR have been discussed and will be addressed
  • Timestamp reliability - how should the API note that timestamps are always relative to the Sofie Server Core system clock (assuming synced over NTP)
    • Leaving this as an open question for now, the problem should be noted in the ReadMe for Live Status Gateway (client needs to be in time sync with Live Status Gateway)

@ianshade
Copy link
Contributor Author

Thank you, Jan and Mint, for your time and valuable insights during the meeting. I believe I have now addressed all comments from the review and brought up during the meeting. I also resolved the merge conflict.

@ianshade ianshade requested a review from jstarpl November 27, 2023 13:31
@jstarpl jstarpl merged commit 33de6dc into nrkno:release50 Nov 30, 2023
34 of 35 checks passed
@nytamin nytamin added the Contribution External contribution label Jun 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Contribution from TV 2 Norge Contributions sponsored by TV 2 Norge (tv2.no) Contribution External contribution
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants