Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

fix(voice): raise FileNotFoundError for missing local path innextcord.FFmpegPCMAudio#1272

Open
Dking08 wants to merge 6 commits intonextcord:masterfrom
Dking08:fix/voice-ffmpeg-filenotfound
Open

fix(voice): raise FileNotFoundError for missing local path innextcord.FFmpegPCMAudio#1272
Dking08 wants to merge 6 commits intonextcord:masterfrom
Dking08:fix/voice-ffmpeg-filenotfound

Conversation

@Dking08
Copy link

@Dking08Dking08 commentedOct 23, 2025
edited
Loading

Summary

Detect and surface ffmpeg failures via exit code checking.

  • Affects:FFmpegPCMAudio,FFmpegOpusAudio (via FFmpegAudio base),AudioPlayer
  • Changes:
    • Added_raise_if_process_failed() method to check ffmpeg exit codes
    • Call error check inFFmpeg*Audio.read() when stream ends unexpectedly
    • AddedAudioPlayer.error andAudioPlayer.has_failed properties for error inspection
    • UpdatedFFmpegPCMAudio docstring to document runtime error behavior

Behavioral impact:

  • Previously, ffmpeg failures (including missing files) would silently result in no audio playback
  • Now, ffmpeg failures raiseClientException with exit code information during playback
  • Users can checkAudioPlayer.has_failed andAudioPlayer.error to inspect playback failures

If applicable, link issue(s):


Initial Approach (Revised - see below)

Click to view original approach (replaced with more robust solution)

Original approach: Proactive file existence checking

The initial implementation added early file existence validation inFFmpegAudio.__init__ using a_looks_like_local_path() helper to distinguish local files from URLs/virtual inputs.

Why it was changed:

  • Flaky and unreliable (could misidentify URLs or ffmpeg virtual inputs)
  • Only caught one type of failure (missing files)
  • Required complex path validation heuristics

Revised approach: Reactive error detection via exit codes

  • More robust - catches ALL ffmpeg failures, not just missing files
  • Simpler code - no complex path heuristics needed
  • Better user experience - clear error messages with exit codes
  • Follows established patterns - error detection at point of failure

Reproduction and verification

Minimal example:

importnextcorddefafter_playback(error):iferror:print(f"Playback failed:{error}")# Missing file will cause ffmpeg to exit with errorsource=nextcord.FFmpegPCMAudio("non-existent-file.m4a")voice_client.play(source,after=after_playback)# Or check during/after playback:ifvoice_client._player.has_failed:print(f"Error:{voice_client._player.error}")

Expected:ClientException raised during.read() when ffmpeg exits with non-zero status, error accessible via AudioPlayer properties.

Additional notes:

  • This approach is more robust than path validation - it catches all ffmpeg failures, not just missing files
  • Error detection happens at the point of failure (during read) rather than construction
  • BothFFmpegPCMAudio andFFmpegOpusAudio benefit from this error handling

This is a Code Change

  • I have tested my changes.
  • I have updated the documentation to reflect the changes.
  • I have runtask pyright and fixed the relevant issues.

Tooling status

  • Lint/format: PASS (task lint)
  • Typecheck: PASS (poetry run python -m pyright → 0 errors)
  • Slotscheck: PASS (task slotscheck)
  • Syntax/build: PASS (Poetry install / compile succeeded)

Risk assessment

Low. Changes improve error visibility by detecting actual ffmpeg failures rather than attempting flaky proactive validation. Error information is now accessible to users via clean API.

@Dking08Dking08 marked this pull request as draftOctober 23, 2025 21:27
@Dking08Dking08force-pushed thefix/voice-ffmpeg-filenotfound branch from4fc607c tob717021CompareOctober 23, 2025 21:41
@Dking08Dking08 changed the titlefix(voice): raise FileNotFoundError for missing local path innextcord.FFmpegPCMAudio (#1229)fix(voice): raise FileNotFoundError for missing local path innextcord.FFmpegPCMAudioOct 23, 2025
@Dking08Dking08 marked this pull request as ready for reviewOctober 23, 2025 21:44
@teaishealthy
Copy link
Collaborator

teaishealthy commentedOct 24, 2025
edited
Loading

works, but im not sure if this is the right approach for this, it seems flaky. it's probably a better idea to leave the task of checking if a file exists to the user.

somewhat confusingly nothing in the codebase ever checks the exit code of ffmpeg ever.

some thoughts:
we could check if ffmpeg hasn't exited in FFmpeg*Audio.read and raise, but this error only gets collected in AudioPlayer._call_after and is printed to stdout, so it has no way of making it to the user

we could add something like has_failed to VoiceClient and AudioPlayer so the user could actually check why the client stopped.

@Dking08
Copy link
Author

Dking08 commentedOct 25, 2025
edited
Loading

@teaishealthy Thanks for the detailed feedback!
Yeah you are right, the earlier approach was kind of proactive and flawed.
I've implemented the changes you suggested - added exit code checking in read() and exposed error/has_failed properties on AudioPlayer.
Now it gives users a reliable way to detect failures without brittle path checks.

ashleney and teaishealthy reacted with rocket emoji

@Dking08Dking08 marked this pull request as draftOctober 25, 2025 11:37
- Remove unreliable file path validation- Add _raise_if_process_failed() method to check ffmpeg exit codes- Expose AudioPlayer.error and has_failed properties- Raise ClientException in read() when ffmpeg exits with non-zero statusReplaces flaky proactive file checks with robust error detection.
@Dking08Dking08force-pushed thefix/voice-ffmpeg-filenotfound branch from003111f tof60ac18CompareOctober 25, 2025 11:49
@Dking08Dking08 marked this pull request as ready for reviewOctober 25, 2025 11:57
@Dking08
Copy link
Author

@teaishealthy
Im looking forward to your feedback on this PR. If everything looks good and no further changes are needed, could you please atleast label it ashacktoberfest-accepted? It would mean a lot and really help me out.

Soheab reacted with laugh emoji

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@teaishealthyteaishealthyAwaiting requested review from teaishealthy

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

nextcord.FFmpegPCMAudio does not throw FileNotFoundError

2 participants

@Dking08@teaishealthy

Comments


[8]ページ先頭

©2009-2026 Movatter.jp