1682206 - Add a capability to disable video rendering to Main window during PIP

Status: NEW

(Toolkit :: Picture-in-Picture, enhancement, P3)

Description

[Sotaro Ikeda [:sotaro]]

Current PIP forwards video frames to both Main window and PIP window, though rendering to Main window is not shown by showing pictureInPictureOverlay.

• https://searchfox.org/mozilla-central/rev/97dd1934d16cada69dbf68e9cda9e9a017459e9e/toolkit/content/widgets/videocontrols.js#1023
• https://searchfox.org/mozilla-central/rev/97dd1934d16cada69dbf68e9cda9e9a017459e9e/toolkit/content/widgets/videocontrols.js#3011

Compositor and WebRender have a capability to skip such hidden video rendering. But it adds extra cpu costs to Compositor/WebRender.

Then it could cause a problem like Bug 1674775. For performance, it seems better to skip video frame forwarding at VideoSink during cloning video if possible.

Comment 1

[Sotaro Ikeda [:sotaro]]

During PIP, video frames are forwarded to 2 video tags, since the PIP uses CloneElementVisually() apis.

• https://searchfox.org/mozilla-central/rev/97dd1934d16cada69dbf68e9cda9e9a017459e9e/dom/html/HTMLVideoElement.cpp#460

Comment 2

[Sotaro Ikeda [:sotaro]]

Attachment: temporal patch - Disble pictureInPictureOverlay

When pictureInPictureOverlay was disabled, I saw video frames were shown also on Main window.

Comment 3

[Sotaro Ikeda [:sotaro]]

Attachment: temporal patch - Disable video rendering during cloning

Comment 4

[Sotaro Ikeda [:sotaro]]

Comment on attachment 9192967 [details] [diff] [review]
temporal patch - Disable video rendering during cloning

Video rendering could be disabled like the patch, though the patch is very rough.

Comment 5

[Sotaro Ikeda [:sotaro]]

:mconley, is there a reason why video frames are also forwarded to main Window? For performance, it seems better to stop forwarding video frames to Main window during cloning video.

Comment 6

[Sotaro Ikeda [:sotaro]]

:mstange, do you have any ideas about a place of stopping forwarding video frames to Main window during showing PIP?

Comment 7

[Sotaro Ikeda [:sotaro]]

When I tried 720p60 video with WebRender on my Linux PC, PIP decreased FPS from 40fps to 20 fps. But When I applied attachment 9192967 [details] [diff] [review], FPS was increased to 50 fps.

Comment 8

[Mike Conley (:mconley) (:⚙️)]

Hey sotaro,

Sorry for the delay. As I recall, there were two reasons why we keep forwarding the frames to the originating window:

1. As I recall, when I added that support, it hadn't been made clear yet whether or not we wanted to keep the video playing in the originating tab (this is what Opera does).
2. I was also being really cautious - I wanted to make sure that if anything went wrong setting up the PiP player window, that we wouldn't accidentally kill the video in the originating tab.

This also reminds me of bug 1605155. Is this a dupe of that one?

Comment 9

[Markus Stange [:mstange]]

Sorry, I don't know this code at all. Forwarding request to somebody who has worked on it before.

Comment 10

[Andreas Pehrson [:pehrsons]]

(In reply to Sotaro Ikeda [:sotaro] from comment #6)

:mstange, do you have any ideas about a place of stopping forwarding video frames to Main window during showing PIP?

Hi Sotaro,

I think this is doable, but some caution is needed, and perhaps some new tests.

There are two modes of the video element that render PiP through separate paths. Playing a MediaStream, and playing anything else.

1. MediaStream:
   In HTMLMediaElement there's mMediaStreamRenderer for rendering audio and video through the media element's VideoFrameContainer. When PiP is enabled there's in addition mSecondaryMediaStreamRenderer for rendering video through the PiP window's VideoFrameContainer.
   Some caution is needed here as the primary renderer is driving some events, for instance after rendering the first frame. This event is important, we cannot regress this.

2. Anything else:
   MediaDecoder->MediaDecoderStateMachine->VideoSink is the hierarchy through which we set up the decoder pipe to render video. VideoSink renders video frames primarily here, and for PiP here. I'm not aware of any caution one would have to take specifically for the MediaDecoder stack, but I don't know this code as well as the above.

A note on general caution needed: js can query the current frame of a video element, for instance by drawing the video element to a canvas. This in turn queries the primary (as in non-PiP) ImageContainer for the current frame. There might be other cases to consider too, mostly among these paths I believe.

It also seems like invalidation of the video element (from the primary VideoFrameContainer) drives invalidation of the secondary (PiP) VideoFrameContainer. That this is needed surprises me a bit but care would need to be taken around this as well.

TL;DR I'm not sure where the cleanest place to block frames to the main window would be. By intuition, given the drawImage case above, I'd primarily aim at ImageContainer, but you know that side of things better than me. Doing it in HTMLMediaElement and the two rendering pipes available there is doable but covers a non-trivial amount of corner cases as outlined above. Doing it VideoFrameContainer could work too, if drawImage et al. could get what they need from VideoFrameContainer instead of ImageContainer.

Comment 11

[Sotaro Ikeda [:sotaro]]

(In reply to Mike Conley (:mconley) (:⚙️) (Away Dec 19, Back Jan 5) from comment #8)

This also reminds me of bug 1605155. Is this a dupe of that one?

Yea, this bug is similar bug, though a focus of bug is a bit different.

Comment 12

[Sotaro Ikeda [:sotaro]]

One simple way is just disconnect video rendering path from main rendering path.

Comment 13

[Sotaro Ikeda [:sotaro]]

Attachment: temporal patch - Disable video rendering during cloning video

With WebRender, video rendering could be disabled simply by removing async image pipeline.

Comment 14

[Sotaro Ikeda [:sotaro]]

From Comment 10 , it seems better to disable video rendering in graphics area. Created Bug 1684194 for WebRender.