WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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+
Details
Formatted Diff
Diff
Patch
(36.91 KB, patch)
2020-07-29 12:19 PDT
,
Jer Noble
no flags
Details
Formatted Diff
Diff
Patch for landing.
(37.21 KB, patch)
2020-07-29 12:51 PDT
,
Jer Noble
no flags
Details
Formatted Diff
Diff
Patch for landing
(37.25 KB, patch)
2020-07-29 14:17 PDT
,
Jer Noble
no flags
Details
Formatted Diff
Diff
Patch for landing
(37.22 KB, patch)
2020-07-29 14:37 PDT
,
Jer Noble
no flags
Details
Formatted Diff
Diff
Show Obsolete
(4)
View All
Add attachment
proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1
2020-07-29 11:58:19 PDT
<
rdar://problem/66284848
>
Jer Noble
Comment 2
2020-07-29 12:07:12 PDT
Created
attachment 405480
[details]
Patch
Jer Noble
Comment 3
2020-07-29 12:19:46 PDT
Created
attachment 405481
[details]
Patch
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.
Top of Page
Format For Printing
XML
Clone This Bug