Bug 213837

Summary: Allow registering VP9 as a VT decoder
Product: WebKit Reporter: youenn fablet <youennf>
Component: MediaAssignee: youenn fablet <youennf>
Status: RESOLVED FIXED    
Severity: Normal CC: eric.carlson, ews-watchlist, glenn, hta, jer.noble, philipj, sergio, tommyw, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Local Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 213778    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch for landing
none
Patch none

Description youenn fablet 2020-07-01 06:00:27 PDT 
Allow registering VP9 as a VT decoder
Comment 1 Radar WebKit Bug Importer 2020-07-01 06:26:01 PDT 
<rdar://problem/64984881>
Comment 2 youenn fablet 2020-07-01 06:42:35 PDT 
Created attachment 403286 [details]
Patch
Comment 3 Eric Carlson 2020-07-01 14:35:44 PDT 
Comment on attachment 403286 [details]
Patch

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

> Source/ThirdParty/libwebrtc/Source/webrtc/sdk/WebKit/WebKitVP9Decoder.cpp:201
> +    if (!CMBlockBufferIsRangeContiguous(encodedBuffer, 0, 0)) {
> +        RTC_LOG(LS_ERROR) << "VP9 decoder: failed to get contiguous buffer";
> +        return kVTParameterErr;
> +    }

You can use CMBlockBufferCreateContiguous(...) if you require a contiguous buffer.

> Source/ThirdParty/libwebrtc/Source/webrtc/sdk/WebKit/WebKitVP9Decoder.cpp:224
> +    return noErr;

Should this return an empty dictionary?

> Source/ThirdParty/libwebrtc/Source/webrtc/sdk/WebKit/WebKitVP9Decoder.cpp:247
> +    VTDecoderSessionEmitDecodedFrame(m_session, m_currentFrame, vtError, 0, nullptr);

Maybe ASSERT(m_currentFrame) here?

> Source/ThirdParty/libwebrtc/Source/webrtc/sdk/WebKit/WebKitVP9Decoder.cpp:279
> +    VTDecoderSessionEmitDecodedFrame(m_session, m_currentFrame, pixelBuffer ? noErr : -1, 0, pixelBuffer);

Ditto.
Comment 4 youenn fablet 2020-07-02 00:31:57 PDT 
Created attachment 403349 [details]
Patch
Comment 5 youenn fablet 2020-07-02 02:10:02 PDT 
Created attachment 403355 [details]
Patch
Comment 6 youenn fablet 2020-07-02 04:36:15 PDT 
Created attachment 403361 [details]
Patch
Comment 7 Jer Noble 2020-07-02 10:21:22 PDT 
Comment on attachment 403361 [details]
Patch

LGTM; I think we could use some more tests that verify HTMLMediaElement.canPlayType('video/mp4; codecs=vp9') and the same for MediaSource.isTypeSupported('video/mp4; codecs=vp9'), but that can be follow-up work.
Comment 8 youenn fablet 2020-07-02 12:31:28 PDT 
(In reply to Jer Noble from comment #7)
> Comment on attachment 403361 [details]
> Patch
> 
> LGTM; I think we could use some more tests that verify
> HTMLMediaElement.canPlayType('video/mp4; codecs=vp9') and the same for
> MediaSource.isTypeSupported('video/mp4; codecs=vp9'), but that can be
> follow-up work.

Will add the tests
I am not sure they will pass though, how would WebKit know the codec name is vp9?
Comment 9 youenn fablet 2020-07-03 00:42:43 PDT 
Created attachment 403445 [details]
Patch for landing
Comment 10 youenn fablet 2020-07-03 05:32:42 PDT 
Created attachment 403453 [details]
Patch
Comment 11 EWS 2020-07-03 07:21:37 PDT 
Committed r263894: <https://trac.webkit.org/changeset/263894>

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