Bug 104581 - Update MediaSource to allow append() calls in "ended" state.
Summary: Update MediaSource to allow append() calls in "ended" state.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Aaron Colwell
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-12-10 11:52 PST by Aaron Colwell
Modified: 2012-12-11 10:24 PST (History)
4 users (show)

See Also:


Attachments
Patch (13.23 KB, patch)
2012-12-11 07:09 PST, Aaron Colwell
no flags Details | Formatted Diff | Diff
Rebase (13.20 KB, patch)
2012-12-11 08:18 PST, Aaron Colwell
no flags Details | Formatted Diff | Diff
Added 0-byte append test (17.15 KB, patch)
2012-12-11 09:45 PST, Aaron Colwell
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Aaron Colwell 2012-12-10 11:52:17 PST
The October 1st update to the Media Source Extensions spec changed the SourceBuffer.append() behavior to allow calls in the "ended" state. These calls will trigger a transition to "open" and allow data to be appended instead of throwing an exception. The current WebKit implementation needs to be updated to reflect this new behavior.
Comment 1 Aaron Colwell 2012-12-11 07:09:34 PST
Created attachment 178796 [details]
Patch
Comment 2 Aaron Colwell 2012-12-11 07:13:49 PST
Comment on attachment 178796 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=178796&action=review

> Source/WebCore/Modules/mediasource/MediaSource.cpp:-289
> -    if (!data->length())

Chromium player implementation was updated to handle this case. m_player needs to see these appends now so that it can properly mirror the state transition.
Comment 3 WebKit Review Bot 2012-12-11 07:52:13 PST
Comment on attachment 178796 [details]
Patch

Attachment 178796 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15280102

New failing tests:
http/tests/media/media-source/video-media-source-append-in-ended-state.html
http/tests/media/media-source/video-media-source-append-with-offset-in-ended-state.html
Comment 4 Aaron Colwell 2012-12-11 08:18:25 PST
Created attachment 178809 [details]
Rebase
Comment 5 Eric Carlson 2012-12-11 09:10:50 PST
Comment on attachment 178809 [details]
Rebase

View in context: https://bugs.webkit.org/attachment.cgi?id=178809&action=review

> Source/WebCore/Modules/mediasource/MediaSource.cpp:293
> +    if (m_readyState == endedKeyword())
> +        setReadyState(openKeyword());
>  
>      if (!m_player->sourceAppend(id, data->data(), data->length())) {
>          ec = SYNTAX_ERR;

Is it necessary to call to m_player->sourceAppend() when data->length() is zero?

It is probably worth having an explicit test for calling append() with no data when state is "ended".
Comment 6 Aaron Colwell 2012-12-11 09:38:56 PST
Comment on attachment 178809 [details]
Rebase

View in context: https://bugs.webkit.org/attachment.cgi?id=178809&action=review

>> Source/WebCore/Modules/mediasource/MediaSource.cpp:293
>>          ec = SYNTAX_ERR;
> 
> Is it necessary to call to m_player->sourceAppend() when data->length() is zero?
> 
> It is probably worth having an explicit test for calling append() with no data when state is "ended".

The call on m_player is necessary so the player implementation can keep its state in sync with m_readyState.

Added a test for the 0-byte case.
Comment 7 Aaron Colwell 2012-12-11 09:45:40 PST
Created attachment 178822 [details]
Added 0-byte append test
Comment 8 WebKit Review Bot 2012-12-11 10:24:46 PST
Comment on attachment 178822 [details]
Added 0-byte append test

Clearing flags on attachment: 178822

Committed r137332: <http://trac.webkit.org/changeset/137332>
Comment 9 WebKit Review Bot 2012-12-11 10:24:50 PST
All reviewed patches have been landed.  Closing bug.