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.

Sign up for GitHub

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

Jump to bottom

TinyPilot reports H.264 failure when remote display is just off #1487

Closed
mtlynch opened this issue Jul 5, 2023 · 1 comment · Fixed by #1619
Closed

TinyPilot reports H.264 failure when remote display is just off #1487

mtlynch opened this issue Jul 5, 2023 · 1 comment · Fixed by #1619
Assignees
@jdeanwallace
Labels
bug Something isn't working medium

Comments

@mtlynch
Copy link
Contributor

mtlynch commented Jul 5, 2023 •
edited
Loading

One issue I've run into a lot lately is that when the remote computer is just powered off and there's no video signal, TinyPilot thinks that H.264 has failed:

image

We should adjust the H.264 -> MJPEG failover to check first whether there's a signal at all. If the reason Janus isn't streaming anything is that there's no image to stream, we shouldn't fail over to MJPEG or present a warning to the user.

To repro

  1. Enable H.264 video on TinyPilot
  2. Turn off target computer

Expected: TinyPilot stays in H.264 mode.

Actual: TinyPilot fails over into MJPEG mode.

Longer demo

shutdown-error.mp4
@mtlynch mtlynch added bug Something isn't working medium labels Jul 5, 2023
@jdeanwallace jdeanwallace self-assigned this Aug 21, 2023
@mtlynch
Copy link
Contributor Author

mtlynch commented Aug 29, 2023

I'm realizing we need to get the 2.6.1 image to the contract manufacturer pretty early so that there's no confusion over which version to use, so I'm punting this one to 2.6.2.

@jdeanwallace jdeanwallace mentioned this issue Sep 6, 2023
Merged
jdeanwallace added a commit that referenced this issue Sep 12, 2023
@jdeanwallace
Maintain h.264 video stream when target machine is switched off (#1619)
9fa1616 
Resolves #1487

This PR refactors our remote-screen WebRTC API to allow media tracks to
be added/removed without automatically enabling/disabling the WebRTC
streaming mode. This gives more responsibility to the (function) caller,
but also gives more control in deciding when to change streaming modes.

Demo video:


https://github.com/tiny-pilot/tinypilot/assets/6730025/d891fbcb-1cea-41fe-b775-948b5a9879e6

Notes
1. We've renamed `enableWebrtcStreamTrack` -> `addWebrtcStreamTrack`
* This method no longer enables WebRTC mode (i.e., shows the video
element)
1. We've renamed `disableWebrtcStreamTrack` -> `removeWebrtcStreamTrack`
* This method no longer disables WebRTC mode (i.e., hides the video
element)
1. WebRTC mode can now both be enabled and have zero media tracks
    * This was the cause of the original bug
1. The remote-screen will still failover to MJPEG when the connection to
Janus fails, but will not try to reconnect until a full page refresh

<a data-ca-tag
href="https://codeapprove.com/pr/tiny-pilot/tinypilot/1619"><img
src="https://codeapprove.com/external/github-tag-allbg.png" alt="Review
on CodeApprove" /></a>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jdeanwallace jdeanwallace

Labels
bug Something isn't working medium
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Maintain h.264 video stream when target machine is switched off tiny-pilot/tinypilot
2 participants
@jdeanwallace @mtlynch