Bug 228947 - Implement SFrameTransform error handling
Summary: Implement SFrameTransform error handling
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebRTC (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: youenn fablet
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-08-10 02:53 PDT by youenn fablet
Modified: 2021-08-12 15:28 PDT (History)
19 users (show)

See Also:


Attachments
Patch (48.16 KB, patch)
2021-08-10 06:46 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (53.34 KB, patch)
2021-08-10 08:53 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 2021-08-10 02:53:44 PDT
Implement SFrameTransform error handling
Comment 1 youenn fablet 2021-08-10 06:46:04 PDT
Created attachment 435253 [details]
Patch
Comment 2 youenn fablet 2021-08-10 08:53:59 PDT
Created attachment 435262 [details]
Patch
Comment 3 youenn fablet 2021-08-11 07:02:40 PDT
Comment on attachment 435262 [details]
Patch

bot error is unrelated
Comment 4 Eric Carlson 2021-08-11 10:11:45 PDT
Comment on attachment 435262 [details]
Patch

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

> Source/WebCore/Modules/mediastream/RTCRtpSFrameTransform.cpp:118
> +static std::optional<Vector<uint8_t>> processFrame(const uint8_t* data, size_t size, RTCRtpSFrameTransformer& transformer, ScriptExecutionContextIdentifier identifier, const WeakPtr<RTCRtpSFrameTransform>& weakTransform)

It might be worth using Span here and the other places you pass around { uint8_t*, size_t } in these classes.
Comment 5 youenn fablet 2021-08-12 02:14:57 PDT
(In reply to Eric Carlson from comment #4)
> Comment on attachment 435262 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=435262&action=review
> 
> > Source/WebCore/Modules/mediastream/RTCRtpSFrameTransform.cpp:118
> > +static std::optional<Vector<uint8_t>> processFrame(const uint8_t* data, size_t size, RTCRtpSFrameTransformer& transformer, ScriptExecutionContextIdentifier identifier, const WeakPtr<RTCRtpSFrameTransform>& weakTransform)
> 
> It might be worth using Span here and the other places you pass around {
> uint8_t*, size_t } in these classes.

Good idea, I'll do the refactoring in a follow-up as this entails changes to transformer and frame classes.
Comment 6 EWS 2021-08-12 02:37:14 PDT
Committed r280958 (240464@main): <https://commits.webkit.org/240464@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 435262 [details].
Comment 7 Radar WebKit Bug Importer 2021-08-12 02:38:17 PDT
<rdar://problem/81837525>
Comment 8 youenn fablet 2021-08-12 02:46:32 PDT
> Good idea, I'll do the refactoring in a follow-up as this entails changes to
> transformer and frame classes.

https://bugs.webkit.org/show_bug.cgi?id=229029