w3c / media-source

Media Source Extensions
https://w3c.github.io/media-source/
Other
268 stars 57 forks source link

Fixed #19, #20, #26: Require remove() for range removal #65

Closed jdsmith3000 closed 8 years ago

jdsmith3000 commented 8 years ago
jdsmith3000 commented 8 years ago

@wolenetz: Please review. Addresses issues #19, #20 & #26.

paulbrucecotton commented 8 years ago

@wolenetz; Can you please review this PR ASAP so we can get issues #19, #20 & #26 dealt with?

wolenetz commented 8 years ago

@paulbrucecotton @jdsmith3000 Yes - this is part of what I'm working through shortly.

jdsmith3000 commented 8 years ago

@wolenetz: Might you review by EoW? Change implements our agreed upon solution. Addresses issues #19, #20 & #26.

wolenetz commented 8 years ago

Yes, I'm looking at this today - got tied up earlier this week in trying out upstreaming a new web-platform-test. Reviews and PRs on spec issues will be coming faster now from me.

wolenetz commented 8 years ago

@jdsmith3000 Looks pretty good. See my comments, above. Let me know when they're addressed and I'll do a final review and merge this if it l*tm. Thanks!

wolenetz commented 8 years ago

@jdsmith3000 Yesterday, I responded to your most recent comments in the line notes, above. I believe we're in agreement on the route forward. Please let me know if you have any further questions. I'd like to review again once you've addressed the line notes, above. Thanks!

jdsmith3000 commented 8 years ago

@Wolenetz Rebased commit picks up these changes based on your feedback:

jdsmith3000 commented 8 years ago

@Wolenetz Please review. I think I've picked up the changes we settled on after discussion. If so, let's finish this and close three issues.

wolenetz commented 8 years ago

Yep - now that I'm back from my brief vacation, I'm reviewing both this and https://github.com/w3c/media-source/pull/73 today.

jdsmith3000 commented 8 years ago

@wolenetz I believe this is now ready to merge.

wolenetz commented 8 years ago

Looks good! Since the EME-discussion about the GH "green button" hasn't settled / I'm not sure I trust the button yet, I'll go ahead and merge this manually, shortly. Thanks for getting this updated, @jdsmith3000

Note to web authors and UA implementors: This spec change may break MSE players which previously depended upon implicit (and ambiguously defined) range removals on duration reductions which would truncate buffered media, and similarly might break players which attempt to abort() asynchronous range-removals. The workaround for the former is now a well-defined explicit remove(), and for the latter, there is no longer allowed any ability for an app to abort() range removals.

plehegar commented 8 years ago

I'm curious: does this change break existing UA implementations and, if yes, do we know the timeline for them to get aligned?

jdsmith3000 commented 8 years ago

Yes, this will require implementations to update. The previous spec was not actually implementable however.

We need to discuss implementation updates in general, since this is not the only issue that could require updates.