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

Temporary fix for video latency #111

Merged
merged 4 commits into from
Jan 10, 2024
Merged

Temporary fix for video latency #111

merged 4 commits into from
Jan 10, 2024

Conversation

amalnanavati
Copy link
Contributor

@amalnanavati amalnanavati commented Jan 10, 2024

Describe this pull request. Link to relevant GitHub issues, if any.

Joint PR with ada_feeding#151

Although #106 improved video streaming by replacing the dependency on web_video_server with direct subscription to compressed images, the latency of this approach depends on the device. On a desktop computer, the video stream has 1-3 sec latency, but on an iPhone 11, it has increasing latency that gets into the tens of seconds.

Looking into it, the most likely reason for this is that roslibjs uses EventEmitter2's emit event, which is synchronous. Thus, even if the subscriber on the ROS side has no queue, a queue builds up on the web app side.

The best solution to this, as documented here, is to implement WebRTC peer-to-peer streaming between the robot and web app. That is documented in issue #112 .

However, given the learning curve and time involved in implementing WebRTC, this PR and its affiliated PR ada_feeding#151 introduces a temporary fix to the problem, in the following ways:

  1. Rate-limits the publication of image topics that the web app subscribes to to 3 Hz. (parameter set both in ada_feeding_perception/config/republisher.yaml and in VideoFeed.jsx)
  2. Every 10 seconds, have the web app unsubscribe and re-subscribe to the topic, to clear any backlog. (parameter set in VideoFeed.jsx)

Note that these parameters can be tuned to find a subjectively optimal setting.

Explain how this pull request was tested, including but not limited to the below checkmarks.

  • Pull the 2 PRs and re-build.
  • Start the robot code and the web app.
  • Go to bite selection, move your hand in and out of the camera, verify that the latency is not more than 1-2 secs.
  • Go to the DetectingFace page, move your head around in front of the camera, verify that the latency is not more than 1-2 secs.

Before creating a pull request

  • Format React code with npm run format
  • [N/A] Format Python code by running python3 -m black . in the top-level of this repository
  • Thoroughly test your code's functionality, including unintended uses.
  • [N/A] Fully test the responsiveness of the feature as documented in the Responsiveness Testing Guidelines. If you deviate from those guidelines, document above why you deviated and what you did instead.
  • [N/A] Consider the user flow between states that this feature introduces, consider different situations that might occur for the user, and ensure that there is no way for the user to get stuck in a loop.

Before merging a pull request

  • Squash all your commits into one (or Squash and Merge)

@amalnanavati amalnanavati merged commit ec73b0e into main Jan 10, 2024
4 checks passed
@amalnanavati amalnanavati deleted the amaln/video_fix branch January 10, 2024 18:18
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.

1 participant