igniterealtime / openfire-pade-plugin

A plugin for Openfire that offers web-based unified communications - chat, groupchat, telephone, audio and video conferencing.
Apache License 2.0
54 stars 29 forks source link

Feature Request: Collaborative Laser-Pointer #77

Closed gjaekel closed 3 years ago

gjaekel commented 3 years ago

Since V1.0, OFMeeting provides the "Confetti"-Tool. By invoking, one may invoke a confetti rain on the own screen and for all othre participants. technical, this uses some kind of broadcast messaging. Inspired by this, I want to propose to develop a new feature: A collaborative laser-pointer, that will allow to highlight some spot in the one hand on the own video, but in the other hand, on the video of any other participant of the meeting.

This should allow a presenter to highlight something for all other, but also to allow a listener to point out to something displayed by the presenter to ease a callback.

The "light pointer" should be a html element, the visual effect of the pointer should be applied by CSS to allow e.g. some kind of bullet circle with 100% opacity surrounded by a dim-out of the other area. Or just a light-green spot. Probably coupled by a CSS animation for in/out.

deleolajide commented 3 years ago

Great idea. Sounds like the beginnings of an augmented reality application for ofmeet.

technical, this uses some kind of broadcast messaging. Inspired by this, I want to propose to develop a new feature:

Yes, it uses a jitsi api called SendCommand that is used for other broadcast features like hand raising.

I am not free to work on this, but I will mark if for anyone else to get involved

deleolajide commented 3 years ago

I have spent some time on this re-using the shared cursors from the together.js project

image

Clicking mouse produces a red circle that lingers for a few seconds

image

It does not work with browse/tab mode view with multiple participants on screen. It also does not work properly with the speaker only video view as the image is inverted and the coordinates calculated are all wrong.

However, it works best with screen sharing

image

I will release what I have done so far in version 1.0.2

gjaekel commented 3 years ago

It does not work with browse/tab mode view with multiple participants on screen. It also does not work properly with the speaker only video view as the image is inverted and the coordinates calculated are all wrong.

This is a good first sprint, nevertheless. Will try it ASAP. Thank you!

cool0707 commented 3 years ago

How about disabling cursor sharing in Tile View?

gjaekel commented 3 years ago
cool0707 commented 3 years ago

When I looked at collab.js, I found that there was code remaining for the keydown and scroll events. Scrolling does not occur in the meeting screen, and there seems to be no need to share key events. Is there any purpose for these codes to be remained?

deleolajide commented 3 years ago

Is there any purpose for these codes to be remained?

The key events are needed to support collaborative data form entry in co-browsing. The original vision behind all this is explained in my 2015 blog - https://discourse.igniterealtime.org/t/realtime-collaboration-with-openfire-meetings/68740

cool0707 commented 3 years ago

I understand that your ultimate goal is to build a collaborative infrastructure with Openfire at its core. However, I don't think we'll be doing any data entry on the meeting screen, is there any impediment to removing the currently non-functional code in collab.js? I'd like to tidy up the code before I solve the problem of the cursor sharing coordinates being incorrect, or the cursor being shared even when a different video is being displayed. I also need to change some of the structure of the 'message', is this a problem?

deleolajide commented 3 years ago

However, I don't think we'll be doing any data entry on the meeting screen, is there any impediment to removing the currently non-functional code in collab.js? I'd like to tidy up the code before I solve the problem of the cursor sharing coordinates being incorrect, or the cursor being shared even when a different video is being displayed. I also need to change some of the structure of the 'message', is this a problem?

None. We can bring back the non-functional code later when needed. Thanks for doing this 👍

deleolajide commented 3 years ago

@cool0707 thanks for the PR and the work you have put into this. We just need to fix a few things and it will be ready for use.

~1. Black color is used for all participants in meeting instead of random colors that were used before~

  1. Sometimes, the red circle when shared cursor is clicked is showing correctly on the local screen, but not on the remote screens of every other participant, the location is not modified (transformed) to match the shared cursor. This happen consistently if you use 2 users and the second person shares his/her cursor. The first person click is ok, the second person click is wrong
gjaekel commented 3 years ago

Black color is used for all participants in call instead of random colors that were used before

For the shared cursors? I don't can confirm!

gjaekel commented 3 years ago

Let me copy the comment at #223 here, because here they are in the right context.

gjaekel commented 3 years ago

Attention:

lib-jitsi-meet.min.js:3580 MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 conference.userLeft listeners added. Use emitter.setMaxListeners() to increase limit
    at l (https://miet.dnb.de/raum/libs/lib-jitsi-meet.min.js:3579:25)
    at a.addListener (https://miet.dnb.de/raum/libs/lib-jitsi-meet.min.js:3663:20)
    at ie.on (https://miet.dnb.de/raum/libs/lib-jitsi-meet.min.js:10472:56)
    at Collab.init (https://miet.dnb.de/raum/collab.js:542:24)
cool0707 commented 3 years ago

@deleolajide

  • Black color is used for all participants in meeting instead of random colors that were used before

I was not able to reproduce this in my local environment. This may be due to conference.setLocalParticipantProperty(), I used to make the cursor color of each participant look the same to everyone and to match the color of the letter avatar. This is just an idea, but since the User ID is a random 8-digit hexadecimal number, the color of the letter avatar and the cursor could be determined reliably by using the top 6 digits of the ID. The User ID is the one you can get with APP.conference.getMyUserId().

  • Sometimes, the red circle when shared cursor is clicked is showing correctly on the local screen, but not on the remote screens of every other participant, the location is not modified (transformed) to match the shared cursor. This happen consistently if you use 2 users and the second person shares his/her cursor. The first person click is ok, the second person click is wrong

I think either the conditional expression or the variable is wrong because the following code determines whether the coordinates should be inverted.

https://github.com/cool0707/openfire-pade-plugin/blob/665f34591322f25bad8e6d1502a79dd039099ab1/web/src/main/webapp/collab.js#L85-L91

@gjaekel Collab.init is supposed to be called only once, so it's strange that the listener is registered 11 times. Memory leaks are a critical issue, so I'll look into it.

gjaekel commented 3 years ago

I was not able to reproduce this in my local environment.

Me, too :)

This may be due to conference.setLocalParticipantProperty(), I used to make the cursor color of each participant look the same to everyone and to match the color of the letter avatar.

I observed that the cursor background color was the same as the pseudo avatar background color. That is what I would expect and should be kept if possible. As said, maybe it's even possible to use a small version of the avatar as pointer tag.

gjaekel commented 3 years ago
        static shouldInvert(videoId) {
          return (
                APP.conference.isLocalId(videoId) &&
                APP.UI.getLargeVideo().containers.camera.localFlipX &&
                !APP.conference.isLocalVideoMuted() &&
                APP.UI.getLargeVideo().state == 'camera');
         }

with other words, this should mean: Invert, if "own video window" and "cam flipped" and "video on" and "cam selected (and not screen-share, presumably). I expect, for the local view, the video is flipped by default to archive the expected "me looks like in a mirror" effect.

deleolajide commented 3 years ago

Black color is used for all participants in call instead of random colors that were used before

Very Strange. I am unable to reproduce anymore

cool0707 commented 3 years ago

Fixed a bug in PR #248. The problem with the incorrect cursor position was an elementary mistake of rewriting the pass-by-reference argument.

The EventEmitter memory leak was a problem caused by Jitsi Meet registering 10 event listeners, which is the upper limit. It seemed to be working fine when ignored, but I changed the limit to 20 just in case.

The black cursor problem may have been caused by timing, so I changed the color to be uniquely determined from the ID when no letter avatar is specified, just in case. Also, when setting the color, it is validated now.

gjaekel commented 3 years ago

may we close this?

@cool0707 Got the first feedback from one of our users today: "comment":"Die neue \"shared cursor\" Funktion ist super!","audio":"5","video":"4"