Bug 205763

Summary: [Media in GPU process] Add remote MIME type cache
Product: WebKit Reporter: Eric Carlson <eric.carlson>
Component: MediaAssignee: Eric Carlson <eric.carlson>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, dino, ews-watchlist, glenn, jer.noble, peng.liu6, philipj, sergio, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 205123    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch for landing
none
Patch for landing none

Eric Carlson
Reported 2020-01-03 18:56:17 PST
Add remote a MIME type cache to decrease the number of times we need to use IPC to answer questions about supported types.
Attachments
Patch (45.63 KB, patch)
2020-01-03 19:16 PST, Eric Carlson 		no flags
DetailsFormatted DiffDiff
Patch (45.52 KB, patch)
2020-01-04 17:02 PST, Eric Carlson 		no flags
DetailsFormatted DiffDiff
Patch (45.66 KB, patch)
2020-01-04 18:17 PST, Eric Carlson 		no flags
DetailsFormatted DiffDiff
Patch (45.64 KB, patch)
2020-01-04 21:45 PST, Eric Carlson 		no flags
DetailsFormatted DiffDiff
Patch (45.86 KB, patch)
2020-01-05 07:47 PST, Eric Carlson 		no flags
DetailsFormatted DiffDiff
Patch for landing (45.95 KB, patch)
2020-01-05 19:53 PST, Eric Carlson 		no flags
DetailsFormatted DiffDiff
Patch for landing (44.85 KB, patch)
2020-01-05 20:39 PST, Eric Carlson 		no flags
DetailsFormatted DiffDiff
Show Obsolete (6) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2020-01-03 18:56:29 PST
<rdar://problem/58312152>
Eric Carlson
Comment 2 2020-01-03 19:16:57 PST
Created attachment 386746 [details] Patch
Jer Noble
Comment 3 2020-01-03 20:23:18 PST
Comment on attachment 386746 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=386746&action=review > Source/WebCore/platform/graphics/MIMETypeCache.cpp:91 > +void MIMETypeCache::setSupportedTypes(const Vector<String>& newTypes) This change restores the broken naming convention and behavior you fixed in your last patch. set() should mean “set”; it doesn’t here. If a client calls add(), then set(), the expectation is that the second call replaces values added by the first. That doesn’t work here. Clients who would like to optimize for the case where the cache is already initialized can just do: `if(cache.isEmpty()) cache.addSupportedTypes();`.
Eric Carlson
Comment 4 2020-01-04 17:02:36 PST
Created attachment 386775 [details] Patch
Eric Carlson
Comment 5 2020-01-04 18:17:25 PST
Created attachment 386778 [details] Patch
Eric Carlson
Comment 6 2020-01-04 21:45:08 PST
Created attachment 386781 [details] Patch
Eric Carlson
Comment 7 2020-01-05 07:47:53 PST
Created attachment 386789 [details] Patch
Dean Jackson
Comment 8 2020-01-05 13:28:26 PST
Comment on attachment 386789 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=386789&action=review > Source/WebCore/platform/graphics/MIMETypeCache.cpp:68 > + MediaPlayerEnums::SupportsType result = MediaPlayerEnums::SupportsType::IsNotSupported; Can this be auto? > Source/WebCore/platform/graphics/MIMETypeCache.cpp:84 > + do { > + if (!isAvailable() || mimeType.isEmpty()) > + break; > + > + auto contentType = ContentType { mimeType }; > + auto containerType = contentType.containerType(); > + if (!supportsContainerType(containerType)) > + break; > + > + result = MediaPlayerEnums::SupportsType::MayBeSupported; > + if (contentType.codecs().isEmpty()) > + break; > + > + if (canDecodeExtendedType(contentType)) > + result = MediaPlayerEnums::SupportsType::IsSupported; > + } while (0); This confused me! > Source/WebCore/platform/graphics/MIMETypeCache.cpp:95 > + m_supportedTypes = HashSet<String, ASCIICaseInsensitiveHash>(); Use { } rather than ()? > Source/WebKit/WebProcess/GPU/media/RemoteMediaPlayerManager.cpp:117 > +static HashMap<MediaPlayerEnums::MediaEngineIdentifier, std::unique_ptr<RemoteMediaPlayerMIMETypeCache>, WTF::IntHash<MediaPlayerEnums::MediaEngineIdentifier>, WTF::StrongEnumHashTraits<MediaPlayerEnums::MediaEngineIdentifier>>& mimeCaches() > { > -// FIXME: Use parameters.mediaMIMETypes. > + static NeverDestroyed<HashMap<MediaPlayerEnums::MediaEngineIdentifier, std::unique_ptr<RemoteMediaPlayerMIMETypeCache>, WTF::IntHash<MediaPlayerEnums::MediaEngineIdentifier>, WTF::StrongEnumHashTraits<MediaPlayerEnums::MediaEngineIdentifier>>> caches; Could we typedef/using this thing?
Eric Carlson
Comment 9 2020-01-05 19:50:20 PST
Comment on attachment 386789 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=386789&action=review >> Source/WebCore/platform/graphics/MIMETypeCache.cpp:68 >> + MediaPlayerEnums::SupportsType result = MediaPlayerEnums::SupportsType::IsNotSupported; > > Can this be auto? Yes, fixed. >> Source/WebCore/platform/graphics/MIMETypeCache.cpp:84 >> + } while (0); > > This confused me! Sorry about that :-) >> Source/WebCore/platform/graphics/MIMETypeCache.cpp:95 >> + m_supportedTypes = HashSet<String, ASCIICaseInsensitiveHash>(); > > Use { } rather than ()? Good point, changed. >> Source/WebKit/WebProcess/GPU/media/RemoteMediaPlayerManager.cpp:117 >> + static NeverDestroyed<HashMap<MediaPlayerEnums::MediaEngineIdentifier, std::unique_ptr<RemoteMediaPlayerMIMETypeCache>, WTF::IntHash<MediaPlayerEnums::MediaEngineIdentifier>, WTF::StrongEnumHashTraits<MediaPlayerEnums::MediaEngineIdentifier>>> caches; > > Could we typedef/using this thing? Yes, fixed.
Eric Carlson
Comment 10 2020-01-05 19:53:07 PST
Created attachment 386804 [details] Patch for landing
Eric Carlson
Comment 11 2020-01-05 20:39:45 PST
Created attachment 386807 [details] Patch for landing
WebKit Commit Bot
Comment 12 2020-01-05 21:47:57 PST
Comment on attachment 386807 [details] Patch for landing Clearing flags on attachment: 386807 Committed r254048: <https://trac.webkit.org/changeset/254048>
WebKit Commit Bot
Comment 13 2020-01-05 21:47:59 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.