Brave, do you k now if we track rotation for desktop capture?

No reproduction on my Win10 laptop(Chrome M67&69) and Chromebook Nautilus(Chrome M68).

Some questions:
- Is it only happen to you on ChromeOS?
- What's the model of your chromebook? Is it device specific issue?
- Does it happen with other Chrome version? Is it a new regression?

Oh, actually the orientation of Chrome itself is wrong too, in your attachment. Screen orientation is locked? I suppose that's the reason.

> - Is it only happen to you on ChromeOS?

I've only tested this on Chromebooks.

> - What's the model of your chromebook? Is it device specific issue?

Reproduced on Pixelbook and Lux.

> - Does it happen with other Chrome version? Is it a new regression?

I've been using the canary channel for the two Chromebooks I mentioned.
I haven't tested this on an older version.


> Oh, actually the orientation of Chrome itself is wrong too, in your attachment. Screen orientation is locked? I suppose that's the reason.

I was holding the device in the portrait mode, so I think the orientation was correct.

The screen orientation wasn't locked (i.e. the screen content rotated automatically based on sensors of the device)

Seems that I can reproduce this on my Nexus Chromebook with 68.0.3440.4 (Official Build) dev (64-bit), but not with Chrome M67 Beta Channel. So it should be a recent regression. Will take a look tomorrow.

satorux@, please help to verify if Stable/Beta channel works on your chromebooks.

Tested it on Pixelbook with the following version:

  Chrome Version       : 67.0.3396.67
  OS Version: 10575.51.0

The screen recording in the portrait mode worked as shown in the attached video.

Also screen sharing via go/present worked with the correct orientation, in the portrait mode as well.

Deleted: test.webm 422 KB

	test.webm 422 KB View Download

miu@, the cl https://chromium.googlesource.com/chromium/src/+/45ed65eaab60ab99686c7e9ea9a8b052bd85cd2c causes another issue on ChromeOS.
And it still happens with ToT #563399, 69.3447.

Hmm...I've examined both the old and the new capture code paths. This shouldn't have changed. Maybe there was a separate change within the display compositor, or in the video stack downstream, around the same time that changed some assumptions. I'll keep looking.

Based on discussions with the various teams, it seems that what has changed is the following: In the old window capture, we were executing copy requests on the content layer (the child of the root layer of the layer tree). In the new window capture, capture can only target whole surfaces (represented by a FrameSinkId), which effectively means the the root layer of the layer tree is being captured. The child of the root layer is the pre-rotated content, whereas the root layer is a -90 degree rotation for the benefit of the screen hardware.

I'm analyzing our options now. It seems two solutions might be:

1. Modify the Ash screen rotator to lock the device in landscape mode while desktop capture is active.

2. (Sort of a hack) Modify FrameSinkVideoCapture to detect capture of the root surface that has it's final composite operation doing a -90 degree rotation, and capture the non-rotated child layer instead. This assumes the layer is available in VIZ (it might not be, I'm not sure yet). Also, this hack would have to be propagated through a lot of code modules since all size/scaling values would have to know to flip their X and Y coordinates for this use case.

From user perspective, #1 is probably a better choice (and it's also probably going to be much simpler in terms of design/code changes). Generally, desktop video capture is set up to capture wider-than-tall size video; and such video is usually displayed on a wider-than-tall size screen (e.g., VC equipment, projector in a lecture hall, etc.). So, IMHO, it makes a lot of sense to lock the source screen in landscape mode: Doing so would improve quality, reduce letterboxing/pillarboxing, less drastic scaling of content on the receiver, etc.

Thank you for the investigation. Tablet-like Chromebooks are often/regularly used in the portrait mode so I think losing the ability of recording/sharing in the portrait mode is unfortunate for these devices.

Agreed with comment #10, we should avoid option 1 as it will be confusing to the user and also does not work on devices like Dru, where the device native orientation is portrait.

How does this impact casting? 

> How does this impact casting? 

As mentioned in the original post, the orientation was wrong in the same way when I presented the whole screen via go/present.

 Issue 862026  has been merged into this issue.

To be honest, logistics are also a concern here. I'm not sure who is available to work on this issue. I could assist with pointers and code reviews, if someone would like to step-up to help work on this.

I looked a bit deeper at the code: Basically, the compositor isn't aware of display rotation. It's entirely a UI abstraction/concept. Meaning, it's Ash code that is controlling and applying a 90-degree rotation transform to a compositing layer, while the compositor itself naively assumes a fixed-orientation display output (unaware of the hardware's actual orientation).

So, the possible solutions are:

1. #1 from comment 9 is still an option.

2. #2 from comment 9 is not feasible due to lack of plumbing of the display rotation info.

3. Add an API to viz::mojom::FrameSinkVideoCapturer to have the VIZ capturer itself execute a rotation. We would have to add to the viz::CopyOutputRequest and GPU shader programs the option to execute a rotation during capture from the compositor. Then, client-side code (in the browser process) would monitor the Ash display rotation changes and notify the capturer via mojo API calls.

Changing to Bug-Regression per comment 6

Hi Omri - raising to P1 since we're getting a ton of complaints about this now.  Can you please help fix this?  Thanks!

+1
Mark, any chance we can get someone from your team look into this? It's been open since May and folks are complaining like crazy.

powerb@ (already cc'd) is PM for the team and can help with triaging and prioritization.  He's likely a better assignee than I am, so routing to him.

Having said that, the team is currently quite shorthanded - we're working on this but solutions aren't immediate.  So prioritization alone probably won't get to a quick resolution.  miu@, you note willingness to do reviews, but do you have suggestions as to who might be able to take on the fix itself?

One other possible solution: We could add something to content::AuraWindowCaptureDevice to listen for screen rotation events (in the case of CrOS desktop capture only), and set the VideoFrame's metadata (ROTATION key). Then, downstream, in content::WebRtcVideoCaptureAdapter::OnFrameCaptured(), where frames are already modified (cropped/scaled) for other concerns, we could also leverage libyuv to rotate the content (https://cs.chromium.org/chromium/src/third_party/libyuv/include/libyuv/rotate.h?rcl=b36c86fdfe746d7be904c3a565b047b24d58087e&l=70).

re c#19: Maybe braveyao@, or somebody on their team who is familiar-enough with the proposal I mentioned in c#20?

Frankly, the suggestion in c#20 is really good to have. But frame captured in a wrong orientation sounds like a separate issue.

Currently content capture doesn't support screen rotation well. As more and more devices with desktop OS (CrOS and Win10 at present) can work in tablet mode now, there are at least two known issues due to screen rotation for screen capture. One is this issue, another one is captured frame in another orientation may be scaled down if it can't match the constraints.

There are some working to do: handle the constraints setting, detect frame rotation, make the resolution chooser do the right choice and etc.

I currently have a complex P0 assignment for this quarter. Can't take other high priority issues right now.