Media source duration change algorithm (step 2 in particular) #3232
Conversation
The test cases buffer media data around a specific presentation timestamp, either by loading the right media segment, or by using the append window mechanism, and then truncate the duration to appropriate values.
New test cases ensure that implementations throw an InvalidStateError exception when the new duration is less than the highest starting presentation timestamp of any buffered coded frames. Some test cases took for granted that setting the duration would set the SourceBuffer.updating attribute. This is no longer the case. Tests updated accordingly. One the test cases assumed that endOfStream() could only adjust the duration downwards. That is no longer true either. Also, some duration tests assumed that a "timeupdate" event would only be fired after the "seeking" event. However, "timeupdate" events are regularly issued during playback and thus such an evet could fire *before* the "seeking" event.
|
Reviewers for this pull request are: @acolwell, @bit, @shishimaru, @sideshowbarker, and @wolenetz. |
|
Critic review: https://critic.hoppipolla.co.uk/r/6661 This is an external review system which you may optionally use for the code review of your pull request. In order to help critic track your changes, please do not make in-place history rewrites (e.g. via |
Some tests expected the API to allow the truncation of the media content duration even if that truncated a buffered range. The spec no longer allows that behavior. See related discussion in: w3c/media-source#102 (comment) This update drops some tests on seeking after truncation, they seem no longer relevant.
|
FF folks landed a conflicting change. I'll work through such conflict after reviewing the rest of these PRs from @tidoust. FYI-@jyavenard and @tidoust: Chromium change https://codereview.chromium.org/2102323002 also updated (probably with similar conflicts) related tests to accommodate the spec change to disallow duration truncating whole coded frames, and to disallow aborting range removals. |
|
Let me know if you need a hand to resolve such conflicts. The commit that changed the test is 23b22d8 |
|
i believe this commit may no longer be necessary as the test was fixed in push ac5ec11 |
|
I'm reviewing this now. Per #3232 (comment), this may now be redundant. |
| test.waitForExpectedEvents(function() | ||
| { | ||
| assert_false(sourceBuffer.updating, "updating"); | ||
| assert_less_than(mediaSource.duration, 5, "duration"); |
There was a problem hiding this comment.
Current mediasource-config-changes.js uses explicit remove(2,...) and is more complete. This part of the this change is done better by current test.
|
There are some parts of this PR which include some new coverage; other parts regress or conflict with similar fixes already made. I'll update this PR to just include the new pieces of coverage noted in my diff comments, above. -edit: I'll make a new PR, since this one originated in @tidoust's private fork. |
…tests#3232 Original author of some of this new coverage is @tidoust. This commit replaces those in PR web-platform-tests#3232, which had become highly conflicting with fixes to mediasource-duration.html and others which landed while web-platform-tests#3232 was in review. This commit also includes further coverage beyond web-platform-tests#3232.
…age_per_review_of_pr_3232 Add duration coverage found missing during review of PR #3232
New test cases ensure that implementations throw an InvalidStateError exception when the new duration is less than the highest starting presentation timestamp of any buffered coded frames.
Some test cases took for granted that setting the duration would set the SourceBuffer.updating attribute. This is no longer the case. Tests updated accordingly.
One the test cases assumed that endOfStream() could only adjust the duration downwards. That seems no longer true either.
Also, some duration tests assumed that a "timeupdate" event would only be fired after the "seeking" event. However, "timeupdate" events are regularly issued during playback and thus such an event could fire before the "seeking" event.