Bug 112301 - Add support for MediaSource::isTypeSupported()
Summary: Add support for MediaSource::isTypeSupported()
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: 2013-03-13 16:54 PDT by Aaron Colwell
Modified: 2013-03-20 10:25 PDT (History)
12 users (show)

See Also:


Attachments
Patch (15.94 KB, patch)
2013-03-13 17:00 PDT, Aaron Colwell
no flags Details | Formatted Diff | Diff
Patch (15.94 KB, patch)
2013-03-14 11:37 PDT, Aaron Colwell
no flags Details | Formatted Diff | Diff
Rebase (16.11 KB, patch)
2013-03-20 09:55 PDT, 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 2013-03-13 16:54:18 PDT
Add support for MediaSource::isTypeSupported()
Comment 1 Aaron Colwell 2013-03-13 17:00:23 PDT
Created attachment 193024 [details]
Patch
Comment 2 WebKit Review Bot 2013-03-13 17:04:57 PDT
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.
Comment 3 Darin Fisher (:fishd, Google) 2013-03-13 17:18:00 PDT
Comment on attachment 193024 [details]
Patch

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

We usually put MIME-type related queries on WebMIMERegistry.  However, I notice that this is not a function on WebCore::MIMERegistry, although I suppose it could be.


> Source/WebKit/chromium/src/MediaSourcePrivateImpl.cpp:46
> +    return WebKit::webKitPlatformSupport()->isTypeSupportedForMediaSource(type, codecs);

nit: webKitPlatformSupport() is deprecated.  You should be using WebKit::Platform::current() instead.
Comment 4 Aaron Colwell 2013-03-14 11:37:48 PDT
Created attachment 193161 [details]
Patch
Comment 5 WebKit Review Bot 2013-03-14 11:50:01 PDT
Comment on attachment 193161 [details]
Patch

Attachment 193161 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://webkit-commit-queue.appspot.com/results/17194230
Comment 6 WebKit Review Bot 2013-03-14 11:56:08 PDT
Comment on attachment 193161 [details]
Patch

Attachment 193161 [details] did not pass cr-linux-debug-ews (chromium-xvfb):
Output: http://webkit-commit-queue.appspot.com/results/17036857
Comment 7 Darin Fisher (:fishd, Google) 2013-03-14 11:57:08 PDT
Comment on attachment 193161 [details]
Patch

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

> Source/Platform/chromium/public/WebMimeRegistry.h:54
> +

tangent: is it possible to resolve the above FIXME now?
Comment 8 Darin Fisher (:fishd, Google) 2013-03-14 11:58:09 PDT
Comment on attachment 193161 [details]
Patch

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

> Source/Platform/chromium/public/WebMimeRegistry.h:55
> +    virtual bool supportsMediaSourceMIMEType(const WebKit::WebString& mimeType, const WebKit::WebString& codecs) = 0;

note: we typically provide default implementations for interfaces that are intended to be implemented by chromium.  that way, the webkit side can land first.
Comment 9 Aaron Colwell 2013-03-14 12:01:45 PDT
Comment on attachment 193161 [details]
Patch

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

>> Source/Platform/chromium/public/WebMimeRegistry.h:54
>> +
> 
> tangent: is it possible to resolve the above FIXME now?

Yes. I'll do this in a separate patch.

>> Source/Platform/chromium/public/WebMimeRegistry.h:55
>> +    virtual bool supportsMediaSourceMIMEType(const WebKit::WebString& mimeType, const WebKit::WebString& codecs) = 0;
> 
> note: we typically provide default implementations for interfaces that are intended to be implemented by chromium.  that way, the webkit side can land first.

I can do that, but then the LayoutTest will fail. Should I add TestExpectation line for this and file a bug to remove it once the Chromium side lands?
Comment 10 Peter Beverloo (cr-android ews) 2013-03-14 12:19:06 PDT
Comment on attachment 193161 [details]
Patch

Attachment 193161 [details] did not pass cr-android-ews (chromium-android):
Output: http://webkit-commit-queue.appspot.com/results/17134605
Comment 11 Aaron Colwell 2013-03-20 09:55:47 PDT
Created attachment 194083 [details]
Rebase
Comment 12 WebKit Review Bot 2013-03-20 10:24:58 PDT
Comment on attachment 194083 [details]
Rebase

Clearing flags on attachment: 194083

Committed r146360: <http://trac.webkit.org/changeset/146360>
Comment 13 WebKit Review Bot 2013-03-20 10:25:03 PDT
All reviewed patches have been landed.  Closing bug.