| Summary: | Make worker RTC insertable streams constructs controlled by webrtc insertable streams feature setting | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | youenn fablet <youennf> | ||||||
| Component: | WebRTC | Assignee: | youenn fablet <youennf> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | alex, benjamin, calvaris, cdumez, eric.carlson, esprehn+autocc, ews-watchlist, glenn, hta, jer.noble, kangil.han, kondapallykalyan, philipj, sam, sergio, tommyw, webkit-bug-importer, youennf | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | WebKit Local Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Bug Depends on: | 219688 | ||||||||
| Bug Blocks: | 219890 | ||||||||
| Attachments: |
|
||||||||
|
Description
youenn fablet
2020-11-23 01:41:37 PST
Created attachment 414796 [details]
Patch
Ping review. Thank you so much for working on making this part of Settings better! Would you mind breaking out the change to Settings into its own patch? I think it would be good to look at it separately. (In reply to Sam Weinig from comment #4) > Thank you so much for working on making this part of Settings better! Would > you mind breaking out the change to Settings into its own patch? I think it > would be good to look at it separately. Sure. Do you have any preliminary comment on the Settings part of the patch? I for instance kept it as a subclass of Settings but contemplated to move it to its own class. Or maybe you have some other idea? (In reply to youenn fablet from comment #5) > (In reply to Sam Weinig from comment #4) > > Thank you so much for working on making this part of Settings better! Would > > you mind breaking out the change to Settings into its own patch? I think it > > would be good to look at it separately. > > Sure. > Do you have any preliminary comment on the Settings part of the patch? > I for instance kept it as a subclass of Settings but contemplated to move it > to its own class. > Or maybe you have some other idea? Subobject seems fine, though I am not sure if it makes sense to constrain it to just the boolean values. I think I would prefer doing a deep (thread safe) copy of all the Settings if possible, just to keep things relatively normalized. (In reply to Sam Weinig from comment #6) > (In reply to youenn fablet from comment #5) > > (In reply to Sam Weinig from comment #4) > > > Thank you so much for working on making this part of Settings better! Would > > > you mind breaking out the change to Settings into its own patch? I think it > > > would be good to look at it separately. > > > > Sure. > > Do you have any preliminary comment on the Settings part of the patch? > > I for instance kept it as a subclass of Settings but contemplated to move it > > to its own class. > > Or maybe you have some other idea? > > Subobject seems fine, though I am not sure if it makes sense to constrain it > to just the boolean values. I think I would prefer doing a deep (thread > safe) copy of all the Settings if possible, just to keep things relatively > normalized. I used the boolean heuristic as a way to restrict to the feature flags that are necessary when creating the global scope. I thought a bit about extending that but it did not seem useful right now so ended up not doing it. (In reply to youenn fablet from comment #7) > (In reply to Sam Weinig from comment #6) > > (In reply to youenn fablet from comment #5) > > > (In reply to Sam Weinig from comment #4) > > > > Thank you so much for working on making this part of Settings better! Would > > > > you mind breaking out the change to Settings into its own patch? I think it > > > > would be good to look at it separately. > > > > > > Sure. > > > Do you have any preliminary comment on the Settings part of the patch? > > > I for instance kept it as a subclass of Settings but contemplated to move it > > > to its own class. > > > Or maybe you have some other idea? > > > > Subobject seems fine, though I am not sure if it makes sense to constrain it > > to just the boolean values. I think I would prefer doing a deep (thread > > safe) copy of all the Settings if possible, just to keep things relatively > > normalized. > > I used the boolean heuristic as a way to restrict to the feature flags that > are necessary when creating the global scope. > I thought a bit about extending that but it did not seem useful right now so > ended up not doing it. There are a number of non-bool settings that will be useful in workers eventually, usually around maximum on minimum sizes (e.g. maximumAccelerated2dCanvasSize for offscreen canvas at some point, etc). Created attachment 416147 [details]
Rebasing
Comment on attachment 416147 [details]
Rebasing
r=me too.
Committed r270842: <https://trac.webkit.org/changeset/270842> All reviewed patches have been landed. Closing bug and clearing flags on attachment 416147 [details]. |