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

Fix recording not stopping #1812

Merged
merged 5 commits into from
May 11, 2022
Merged

Fix recording not stopping #1812

merged 5 commits into from
May 11, 2022

Conversation

Arri98
Copy link
Contributor

@Arri98 Arri98 commented Apr 25, 2022

There was an error where when recording where the recording would not stop and files kept growing even though licode gave no error. What I found is that the promise that removed the ExternalOutput was always pending and never resolved and the callback was never executed but, the recording would stop on most occasions. After removing the lines shown in the PR the callback is always called and the recording stops all the times I tested it.

To check this you check that the log inside the callback in the removeExternalOutput() function on line 115 of erizo_controller/erizoJS/models/Publisher.js is never reached.

I don´t know the root issue of this problem, and my manual testing is limited so this might cause other issues with removing that check. I will leave this PR as a draft, as a point of discussion and potential fix until a better solution is found or further testing is done.

[] It needs and includes Unit Tests

Changes in Client or Server public APIs

[] It includes documentation for these changes in /doc.

@Arri98
Copy link
Contributor Author

Arri98 commented Apr 28, 2022

After further investigation i have found the root of the problem. In the ExternalOuput, when closing it, we would wait for the recording thread to finish before freeing memory allocated to libav componentes. However, . The file im going to comment is /src/erizo/media/ExternalOutput.cpp.

In line 110 and 111 we unlock the recordingThread and wait for it to finish. However as the recording_ flag is never set to false it never leaves the loop in line 545. Queues are not depleated and libav componentes are not liberated in subsecuent lines. The debug message in line 134 never appears as we never reach this point.

Also as we never closed the externalOuput, in the AsyncCloser class inside the ExternalOutput.cc we never fulfilled the promise, so the callback was also never executed.

For the these reasons the audio and video queues would never deplete and the last seconds of the recording were lost. Also there was a small memory leaks and some open connections after closing recorders.

@Arri98 Arri98 marked this pull request as ready for review May 3, 2022 10:28
Copy link
Contributor

@lodoyun lodoyun left a comment

Choose a reason for hiding this comment

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

This look good to me 👍 Good catch

@lodoyun lodoyun merged commit 19b1c47 into lynckia:master May 11, 2022
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.

2 participants