Skip to content
This repository has been archived by the owner on Sep 19, 2024. It is now read-only.

[RTC-228] Don't send new_tracks notification with empty list #269

Merged
merged 1 commit into from
May 15, 2023

Conversation

mickel8
Copy link
Contributor

@mickel8 mickel8 commented May 15, 2023

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
Copy link

codecov bot commented May 15, 2023

Codecov Report

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

❗ 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%) ⬆️

... 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.

@mickel8 mickel8 changed the title Don't send new_tracks notification with empty list [RTC-228] Don't send new_tracks notification with empty list May 15, 2023
@mickel8 mickel8 marked this pull request as ready for review May 15, 2023 11:09
@mickel8 mickel8 requested a review from LVala May 15, 2023 11:09
@mickel8 mickel8 merged commit 65b3347 into master May 15, 2023
@mickel8 mickel8 deleted the new-tracks branch May 15, 2023 13:14
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants