Skip to content

Conversation

bmsan
Copy link
Contributor

@bmsan bmsan commented Mar 14, 2021

fixes #561

Copy link
Member

@thijstriemstra thijstriemstra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks! there are some trailing spaces, see https://github.com/collab-project/videojs-record/pull/562/files

@thijstriemstra
Copy link
Member

thijstriemstra commented Mar 14, 2021

Does this stop call introduce any flickering or interruption in the video preview stream?

@bmsan
Copy link
Contributor Author

bmsan commented Mar 14, 2021

I ran two types of tests described below:

Test1: In change-video-input with 2 webcams connected, change fast the input device two times(before the webstream starts to get streamed).
Test2: Call getDevice() multiple times( I called this from the browser console
   eg: `player.record().getDevice(); player.record().getDevice()`

For both tests flickering is observed but in both pre-pull request and post-pull request versions. But I don't think there is any extra flickering induced by this change.

More exactly for Test1 if you select very fast camera2 & then camera1 => camera2 will be seen for a fraction of a second, then background color, then camera1. (But as stated above the behavior, in terms of flickering, is the same regardless of this pull-request)

I didn't observe any visual difference between the two code versions, only the fact the camera is released(with the pull-request).

@thijstriemstra thijstriemstra changed the title Fixed Media Stream resource leak. See ISSUE-#561 getDevice: stop previous stream if it is active Mar 22, 2021
@thijstriemstra thijstriemstra changed the title getDevice: stop previous stream if it is active fix stopping stream if it is active when using getDevice Mar 22, 2021
Copy link
Member

@thijstriemstra thijstriemstra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

@thijstriemstra thijstriemstra merged commit c26de7a into collab-project:master Mar 22, 2021
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 this pull request may close these issues.

Safeguard against a potential resource leak(media streams) due to user missuse
2 participants