Bug 213837 - Allow registering VP9 as a VT decoder
Summary: Allow registering VP9 as a VT decoder
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: youenn fablet
URL:
Keywords: InRadar
Depends on: 213778
Blocks:
  Show dependency treegraph
 
Reported: 2020-07-01 06:00 PDT by youenn fablet
Modified: 2020-07-03 07:21 PDT (History)
9 users (show)

See Also:


Attachments
Patch (93.05 KB, patch)
2020-07-01 06:42 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (254.47 KB, patch)
2020-07-02 00:31 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (256.30 KB, patch)
2020-07-02 02:10 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (256.30 KB, patch)
2020-07-02 04:36 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch for landing (256.71 KB, patch)
2020-07-03 00:42 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (256.53 KB, patch)
2020-07-03 05:32 PDT, youenn fablet
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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].