OPENMEETINGS-2278 fix restart of recording and ghost KStream

[sebawagner]

OPENMEETINGS-2278

• Add functionality to stop the sharing as part of recording stop as it will stop the MediaStream on stop of recording.
• This also clears the KStream that goes ghost and enables restarting the recording.
• Some further work required on the UI to prevent starting to screenshare while recording potentially required.

This is a first Draft, please allow me to explain some of the changes.

And if agreed I can try to add some unit testing. (Or at least figure out if that is possible)

With this fix the effect is:

• If stopping the recording it will also stop the sharing of the screen. Imho this happened already anyway.
  • You can restart the recording after stopping it. And it will work fine.
• It doesn't have any ghost KStream objects, they are created and dropped correctly

I tested:

• Start Recording, Stop Recording (Creates MP4 file) AND restart and stop (creates MP4 file) AND leave room re-enter, start recording, stop recording (Creates MP4 files)
• Start sharing, start recording, stop recording, (implicit stopped sharing, changes sharing button to stop)
• Repeat above => No empty KStream/ghost are created:

Caveat: It stops the sharing when stopping the recording. But I'm not sure but I saw the MediaStream destroy event when stopping recording. And I think this is a usability issue that you can't really independently start/stop sharing and recording. But maybe we would need a bigger refactoring to solve this.

Other notes: I would like to explore how to use the DestroyObject Kurento EventListener to create debug logs and to explore how we could be using this event to trace ghost KStream objects.

[solomax]

I would say If stopping the recording it will also stop the sharing of the screen is not the best option I would write some tests and try to avoid this it might me extremely annoying is all participants in the room will have to re-open screen-sharing just because recording has been stopped

[sebawagner]

@solomax it works now!

You can enable the recoridng and it will just start recoring, you can stop it and it will stop recording and only stop sharing if sharing was disabled.

However if you start recording, and then enable sharing while recording, and then stop recording, it will continue to share screen.

You can also share screen and start/stop recording as many times as you want.

The issue with the ghost KStream was because the video-util close closed the stream on the first try. The logic on client side was closing the stream in a way so you couldn't restart. Stopping from Server side actually works better. I'm not quite sure what happens client side its a also a bit confusing, cause if you "just record" on server side you still have a screen-share. Even though you don't create an screen-share event. Its kind of like hidden on the server, until you send the share event. So I think controlling the logic from the server logic seems to work better.

I also did check with a 2nd browser to make sure all screen-share events come through. And also no Zero size or ghost KStreams are created.

[sebawagner]

I would say If stopping the recording it will also stop the sharing of the screen is not the best option

=> That is fixed now. It won't stop the sharing - except - sharing wasn't started. Then it will stop the stream. If you start however the sharing while recording then sharing will continue.

[sebawagner]

@solomax r u happy with it? I can't find or think of any regression introduced by it

[sebawagner]

Please let me know on the approach. There is really not much that changed. I should simply now work as expected.

If you're happy with the PR, I start thinking of modifying some of the tests in TestRoomFlowMocked to see how to verify it.

[solomax]

@sebawagner sorry for the long review, I'm on vacation so trying to relax :)

[sebawagner]

Thanks! Take care!

[solomax]

If you're happy with the PR, I start thinking of modifying some of the tests in TestRoomFlowMocked to see how to verify it.

please do not use tests from TestRoomFlowMocked these are for audio/video setup dialog please create separate class (common code can be unified later, if any)