Remove zero-copy path from CreateImageFromVideoFrame [chromium/src : main]
0 views
Skip to first unread message
Vasiliy Telezhnikov (Gerrit)
Oct 17, 2025, 10:37:25 AM (2 days ago) Oct 17
to Colin Blundell, Chromium LUCI CQ, chromium...@chromium.org, Dirk Schulze, CJ DiMeglio, Stephen Chenney, srirama chandra sekhar, blink-rev...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, drott+bl...@chromium.org, eric.c...@apple.com, feature-me...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, kinuko...@chromium.org
Attention needed from Colin Blundell
Vasiliy Telezhnikov added 1 comment![]( "Open in Gerrit")
Patchset-level comments
Related details
Attention is currently required from:
- Colin Blundell
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Colin Blundell (Gerrit)
Oct 17, 2025, 11:21:02 AM (2 days ago) Oct 17
to Vasiliy Telezhnikov, Colin Blundell, Chromium LUCI CQ, chromium...@chromium.org, Dirk Schulze, CJ DiMeglio, Stephen Chenney, srirama chandra sekhar, blink-rev...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, drott+bl...@chromium.org, eric.c...@apple.com, feature-me...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, kinuko...@chromium.org
Attention needed from Vasiliy Telezhnikov
Colin Blundell voted and added 2 comments![]( "Open in Gerrit")
Votes added by Colin Blundell
| Code-Review | +1 |
2 comments
Patchset-level comments
Colin Blundell . resolved
Thanks!
File third_party/blink/renderer/modules/webcodecs/video_frame.cc
Line 1579, Patchset 2 (Parent):
// We disable zero copy images since the ImageBitmap spec says created bitmapsColin Blundell . unresolved
nit: delete this comment
Related details
Attention is currently required from:
- Vasiliy Telezhnikov
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
AI Code Reviewer (Gerrit)
Oct 17, 2025, 2:03:36 PM (2 days ago) Oct 17
to Vasiliy Telezhnikov, Colin Blundell, Chromium LUCI CQ, chromium...@chromium.org, Dirk Schulze, CJ DiMeglio, Stephen Chenney, srirama chandra sekhar, blink-rev...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, drott+bl...@chromium.org, eric.c...@apple.com, feature-me...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, kinuko...@chromium.org
Attention needed from Vasiliy Telezhnikov
AI Code Reviewer added 1 comment![]( "Open in Gerrit")
File third_party/blink/renderer/platform/graphics/video_frame_image_util.h
Line 79, Patchset 3 (Latest):
scoped_refptr<media::VideoFrame> frame,AI Code Reviewer . unresolved
The parameter names `frame`, `resource_provider`, and `video_renderer` are clear from their types and can be omitted from the function declaration in the header file. (Blink Style Guide: Naming - May leave obvious parameter names out of function declarations)
_To keep this interaction as brief and non-intrusive as possible, please consider responding with one of following options:
**Done** | **Won't fix**: reason | **b/<bug_id>** | **Invalid:** reason
This comment was generated by [Experimental Blink C++ Code Review Agent](http://go/blink-c++-code-review-agent).AI reviews can sometimes be inaccurate; We appreciate your 🙏 feedback 🙏 to help us improve.[File a bug](http://go/blink-c++-code-review-agent-feedback) | [Provide feedback on chat](https://chat.google.com/room/AAQA0zhQHe0?cls=4) | [Opt-out](https://ganpati2.corp.google.com/group/peep-genai-blink-agent-optout.prod)_
Related details
Attention is currently required from:
- Vasiliy Telezhnikov
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Vasiliy Telezhnikov (Gerrit)
Oct 17, 2025, 2:04:45 PM (2 days ago) Oct 17
to Dale Curtis, AI Code Reviewer, Colin Blundell, Chromium LUCI CQ, chromium...@chromium.org, Dirk Schulze, CJ DiMeglio, Stephen Chenney, srirama chandra sekhar, blink-rev...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, drott+bl...@chromium.org, eric.c...@apple.com, feature-me...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, kinuko...@chromium.org
Attention needed from Dale Curtis
Vasiliy Telezhnikov voted and added 2 comments![]( "Open in Gerrit")
Votes added by Vasiliy Telezhnikov
| Commit-Queue | +1 |
2 comments
Patchset-level comments
File third_party/blink/renderer/modules/webcodecs/video_frame.cc
Line 1579, Patchset 2 (Parent):
// We disable zero copy images since the ImageBitmap spec says created bitmapsColin Blundell . resolved
Vasiliy Telezhnikovnit: delete this comment
Done
Related details
Attention is currently required from:
- Dale Curtis
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Vasiliy Telezhnikov (Gerrit)
Oct 17, 2025, 2:06:46 PM (2 days ago) Oct 17
to Dale Curtis, AI Code Reviewer, Colin Blundell, Chromium LUCI CQ, chromium...@chromium.org, Dirk Schulze, CJ DiMeglio, Stephen Chenney, srirama chandra sekhar, blink-rev...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, drott+bl...@chromium.org, eric.c...@apple.com, feature-me...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, kinuko...@chromium.org
Attention needed from Dale Curtis
Vasiliy Telezhnikov added 1 comment![]( "Open in Gerrit")
File third_party/blink/renderer/platform/graphics/video_frame_image_util.h
The parameter names `frame`, `resource_provider`, and `video_renderer` are clear from their types and can be omitted from the function declaration in the header file. (Blink Style Guide: Naming - May leave obvious parameter names out of function declarations)
_To keep this interaction as brief and non-intrusive as possible, please consider responding with one of following options:
**Done** | **Won't fix**: reason | **b/<bug_id>** | **Invalid:** reasonThis comment was generated by [Experimental Blink C++ Code Review Agent](http://go/blink-c++-code-review-agent).AI reviews can sometimes be inaccurate; We appreciate your 🙏 feedback 🙏 to help us improve.[File a bug](http://go/blink-c++-code-review-agent-feedback) | [Provide feedback on chat](https://chat.google.com/room/AAQA0zhQHe0?cls=4) | [Opt-out](https://ganpati2.corp.google.com/group/peep-genai-blink-agent-optout.prod)_
Vasiliy Telezhnikov
Won't fix: this CL doesn't change names.
Related details
Attention is currently required from:
- Dale Curtis
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Dale Curtis (Gerrit)
Oct 17, 2025, 2:12:02 PM (2 days ago) Oct 17
to Vasiliy Telezhnikov, AI Code Reviewer, Colin Blundell, Chromium LUCI CQ, chromium...@chromium.org, Dirk Schulze, CJ DiMeglio, Stephen Chenney, srirama chandra sekhar, blink-rev...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, drott+bl...@chromium.org, eric.c...@apple.com, feature-me...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, kinuko...@chromium.org
Attention needed from Vasiliy Telezhnikov
Dale Curtis voted and added 1 comment![]( "Open in Gerrit")
Votes added by Dale Curtis
| Code-Review | +1 |
1 comment
Patchset-level comments
Dale Curtis . resolved
Thanks. Would be nice to get this working, but agree with deleting for now.
Related details
Attention is currently required from:
- Vasiliy Telezhnikov
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Vasiliy Telezhnikov (Gerrit)
Oct 17, 2025, 3:53:49 PM (2 days ago) Oct 17
to Dale Curtis, AI Code Reviewer, Colin Blundell, Chromium LUCI CQ, chromium...@chromium.org, Dirk Schulze, CJ DiMeglio, Stephen Chenney, srirama chandra sekhar, blink-rev...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, drott+bl...@chromium.org, eric.c...@apple.com, feature-me...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, kinuko...@chromium.org
Vasiliy Telezhnikov voted and added 1 comment![]( "Open in Gerrit")
Votes added by Vasiliy Telezhnikov
| Commit-Queue | +2 |
1 comment
Patchset-level comments
Vasiliy Telezhnikov . resolved
Thanks for the reviews.
Related details
Attention set is empty
Chromium LUCI CQ (Gerrit)
Oct 17, 2025, 3:57:06 PM (2 days ago) Oct 17
to Vasiliy Telezhnikov, Dale Curtis, AI Code Reviewer, Colin Blundell, chromium...@chromium.org, Dirk Schulze, CJ DiMeglio, Stephen Chenney, srirama chandra sekhar, blink-rev...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, drott+bl...@chromium.org, eric.c...@apple.com, feature-me...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, kinuko...@chromium.org
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Commit message:
Remove zero-copy path from CreateImageFromVideoFrame
All production use-cases pass non-empty destination rect [1][2][3][4],
so the code is essentially dead. There was few attempts to enable it for
various contexts, but more work is required to make it shippable and it
was in this state since the beginning.
We can revisit this after our shared image refactoring is done, but it's
easier to remove now.
[1]
https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/html/media/html_video_element.cc;drc=c5fe0d8a9bd7720d7c8738130a24ac7597178c87;l=616
[2]
https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/modules/webcodecs/video_frame.cc;drc=c5fe0d8a9bd7720d7c8738130a24ac7597178c87;l=1478
[3]
https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/modules/webcodecs/video_frame.cc;drc=c5fe0d8a9bd7720d7c8738130a24ac7597178c87;l=1584
[4]
https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/modules/webgl/webgl_rendering_context_base.cc;drc=c5fe0d8a9bd7720d7c8738130a24ac7597178c87;l=6616
Change-Id: Ifd9da8d2481ec720361c53abc7f30e2effc195e9
Reviewed-by: Dale Curtis <dalec...@chromium.org>
Reviewed-by: Colin Blundell <blun...@chromium.org>
Commit-Queue: Vasiliy Telezhnikov <vas...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1531607}
Files:
- M third_party/blink/renderer/core/html/media/html_video_element.cc
- M third_party/blink/renderer/modules/webcodecs/video_frame.cc
- M third_party/blink/renderer/modules/webgl/webgl_rendering_context_base.cc
- M third_party/blink/renderer/platform/graphics/video_frame_image_util.cc
- M third_party/blink/renderer/platform/graphics/video_frame_image_util.h
- M third_party/blink/renderer/platform/graphics/video_frame_image_util_test.cc
Change size: M
Delta: 6 files changed, 48 insertions(+), 125 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Dale Curtis, +1 by Colin Blundell
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages