Bug 222982 – Update RTCRtpScriptTransform to the latest version of the spec

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

[youenn fablet]

Update RTCRtpScriptTransform to the latest version of the spec

[youenn fablet]

Created attachment 422716 [details]
Patch

[youenn fablet]

Created attachment 422821 [details]
Patch

[youenn fablet]

Comment on attachment 422821 [details]
Patch

Wincairo might be a unified build issue

[Eric Carlson]

Comment on attachment 422821 [details]
Patch

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

> Source/WebCore/ChangeLog:10
> +        Add support for options parameter provided in RTCRtpScriptTrtansform constructor.

s/RTCRtpScriptTrtansform/RTCRtpScriptTransform/

> Source/WebCore/Modules/mediastream/RTCRtpScriptTransformer.cpp:55
> +    auto transformer = adoptRef(*new RTCRtpScriptTransformer(context, WTFMove(options), WTFMove(port), readable.releaseReturnValue(), WTFMove(readableSource)));

Is there any reason to not have the constructor get the SimpleReadableStreamSource from the ReadableStream?

> Source/WebCore/Modules/mediastream/RTCRtpScriptTransformer.cpp:104
> +            return writable;

I thought this was wrong at first because of the variable name, something like `writableOrException` might make it clearer.

[youenn fablet]

(In reply to Eric Carlson from comment #4)
> Comment on attachment 422821 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=422821&action=review
> 
> > Source/WebCore/ChangeLog:10
> > +        Add support for options parameter provided in RTCRtpScriptTrtansform constructor.
> 
> s/RTCRtpScriptTrtansform/RTCRtpScriptTransform/

OK

> > Source/WebCore/Modules/mediastream/RTCRtpScriptTransformer.cpp:55
> > +    auto transformer = adoptRef(*new RTCRtpScriptTransformer(context, WTFMove(options), WTFMove(port), readable.releaseReturnValue(), WTFMove(readableSource)));
> 
> Is there any reason to not have the constructor get the
> SimpleReadableStreamSource from the ReadableStream?

ReadableStream can take any ReadableStreamSource but we want to keep the fact it is a SimpleReadableStreamSource.

> > Source/WebCore/Modules/mediastream/RTCRtpScriptTransformer.cpp:104
> > +            return writable;
> 
> I thought this was wrong at first because of the variable name, something
> like `writableOrException` might make it clearer.

OK

[youenn fablet]

Created attachment 423043 [details]
Patch for landing

[youenn fablet]

Created attachment 423052 [details]
Win fix try

[youenn fablet]

Don, Hironori-san, can you help me with wincairo failure?
Might be a unified build issue.

[Fujii Hironori]

Thank you for letting me know that. It looks a simple problem. Will check.

[Fujii Hironori]

Created attachment 423080 [details]
Patch

[Fujii Hironori]

It turned out this isn't a simple problem. I'll look into into it again next week.
I added some header inclusions to WebGLRenderingContextBase.h, and fixed recursive inclusion of WebGLTransformFeedback.h.
This patch can compile for WinCairo.
Hmm, but, it can't compile for other ports.

[EWS]

commit-queue failed to commit attachment 423052 [details] to WebKit repository. To retry, please set cq+ flag again.

[EWS]

commit-queue failed to commit attachment 423052 [details] to WebKit repository. To retry, please set cq+ flag again.

[EWS]

Committed r274385: <https://commits.webkit.org/r274385>

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

[Radar WebKit Bug Importer]

<rdar://problem/75392479>