fix: convert qq_official voice messages to wav#6944
fix: convert qq_official voice messages to wav#69441zzxy1 wants to merge 2 commits intoAstrBotDevs:masterfrom
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the QQ Official message event handling by introducing robust audio format management. It ensures that all voice messages are in the required WAV format before being converted to Tencent Silk, preventing potential issues with unsupported audio types and improving the reliability of voice message delivery. The changes include detection, conversion, and proper cleanup of temporary files, all backed by new test cases. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request enhances the QQOfficialMessageEvent to support non-WAV audio records by converting them to WAV format before processing them into Tencent Silk. It introduces a new utility method _is_wav_audio_file and includes tests for both WAV and non-WAV audio handling. The review comments suggest converting the newly added file I/O operations (os.path.exists, os.remove, and open()) to their asynchronous aiofiles.os counterparts to prevent blocking the asyncio event loop.
| if converted_record_wav_path and os.path.exists( | ||
| converted_record_wav_path | ||
| ): | ||
| try: | ||
| os.remove(converted_record_wav_path) | ||
| except OSError as e: | ||
| logger.warning( | ||
| f"[QQOfficial] failed to remove converted audio file: {e}" | ||
| ) |
There was a problem hiding this comment.
This block uses synchronous file operations (os.path.exists and os.remove) which can block the asyncio event loop. Please use the asynchronous versions from aiofiles.os. You will need to import aiofiles.os.
| if converted_record_wav_path and os.path.exists( | |
| converted_record_wav_path | |
| ): | |
| try: | |
| os.remove(converted_record_wav_path) | |
| except OSError as e: | |
| logger.warning( | |
| f"[QQOfficial] failed to remove converted audio file: {e}" | |
| ) | |
| if converted_record_wav_path and await aiofiles.os.path.exists( | |
| converted_record_wav_path | |
| ): | |
| try: | |
| await aiofiles.os.remove(converted_record_wav_path) | |
| except OSError as e: | |
| logger.warning( | |
| f"[QQOfficial] failed to remove converted audio file: {e}" | |
| ) |
| @staticmethod | ||
| def _is_wav_audio_file(file_path: str) -> bool: | ||
| try: | ||
| with open(file_path, "rb") as f: | ||
| header = f.read(12) | ||
| except OSError: | ||
| return False | ||
| return len(header) >= 12 and header[:4] == b"RIFF" and header[8:12] == b"WAVE" |
There was a problem hiding this comment.
The _is_wav_audio_file method performs synchronous file I/O using open(), which can block the asyncio event loop. Since aiofiles is already used in the project, it's better to use it here for non-blocking I/O. This method should be converted to an async method, and the call site updated with await.
| @staticmethod | |
| def _is_wav_audio_file(file_path: str) -> bool: | |
| try: | |
| with open(file_path, "rb") as f: | |
| header = f.read(12) | |
| except OSError: | |
| return False | |
| return len(header) >= 12 and header[:4] == b"RIFF" and header[8:12] == b"WAVE" | |
| @staticmethod | |
| async def _is_wav_audio_file(file_path: str) -> bool: | |
| try: | |
| async with aiofiles.open(file_path, "rb") as f: | |
| header = await f.read(12) | |
| except OSError: | |
| return False | |
| return len(header) >= 12 and header[:4] == b"RIFF" and header[8:12] == b"WAVE" |
Summary
Testing
Closes #6509
Summary by Sourcery
Handle QQ Official voice messages that are not already WAV by converting them before Silk encoding and cleaning up temporary files.
Bug Fixes:
Enhancements:
Tests: