-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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(Ads): Allow use a custom playhead tracker in CS #5543
feat(Ads): Allow use a custom playhead tracker in CS #5543
Conversation
Incremental code coverage: No instrumented code was changed. |
* | ||
* @description | ||
* Ads configuration. | ||
* | ||
* @property {boolean} customPlayheadTracker | ||
* If this is <code>true</code>, we create a custom playhead tracker for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be nice to have some details here on why/when you might want this. In the PR description, too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I have to admit I'm not sure what this has to do with having multiple video elements?
I tried the change out and it doesn't seem to result in the ad container having a different DOM structure than the regular method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The structure is the same, but the playhead tracker is used to get the position of the original video. If you can only have one video, if you delete it to leave the video to the ad, you stop having the position of the original video, so you can no longer track the position to launch the ads (vmap case)
Looks fine otherwise. |
With the following code implemented on the application side it is possible to support #2792 Using
The idea is to implement this within ShakaPlayer itself in a future release. |
Related to #2792
This is useful because it allows you to implement the use of IMA on platforms that do not support multiple video elements. See #5543 (comment)