fix(core): fix Record.convert_to_file_path saving audio as .jpg#6868
fix(core): fix Record.convert_to_file_path saving audio as .jpg#6868BillionClaw wants to merge 1 commit intoAstrBotDevs:masterfrom
Conversation
When Record.audio file is downloaded via HTTP URL, save_temp_img() was used which always saves with .jpg extension. This corrupts non-JPG audio files (MP3, OGG, AAC, etc.). Add download_audio_by_url() and save_temp_audio() that save audio data without image processing, using .audio extension to avoid format corruption. Fixes: Record component convert_to_file_path for non-WAV audio URLs.
dosubot
bot
added
the
size:M
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 resolves a critical issue where audio and voice messages sent to the QQ bot were not being processed correctly after a version upgrade. The root cause was identified as Record.convert_to_file_path() incorrectly saving audio files with a .jpg extension due to using an image download utility. The fix introduces dedicated functions for saving and downloading audio files, ensuring they retain their proper format and extension, thereby enabling the QQ bot to correctly upload and process these media types. 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
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The docstrings for
save_temp_audioanddownload_audio_by_urlmention a "proper"/"preserving" extension, but the implementation always uses a.audiosuffix without inspecting the URL or headers; either adjust the behavior to actually preserve the extension or clarify the docstrings to avoid confusion. - The SSL handling and retry logic in
download_audio_by_urlduplicates the pattern used indownload_image_by_url; consider extracting a shared helper for URL downloads to reduce repetition and keep future fixes centralized. - The
except Exception as e: raise eindownload_audio_by_urlis redundant and can be removed (or replaced with a more specific log/wrap) since it doesn’t change the default exception behavior.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The docstrings for `save_temp_audio` and `download_audio_by_url` mention a "proper"/"preserving" extension, but the implementation always uses a `.audio` suffix without inspecting the URL or headers; either adjust the behavior to actually preserve the extension or clarify the docstrings to avoid confusion.
- The SSL handling and retry logic in `download_audio_by_url` duplicates the pattern used in `download_image_by_url`; consider extracting a shared helper for URL downloads to reduce repetition and keep future fixes centralized.
- The `except Exception as e: raise e` in `download_audio_by_url` is redundant and can be removed (or replaced with a more specific log/wrap) since it doesn’t change the default exception behavior.
## Individual Comments
### Comment 1
<location path="astrbot/core/utils/io.py" line_range="66-70" />
<code_context>
return p
+def save_temp_audio(audio_data: bytes) -> str:
+ """Save audio data to a temporary file with a proper extension."""
+ temp_dir = get_astrbot_temp_path()
+ timestamp = f"{int(time.time())}_{uuid.uuid4().hex[:8]}"
+ p = os.path.join(temp_dir, f"recordseg_{timestamp}.audio")
+ with open(p, "wb") as f:
+ f.write(audio_data)
</code_context>
<issue_to_address>
**issue:** Function docstring claims a proper extension is used, but the implementation always writes `.audio`.
Please either (a) change the implementation to accept/use an extension or filename hint so it truly uses a “proper” extension, or (b) update the docstring to state that the file is always saved with a `.audio` extension, so callers don’t rely on the extension to infer the actual audio type.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| def save_temp_audio(audio_data: bytes) -> str: | ||
| """Save audio data to a temporary file with a proper extension.""" | ||
| temp_dir = get_astrbot_temp_path() | ||
| timestamp = f"{int(time.time())}_{uuid.uuid4().hex[:8]}" | ||
| p = os.path.join(temp_dir, f"recordseg_{timestamp}.audio") |
There was a problem hiding this comment.
issue: Function docstring claims a proper extension is used, but the implementation always writes .audio.
Please either (a) change the implementation to accept/use an extension or filename hint so it truly uses a “proper” extension, or (b) update the docstring to state that the file is always saved with a .audio extension, so callers don’t rely on the extension to infer the actual audio type.
dosubot
bot
added
the
area:core
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly addresses an issue where audio files from URLs were being saved with an incorrect .jpg extension. The introduction of dedicated functions for handling audio downloads is a good approach. My review includes a suggestion to refactor the new download_audio_by_url function to improve maintainability by reducing code duplication and to correct its docstring for clarity.
| async def download_audio_by_url(url: str) -> str: | ||
| """Download audio from URL, preserving extension. Returns local file path.""" | ||
| try: | ||
| ssl_context = ssl.create_default_context(cafile=certifi.where()) | ||
| connector = aiohttp.TCPConnector(ssl=ssl_context) | ||
| async with aiohttp.ClientSession( | ||
| trust_env=True, | ||
| connector=connector, | ||
| ) as session: | ||
| async with session.get(url) as resp: | ||
| data = await resp.read() | ||
| return save_temp_audio(data) | ||
| except (aiohttp.ClientConnectorSSLError, aiohttp.ClientConnectorCertificateError): | ||
| logger.warning( | ||
| f"SSL certificate verification failed for {url}. " | ||
| "Disabling SSL verification (CERT_NONE) as a fallback." | ||
| ) | ||
| ssl_context = ssl.create_default_context() | ||
| ssl_context.check_hostname = False | ||
| ssl_context.verify_mode = ssl.CERT_NONE | ||
| async with aiohttp.ClientSession() as session: | ||
| async with session.get(url, ssl=ssl_context) as resp: | ||
| data = await resp.read() | ||
| return save_temp_audio(data) | ||
| except Exception as e: | ||
| raise e |
There was a problem hiding this comment.
This new function download_audio_by_url contains a significant amount of duplicated code for handling SSL fallbacks, which is also present in other download functions like download_image_by_url. This makes the code harder to maintain. Additionally, the docstring is misleading as it claims to "preserve extension", but the implementation saves the file with a hardcoded .audio extension via save_temp_audio.
To improve maintainability and correctness, you could refactor this function to reduce duplication and update the docstring. Here's a suggested refactoring that uses a nested helper function to avoid repeating the download logic:
async def download_audio_by_url(url: str) -> str:
"""Download audio from URL. Returns local file path."""
async def _download_and_save(session, **kwargs):
async with session.get(url, **kwargs) as resp:
resp.raise_for_status()
data = await resp.read()
return save_temp_audio(data)
try:
ssl_context = ssl.create_default_context(cafile=certifi.where())
connector = aiohttp.TCPConnector(ssl=ssl_context)
async with aiohttp.ClientSession(
trust_env=True,
connector=connector,
) as session:
return await _download_and_save(session)
except (aiohttp.ClientConnectorSSLError, aiohttp.ClientConnectorCertificateError):
logger.warning(
f"SSL certificate verification failed for {url}. "
"Disabling SSL verification (CERT_NONE) as a fallback."
)
ssl_context = ssl.create_default_context()
ssl_context.check_hostname = False
ssl_context.verify_mode = ssl.CERT_NONE
async with aiohttp.ClientSession() as session:
return await _download_and_save(session, ssl=ssl_context)1 participant
Summary
Fixes issue #6853: After upgrading from 4.19.5 to 4.20+, sending voice or audio files to the QQ bot produces no response.
Root Cause
When a
Recordmessage component downloads an audio file from a URL,convert_to_file_path()calleddownload_image_by_url()which always saves files with.jpgextension viasave_temp_img(). This corrupts non-JPG audio formats (MP3, OGG, AAC, silk, etc.).The QQ official bot's
upload_group_and_c2c_media()then fails because the file extension doesn't match the actual content type.Fix
save_temp_audio()inastrbot/core/utils/io.py- saves audio bytes with.audioextension (no PIL/Image processing)download_audio_by_url()inastrbot/core/utils/io.py- downloads audio from URL using the new saverRecord.convert_to_file_path()to usedownload_audio_by_url()instead ofdownload_image_by_url()for HTTP URLsThis is a complementary fix to PR #6867 (which fixes non-WAV audio conversion in
_parse_to_qqofficial()). Both fixes are needed: PR #6867 ensures the audio is in WAV format for Tencent Silk encoding, while this fix ensures the audio file is saved correctly from URL without extension corruption.Testing
download_audio_by_url()saves files without PIL image processingdownload_image_by_url()unchanged)Closes #6853
Summary by Sourcery
Fix audio record handling so that audio downloaded from URLs is saved correctly and can be processed by the QQ bot.
Bug Fixes:
Enhancements: