Skip to content

Conversation

@DoctorBud
Copy link
Collaborator

  • Adds 'make run-pretty' option to format log output in a more human-readable way.
  • Fixes typos in a couple places
  • Adds static/examples/mirror.html, which synthesizes a video source and acts as a peer.
  • Adds links to other examples to bottom of static/index.html
  • Adds js/mirrormedia.js which uses a Canvas to create a peer video stream.
  • In peering.js, add a 1-second delay before manually firing .play() on a newly created video element. This resolves random issues where even an autoplay video element doesn't start playing automatically.

…readable way.

- Fixes typos in a couple places
- Adds static/examples/mirror.html, which synthesizes a video source and acts as a peer.
- Adds links to other examples to bottom of static/index.html
- Adds js/mirrormedia.js which uses a Canvas to create a peer video stream.
- In peering.js, add a 1-second delay before manually firing .play() on a newly created video element. This resolves random issues where even an autoplay video element doesn't start playing automatically.

run-pretty: build
# open http://localhost:3001
ENV=local ./backend 2>&1 | jq --raw-output '[("# " + .level + (" " * 10))[0:10], (.func + (" " * 20))[0:20], .msg] | @tsv'
Copy link
Owner

Choose a reason for hiding this comment

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

Why not have the local env var change the logging formatter used? https://github.com/ryanrolds/club/blob/master/golang/cmd/backend/logging.go#L21

Comment on lines +82 to +85
setTimeout(() => {
video.play()
}, 1000)

Copy link
Owner

Choose a reason for hiding this comment

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

Any more details on the race condition that requires this? Are we not emitting an event somewhere? Maybe the issue is the lack of handling of the negotiationneeded, which the frontend should be handling correctly now.

@ryanrolds ryanrolds self-requested a review July 18, 2020 04:25
Copy link
Owner

@ryanrolds ryanrolds left a comment

Choose a reason for hiding this comment

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

Requesting the above changes.

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