Skip to content

enhancement: record audio and video from webcam simultaneously#2721

Merged
abidlabs merged 9 commits into
gradio-app:mainfrom
MandarGogate:main
Dec 26, 2022
Merged

enhancement: record audio and video from webcam simultaneously#2721
abidlabs merged 9 commits into
gradio-app:mainfrom
MandarGogate:main

Conversation

@jeeanribeiro
Copy link
Copy Markdown
Contributor

Description

From @MandarGogate issue:

Is your feature request related to a problem? Please describe.
I want to build a demo for machine learning model that processes Audio-Visual data from webcam. The current Video component records from webcam without audio.
Describe the solution you'd like
Since the Video component uses MediaRecorder API it can record audio and video simultaneously.

Changes:

- Enables audio in Webcam.svelte inside `access_webcam` function

Closes: #2704

Checklist:

  • I have performed a review of the code
  • I have added a short summary of my change to the CHANGELOG.md
  • The code follows the style guidelines of this project

@jeeanribeiro
Copy link
Copy Markdown
Contributor Author

@MandarGogate
I think that for this PR to be mergeable, it just needs the summary of the changes added in CHANGELOG.md, you can check the instructions about it here: https://github.com/gradio-app/gradio/blob/main/.github/PULL_REQUEST_TEMPLATE.md
As I'm trying to merge your fork, I don't have the permission to do this change

@abidlabs
Waiting for your review 😄

@pngwn
Copy link
Copy Markdown
Member

pngwn commented Nov 25, 2022

We should probably make this an option to ensure the author has choice and that users aren't bombarded with permission requests when they run apps. Many models don't need or can't use both audio and video inputs.

@abidlabs
Copy link
Copy Markdown
Member

Agreed with @pngwn‘s suggestion — ideally we have a Boolean parameter e.g. “include_audio” which can be True by default and applies to the webcam scenario but also to the regular Video component. In other words, if it is False then the audio from an uploaded Video would be removed as well

@MandarGogate
Copy link
Copy Markdown
Contributor

I have added include_audio parameter. You can try it yourself using https://gist.github.com/MandarGogate/d49d1fd871b85c9840d5df7656c10c41

Copy link
Copy Markdown
Member

@pngwn pngwn left a comment

Choose a reason for hiding this comment

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

Looks good to me. Left a few suggestions for the wording of the change log and docstring.

Comment thread CHANGELOG.md Outdated
Comment thread gradio/components.py Outdated
MandarGogate and others added 2 commits November 26, 2022 15:50
Co-authored-by: pngwn <hello@pngwn.io>
Co-authored-by: pngwn <hello@pngwn.io>
@pngwn
Copy link
Copy Markdown
Member

pngwn commented Nov 27, 2022

Is defaulting to True a breaking change? Are there any times when the presence of an audio track will cause the predict function/ model that is processing the video to choke + fail?

@abidlabs
Copy link
Copy Markdown
Member

Is defaulting to True a breaking change? Are there any times when the presence of an audio track will cause the predict function/ model that is processing the video to choke + fail?

Good point, let's test with a few Spaces that currently take in a webcam video. I believe @nateraw has built some...

@abidlabs
Copy link
Copy Markdown
Member

Sorry for the late response @jeeanribeiro. I tested this out and it turns out setting include_audio to be True by default will be a breaking change because some users have webcam -> video demos where they do not expect the webcam to be recording the video. My suggestion would be to have include_audio be None, which gets set to True by default if source is upload but False if source is webcam. Would you be able to make that change and merge in the latest changes from main. Happy to re-review

cc @pngwn @aliabid94

@MandarGogate
Copy link
Copy Markdown
Contributor

@abidlabs I have updated the default behaviour of include_audio and merged latest changes from main

@abidlabs
Copy link
Copy Markdown
Member

Tested and works great. Couple of tests need to be fixed but I can go ahead and do that -- thanks for this addition to the gradio library @jeeanribeiro!

@abidlabs abidlabs merged commit 24f413c into gradio-app:main Dec 26, 2022
This was referenced Dec 26, 2022
abidlabs added a commit that referenced this pull request Dec 26, 2022
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.

Recording Audio and Video from webcam simultaneously

4 participants