A MediaSource object is needed in order to implement the new object-oriented MediaSource API: http://dvcs.w3.org/hg/html-media/raw-file/tip/media-source/media-source.html#mediasource
Created attachment 154181 [details] Patch
Comment on attachment 154181 [details] Patch Attachment 154181 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13350072 New failing tests: fast/loader/loadInProgress.html fast/inline/positionedLifetime.html fast/loader/unload-form-post-about-blank.html
Created attachment 154234 [details] Archive of layout-test-results from gce-cr-linux-08 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: gce-cr-linux-08 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid
Ping! Pretty sure those test failures aren't related. I'll upload a fresh patch this afternoon to be sure. But this should be ready for review as is.
Created attachment 156579 [details] Patch
Comment on attachment 156579 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=156579&action=review > Source/WebCore/CMakeLists.txt:2667 > + Modules/mediasource/MediaSource.idl Could you move this to existing file list? Because, r124730 removed ENALBE_MEDIA_SOURCE in CMakeLists.txt > Source/WebCore/CMakeLists.txt:2672 > + Modules/mediasource/MediaSource.cpp ditto.
Comment on attachment 156579 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=156579&action=review >> Source/WebCore/CMakeLists.txt:2667 >> + Modules/mediasource/MediaSource.idl > > Could you move this to existing file list? Because, r124730 removed ENALBE_MEDIA_SOURCE in CMakeLists.txt Sure thing, thanks for catching that!
Created attachment 156590 [details] fixing CMakeLists.txt
(In reply to comment #7) > Sure thing, thanks for catching that! Thanks, looks fine in CMake.
Comment on attachment 156590 [details] fixing CMakeLists.txt View in context: https://bugs.webkit.org/attachment.cgi?id=156590&action=review > Source/WebCore/Modules/mediasource/MediaSource.cpp:97 > + RefPtr<SourceBuffer> buffer = SourceBuffer::create(String::number(++m_nextSourceBufferId), this); Does this need to deal with m_nextSourceBufferId wrapping around? > Source/WebCore/Modules/mediasource/MediaSourceRegistry.cpp:44 > + ASSERT(isMainThread()); Why is this ASSERT necessary? A comment would be helpful.
Committed r124780: <http://trac.webkit.org/changeset/124780>
Re-opened since this is blocked by 93294
r124798 fixed the Android build problem caused by this patch.
Re-opened since this is blocked by 93340
Created attachment 157002 [details] Patch Lets try that again. This patch also includes the addition of a sourceURLs list in PublicURLManager to avoid the problems encounted with the last patch.
*** Bug 93285 has been marked as a duplicate of this bug. ***
(In reply to comment #10) > (From update of attachment 156590 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=156590&action=review > > > Source/WebCore/Modules/mediasource/MediaSource.cpp:97 > > + RefPtr<SourceBuffer> buffer = SourceBuffer::create(String::number(++m_nextSourceBufferId), this); > > Does this need to deal with m_nextSourceBufferId wrapping around? > You forgot to comment on this question.
Comment on attachment 156590 [details] fixing CMakeLists.txt View in context: https://bugs.webkit.org/attachment.cgi?id=156590&action=review Oops, forgot to click Publish. >>> Source/WebCore/Modules/mediasource/MediaSource.cpp:97 >>> + RefPtr<SourceBuffer> buffer = SourceBuffer::create(String::number(++m_nextSourceBufferId), this); >> >> Does this need to deal with m_nextSourceBufferId wrapping around? > > You forgot to comment on this question. Yes. We will handle this on the chromium side by checking for duplicate source ids and throwing a ReachedIdLimit. >> Source/WebCore/Modules/mediasource/MediaSourceRegistry.cpp:44 >> + ASSERT(isMainThread()); > > Why is this ASSERT necessary? A comment would be helpful. Done.
(In reply to comment #18) > (From update of attachment 156590 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=156590&action=review > > Oops, forgot to click Publish. > > >>> Source/WebCore/Modules/mediasource/MediaSource.cpp:97 > >>> + RefPtr<SourceBuffer> buffer = SourceBuffer::create(String::number(++m_nextSourceBufferId), this); > >> > >> Does this need to deal with m_nextSourceBufferId wrapping around? > > > > You forgot to comment on this question. > > Yes. We will handle this on the chromium side by checking for duplicate source ids and throwing a ReachedIdLimit. > Why do it in platform specific code? This will require every port that implements the feature to duplicate the check.
(In reply to comment #19) > (In reply to comment #18) > > (From update of attachment 156590 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=156590&action=review > > > > Oops, forgot to click Publish. > > > > >>> Source/WebCore/Modules/mediasource/MediaSource.cpp:97 > > >>> + RefPtr<SourceBuffer> buffer = SourceBuffer::create(String::number(++m_nextSourceBufferId), this); > > >> > > >> Does this need to deal with m_nextSourceBufferId wrapping around? > > > > > > You forgot to comment on this question. > > > > Yes. We will handle this on the chromium side by checking for duplicate source ids and throwing a ReachedIdLimit. > > > Why do it in platform specific code? This will require every port that implements the feature to duplicate the check. Good point. As per our offline discussion, I will file a bug to address this. Will probably use size_t for the type of m_nextSourceBufferId and cap the number of SourceBuffers that may be added at the size of the sourceBuffers vector.
Comment on attachment 157002 [details] Patch Clearing flags on attachment: 157002 Committed r124953: <http://trac.webkit.org/changeset/124953>
All reviewed patches have been landed. Closing bug.