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

Hide the waveform when playing recorded audio if show_recording_waveform is False #10405

Merged
merged 17 commits into from
Jan 24, 2025

Conversation

abidlabs
Copy link
Member

@abidlabs abidlabs commented Jan 22, 2025

For large audio files, its useful to provide a way to use the default browser controls to reduce latency and memory consumption.

Closes: #6801

Test all the reasonable combinations:

import gradio as gr

with gr.Blocks() as demo:
    gr.Audio(
        sources=["microphone"],
        waveform_options=gr.WaveformOptions(
            show_recording_waveform=True,
            show_controls=True,
        ),
    )
    gr.Audio(
        sources=["microphone"],
        waveform_options=gr.WaveformOptions(
            show_recording_waveform=True,
            show_controls=False,
        ),
    )
    gr.Audio(
        sources=["microphone"],
        waveform_options=gr.WaveformOptions(
            show_recording_waveform=False,
            show_controls=True,
        ),
    )
    gr.Audio(
        sources=["microphone"],
        waveform_options=gr.WaveformOptions(
            show_recording_waveform=True,
        ),
    )
    gr.Audio(
        sources=["microphone"],
        waveform_options=gr.WaveformOptions(
            show_recording_waveform=False,
        ),
    )

demo.launch()

@gradio-pr-bot
Copy link
Collaborator

gradio-pr-bot commented Jan 22, 2025

🪼 branch checks and previews

Name Status URL
Spaces ready! Spaces preview
Website building...
Storybook ready! Storybook preview
🦄 Changes detected! Details

Install Gradio from this PR

pip install https://gradio-pypi-previews.s3.amazonaws.com/1dcf118600dc276755104f02df520cb39df6ec30/gradio-5.13.0-py3-none-any.whl

Install Gradio Python Client from this PR

pip install "gradio-client @ git+https://github.com/gradio-app/gradio@1dcf118600dc276755104f02df520cb39df6ec30#subdirectory=client/python"

Install Gradio JS Client from this PR

npm install https://gradio-npm-previews.s3.amazonaws.com/1dcf118600dc276755104f02df520cb39df6ec30/gradio-client-1.10.0.tgz

Use Lite from this PR

<script type="module" src="https://gradio-lite-previews.s3.amazonaws.com/1dcf118600dc276755104f02df520cb39df6ec30/dist/lite.js""></script>

@gradio-pr-bot
Copy link
Collaborator

gradio-pr-bot commented Jan 22, 2025

🦄 change detected

This Pull Request includes changes to the following packages.

Package Version
@gradio/audio patch
@gradio/chatbot patch
@gradio/multimodaltextbox patch
gradio patch
  • Maintainers can select this checkbox to manually select packages to update.

With the following changelog entry.

Hide the waveform when playing recorded audio if show_recording_waveform is False

Maintainers or the PR author can modify the PR title to modify this entry.

Something isn't right?

  • Maintainers can change the version label to modify the version bump.
  • If the bot has failed to detect any changes, or if this pull request needs to update multiple packages to different versions or requires a more comprehensive changelog entry, maintainers can update the changelog file directly.

skip_length: The percentage (between 0 and 100) of the audio to skip when clicking on the skip forward / skip backward buttons. Defaults to 5.
sample_rate: The output sample rate (in Hz) of the audio after editing. Defaults to 44100.
show_recording_waveform: Whether to show the waveform when recording audio or playing audio.
show_controls: Whether to show the standard HTML audio player below the waveform when recording audio or playing audio. By default, this is set to False if `show_recording_waveform` is True, and vice versa.
Copy link
Member Author

Choose a reason for hiding this comment

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

I think this is the most sensible way to handle the case when show_recording_waveform=False without adding a new parameter.

@@ -36,7 +36,10 @@
export let show_download_button: boolean;
export let show_share_button = false;
export let editable = true;
export let waveform_options: WaveformOptions = {};
export let waveform_options: WaveformOptions = {
Copy link
Member Author

Choose a reason for hiding this comment

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

Added to preserve behavior for unit tests

@@ -195,6 +195,10 @@
border: 1px solid var(--block-border-color);
}
.duration-button {
Copy link
Member Author

Choose a reason for hiding this comment

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

Just a styling tweak for consistency:

image

@abidlabs abidlabs force-pushed the show_recording_waveform branch from 79794df to 9922b12 Compare January 22, 2025 06:30
@abidlabs abidlabs force-pushed the show_recording_waveform branch from a0cdd44 to 00f3622 Compare January 22, 2025 06:35
@abidlabs abidlabs force-pushed the show_recording_waveform branch from a0a92fe to 16770c2 Compare January 22, 2025 06:36
@@ -54,7 +54,10 @@
export let file_count: "single" | "multiple" | "directory" = "multiple";
export let max_plain_text_length = 1000;
export let waveform_settings: Record<string, any>;
export let waveform_options: WaveformOptions = {};
export let waveform_options: WaveformOptions = {
Copy link
Member Author

@abidlabs abidlabs Jan 22, 2025

Choose a reason for hiding this comment

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

waveform_options and waveform_settings shouldn't even be props here, but when I started refactoring this, the PR quickly got out of hand. Will refactor the audio out of multimodaltextbox here in a separate PR

Copy link
Collaborator

Choose a reason for hiding this comment

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

We should add waveform_options as a param to multimodaltextbox.

@abidlabs abidlabs marked this pull request as ready for review January 23, 2025 18:41
Copy link
Collaborator

@dawoodkhan82 dawoodkhan82 left a comment

Choose a reason for hiding this comment

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

Tested all combinations and look good to me.

One small nit: when both show_controls and show_recording_waveform areTrue, when changing the playback speed on one, it doesn't show the new playback speed on the others' controls.

@@ -54,7 +54,10 @@
export let file_count: "single" | "multiple" | "directory" = "multiple";
export let max_plain_text_length = 1000;
export let waveform_settings: Record<string, any>;
export let waveform_options: WaveformOptions = {};
export let waveform_options: WaveformOptions = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should add waveform_options as a param to multimodaltextbox.

@dawoodkhan82
Copy link
Collaborator

Also not sure when anyone would have both show_controls and show_recording_waveform set to True. Should we even allow both to show. Or maybe just default to one and ignore the other.

@abidlabs
Copy link
Member Author

Thanks @dawoodkhan82, good points, I'll just deprecate show_controls. If show_recording_waveform=True, then the waveform shows, if its False, the default controls show. That should address both points.

@abidlabs abidlabs added the v: patch A change that requires a patch release label Jan 24, 2025
@abidlabs
Copy link
Member Author

Ok made the change, thanks @dawoodkhan82!

@abidlabs abidlabs merged commit 92dda15 into main Jan 24, 2025
23 checks passed
@abidlabs abidlabs deleted the show_recording_waveform branch January 24, 2025 03:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v: patch A change that requires a patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hide the waveform when playing recorded audio if show_recording_waveform is False
3 participants