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

add calls to flush_audio for say() and rtvi action #492

Merged
merged 2 commits into from
Sep 25, 2024

Conversation

kwindla
Copy link
Contributor

@kwindla kwindla commented Sep 23, 2024

Call flush_audio() after direct calls to TTS say() and when processing TTSSpeakFrames.

@kwindla kwindla requested a review from aconchillo September 23, 2024 02:06
@@ -186,6 +186,7 @@ async def run_tts(self, text: str) -> AsyncGenerator[Frame, None]:

async def say(self, text: str):
await self.process_frame(TextFrame(text=text), FrameDirection.DOWNSTREAM)
await self.flush_audio()
Copy link
Contributor

@aconchillo aconchillo Sep 23, 2024

Choose a reason for hiding this comment

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

flush_audio is only available in AsynTTSService since I thought it only made sense there. We have two options:

  1. Add flush_audio to TTSService
  2. Override say() in AsyncTTSService with
async def say(self, text: str):
    super().say(text)
    await self.flush_audio()

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think there's anything to flush in TTSService, so maybe option 2 is better?

@@ -235,6 +236,7 @@ async def process_frame(self, frame: Frame, direction: FrameDirection):
await self.push_frame(frame, direction)
elif isinstance(frame, TTSSpeakFrame):
await self._push_tts_frames(frame.text)
await self.flush_audio()
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here.

    async def process_frame(self, frame: Frame, direction: FrameDirection):
        super().process_frame(frame, direction)
        if isinstance(frame, TTSSpeakFrame):
            await self.flush_audio()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should be await super().process_frame(frame, direction), right?

(An await in front of the call to the super class's method.)

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, sorry.

@kwindla kwindla force-pushed the khk/flush-more-audio branch from cb11b6f to 3621fce Compare September 25, 2024 16:20
@kwindla kwindla requested a review from aconchillo September 25, 2024 16:21
@@ -278,6 +282,11 @@ async def cancel(self, frame: CancelFrame):
await self._stop_frame_task
self._stop_frame_task = None

async def process_frame(self, frame: Frame, direction: FrameDirection):
super().process_frame(frame, direction)
Copy link
Contributor

Choose a reason for hiding this comment

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

We are still missing the await here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Egads. Sorry. I clearly do not know how to use computers.

@kwindla kwindla requested a review from aconchillo September 25, 2024 17:59
@aconchillo
Copy link
Contributor

LGTM!

@kwindla kwindla merged commit 8f2941c into main Sep 25, 2024
3 checks passed
@aconchillo aconchillo deleted the khk/flush-more-audio branch October 23, 2024 20:55
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.

2 participants