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-228] Don't send new_tracks notification with empty list #269

Closed mickel8 closed 1 year ago

mickel8 commented 1 year ago

Sending {:new_tracks, []} was causing EndpointBin in the webrtc plugin thinking it should send offerData.

See https://github.com/jellyfish-dev/membrane_webrtc_plugin/blob/master/lib/membrane_webrtc_plugin/endpoint_bin.ex#L689

offerData triggers client SDK to send SDP offer and the whole process of establishing the connection starts. Because there are no tracks in the SDP, ICE fails (we probably generate wrong SDP) and the whole process is repeated.

The initial idea was not to send offerData when there are no tracks on the server side so I fixed it.

codecov[bot] commented 1 year ago

Codecov Report

Merging #269 (6a1d7c2) into master (267235e) will decrease coverage by 0.06%. The diff coverage is 100.00%.

:exclamation: Current head 6a1d7c2 differs from pull request most recent head ae4149f. Consider uploading reports for the commit ae4149f to get more accurate results

@@            Coverage Diff             @@
##           master     #269      +/-   ##
==========================================
- Coverage   60.21%   60.15%   -0.06%     
==========================================
  Files          43       43              
  Lines        2041     2043       +2     
==========================================
  Hits         1229     1229              
- Misses        812      814       +2     
Impacted Files Coverage Δ
lib/membrane_rtc_engine/engine.ex 76.61% <100.00%> (+0.19%) :arrow_up:

... and 4 files with indirect coverage changes


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 65b25eb...ae4149f. Read the comment docs.