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

Extend API #170

Merged
merged 8 commits into from
Mar 21, 2024
Merged

Extend API #170

merged 8 commits into from
Mar 21, 2024

Conversation

martinstark
Copy link
Collaborator

@martinstark martinstark commented Mar 8, 2024

Adds the following:

  • Ability to fetch the current timestamp including stitched ad durations when viewing VODs. CAUTION: this timestamp may refer to different relative positions on each video start, depending on length of stitched ads
  • Ability to fetch the duration including stitched ad durations when viewing VODs. CAUTION: this timestamp may refer to different relative positions on each video start, depending on length of stitched ads

to test, call:

player.getCurrentTime(TimeMode.AbsoluteTime);
player.getDuration(TimeMode.AbsoluteTime);

If the player is currently playing an ad, by default, the methods would return the position within the ad and the duration of the ad respectively. If the TimeMode.AbsoluteTime parameter is added, the methods will instead return the absolute time, ignoring ongoing ads.

Base automatically changed from fix-infinite-break-finished to develop March 13, 2024 07:28
Copy link
Member

@dweinber dweinber left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution. A few comments to address, and please add a CHANGELOG.md entry - thanks!

src/ts/BitmovinYospacePlayerAPI.ts Outdated Show resolved Hide resolved
src/ts/BitmovinYospacePlayerAPI.ts Outdated Show resolved Hide resolved
src/ts/BitmovinYospacePlayerAPI.ts Outdated Show resolved Hide resolved
@martinstark
Copy link
Collaborator Author

martinstark commented Mar 14, 2024

@dweinber changelog is included in #171 , if that is okay. It could be merged into this branch before merging to develop. The last 3 PRs were all part of the same body of work, but it was desired that PRs were created separately.

@dweinber
Copy link
Member

Thanks @martinstark, I've started a conversation around existing vs new APIs separately.

Re: Changelog:
I believe what's done in this PR should also be listed by this PR 🙂 (i.e. just the addition of the new APIs should be listed as part of this PR)

@martinstark
Copy link
Collaborator Author

Re: Changelog:
I believe what's done in this PR should also be listed by this PR 🙂 (i.e. just the addition of the new APIs should be listed as part of this PR)

Sure! Since the PRs are built on top of each other I'll have a small rebase/force-push party to get it sorted once we agree to the finalized API. I'll move the changelog to the appropriate PR when that happens.

@dweinber
Copy link
Member

Ah ok, missed that. I thought we'd merge this one, and then 171 would be just done against develop as well.

All good then, don't worry about the changelog.

@martinstark
Copy link
Collaborator Author

Since I'll likely need to make changes here I'll be doing the rebasing regardless, so I can make sure the changelog is in the correct place as part of that

@dweinber
Copy link
Member

Hey @martinstark,
As discussed offline, please apply my review comments.

@martinstark martinstark force-pushed the extend-api branch 2 times, most recently from 7c042d7 to 037ad18 Compare March 21, 2024 10:59
@martinstark
Copy link
Collaborator Author

martinstark commented Mar 21, 2024

@dweinber new commits added, pr description updated

Copy link
Member

@dweinber dweinber left a comment

Choose a reason for hiding this comment

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

Code looks good, thanks for the updates!

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
martinstark and others added 3 commits March 21, 2024 14:04
Co-authored-by: Daniel Weinberger <dweinber@users.noreply.github.com>
Co-authored-by: Daniel Weinberger <dweinber@users.noreply.github.com>
Co-authored-by: Daniel Weinberger <dweinber@users.noreply.github.com>
@dweinber dweinber merged commit 2c95f3c into develop Mar 21, 2024
2 checks passed
@dweinber dweinber deleted the extend-api branch March 21, 2024 13:09
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.

2 participants