-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Fix for recording audio and screensharing streams #1265
Conversation
I've compiled and tested those changes in this PR. |
@zafergurel I've made a PR in your fork that should pass the tests: https://github.com/advancity/licode/pull/2 |
@nvazquezg Thank you. 👍 |
@zafergurel I found no problems with the recordings of video-and-audio streams and audio-only streams, there is also no delay in the start of the recording 👍 But the screen (video-only) is not working in my tests: The logs show: And this makes true this condition, that invalidates the output: Any ideas to fix this situation? |
@zafergurel This problem was in my client side, I was creating the screenshare stream with |
@nvazquezg Very good. 👍
|
@nvazquezg I still have missing parts in the recording file. I use the docker container. |
@zafergurel With your PR, the recordings don't miss any audio or video at the beginning. The last ~5 seconds of the recording are missing, though, but it is close enough for my use case ATM. I'm compiling directly from source, not using docker yet. |
@nvazquezg I have not tested those PRs yet. I'll check them out as soon as I find time. |
Also waiting for this PR. |
Any updates on this? |
This fix still doesn't allow recording screen sharing with audio enabled :( |
any update on this pull |
This is fixed by #1808. So, it's time to close it. |
Description
ExternalOutput does not record audio-only or video-only (screensharing) streams.
The changes introduced in this commit lets ExternalOutput to be aware that the stream is an audio-only or video-only stream.
Also please look at #1254 .
The comments about this PR made this PR possible.
Issues #1203 and #1185 are related and can be fixed with this PR.
[x] It needs and includes Unit Tests
Changes in Client or Server public APIs
ExternalOutput in erizoAPI has been changed.
A new method named setHasAudioAndVideo has been added to ExternalOutput (ExternalOutput.h and ExternalOutput.cc).