Skip to content
This repository has been archived by the owner on Mar 1, 2024. It is now read-only.

Update SignallingWebServer platform scripts to support Mac #389

Merged
merged 4 commits into from Oct 19, 2023

Conversation

Belchy06
Copy link
Collaborator

Relevant components:

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

Problem statement:

Although PixelStreaming doesn't currently support Mac as a platform, this doesn't mean that the infrastructure can't be run on Mac (x86_64 / Arm64) devices.

Solution

This PR modifies the bash platform scripts to first check for the OS and architecture before grabbing the appropriate Node binaries.

Additionally, I've forked CoTurn and created binaries for each Mac architecture. This platform scripts now grab the correct binary for the machine's architecture as well.

Documentation

No documentation updates as we didn't explicitly state that we didn't support Mac.

Copy link
Contributor

@lukehb lukehb left a comment

Choose a reason for hiding this comment

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

Left some comments about default assuming we are on Linux if we are not on Darwin.

SignallingWebServer/platform_scripts/bash/setup.sh Outdated Show resolved Hide resolved
SignallingWebServer/platform_scripts/bash/setup.sh Outdated Show resolved Hide resolved
Copy link
Contributor

@lukehb lukehb left a comment

Choose a reason for hiding this comment

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

Requesting we default assume Linux if not on Darwin.

@Belchy06 Belchy06 requested a review from lukehb October 19, 2023 01:16
Copy link
Contributor

@lukehb lukehb left a comment

Choose a reason for hiding this comment

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

Let's grab the coturn bins from this repo instead of your personal repo (same with the Windows one grabbing from Matt's repo).

SignallingWebServer/platform_scripts/bash/setup.sh Outdated Show resolved Hide resolved
Copy link
Contributor

@lukehb lukehb left a comment

Choose a reason for hiding this comment

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

lgtm

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants