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

Create transceivers on set_remote_description #2

Merged
merged 2 commits into from
Oct 23, 2023
Merged

Conversation

mickel8
Copy link
Member

@mickel8 mickel8 commented Oct 18, 2023

Definitely not perfect and for sure bugged but maybe this can be a base for further work.

The idea is that we iterate over mlines in SDP, we try to find a transceiver for a given mline, if it exists we potentially update its configuraiton. If it doesn't exist we create a new one. At the end we compare previous transceivers with new transceivers and try to find new MediaStreamTracks

@mickel8 mickel8 force-pushed the rtp-transceiver branch 2 times, most recently from c28efe0 to addb72f Compare October 19, 2023 17:15
@codecov
Copy link

codecov bot commented Oct 19, 2023

Codecov Report

Attention: 5 lines in your changes are missing coverage. Please review.

Files Coverage Δ
lib/ex_webrtc/media_stream_track.ex 100.00% <100.00%> (ø)
lib/ex_webrtc/peer_connection/configuration.ex 50.00% <ø> (ø)
lib/ex_webrtc/rtp_transceiver.ex 100.00% <100.00%> (ø)
lib/ex_webrtc/utils.ex 0.00% <0.00%> (ø)
lib/ex_webrtc/peer_connection.ex 35.64% <84.21%> (ø)

... and 1 file with indirect coverage changes

📢 Thoughts on this report? Let us know!.

@mickel8 mickel8 force-pushed the rtp-transceiver branch 7 times, most recently from 2d395d8 to a1ed9a1 Compare October 19, 2023 18:12
@mickel8 mickel8 marked this pull request as ready for review October 19, 2023 18:20
@mickel8 mickel8 requested a review from LVala October 19, 2023 18:26
lib/ex_webrtc/rtp_transceiver.ex Show resolved Hide resolved
Comment on lines +321 to +323
for track <- new_remote_tracks do
notify(state.owner, {:track, track})
end
Copy link
Member

Choose a reason for hiding this comment

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

I suppose you know that and this is only temporary, but I believe this notification should appear when only tracks are negotiated.

Copy link
Member Author

Choose a reason for hiding this comment

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

Now we accept everything :D

@mickel8 mickel8 merged commit d19d6c9 into master Oct 23, 2023
4 checks passed
@mickel8 mickel8 deleted the rtp-transceiver branch October 23, 2023 08:15
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