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

Add initial implementation of craete_offer, apply_local_description and add_transceiver #6

Merged
merged 5 commits into from
Oct 27, 2023

Conversation

LVala
Copy link
Member

@LVala LVala commented Oct 25, 2023

No description provided.

@LVala LVala self-assigned this Oct 25, 2023
@codecov
Copy link

codecov bot commented Oct 25, 2023

Codecov Report

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

Files Coverage Δ
lib/ex_webrtc/sdp_utils.ex 87.27% <90.90%> (+5.45%) ⬆️
lib/ex_webrtc/rtp_transceiver.ex 58.97% <0.00%> (-3.19%) ⬇️
lib/ex_webrtc/peer_connection.ex 24.83% <5.00%> (-14.38%) ⬇️

📢 Thoughts on this report? Let us know!.

@LVala LVala marked this pull request as ready for review October 25, 2023 12:23
@LVala LVala requested a review from mickel8 October 25, 2023 12:26
{:fingerprint, Keyword.fetch!(config, :fingerprint)},
{:setup, Keyword.fetch!(config, :setup)},
:rtcp_mux
] ++ if(Keyword.get(config, :rtcp, false), do: [{"rtcp", "9 IN IP4 0.0.0.0"}], else: [])
Copy link
Member

Choose a reason for hiding this comment

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

Nitpick but if that's not a big problem I would put this as first attribute.
This way we will have something like this

m=audio 9 UDP/TLS/RTP/SAVPF 111 63 9 0 8 13 110 126
c=IN IP4 0.0.0.0
a=rtcp:9 IN IP4 0.0.0.0

so all address related lines next to each other

else
for mline <- state.current_remote_desc.media,
{:mid, mid} <- ExSDP.Media.get_attribute(mline, :mid),
{mid, _} <- Integer.parse(mid) do
Copy link
Member

Choose a reason for hiding this comment

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

Use String.to_integer

Copy link
Member Author

Choose a reason for hiding this comment

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

why tho? String.to_integer might rise and I wanted to avoid that.

Comment on lines 474 to 475
max_mid = Enum.max(crd_mids ++ tsc_mids, &>=/2, fn -> -1 end) + 1
max(state.next_mid, max_mid + 1)
Copy link
Member

Choose a reason for hiding this comment

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

Looks like one +1 is not needed

};

const mode = "passive"
Copy link
Member

Choose a reason for hiding this comment

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

Very nice example modification 👍

@@ -35,7 +41,10 @@ defmodule ExWebRTC.PeerConnection do
:dtls_buffered_packets,
dtls_finished: false,
transceivers: [],
signaling_state: :stable
signaling_state: :stable,
next_mid: 0,
Copy link
Member

Choose a reason for hiding this comment

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

IMO this can be deleted

@LVala LVala merged commit 7c41057 into master Oct 27, 2023
2 of 4 checks passed
@LVala LVala deleted the create-offer branch October 27, 2023 07:59
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