Bug 219148 – Add support for RTCRtpScriptTransform

• Status: RESOLVED
• Resolution: FIXED
• Component: WebRTC

[youenn fablet]

Add support for RTCRtpScriptTransform

[youenn fablet]

Created attachment 414553 [details]
Patch

[youenn fablet]

Created attachment 414556 [details]
Patch

[youenn fablet]

Created attachment 414557 [details]
Patch

[youenn fablet]

Created attachment 414559 [details]
Patch

[youenn fablet]

Created attachment 414560 [details]
Patch

[youenn fablet]

Created attachment 414564 [details]
Patch

[Eric Carlson]

Comment on attachment 414564 [details]
Patch

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

> Source/WebCore/Modules/mediastream/RTCRtpScriptTransform.cpp:117
> +        m_backend = makeRef(backend);

You only set `m_backend` if `setupTransformer` fails?

> Source/WebCore/Modules/mediastream/RTCRtpScriptTransformer.cpp:174
> +            auto value = isAudio ? toJS(&globalObject, &globalObject, RTCEncodedAudioFrame::create(WTFMove(frame))) : toJS(&globalObject, &globalObject, RTCEncodedAudioFrame::create(WTFMove(frame)));

Don’t you want to create a Video frame when `isAudio` is false?

[youenn fablet]

(In reply to Eric Carlson from comment #7)
> Comment on attachment 414564 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=414564&action=review
> 
> > Source/WebCore/Modules/mediastream/RTCRtpScriptTransform.cpp:117
> > +        m_backend = makeRef(backend);
> 
> You only set `m_backend` if `setupTransformer` fails?

Yes, it means the transformer is not yet there but will be soon.
So we keep a ref to m_backend so that we can call setupTransformer at the time the transformer is hooked to its transform.

> > Source/WebCore/Modules/mediastream/RTCRtpScriptTransformer.cpp:174
> > +            auto value = isAudio ? toJS(&globalObject, &globalObject, RTCEncodedAudioFrame::create(WTFMove(frame))) : toJS(&globalObject, &globalObject, RTCEncodedAudioFrame::create(WTFMove(frame)));
> 
> Don’t you want to create a Video frame when `isAudio` is false?

Good catch, will add a test for it.

[youenn fablet]

Created attachment 414659 [details]
Patch for landing

[EWS]

Committed r270107: <https://trac.webkit.org/changeset/270107>

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

[Radar WebKit Bug Importer]

<rdar://problem/71623266>