RESOLVED FIXED 214928
Support HDR decode in SW VP9
https://bugs.webkit.org/show_bug.cgi?id=214928
Summary Support HDR decode in SW VP9
Jer Noble
Reported 2020-07-29 11:57:55 PDT
Support HDR decode in SW VP9
Attachments
Patch (24.17 KB, patch)
2020-07-29 12:07 PDT, Jer Noble
eric.carlson: review+
Patch (36.91 KB, patch)
2020-07-29 12:19 PDT, Jer Noble
no flags
Patch for landing. (37.21 KB, patch)
2020-07-29 12:51 PDT, Jer Noble
no flags
Patch for landing (37.25 KB, patch)
2020-07-29 14:17 PDT, Jer Noble
no flags
Patch for landing (37.22 KB, patch)
2020-07-29 14:37 PDT, Jer Noble
no flags
Radar WebKit Bug Importer
Comment 1 2020-07-29 11:58:19 PDT
Jer Noble
Comment 2 2020-07-29 12:07:12 PDT
Jer Noble
Comment 3 2020-07-29 12:19:46 PDT
Eric Carlson
Comment 4 2020-07-29 12:45:32 PDT
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 5 2020-07-29 12:51:54 PDT
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
Comment 6 2020-07-29 12:51:55 PDT
Created attachment 405489 [details] Patch for landing.
Jer Noble
Comment 7 2020-07-29 12:53:09 PDT
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 8 2020-07-29 13:20:47 PDT
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
Comment 9 2020-07-29 14:17:15 PDT
Created attachment 405499 [details] Patch for landing
Jer Noble
Comment 10 2020-07-29 14:37:14 PDT
Created attachment 405503 [details] Patch for landing
EWS
Comment 11 2020-07-29 18:01:10 PDT
Committed r265073: <https://trac.webkit.org/changeset/265073> All reviewed patches have been landed. Closing bug and clearing flags on attachment 405503 [details].
Note You need to log in before you can comment on or make changes to this bug.