Skip to content

Conversation

@kajarenc
Copy link
Collaborator

@kajarenc kajarenc commented Nov 16, 2022

📚 Context

We request video stream from a browser using the width prop, but it could have been changed very often (when we resize the window), which was leading to spamming the system queue of requests to the web camera. As a solution, we use debouncing width change handler and only request new video streams with constraints based on the width that stabilized after window resizes.

  • What kind of change does this PR introduce?

    • Bugfix
    • Feature
    • Refactoring
    • Other, please describe:

🧠 Description of Changes

  • Add bullet points summarizing your changes here

    • This is a breaking API change
    • This is a visible (user-facing) change

Revised:

Insert screenshot of your updated UI/code here

Current:

Insert screenshot of existing UI/code here

🧪 Testing Done

  • Screenshots included
  • Added/Updated unit tests
  • Added/Updated e2e tests

🌐 References

Does this depend on other work, documents, or tickets?


Contribution License Agreement

By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.

* fix camera jitter bug
@kajarenc kajarenc added the security-assessment-completed Security assessment has been completed for PR label Nov 24, 2022
@kajarenc kajarenc changed the title [WIP] Update react-webcam dependency Fix #5661, update react-webcam dependency Nov 24, 2022
@kajarenc kajarenc marked this pull request as ready for review November 28, 2022 12:32
Copy link
Collaborator

@vdonato vdonato left a comment

Choose a reason for hiding this comment

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

LGTM

It'd be nice to figure out a way to unit test this if possible, but I'm not entirely sure if there's a way of doing so that's not a ton of unwarranted effort, so I wouldn't try too hard to figure something out.

@kajarenc
Copy link
Collaborator Author

kajarenc commented Dec 2, 2022

LGTM

It'd be nice to figure out a way to unit test this if possible, but I'm not entirely sure if there's a way of doing so that's not a ton of unwarranted effort, so I wouldn't try too hard to figure something out.

Yes, I agree, for this specific case (working with browser API to access the camera and event for browser resize) it is really tricky to come up with unit tests.

@kajarenc kajarenc merged commit e3b101a into streamlit:develop Dec 2, 2022
tconkling added a commit that referenced this pull request Dec 2, 2022
* develop:
  Fix #5661, update react-webcam dependency (#5711)
  metrics_util_test.test_public_api_commands: minor cleanup (#5801)
  Release 1.15.2 (#5800)
tconkling added a commit to tconkling/streamlit that referenced this pull request Dec 2, 2022
# By Karen Javadyan (1) and others
# Via GitHub (1) and Tim Conkling (1)
* feature/ReleaseCacheV2:
  Fix streamlit#5661, update react-webcam dependency (streamlit#5711)
  metrics_util_test.test_public_api_commands: minor cleanup (streamlit#5801)
  Release 1.15.2 (streamlit#5800)

# Conflicts:
#	lib/tests/streamlit/runtime/metrics_util_test.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

security-assessment-completed Security assessment has been completed for PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

camera_input repeatedly reopens webcam as window width changes (in wide layout)

2 participants