Bug 214928 – Support HDR decode in SW VP9

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Jer Noble]

Support HDR decode in SW VP9

[Radar WebKit Bug Importer]

<rdar://problem/66284848>

[Jer Noble]

Created attachment 405480 [details]
Patch

[Jer Noble]

Created attachment 405481 [details]
Patch

[Eric Carlson]

Comment on attachment 405480 [details]
Patch

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

r=me once the bots are happy

> Source/ThirdParty/libwebrtc/Source/webrtc/sdk/WebKit/WebKitVP9Decoder.cpp:277
> +        CFTypeRef configurationRecord = CFDictionaryGetValue((CFDictionaryRef)extensionAtoms, CFSTR("vpcC"));

`auto configurationRecord = static_cast<CFDataRef>(CFDictionaryGetValue(...`

> Source/ThirdParty/libwebrtc/Source/webrtc/sdk/WebKit/WebKitVP9Decoder.cpp:281
> +        auto configurationRecordSize = CFDataGetLength((CFDataRef)configurationRecord);

The cast isn't necessary with the above.

> Source/ThirdParty/libwebrtc/Source/webrtc/sdk/WebKit/WebKitVP9Decoder.cpp:285
> +        auto configurationRecordData = CFDataGetBytePtr((CFDataRef)configurationRecord);

Ditto

> Source/ThirdParty/libwebrtc/Source/webrtc/sdk/WebKit/WebKitVP9Decoder.cpp:305
> +        CFMutableArrayRef cfPixelFormats = CFArrayCreateMutable(kCFAllocatorDefault, 2, &kCFTypeArrayCallBacks);

'auto cfPixelFormats = ...`

> Source/ThirdParty/libwebrtc/Source/webrtc/sdk/WebKit/WebKitVP9Decoder.cpp:310
> +        CFNumberRef borderPixelsValue = createNumber(32);

`auto borderPixelsValue = ...`

> Source/ThirdParty/libwebrtc/Source/webrtc/sdk/WebKit/WebKitVP9Decoder.cpp:326
> +        CFDictionaryRef attributes = CFDictionaryCreate(kCFAllocatorDefault, keys, values, std::size(keys), &kCFTypeDictionaryKeyCallBacks, &kCFTypeDictionaryValueCallBacks);

`auto attributes = ...`

> Source/ThirdParty/libwebrtc/Source/webrtc/sdk/WebKit/WebKitVP9Decoder.cpp:342
> +    CFRelease(pixelBufferAttributes);

Double release!

[Darin Adler]

Comment on attachment 405480 [details]
Patch

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

>> Source/ThirdParty/libwebrtc/Source/webrtc/sdk/WebKit/WebKitVP9Decoder.cpp:305
>> +        CFMutableArrayRef cfPixelFormats = CFArrayCreateMutable(kCFAllocatorDefault, 2, &kCFTypeArrayCallBacks);
> 
> 'auto cfPixelFormats = ...`

Why not use CFArrayCreate instead of CRArrayCreateMutable?

> Source/ThirdParty/libwebrtc/Source/webrtc/sdk/WebKit/WebKitVP9Decoder.cpp:308
> +        CFRelease(formatNumber);

If this was WebKit code, not libwebtrc, I would suggest using adoptCF instead of CFRelease. It’s hard to get reference counting right the old manual way.

[Jer Noble]

Created attachment 405489 [details]
Patch for landing.

[Jer Noble]

Comment on attachment 405480 [details]
Patch

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

>> Source/ThirdParty/libwebrtc/Source/webrtc/sdk/WebKit/WebKitVP9Decoder.cpp:277
>> +        CFTypeRef configurationRecord = CFDictionaryGetValue((CFDictionaryRef)extensionAtoms, CFSTR("vpcC"));
> 
> `auto configurationRecord = static_cast<CFDataRef>(CFDictionaryGetValue(...`

Yeah, much better.

>> Source/ThirdParty/libwebrtc/Source/webrtc/sdk/WebKit/WebKitVP9Decoder.cpp:305
>> +        CFMutableArrayRef cfPixelFormats = CFArrayCreateMutable(kCFAllocatorDefault, 2, &kCFTypeArrayCallBacks);
> 
> 'auto cfPixelFormats = ...`

Ok.

>> Source/ThirdParty/libwebrtc/Source/webrtc/sdk/WebKit/WebKitVP9Decoder.cpp:310
>> +        CFNumberRef borderPixelsValue = createNumber(32);
> 
> `auto borderPixelsValue = ...`

Ok.

>> Source/ThirdParty/libwebrtc/Source/webrtc/sdk/WebKit/WebKitVP9Decoder.cpp:326
>> +        CFDictionaryRef attributes = CFDictionaryCreate(kCFAllocatorDefault, keys, values, std::size(keys), &kCFTypeDictionaryKeyCallBacks, &kCFTypeDictionaryValueCallBacks);
> 
> `auto attributes = ...`

Ok.

>> Source/ThirdParty/libwebrtc/Source/webrtc/sdk/WebKit/WebKitVP9Decoder.cpp:342
>> +    CFRelease(pixelBufferAttributes);
> 
> Double release!

Ooh, that's bad!

[Peng Liu]

Comment on attachment 405489 [details]
Patch for landing.

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

> Source/WebCore/ChangeLog:9
> +        CMFormatDescription attached to each incoming video fram.

Nit: s/fram/frame.

> Source/WebCore/ChangeLog:13
> +        in this situations.

Nit: s/this/those.

[Jer Noble]

Created attachment 405499 [details]
Patch for landing

[Jer Noble]

Created attachment 405503 [details]
Patch for landing

[EWS]

Committed r265073: <https://trac.webkit.org/changeset/265073>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 405503 [details].