Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

last-n is set to 1 when entering PiP, isn't raised back when exiting PiP #10257

Closed
lultimouomo opened this issue Oct 26, 2021 · 4 comments · Fixed by #11425
Closed

last-n is set to 1 when entering PiP, isn't raised back when exiting PiP #10257

lultimouomo opened this issue Oct 26, 2021 · 4 comments · Fixed by #11425

Comments

@lultimouomo
Copy link

Description:

Both on Android and iOS, entering PiP sets last-n to 1, but exiting PiP doesn't raise it back, which causes a bunch of video-less rectangles to be shown in grid mode.
I think what is happening is the following: by entering PiP the window becomes smaller than REDUCED_UI_THRESHOLD, this causes the UI to toggle to reduced mode, which disables the filmstrip.
lastn middleware is listening for fimstrip changes, and calls _updateLastN, which finds that filmStrip is not enabled and sets last-n to 1.

When PiP is exited, the filmstrip is enabled again and _updateLastN is called; but since by default it will use the last selected value, it just sets last-n again to 1.

The only way to exit the impasse is enter audio-only mode; this sets last-n to 0, which is falsey and therefore is not considered when computing lastNSelected

This happens both when using the SDK in a 3rd party app and when using the official Jitsi app (at least on Android, on iOS I only tried the SDK)

Steps to reproduce:

  1. Open Jitsi app on mobile
  2. Join conference with at least two other participants
  3. Enter PiP
  4. Exit PiP

Expected behavior:

When exiting PiP, last-n is restored to the initial value and you can see multiple video streams

Actual behavior:

last-n is set again to 1 (as shown by android logcat), only your own video + one other participant is shown

Server information:

Happens both on self-hosted server and meet.jit.si

Client information:

  • Browser / app version: Happens on Jitsi Meet for Android 21.4.1, Android SDK 3.10.2, iOS SDK 3.10.4
  • Operating System:

Additional information:

A possible solution would be to store the requested last-n separately from the one that is applied, which could be lower because of audio-only, PiP, etc.

@saghul
Copy link
Member

saghul commented Oct 26, 2021

Great analysis! @tmoldovan8x8 can you PTAL when you get a chance?

@stale
Copy link

stale bot commented Apr 18, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix Issue won't be fixed label Apr 18, 2022
@lultimouomo
Copy link
Author

Any chance someone can take a look at the open PR fixing this issue?

@stale stale bot removed the wontfix Issue won't be fixed label Apr 19, 2022
@saghul
Copy link
Member

saghul commented Apr 25, 2022

I just left a review.

saghul added a commit to saghul/jitsi-meet that referenced this issue Apr 25, 2022
If last N goes down to 1 it will be stuck there since it's > 0 and will
be our `lastNSelected`. When limits are applied we'll take the minimum,
so it will end up being 1.

Once can end up in last N being 1 by several means, the more obvious one
by entering Picture-in-Picture mode on mobile.

Fix it by not using the previous last N value for the current
calculation, at all.

Fixes: jitsi#10257
Closes: jitsi#10491
saghul added a commit that referenced this issue Apr 25, 2022
If last N goes down to 1 it will be stuck there since it's > 0 and will
be our `lastNSelected`. When limits are applied we'll take the minimum,
so it will end up being 1.

Once can end up in last N being 1 by several means, the more obvious one
by entering Picture-in-Picture mode on mobile.

Fix it by not using the previous last N value for the current
calculation, at all.

Fixes: #10257
Closes: #10491
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants