Summary: | Add MediaCapabilities support for DolbyVision codecs. | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jer Noble <jer.noble> | ||||||||||
Component: | New Bugs | Assignee: | Jer Noble <jer.noble> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | commit-queue, eric.carlson, ews-watchlist, glenn, philipj, sergio, webkit-bug-importer | ||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||
Version: | WebKit Nightly Build | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Attachments: |
|
Description
Jer Noble
2019-10-18 16:19:50 PDT
Created attachment 381343 [details]
Patch
Comment on attachment 381343 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=381343&action=review > Source/WebCore/platform/graphics/HEVCUtilities.cpp:178 > + switch (profileID) { > + case 9: return codecName == "avc1" || codecName == "avc3"; > + default: return codecName == "hvc1" || codecName == "hev1"; Nit: a switch statement with one case and a default is odd, why not just use: if (profileID == 9) return codecName == "avc1" || codecName == "avc3"; return codecName == "hvc1" || codecName == "hev1"; > Source/WebCore/platform/graphics/HEVCUtilities.cpp:189 > + if (nextElement == codecSplit.end()) > + return WTF::nullopt; if (nextElement == codecSplit.end() || ++nextElement == codecSplit.end()) return WTF::nullopt; > Source/WebCore/platform/graphics/cocoa/HEVCUtilitiesCocoa.mm:196 > + auto& map = codecStringToCodecTypeMap(); > + auto findResult = map.find(parameters.codecName); > + if (findResult == map.end()) > + return false; > + auto codecType = findResult->value; Nit: Any reason to not wrap this in a function like you did with profileIDForAlphabeticDoViProfile and codecStringForDoViCodecType? Comment on attachment 381343 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=381343&action=review >> Source/WebCore/platform/graphics/HEVCUtilities.cpp:178 >> + default: return codecName == "hvc1" || codecName == "hev1"; > > Nit: a switch statement with one case and a default is odd, why not just use: > > if (profileID == 9) > return codecName == "avc1" || codecName == "avc3"; > > return codecName == "hvc1" || codecName == "hev1"; Sure. >> Source/WebCore/platform/graphics/HEVCUtilities.cpp:189 >> + return WTF::nullopt; > > if (nextElement == codecSplit.end() || ++nextElement == codecSplit.end()) > return WTF::nullopt; No this would skip the codec type portion of the string. Need that later. >> Source/WebCore/platform/graphics/cocoa/HEVCUtilitiesCocoa.mm:196 >> + auto codecType = findResult->value; > > Nit: Any reason to not wrap this in a function like you did with profileIDForAlphabeticDoViProfile and codecStringForDoViCodecType? I only split it out before because I ended up with multiple “auto& map =...” statements. But yes, I should convert this into a static query instead. Created attachment 381352 [details]
Patch for landing
Created attachment 381435 [details]
Patch for landing
Created attachment 381441 [details]
Patch for landing
Comment on attachment 381441 [details] Patch for landing Clearing flags on attachment: 381441 Committed r251396: <https://trac.webkit.org/changeset/251396> All reviewed patches have been landed. Closing bug. |