Bug 213837 – Allow registering VP9 as a VT decoder

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[youenn fablet]

Allow registering VP9 as a VT decoder

[Radar WebKit Bug Importer]

<rdar://problem/64984881>

[youenn fablet]

Created attachment 403286 [details]
Patch

[Eric Carlson]

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.

[youenn fablet]

Created attachment 403349 [details]
Patch

[youenn fablet]

Created attachment 403355 [details]
Patch

[youenn fablet]

Created attachment 403361 [details]
Patch

[Jer Noble]

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.

[youenn fablet]

(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?

[youenn fablet]

Created attachment 403445 [details]
Patch for landing

[youenn fablet]

Created attachment 403453 [details]
Patch

[EWS]

Committed r263894: <https://trac.webkit.org/changeset/263894>

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