[webkit-reviews] review granted: [Bug 203170] Add MediaCapabilities support for DolbyVision codecs. : [Attachment 381343] Patch

[bugzilla-daemon at webkit.org]

Eric Carlson <eric.carlson at apple.com> has granted Jer Noble
<jer.noble at apple.com>'s request for review:
Bug 203170: Add MediaCapabilities support for DolbyVision codecs.
https://bugs.webkit.org/show_bug.cgi?id=203170

Attachment 381343: Patch

https://bugs.webkit.org/attachment.cgi?id=381343&action=review




--- Comment #3 from Eric Carlson <eric.carlson at apple.com> ---
Comment on attachment 381343
  --> https://bugs.webkit.org/attachment.cgi?id=381343
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?