Summary: | [WinCairo] Support more video formats. | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | peavo | ||||||
Component: | WebCore Misc. | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | achristensen, bfulgham | ||||||
Priority: | P2 | ||||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
peavo
2016-01-08 06:24:31 PST
Created attachment 268541 [details]
Patch
Comment on attachment 268541 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=268541&action=review Functionality looks good, there are just a few optimizations and reorganizations we need to do. > Source/WebCore/platform/graphics/win/MediaPlayerPrivateMediaFoundation.cpp:124 > + static HashSet<String> cachedTypes; Use NeverDestroyed instead of static. Also, you should probably make this in a separate function like gstreamer's mimeTypeCache(), except have it just return a reference, not a whole HashSet. > Source/WebCore/platform/graphics/win/MediaPlayerPrivateMediaFoundation.cpp:134 > + HRESULT hr = MFGetSupportedMimeTypesPtr()(&propVarMimeTypeArray); This will crash on WindowsXP. I don't think that's a problem, because we don't support XP any more. > Source/WebCore/platform/graphics/win/MediaPlayerPrivateMediaFoundation.cpp:156 > + HashSet<String> types; > + getSupportedTypes(types); We should't copy the whole HashSet every time we want to check if one type is supported. > Source/WebCore/platform/graphics/win/MediaPlayerPrivateMediaFoundation.cpp:158 > + for (HashSet<String>::iterator it = types.begin(); it != types.end(); ++it) { This is what HashSet.contains is for. No need to iterate. Thanks for reviewing, will update the patch :) Created attachment 268569 [details]
Patch
Comment on attachment 268569 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=268569&action=review > Source/WebCore/platform/graphics/win/MediaPlayerPrivateMediaFoundation.cpp:142 > + for (int i = 0; i < mimeTypeArray.cElems; i++) cElems is an unsigned int. Please use unsigned for i. Committed r194787: <http://trac.webkit.org/changeset/194787>. |