[Region Capture][cropTo][#6] Plumb crop message to Viz [chromium/src : main]
Elad Alon (Gerrit)
Attention is currently required from: Daniel Cheng, Avi Drissman, Jordan Bayles, mark a. foltz.
Elad Alon would like Daniel Cheng and Avi Drissman to review this change.
[Region Capture][cropTo][#6] Plumb crop message to Viz
Overall feature:
----------------
Region Capture is a feature allowing the cropping of self-capture video
tracks to track the coordinates of an HTMLElement in the browsing
context.
Public design-doc:
https://docs.google.com/document/d/1dULARMnMZggfWqa_Ti_GrINRNYXGIli3XK9brzAKEV8/edit?usp=sharing
Spec draft (very WIP): https://eladalon1983.github.io/region-capture/
(This will be finished soon and moved to the WICG; hopefully to be
eventually adopted by the W3C.)
This CL sub-chain:
------------------
The Region Capture feature introduces two new APIs:
* navigator.mediaDevices.produceCropId()
* BrowserCaptureMediaStreamTrack.cropTo()
This CL sub-chain deals with the latter.
This CL:
--------
Plumb the crop message (and callback) all the
way to FrameSinkVideoCaptureDevice.
Next CLs:
---------
* Invoke the callback with a rejection message if not self-capture.
* Send the message onwards to viz::ClientFrameSinkVideoCapturer.
Bug: 1247761
Change-Id: Id86be452ab19aad39ba71a622cd9a689be481d84
---
M content/public/browser/video_capture_device_launcher.h
M content/browser/renderer_host/media/mock_video_capture_provider.h
M content/browser/renderer_host/media/video_capture_manager.h
M media/capture/video/video_capture_device.cc
M media/capture/BUILD.gn
M media/capture/video/video_capture_device.h
M content/browser/renderer_host/media/service_launched_video_capture_device.h
M content/browser/media/capture/frame_sink_video_capture_device.h
M content/browser/renderer_host/media/video_capture_controller.h
M content/browser/media/capture/frame_sink_video_capture_device.cc
M content/browser/renderer_host/media/video_capture_manager.cc
M content/browser/renderer_host/media/video_capture_host.cc
M content/browser/renderer_host/media/in_process_launched_video_capture_device.h
M content/browser/renderer_host/media/service_launched_video_capture_device.cc
M content/browser/renderer_host/media/in_process_launched_video_capture_device.cc
M content/browser/renderer_host/media/fake_video_capture_device_launcher.cc
M third_party/blink/renderer/modules/mediastream/browser_capture_media_stream_track.cc
M media/capture/mojom/video_capture_types.mojom
M components/mirroring/browser/single_client_video_capture_host_unittest.cc
M content/browser/renderer_host/media/video_capture_controller.cc
20 files changed, 207 insertions(+), 4 deletions(-)
To view, visit change 3232118. To unsubscribe, or for help writing mail filters, visit settings.
Elad Alon (Gerrit)
Attention is currently required from: Daniel Cheng, Avi Drissman, Jordan Bayles, mark a. foltz.
1 comment:
Patchset:
PTAL
Adding owners as follows:
mfoltz:
*
jophba:
*
avi:
content/browser/renderer_host/media/fake_video_capture_device_launcher.cc
content/browser/renderer_host/media/in_process_launched_video_capture_device.cc
content/browser/renderer_host/media/in_process_launched_video_capture_device.h
content/browser/renderer_host/media/mock_video_capture_provider.h
content/browser/renderer_host/media/service_launched_video_capture_device.cc
content/browser/renderer_host/media/service_launched_video_capture_device.h
content/browser/renderer_host/media/video_capture_controller.cc
content/browser/renderer_host/media/video_capture_controller.h
content/browser/renderer_host/media/video_capture_host.cc
content/browser/renderer_host/media/video_capture_manager.cc
content/browser/renderer_host/media/video_capture_manager.h
content/public/browser/video_capture_device_launcher.h
dcheng:
media/capture/mojom/video_capture_types.mojom
To view, visit change 3232118. To unsubscribe, or for help writing mail filters, visit settings.
Avi Drissman (Gerrit)
Attention is currently required from: Daniel Cheng, Jordan Bayles, Elad Alon, mark a. foltz.
Patch set 2:Code-Review +1
Jordan Bayles (Gerrit)
Attention is currently required from: Daniel Cheng, Elad Alon, mark a. foltz.
Patch set 2:Code-Review +1
1 comment:
File media/capture/video/video_capture_device.cc:
Patch Set #2, Line 66:
base::Token crop_id,Should we use a RegionCaptureCropId alias here?
To view, visit change 3232118. To unsubscribe, or for help writing mail filters, visit settings.
mark a. foltz (Gerrit)
Attention is currently required from: Daniel Cheng, Elad Alon.
6 comments:
Patchset:
Sending partial comments. I may have time to get through the other files later tonight.
File content/browser/media/capture/frame_sink_video_capture_device.h:
Patch Set #2, Line 24:
#include "media/capture/mojom/video_capture_types.mojom.h"Is this necessary?
File content/browser/media/capture/frame_sink_video_capture_device.cc:
Patch Set #2, Line 19:
#include "base/token.h"Duplicate #include
Patch Set #2, Line 188:
// TOOD(crbug.com/1247761): Reject if not self-capture.Nit: Typo in TODO.
The FSVCD doesn't really know about tabs (WebContents), as it's also used for Aura windows. So, this TODO probably belongs in WebContentsVideoCaptureDevice.
All of the logic that deals with crop IDs should probably live there as well now that I think about it.
Patch Set #2, Line 190:
// TODO(crbug.com/1247761): Forward this onwards to Viz.Is this not implemented in this patch because the new mojo API for the crop ID is not exposed yet through components/viz/host?
File content/browser/renderer_host/media/in_process_launched_video_capture_device.cc:
Patch Set #2, Line 126:
device_task_runner_->PostTask(base::PostTaskAndReplyWithResult?
To view, visit change 3232118. To unsubscribe, or for help writing mail filters, visit settings.
mark a. foltz (Gerrit)
Attention is currently required from: Daniel Cheng, Jordan Bayles, Elad Alon.
1 comment:
File media/capture/video/video_capture_device.cc:
Patch Set #2, Line 66:
base::Token crop_id,Should we use a RegionCaptureCropId alias here?
Last I checked this was declared in viz/, which isn't allowed to be used in media/. Maybe we should move the alias to e.g. media/base so it can be used in media/, viz/ and content/. (As a post-launch cleanup.)
To view, visit change 3232118. To unsubscribe, or for help writing mail filters, visit settings.
mark a. foltz (Gerrit)
Attention is currently required from: Daniel Cheng, Jordan Bayles, Elad Alon.
3 comments:
Patchset:
It looks like the Crop() API will only really work on WebContentsVideoCaptureDevice, yet it must be implemented across every capture device. Is there a shared base class where you could put a no-op implementation?
File content/browser/renderer_host/media/mock_video_capture_provider.h:
Patch Set #2, Line 8:
#include "base/callback.h"Here and elsehwere: Use base/callback_forward.h if you don't need the full definition of the base::Callback templates.
File content/browser/renderer_host/media/service_launched_video_capture_device.cc:
Patch Set #2, Line 85:
// TODO(crbug.com/1247761): Implement.Any idea how to implement this? I don't see anything this object owns that would support CropTo().
To view, visit change 3232118. To unsubscribe, or for help writing mail filters, visit settings.
Elad Alon (Gerrit)
Attention is currently required from: Daniel Cheng, Jordan Bayles, mark a. foltz.
9 comments:
Patchset:
It looks like the Crop() API will only really work on WebContentsVideoCaptureDevice, yet it must be […]
There are two inheritance trees that are relevant, AFAICT:
1. VideoCaptureDevice has 14 sub-classes. There, I did have a default implementation that returns kUnsupportedCaptureDevice.
2 LaunchedVideoCaptureDevice has 4 sub-classes, but only 2 of which are non-mocks. There it's reasonable to keep it pure virtual. Or wdyt?
File content/browser/media/capture/frame_sink_video_capture_device.h:
Patch Set #2, Line 24:
#include "media/capture/mojom/video_capture_types.mojom.h"Is this necessary?
I think so, based on this:
https://google.github.io/styleguide/cppguide.html#Include_What_You_Use
"Do not rely on transitive inclusions."
File content/browser/media/capture/frame_sink_video_capture_device.cc:
Patch Set #2, Line 19:
#include "base/token.h"Duplicate #include
Beg pardon? Where's the other?
Patch Set #2, Line 188:
// TOOD(crbug.com/1247761): Reject if not self-capture.
Nit: Typo in TODO. […]
Typo fixed.
This CL introduces plumbing that intends to take the cropTo message all the way to Viz. No logic is introduced by this CL. I can either remove this TODO completely or move it, but if I move it, I'd prefer to do so to another file in the current CL. Wdyt?
Patch Set #2, Line 190:
// TODO(crbug.com/1247761): Forward this onwards to Viz.
Is this not implemented in this patch because the new mojo API for the crop ID is not exposed yet th […]
The next CL in the chain will pick up where this TODO stopped, forwarding the message on to viz::ClientFrameSinkVideoCapturer.
File content/browser/renderer_host/media/in_process_launched_video_capture_device.cc:
Patch Set #2, Line 126:
device_task_runner_->PostTask(base::PostTaskAndReplyWithResult?
IIANM, that would require changing VideoCaptureDevice::Crop() to return media::mojom::CropRequestResult synchronously. That'd mean some Crop() methods in the CL-chain will return synchronously, and some will be Crop(id, callback). I prefer the uniformity. (It bears mentioning that a process hop over to Viz is upcoming in the next CLs in the chain.) Wdyt?
File content/browser/renderer_host/media/mock_video_capture_provider.h:
Patch Set #2, Line 8:
#include "base/callback.h"
Here and elsehwere: Use base/callback_forward. […]
Done
File content/browser/renderer_host/media/service_launched_video_capture_device.cc:
Patch Set #2, Line 85:
// TODO(crbug.com/1247761): Implement.Any idea how to implement this? I don't see anything this object owns that would support CropTo().
I am not yet sure what this really is, so I want to keep it kNotImplemented for the MVP, as the InProcess variant seems the only relevant one. I might need to rephrase this TODO as "research and either implement or leave intentionally unimplemented." Wdyt?
File media/capture/video/video_capture_device.cc:
Patch Set #2, Line 66:
base::Token crop_id,
Last I checked this was declared in viz/, which isn't allowed to be used in media/. […]
+1 for post-launch cleanup of the alias.
To view, visit change 3232118. To unsubscribe, or for help writing mail filters, visit settings.
mark a. foltz (Gerrit)
Attention is currently required from: Daniel Cheng, Jordan Bayles, Elad Alon.
5 comments:
Patchset:
There are two inheritance trees that are relevant, AFAICT: […]
That seems reasonable. 14 subclasses is a lot!
File content/browser/media/capture/frame_sink_video_capture_device.cc:
Patch Set #2, Line 19:
#include "base/token.h"
Beg pardon? Where's the other?
frame_sink_video_capture_device.h
Patch Set #2, Line 188:
// TOOD(crbug.com/1247761): Reject if not self-capture.
Typo fixed. […]
The right file, if I understand your TODOs correctly, is web_contents_video_capture_device.cc. That is where you would check for self-capture.
You could pass the CropId to Viz here, but then a caller would be able to set a CropId when capturing Aura windows, which would either be a no-op (best case scenario) or an invalid use of the API and CHECK-fail somewhere down the line. Maybe Jordan has an opinion as to whether this use-case is okay.
File content/browser/renderer_host/media/in_process_launched_video_capture_device.cc:
Patch Set #2, Line 126:
device_task_runner_->PostTask(
IIANM, that would require changing VideoCaptureDevice::Crop() to return media::mojom::CropRequestRes […]
What is "IIANM"?
base::PostTaskAndReplyWithResult takes two callbacks: one to run the task, and the other to consume the reply on the caller's thread. No synchronous behavior is required.
Examples here:
https://chromium.googlesource.com/chromium/src/+/refs/heads/main/docs/threading_and_tasks.md#keeping-the-browser-responsive
File content/browser/renderer_host/media/service_launched_video_capture_device.cc:
Patch Set #2, Line 85:
// TODO(crbug.com/1247761): Implement.
I am not yet sure what this really is, so I want to keep it kNotImplemented for the MVP, as the InPr […]
That's fine. Maybe update the TODO to "Implement if necessary".
To view, visit change 3232118. To unsubscribe, or for help writing mail filters, visit settings.
Elad Alon (Gerrit)
Attention is currently required from: Daniel Cheng, Jordan Bayles, mark a. foltz.
4 comments:
File content/browser/media/capture/frame_sink_video_capture_device.cc:
Patch Set #2, Line 19:
#include "base/token.h"
frame_sink_video_capture_device. […]
Transitive #include-s are discouraged. (https://chromium-review.googlesource.com/c/chromium/src/+/3232118/comments/07d5a594_808771e4)
It specifies: "This also applies to related headers - foo.cc should include bar.h if it uses a symbol from it even if foo.h includes bar.h."
Patch Set #2, Line 188:
// TOOD(crbug.com/1247761): Reject if not self-capture.
The right file, if I understand your TODOs correctly, is web_contents_video_capture_device.cc. […]
- Windows would not have an attached crop-IDs, I believe...?
- Please note that longer term, we do want to support cropping to a capture of a different tab. It's just not part of this launch. The functionality of rejecting non-self-capture is intended to be temporary. (We need additional Privacy+Security approvals to add that functionality, but no additional logic - in fact, less logic.)
- Since web_contents_video_capture_device.cc is not currently part of this CL, and since I am not 100% sure if we'd end up special-casing self-capture through that file or otherwise, I'd prefer to either remove the TODO completely, or plug it into any of the files that are already part of this CL (e.g. this file). Wdyt?
File content/browser/renderer_host/media/in_process_launched_video_capture_device.cc:
Patch Set #2, Line 126:
device_task_runner_->PostTask(
What is "IIANM"? […]
IIANM = if I am not mistaken
Thanks, I am familiar with PostTaskAndReplyWithResult. I was saying the following:
* Currently, almost all of the X::Crop methods return void, and take on the same two parameters. So:
void Crop(Token, Callback<void(CropRequestResult)>) // All
* If I start using PostTaskAndReplyWithResult, some X::Crop methods would retain the old shape, but some would be of this shape:
CropRequestResult Crop(Token) // Some
void Crop(Token, Callback<void(CropRequestResult)>) // Some
This style inconsistency seems to me unfavorable. I wanted to run this consideration by you before accepting your suggestion. Wdyt?
File content/browser/renderer_host/media/service_launched_video_capture_device.cc:
Patch Set #2, Line 85:
// TODO(crbug.com/1247761): Implement.
That's fine. Maybe update the TODO to "Implement if necessary".
Done
To view, visit change 3232118. To unsubscribe, or for help writing mail filters, visit settings.
mark a. foltz (Gerrit)
Attention is currently required from: Daniel Cheng, Jordan Bayles, Elad Alon.
Patch set 4:Code-Review +1
3 comments:
File content/browser/media/capture/frame_sink_video_capture_device.cc:
Patch Set #2, Line 19:
#include "base/token.h"
Transitive #include-s are discouraged. (https://chromium-review.googlesource. […]
While this is correct according to the reading of the guide, in this case the .cc will always get this header because Token is used in a declaration, and there's no risk of transitive breakage since .ccs aren't included. I don't bother with these sort of redundant includes, personally.
Patch Set #2, Line 188:
// TOOD(crbug.com/1247761): Reject if not self-capture.
* Windows would not have an attached crop-IDs, I believe...? […]
Now that I think about it, you could add an API here to determine compatibility with cropping (i.e. SupportsRegionCapture()) that you override in WCVCD and AWVCD.
AWVCD would hard-return false and WCVDC would check for self-capture or whatever else it needs to do. I kind of like that because it allows us to keep the ClientFrameSinkVideoCapturer private here.
If that is what you had in mind, then no concerns, and sorry for the back and forth.
File content/browser/renderer_host/media/in_process_launched_video_capture_device.cc:
Patch Set #2, Line 126:
device_task_runner_->PostTask(
IIANM = if I am not mistaken […]
Ah, I see your point now, in this case you want to consistently pass a continuation callback between the two threads. I'm not aware of a helper method in base:: for this particular pattern.
To view, visit change 3232118. To unsubscribe, or for help writing mail filters, visit settings.
mark a. foltz (Gerrit)
Attention is currently required from: Daniel Cheng, Jordan Bayles, Elad Alon.
Patch set 10:Code-Review +1
1 comment:
File media/capture/mojom/video_capture_types.mojom:
Patch Set #10, Line 363:
kUnsupportedCaptureDevice,In the interest in landing this more quickly, would it be possible to use kErrorGeneric for the time being and add this new enum value in a later patch?
To view, visit change 3232118. To unsubscribe, or for help writing mail filters, visit settings.
Elad Alon (Gerrit)
Attention is currently required from: Daniel Cheng, Jordan Bayles.
1 comment:
File media/capture/mojom/video_capture_types.mojom:
Patch Set #10, Line 363:
kUnsupportedCaptureDevice,
In the interest in landing this more quickly, would it be possible to use kErrorGeneric for the time […]
Definitely, but I am blocked on dcheng@ for the review of the underlying CL (#5) anyway. Tomorrow morning:
- If I see that he's approved both CLs - great, no change needed in this CL.
- If he's approved #5 but not this one (#6), I'll make the change you've just suggested in order to remove this file from the CL.
- If he's approved neither - we'll see, depending on what made him not approve.
To view, visit change 3232118. To unsubscribe, or for help writing mail filters, visit settings.
Daniel Cheng (Gerrit)
Attention is currently required from: Jordan Bayles, Elad Alon.
3 comments:
File content/browser/renderer_host/media/in_process_launched_video_capture_device.cc:
Patch Set #10, Line 125:
// guaranteed to run before the task that destroys the |device|.base::SequenceBound might be helpful here for simplifying things.
File content/browser/renderer_host/media/video_capture_manager.cc:
Patch Set #10, Line 520:
IsControllerPointerValidIt's kind of unclear to me what this check is trying to guard against.
If it's trying to protect against null pointers (which is what the NOTREACHED() on line 502 seems to indicate), then a null pointer check would be clearer.
If it's trying to protect against an already deleted controller, this seems inherently dangerous/racy if there's any post tasks involved (the address controller points to could have been reused)
If it's trying to protect against a controller that's not in controllers_, it woudl be helpful for there to be some clarity about why this might be the case.
Do you know which it is here?
File media/capture/video/video_capture_device.cc:
Patch Set #10, Line 7:
#include "base/callback_forward.h"Nit: callback.h (we use the callback below, so a forward declaration won't suffice)
To view, visit change 3232118. To unsubscribe, or for help writing mail filters, visit settings.
Elad Alon (Gerrit)
Attention is currently required from: Daniel Cheng, Jordan Bayles.
3 comments:
File content/browser/renderer_host/media/in_process_launched_video_capture_device.cc:
Patch Set #10, Line 125:
// guaranteed to run before the task that destroys the |device|.base::SequenceBound might be helpful here for simplifying things.
Good for a follow-up. I see you've posted this as resolved, so I assume that's your intention.
File content/browser/renderer_host/media/video_capture_manager.cc:
Patch Set #10, Line 520:
IsControllerPointerValid
It's kind of unclear to me what this check is trying to guard against. […]
IsControllerPointerValid() does not seem to check for nullness, only for membership in |controllers_|. I do not know who named it this way or why the code on line 502 uses a misleading message. I think these are good things to follow up on separately. These are pre-existing issues in the class.
The current CL checks for membership in |controllers_|. It does so using IsControllerPointerValid() because that's the idiomatic thing to do in the context of this class. |controller| might be missing from |controllers_| if it's been asynchronously removed. It's not immediately obvious to me why some methods in this class are confident enough things should not happen, and use NOTREACHED, while others (e.g. method below) treat this as a viable possibility. I think it's safest and clearest to handle this with a soft failure.
File media/capture/video/video_capture_device.cc:
Patch Set #10, Line 7:
#include "base/callback_forward.h"
Nit: callback. […]
Done
To view, visit change 3232118. To unsubscribe, or for help writing mail filters, visit settings.
Jordan Bayles (Gerrit)
Attention is currently required from: Daniel Cheng, Elad Alon.
Patch set 13:Code-Review +1
1 comment:
Patchset:
Fixed up the merge conflict, should be good for another look.
@dcheng, PTAL.
To view, visit change 3232118. To unsubscribe, or for help writing mail filters, visit settings.
Daniel Cheng (Gerrit)
Attention is currently required from: Elad Alon.
1 comment:
File content/browser/renderer_host/media/video_capture_manager.cc:
Patch Set #10, Line 520:
IsControllerPointerValid
IsControllerPointerValid() does not seem to check for nullness, only for membership in |controllers_|.
Well, it does, but perhaps as an unintended side effect. Presumably nullptr is never in controllers_.
The real issue is some of this work is triggered via IPC, right? Therefore, it's important to rule out #2 above (which would be a potentially exploitable memory safety issue), but this requires understanding the rationale behind the original check.
To view, visit change 3232118. To unsubscribe, or for help writing mail filters, visit settings.
Elad Alon (Gerrit)
Attention is currently required from: Daniel Cheng, Jordan Bayles.
1 comment:
File content/browser/renderer_host/media/video_capture_manager.cc:
Patch Set #10, Line 520:
IsControllerPointerValid
> IsControllerPointerValid() does not seem to check for nullness, only for membership in |controller […]
Line 518 has DCHECK(controller), as do pre-existing methods of this class that NOTREACHED after checking IsControllerPointerValid(). I think it's very unlikely that someone would push nullptr into |controllers_|, so our code would soft-fail here even if we go past the DCHECK - which in Release we would - and I don't see anything to worry about.
To view, visit change 3232118. To unsubscribe, or for help writing mail filters, visit settings.
Daniel Cheng (Gerrit)
Attention is currently required from: Jordan Bayles, Elad Alon.
1 comment:
File content/browser/renderer_host/media/video_capture_manager.cc:
Patch Set #10, Line 520:
IsControllerPointerValid
Line 518 has DCHECK(controller), as do pre-existing methods of this class that NOTREACHED after chec […]
I'm not worried about a null deref. That's not considered a security issue.
I'm concerned that this is trying to check for a pointer to a deleted controller somehow—if the memory address is reused for a new controller allocation at the same address, then this is potentially unsafe.
To view, visit change 3232118. To unsubscribe, or for help writing mail filters, visit settings.
Elad Alon (Gerrit)
Attention is currently required from: Daniel Cheng, Jordan Bayles.
1 comment:
File content/browser/renderer_host/media/video_capture_manager.cc:
Patch Set #10, Line 520:
IsControllerPointerValid
I'm not worried about a null deref. That's not considered a security issue. […]
I've bypassed VideoCaptureManager. PTAL.
(Follow-up bug: crbug.com/1264113)
To view, visit change 3232118. To unsubscribe, or for help writing mail filters, visit settings.
Elad Alon (Gerrit)
Attention is currently required from: Daniel Cheng, Jordan Bayles.
Patch set 16:Auto-Submit +1
Daniel Cheng (Gerrit)
Attention is currently required from: Jordan Bayles, Elad Alon.
1 comment:
File content/browser/renderer_host/media/video_capture_host.cc:
Patch Set #16, Line 313:
// thereby rejecting (a) unknown crop-IDs and (b) other-tab-crops.Sorry, one last question here: is it benign to specify the crop ID for another tab? It can't lead to the capture somehow capturing the wrong content, because looking up the crop region would fail right?
To view, visit change 3232118. To unsubscribe, or for help writing mail filters, visit settings.
Elad Alon (Gerrit)
Attention is currently required from: Daniel Cheng, Jordan Bayles.
1 comment:
File content/browser/renderer_host/media/video_capture_host.cc:
Patch Set #16, Line 313:
// thereby rejecting (a) unknown crop-IDs and (b) other-tab-crops.
Sorry, one last question here: is it benign to specify the crop ID for another tab? It can't lead to […]
This is all behind a command-line flag, and we don't intend to remove the command-line flag until this TODO is removed (at which point we'll go into OT - which has been approved). As of now, the code for actual cropping is in another CL, also under review, so right now nothing will happen.
The intention for the final, put-together work that is targeting m97:
- Looking up a crop-ID originating from another tab fails with the same failure as an arbitrarily randomized crop-ID which was never assigned by the browser.
- Looking up a crop-ID originating from the current tab crops - assuming the capture *is* of the current tab.
The intention for follow-up work, in several months, subject to its own launch review, etc., is:
- Crop-IDs may be applied even cross-tab; they just need to be associated with the *captured-tab*. That is to say, if A captures B, then only B's crop IDs are legal, regardless of whether A==B or A!=B.
To view, visit change 3232118. To unsubscribe, or for help writing mail filters, visit settings.
Daniel Cheng (Gerrit)
Attention is currently required from: Jordan Bayles, Elad Alon.
Patch set 16:Code-Review +1Commit-Queue +2
1 comment:
Patchset:
LGTM. I looked at the next patchset briefly as well and made a few comments there.
To view, visit change 3232118. To unsubscribe, or for help writing mail filters, visit settings.
Elad Alon (Gerrit)
Attention is currently required from: Jordan Bayles.
Patch set 16:Commit-Queue +2
1 comment:
Patchset:
LGTM. I looked at the next patchset briefly as well and made a few comments there.
Thanks!
To view, visit change 3232118. To unsubscribe, or for help writing mail filters, visit settings.
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted this change.
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3232118
Auto-Submit: Elad Alon <elad...@chromium.org>
Reviewed-by: Daniel Cheng <dch...@chromium.org>
Reviewed-by: Jordan Bayles <jop...@chromium.org>
Reviewed-by: mark a. foltz <mfo...@chromium.org>
Reviewed-by: Avi Drissman <a...@chromium.org>
Commit-Queue: Daniel Cheng <dch...@chromium.org>
Commit-Queue: Elad Alon <elad...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#936048}
---
M content/public/browser/video_capture_device_launcher.h
M content/browser/renderer_host/media/mock_video_capture_provider.h
M media/capture/video/video_capture_device.cc
M media/capture/BUILD.gn
M media/capture/video/video_capture_device.h
M content/browser/renderer_host/media/service_launched_video_capture_device.h
M content/browser/media/capture/frame_sink_video_capture_device.h
M content/browser/renderer_host/media/video_capture_controller.h
M content/browser/media/capture/frame_sink_video_capture_device.cc
M content/browser/renderer_host/media/video_capture_host.cc
M content/browser/renderer_host/media/in_process_launched_video_capture_device.h
M content/browser/renderer_host/media/service_launched_video_capture_device.cc
M content/browser/renderer_host/media/in_process_launched_video_capture_device.cc
M content/browser/renderer_host/media/fake_video_capture_device_launcher.cc
M third_party/blink/renderer/modules/mediastream/browser_capture_media_stream_track.cc
M media/capture/mojom/video_capture_types.mojom
M components/mirroring/browser/single_client_video_capture_host_unittest.cc
M content/browser/renderer_host/media/video_capture_controller.cc
18 files changed, 192 insertions(+), 4 deletions(-)