Skip to content

Initial version of sending from a file #30

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

Merged
merged 2 commits into from
Dec 8, 2023
Merged

Initial version of sending from a file #30

merged 2 commits into from
Dec 8, 2023

Conversation

mickel8
Copy link
Member

@mickel8 mickel8 commented Nov 30, 2023

Things that are still missing:

  • SSRC is hardcoded
  • we don't check whether the other side didn't reject our mline
  • we need to update extension ids if the other side sends its offer first
  • unit tests

Copy link

codecov bot commented Nov 30, 2023

Codecov Report

Merging #30 (a0379ba) into master (8693629) will increase coverage by 0.69%.
The diff coverage is 95.23%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master      #30      +/-   ##
==========================================
+ Coverage   84.43%   85.13%   +0.69%     
==========================================
  Files          12       14       +2     
  Lines         559      592      +33     
==========================================
+ Hits          472      504      +32     
- Misses         87       88       +1     
Files Coverage Δ
lib/ex_webrtc/peer_connection/configuration.ex 82.35% <ø> (ø)
lib/ex_webrtc/rtp/vp8_payloader.ex 100.00% <100.00%> (ø)
lib/ex_webrtc/rtp_sender.ex 100.00% <100.00%> (ø)
lib/ex_webrtc/rtp_transceiver.ex 92.59% <100.00%> (+0.28%) ⬆️
lib/ex_webrtc/peer_connection.ex 80.93% <86.66%> (+0.06%) ⬆️

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 8693629...a0379ba. Read the comment docs.

@mickel8 mickel8 changed the title wip vp8 payloader Sending from a file Dec 1, 2023
@mickel8 mickel8 mentioned this pull request Dec 1, 2023
63 tasks
@mickel8 mickel8 force-pushed the send-file branch 15 times, most recently from 1e530ec to c7d1ee5 Compare December 7, 2023 14:51
@mickel8 mickel8 changed the title Sending from a file Initial sending from a file Dec 7, 2023
@mickel8 mickel8 changed the title Initial sending from a file Initial version of sending from a file Dec 7, 2023
@mickel8 mickel8 force-pushed the send-file branch 4 times, most recently from e32dc24 to 2aff383 Compare December 7, 2023 15:25
@mickel8 mickel8 marked this pull request as ready for review December 7, 2023 15:29
@mickel8 mickel8 requested a review from LVala December 7, 2023 15:29
end

defp handle_webrtc_message({:connection_state_change, :connected} = msg, state) do
Logger.info("#{inspect(msg)}")
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if this is useful.

Copy link
Member Author

Choose a reason for hiding this comment

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

It is more readable when we see in logs:

Connected
Starting sending ivf file

than

Starting sending ivf file


{rtp_packets, last_timestamp} =
Enum.map_reduce(rtp_packets, state.last_timestamp, fn rtp_packet, last_timestamp ->
# we hardcode 3000 as we know the video is in 30 FPS
Copy link
Member

Choose a reason for hiding this comment

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

I would elaborat on this a bit more and include that we got this value from
dividing VP8 clock rate by the frame rate (90000/30 == 3000).

Comment on lines 16 to 20
pc.ontrack = event => {
const videoPlayer = document.getElementById("videoPlayer");
videoPlayer.srcObject = event.streams[0];

};
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
pc.ontrack = event => {
const videoPlayer = document.getElementById("videoPlayer");
videoPlayer.srcObject = event.streams[0];
};
pc.ontrack = event => {
const videoPlayer = document.getElementById("videoPlayer");
videoPlayer.srcObject = event.streams[0];
};

@mickel8 mickel8 merged commit 898d6b3 into master Dec 8, 2023
@mickel8 mickel8 deleted the send-file branch December 8, 2023 11:32
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