Make it possible to have more than one Picture-in-Picture window
Categories
(Toolkit :: Video/Audio Controls, enhancement, P3)
Tracking
()
Tracking | Status | |
---|---|---|
firefox84 | --- | fixed |
People
(Reporter: mconley, Assigned: whjones526)
References
(Blocks 1 open bug)
Details
Attachments
(3 files)
We currently have a limitation where we can only have a single Picture-in-Picture window open at a time.
That limitation is artificial - in theory, we could certainly modify things to allow multiple Picture-in-Picture windows. This would also differentiate us from the other browsers.
Comment 1•4 years ago
|
||
Waiting for this too. Opera and Vivaldi users are asking for this feature too. The Picture-in-Picture Web API allow browsers to implement this. It's a matter of choice.
Reporter | ||
Updated•4 years ago
|
Reporter | ||
Comment 3•4 years ago
|
||
(For context for the people watching this bug, Hunter is a student at MSU who is going to hack on this)
Hi Hunter,
So this is something we want to try to offer to our users. We don't fully understand the use cases yet, but we'd like to allow users to opt-in to this functionality to see whether or not this is truly useful to people.
So we'll be defining a preference for this functionality that will default to false
. That will mean that by default, we will continue with the old behaviour until we make a decision on whether or not to ship multiple PiP player windows enabled by default.`
You can add a pref here:
Maybe call it media.videocontrols.picture-in-picture.multiple-players.enabled
, and set it to false
.
That'll get us started. Next, we need to find all of the places in the code that assume that there's only a single Picture-in-Picture player window, and see how they work.
Here's one, for example: https://searchfox.org/mozilla-central/rev/8f97d6d022bd18d4c3138bf5f13f4144ac334d82/toolkit/components/pictureinpicture/PictureInPicture.jsm#122-148
so I'd start by examining this file, and also reading the documentation in https://phabricator.services.mozilla.com/D49479 if you haven't already, to get a sense of what PictureInPicture.jsm does and how it works.
I think ultimately we're going to need some kind of key-value mapping where each player window is a value, and... something that represents an originating video is the key. I'm going to have a think on how that'll work, but in the meantime, please study the above.
Reporter | ||
Comment 4•4 years ago
|
||
Hunter gave me a great idea - we could use the PictureInPictureParent
as the key! A unique instance of these gets created for every spawned player window.
We have to be careful here - there's a kind of weird dual-nature for PictureInPictureParent
where one gets instantiated when a video first asks to be opened in PiP mode, and then another one that gets created when the player window is opened. I think we want to use the latter one as the key, because this is the one that ultimately is responsible for doing stuff like muting the video, noticing if the originating video goes away, etc. It's the more "interesting" one that actually has messaging go back and forth over it after the player window opens.
So here's how I recommend proceeding with this:
- Add a WeakMap in PictureInPicture.jsm that maps PictureInPictureParent's to player windows. I recommend a WeakMap so that we don't accidentally keep a PictureInPictureParent actor alive if we fail to do correct bookkeeping to clear it out.
- It's actually the player.js file that figures out which PictureInPictureParent actor it wants to use here: https://searchfox.org/mozilla-central/rev/8f97d6d022bd18d4c3138bf5f13f4144ac334d82/toolkit/components/pictureinpicture/content/player.js#125-128, so what I recommend next is to have PictureInPicture.jsm expose a method that player.js can call to set the PictureInPictureParent actor and window into that WeakMap.
- If the
multiple-players.enabled
pref is true, then we should skip closing pre-existing player windows at the start ofhandlePictureInPictureRequest
. - Rename
this.getWeakPipPlayer
intothis.getPlayerWindow
, and have it take an actor argument. That methods job will then be to return the player window in the WeakMap that is mapped to from the associated PictureInPictureParent. - Update all pathways that call
this.getPlayerWindow
to pass in the PictureInPictureParent actor. This might involve a bit of plumbing, since callsites ofthis.getPlayerWindow
might not all have the actor in their scope - so you might have to pass it in from an earlier function in the callstack (Gijs or I can help you sort that out, certainly).
Seem the 1 pip is theorical, so let's change it.
I would love to have this ( https://nextcloud.rkn.ovh/index.php/s/oBbwpNZsWc3rYAT ) ... would be nice to have 2 or more pip !
Add this to the possibility of hdmi-to-tv or chromecast !!!!
Reporter | ||
Comment 6•4 years ago
|
||
I've added some documentation for the component in bug 1589158, which should land shortly. We will likely need to update that documentation as part of the work in this bug, since we'll be refactoring some of the internal components a little bit.
Assignee | ||
Comment 7•4 years ago
|
||
Assignee | ||
Comment 8•4 years ago
|
||
Depends on D90478
Assignee | ||
Comment 9•4 years ago
|
||
Updated•4 years ago
|
Comment 10•4 years ago
|
||
Comment 11•4 years ago
|
||
bugherder |
Comment 12•4 years ago
|
||
It's a great day. Thanks. I was waiting for this feature for a long time. It's is really useful.
Comment 13•4 years ago
|
||
Thank you so much for this, I downloaded Nightly just for this feature.
Description
•