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

Updated file transfer receive code. #371

Merged

Conversation

mcottontensor
Copy link
Collaborator

@mcottontensor mcottontensor commented Dec 17, 2024

Relevant components:

  • Signalling server
  • Common library
  • Frontend library
  • Frontend UI library
  • Matchmaker
  • Platform scripts
  • SFU

Problem statement:

The code for reading file content over the data channel was confusing and hard to decipher leading to accusations of it being incorrect.

Solution

Renamed some variables and added some more descriptive constants to allow easier readability.

@mcottontensor mcottontensor marked this pull request as ready for review December 18, 2024 01:06
@mcottontensor
Copy link
Collaborator Author

For ref. #370

@mcottontensor mcottontensor mentioned this pull request Dec 18, 2024
7 tasks
file.receiving = false;
file.valid = true;
Logger.Info('Received complete file');
const transferDuration = new Date().getTime() - file.timestampStart;
const transferBitrate = Math.round((file.size * 16 * 1024) / transferDuration);
const transferBitrate = Math.round((file.chunks * maxPayloadSize) / transferDuration);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Think maxPayloadSize should be maxMessageSize. The transfer bitrate calculation should still include the 5 byte header because even though it wasn't used in the file contents, it was still transferred data.

@mcottontensor
Copy link
Collaborator Author

💚 All backports created successfully

Status Branch Result
UE5.3
UE5.4
UE5.5

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

mcottontensor added a commit that referenced this pull request Dec 18, 2024
[UE5.3] Merge pull request #371 from mcottontensor/file-transfer-clarity
mcottontensor added a commit that referenced this pull request Dec 18, 2024
[UE5.4] Merge pull request #371 from mcottontensor/file-transfer-clarity
mcottontensor added a commit that referenced this pull request Dec 18, 2024
[UE5.5] Merge pull request #371 from mcottontensor/file-transfer-clarity
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