fishjam-dev / membrane_rtc_engine

Customizable Real-time Communication Engine/SFU library focused on WebRTC.
Apache License 2.0
141 stars 13 forks source link

[RTC-315] Fix RC on removing tracks #308

Closed sgfn closed 1 year ago

sgfn commented 1 year ago

This description is outdated. Refer to the discussion below for more info.

[OUTDATED] The bug outlined in RTC-315 had to do with the remove_child: tee action -- in some cases, this action would execute too quickly, causing the endpoint pad to get unlinked twice later on. The bug could not be reproduced with this fix: returning the action from the handle_child_pad_removed callback of the given endpoint.

The Core team promised to make the error message more descriptive in the future.

codecov[bot] commented 1 year ago

Codecov Report

Merging #308 (de5ab3c) into master (ebafff0) will increase coverage by 0.10%. The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master     #308      +/-   ##
==========================================
+ Coverage   53.25%   53.36%   +0.10%     
==========================================
  Files          39       39              
  Lines        1983     1979       -4     
==========================================
  Hits         1056     1056              
+ Misses        927      923       -4     
Files Changed Coverage Δ
webrtc/lib/webrtc_endpoint.ex 0.00% <0.00%> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update ebafff0...de5ab3c. Read the comment docs.

sgfn commented 1 year ago

For future reference: The changes in this PR are the exact opposite from most of the changes in the following PR: https://github.com/jellyfish-dev/membrane_rtc_engine/pull/192

sgfn commented 1 year ago

We agreed with @Rados13 that this proposed solution should work for now (the issue couldn't be reproduced). If any other problems come up, we'll have to come back to it

sgfn commented 1 year ago

Turns out this may very well be a membrane_core bug. Ref: https://github.com/membraneframework/membrane_core/pull/590

sgfn commented 1 year ago

Confirmed membrane_core bug. Will get fixed in 0.12.8