Closed Bug 1589680 Opened 5 years ago Closed 4 years ago

Make it possible to have more than one Picture-in-Picture window

Categories

(Toolkit :: Video/Audio Controls, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
84 Branch
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.

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.

Flags: needinfo?(mconley)
Blocks: 1662870
Assignee: nobody → whjones526
Flags: needinfo?(mconley)

(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:

https://searchfox.org/mozilla-central/rev/8f97d6d022bd18d4c3138bf5f13f4144ac334d82/modules/libpref/init/all.js#427

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

and this one: https://searchfox.org/mozilla-central/rev/8f97d6d022bd18d4c3138bf5f13f4144ac334d82/toolkit/components/pictureinpicture/PictureInPicture.jsm#188-202

this line: https://searchfox.org/mozilla-central/rev/8f97d6d022bd18d4c3138bf5f13f4144ac334d82/toolkit/components/pictureinpicture/PictureInPicture.jsm#233

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.

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:

  1. 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.
  2. 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.
  3. If the multiple-players.enabled pref is true, then we should skip closing pre-existing player windows at the start of handlePictureInPictureRequest.
  4. Rename this.getWeakPipPlayer into this.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.
  5. Update all pathways that call this.getPlayerWindow to pass in the PictureInPictureParent actor. This might involve a bit of plumbing, since callsites of this.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 !!!!

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.

Depends on: 1667409
Depends on: 1667840
Attachment #9182246 - Attachment description: Bug 1589680: Minimal multi-pip implementation for Alpha demo → Bug 1589680: Added support for multiple, concurrent Picture-in-Picture windows r?mconley
Pushed by mconley@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/df8b842657ac Added support for multiple, concurrent Picture-in-Picture windows r=mconley
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 84 Branch

It's a great day. Thanks. I was waiting for this feature for a long time. It's is really useful.

Thank you so much for this, I downloaded Nightly just for this feature.

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: