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

fix: Fix some phones unable to play audio #2728

Merged
merged 1 commit into from
Mar 28, 2025
Merged

Conversation

shaohuzhang1
Copy link
Contributor

fix: Fix some phones unable to play audio

Copy link

f2c-ci-robot bot commented Mar 28, 2025

Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Copy link

f2c-ci-robot bot commented Mar 28, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

// 标签朗读
this.statusList[index] = AudioStatus.READY
audioElement.pause()
}
}
}
getTextList(text: string, is_end: boolean) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are several issues and suggestions with the provided code:

  1. Multiple Instances of Audio Elements: The line <audio ref="audioPlayer" v-for="item in audioList" ...> generates multiple audio elements on each render, which is inefficient and could lead to performance problems.

  2. Conditional Rendering: Ensure that only one instance of div ref="audioCiontainer" is used unless you need dynamic rendering based on some condition.

  3. Speech Synthesis vs HTML Audio Element: The logic is inconsistent between handling SpeechSynthesisUtterance and HTMLAudioElement instances. This leads to redundant checks and can be streamlined.

  4. Error Handling: There's no error handling when creating a new HTMLAudioElement, which should use document.createElement('audio') instead of assigning it directly to a ref using Vue.js syntax.

  5. State Management: The statusList management seems scattered around different methods without consistent usage guidelines. Consider consolidating state management for better readability and maintainability.

  6. Method Names and Signatures: Method names like pause and logic inside them might not work as expected because they don't handle certain states correctly (e.g., PLAY_INT and other enum values).

  7. Code Style: There are minor style improvements that could enhance readability and consistency throughout the file.

Here are cleaned-up version recommendations:

// Remove the repeated div element and single instance control
<div ref="audioPanel">
  <!-- El button -->
</div>

<!-- Single container for all audio elements -->
<div ref="audioContainer"></div>

<script lang="ts">
import { defineComponent } from 'vue';
class AudioManage {
  textList: string[] = [];
  ttsType: 'TTS' | 'TAG_READING' = ''; // Add type annotation here
  statusList: number[] = []; // Define statuses

  async loadAudioItems() {
    await createHtmlElements(this.textList.map(item =>
      this.createAudioElement(item)
    ));
  }

  private createAudioElement(text: string): HTMLElement {
    const audio = document.createElement('audio');
    audio.controls = true;
    audio.hidden = true;
    const utterance = new SpeechSynthesisUtterance(text);
    // Handle playback events if needed
    return audio;
  }
}

function createHtmlElements(elements: HTMLElement[]): Array<HTMLDivElement> {
  const divContainer = document.createElement('div');
  const parentEl = document.getElementById('parentElementId'); // Replace with actual ID
  elements.forEach((element) => {
    divContainer.appendChild(element);
  });
  parentEl.appendChild(divContainer);
}
</script>

Please adapt the above changes according to your project structure and requirements.

@shaohuzhang1 shaohuzhang1 merged commit bf91579 into main Mar 28, 2025
4 checks passed
@shaohuzhang1 shaohuzhang1 deleted the pr@main@fix_audio branch March 28, 2025 10:24
if (window.speechSynthesis) {
window.speechSynthesis.cancel()
}

window.sendMessage = sendMessage
bus.on('on:transcribing', (status: boolean) => {
transcribing.value = status
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The code snippet provided is almost correct, with only one minor improvement. The speechSynthesis object might not be defined in some environments when the script runs, so adding a null check before calling its method helps prevent errors.

Optimization Suggestion:

Add an early return after checking if window.speechSynthesis exists to avoid unnecessary code execution.

Here's the updated code:

@@ -523,7 +523,10 @@ onMounted(() => {
     let userFormData = JSON.parse(localStorage.getItem(`${accessToken}userForm`) || '{}')
     form_data.value = userFormData
   }
+  if (window.speechSynthesis) {
     window.speechSynthesis.cancel() // Ensure this happens safely
-  }
+    window.sendMessage = sendMessage;
+    bus.on('on:transcribing', (status: boolean) => {
+      transcribing.value = status;
+    });
   }

This change ensures that the rest of the function can proceed without relying implicitly on the presence of window.speechSynthesis.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant