Skip to content

Added clarification on video content position / total length #993

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

Merged
merged 5 commits into from
Jul 14, 2020

Conversation

Isuru-F
Copy link
Contributor

@Isuru-F Isuru-F commented Jul 8, 2020

Proposed changes

Updating Video Spec Content vs Playback property descriptions on position and total_length to reflect the following

1 - Content * properties = Properties on the Asset (excluding Ads)
2 - Playback* properties = Properties on the Playback (Asset + ads)
3 - Ads* Properties = Properties on the Ad Asset.

If the above is true #1 has a bug in the docs that this PR will address.

Merge timing

  • ASAP once approved

Related issues (optional)

@Isuru-F Isuru-F requested a review from kdaswani July 8, 2020 05:27
@sanscontext
Copy link
Contributor

This looks good to me with a minor copy edit, but I'm not sure how to validate that this is true? Is there someone we can tag here to confirm?

@kdaswani
Copy link
Contributor

kdaswani commented Jul 8, 2020

@Isuru-F To confirm, this is only updating the position description under "Playback"? I see that for total_length we already say something similar about including the ad duration. In the "Video Playback Completed" example the final position equals the total_length so that validates this change in my opinion. Not sure if you have anyone else that could confirm this?

Co-authored-by: LRubin <sanscontext@users.noreply.github.com>
@sanscontext
Copy link
Contributor

Hey folks, I'll merge this EOD unless someone has an objection.

@sanscontext sanscontext merged commit de47416 into master Jul 14, 2020
@sanscontext sanscontext deleted the fix-video-spec-errors branch July 14, 2020 18:26
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