Skip to content

Conversation

@mickel8
Copy link
Contributor

@mickel8 mickel8 commented Dec 12, 2023

A couple of things still won't work e.g.:

  • creating transceiver, receiving an offer, applaying this offer, creating answer etc. and then creating offer on our side. When we create our offer, we will use the transceiver configured with configuration that could be updated after receiving remote offer
  • we always call update when we receive remote offer so we don't check wheter ext ids are not re-mapped
  • we don't check wheter ext ids are the same across mlines

but I think this is good enough improvement to be merged

@codecov
Copy link

codecov bot commented Dec 12, 2023

Codecov Report

Merging #37 (5242870) into master (604ba89) will increase coverage by 0.61%.
The diff coverage is 96.90%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master      #37      +/-   ##
==========================================
+ Coverage   86.72%   87.34%   +0.61%     
==========================================
  Files          14       14              
  Lines         633      664      +31     
==========================================
+ Hits          549      580      +31     
  Misses         84       84              
Files Coverage Δ
lib/ex_webrtc/peer_connection.ex 84.67% <100.00%> (+0.12%) ⬆️
lib/ex_webrtc/peer_connection/demuxer.ex 92.59% <100.00%> (ø)
lib/ex_webrtc/rtp_transceiver.ex 91.37% <100.00%> (-0.43%) ⬇️
lib/ex_webrtc/sdp_utils.ex 88.88% <100.00%> (+0.55%) ⬆️
lib/ex_webrtc/peer_connection/configuration.ex 89.85% <94.54%> (+7.50%) ⬆️

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 604ba89...5242870. Read the comment docs.

@mickel8 mickel8 force-pushed the update-config branch 13 times, most recently from 0fd7395 to 6866f25 Compare December 13, 2023 17:24
@mickel8 mickel8 marked this pull request as ready for review December 13, 2023 17:36
Comment on lines -204 to -212
@spec get_extensions(ExSDP.t()) :: %{(id :: non_neg_integer()) => extension() | :unknown}
@spec get_extensions(ExSDP.t()) :: [Extmap.t()]
def get_extensions(sdp) do
# we assume that, if extension is present in multiple mlines, the IDs are the same (RFC 8285)
sdp.media
|> Enum.flat_map(&ExSDP.Media.get_attributes(&1, :extmap))
|> Map.new(fn extmap ->
# TODO: handle direction and extension attributes
ext = urn_to_extension(extmap.uri)
{extmap.id, ext}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed the implementation as:

  • now I can reuse get_extensions in Configuration
  • for now it seems to be good enough to operate on ExSDP.Attribute.Extmap instead of {Extension.SourceDescription, :mid/:cname}.

@mickel8 mickel8 requested a review from LVala December 13, 2023 17:36
@mickel8 mickel8 force-pushed the update-config branch 3 times, most recently from 3577bf1 to 9cdab00 Compare December 13, 2023 17:40
@mickel8 mickel8 merged commit 490f43a into master Dec 14, 2023
@mickel8 mickel8 deleted the update-config branch December 14, 2023 13:15
@mickel8 mickel8 mentioned this pull request Dec 14, 2023
63 tasks
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.

3 participants