fix(Ads): Limit interstitial duration to actual duration if available #7480
Conversation
avelad
added
type: bug
avelad
force-pushed
the
interstitial-duration
branch
from
5ac4e55 to
c4431ad
Compare
|
Incremental code coverage: 75.00% |
| @@ -675,11 +675,25 @@ shaka.ads.InterstitialAdManager = class { | |||
| }); | |||
| try { | |||
| this.updatePlayerConfig_(); | |||
| // playRangeEnd in src= causes the ended event not to be fired when that | |||
There was a problem hiding this comment.
I don't understand this comment in context of the next paragraph of code. Could you improve the comments in this whole try/catch? Happy to have a conversation about it if it helps you figure out what is missing from the comments.
There was a problem hiding this comment.
For src= we use https://www.w3.org/TR/media-frags/ whose implementation in Safari implies that when the playRangeEnd time is reached, if the stream has not finished, what it does is pause the content, but does not emit the ended event.
There was a problem hiding this comment.
IIUC, you are saying that this trick of using play range to limit the interstitial will not trigger an inappropriate 'ended' event, so we can then go back to the main content. Is that correct?
If that's right, I would add to this comment:
"So we can still return to the main content after that."
…#7480) Currently, if the interstitial lasts 30 and we have a stream that lasts 31 seconds, we would go back to live after the interstitial, but with 1 extra second of latency. This PR solves this by limiting the play range to 30.
…#7480) Currently, if the interstitial lasts 30 and we have a stream that lasts 31 seconds, we would go back to live after the interstitial, but with 1 extra second of latency. This PR solves this by limiting the play range to 30.
…#7480) Currently, if the interstitial lasts 30 and we have a stream that lasts 31 seconds, we would go back to live after the interstitial, but with 1 extra second of latency. This PR solves this by limiting the play range to 30.
Currently, if the interstitial lasts 30 and we have a stream that lasts 31 seconds, we would go back to live after the interstitial, but with 1 extra second of latency. This PR solves this by limiting the play range to 30.