Skip to content

Fix stdout pipe deadlock in FFmpeg conversion#732

Open
HapppppyMoon wants to merge 1 commit into
Tichau:integrationfrom
HapppppyMoon:fix/stdout-pipe-deadlock
Open

Fix stdout pipe deadlock in FFmpeg conversion#732
HapppppyMoon wants to merge 1 commit into
Tichau:integrationfrom
HapppppyMoon:fix/stdout-pipe-deadlock

Conversation

@HapppppyMoon
Copy link
Copy Markdown

Summary

This PR fixes a critical bug where FFmpeg conversions hang indefinitely due to a stdout pipe deadlock. The -progress pipe:1 flag writes progress data to stdout, but the application only reads stderr — causing the pipe buffer to fill and FFmpeg to block on write.

Key changes

  • Removed -progress pipe:1 from FFmpeg base arguments
  • Disabled RedirectStandardOutput since stdout is never consumed
  • Progress tracking continues to work via stderr legacy format parsing (already implemented)

Root cause

Component Before After
baseArgs (line 91) "-n -progress pipe:1" "-n"
RedirectStandardOutput (line 82) true false

-progress pipe:1 sends structured progress to stdout, but Convert() only reads StandardError. Once the stdout pipe buffer fills (~4-64KB), FFmpeg blocks on write → process hangs → output file never finalized (no moov atom for MP4) → unplayable file.

Impact

  • Conversions no longer hang mid-process
  • Output files are properly finalized
  • No change in progress bar behavior — ParseFFMPEGOutput already parses stderr progress (size=... time=... bitrate=...)

Testing

  • Verified that the progress regex matches FFmpeg's default stderr output
  • Confirmed no other code references StandardOutput in the conversion pipeline
  • Affected file: ConversionJob_FFMPEG.cs (2 lines changed)

- ffmpeg was given -progress pipe:1 which writes progress data to stdout
- stdout was redirected but never read, filling the pipe buffer
- Once the buffer filled, ffmpeg blocked on write and hung indefinitely
- Output files had no moov atom written, resulting in unplayable files
- Progress parsing already works via stderr legacy format (size=... time=...)
- previously: `-n -progress pipe:1` with `RedirectStandardOutput = true`
- now: `-n` with `RedirectStandardOutput = false`
Copilot AI review requested due to automatic review settings March 23, 2026 07:03
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses an FFmpeg conversion hang caused by redirecting stdout while emitting progress data to pipe:1 (stdout) without consuming it, leading to a blocked FFmpeg process once the stdout pipe buffer fills.

Changes:

  • Removed -progress pipe:1 from FFmpeg base arguments.
  • Disabled RedirectStandardOutput for the FFmpeg process since stdout is not read.
  • Kept progress tracking via existing stderr parsing (size=... time=... bitrate=... format).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

UCHIHAHA103 pushed a commit to UCHIHAHA103/FileConverter that referenced this pull request Apr 26, 2026
Language fix (upstream Tichau#593 Tichau#606 Tichau#609 Tichau#646 Tichau#667 Tichau#673 Tichau#690 Tichau#692 Tichau#735 Tichau#737 Tichau#750):
- Helpers.GetSupportedCultures now probes both the legacy Languages\<culture>\
  folder and the standard .NET satellite path <exe>\<culture>\, so language
  detection no longer silently breaks when the robocopy post-build step fails.
- Replace the fragile robocopy /MOVE post-build step with a plain copy so the
  default .NET layout is preserved alongside the Languages\ layout.
- Guard Settings.ApplicationLanguage setter + SettingsViewModel Save/Close with
  try/catch so a culture apply failure no longer silently closes the Settings
  window without persisting changes.

FFmpeg fix (upstream Tichau#749 Tichau#740 Tichau#739 Tichau#716 Tichau#703 Tichau#700, cherry-picked from PR Tichau#732
by HapppppyMoon):
- Remove -progress pipe:1 and disable RedirectStandardOutput, which were
  filling the unread stdout pipe buffer and deadlocking ffmpeg in v2.2.
- Progress parsing still works via the legacy stderr size=... time=... format.
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