Skip to content

Conversation

@richiemcilroy
Copy link
Member

@richiemcilroy richiemcilroy commented Jan 12, 2026

Adds an experimental option to force the use of the FFmpeg decoder for video exports on macOS, along with related changes to error handling and diagnostics. The main goal is to provide a fallback for users experiencing hardware decoder issues, improving reliability and user experience for MP4 exports.

Greptile Overview

Greptile Summary

This PR adds an experimental option to force FFmpeg decoder usage for MP4 video exports on macOS, addressing hardware decoder reliability issues. The implementation includes:

Key Changes:

  • UI Toggle: Added macOS-only, MP4-only toggle in ExportPage to force FFmpeg decoder
  • Automatic Fallback: On macOS, AVAssetReader automatically falls back to FFmpeg if initialization fails
  • Retry Logic: Export automatically retries with FFmpeg decoder if frame decode errors occur
  • Global State Management: Uses AtomicBool to control decoder selection across the rendering pipeline

Architecture:
The force_ffmpeg_decoder flag flows from UI → export settings → global state, affecting decoder selection in spawn_decoder(). On macOS, the decoder priority is: forced FFmpeg (if enabled) → AVAssetReader → FFmpeg fallback. Automatic retry occurs when frame decode errors are detected without the flag set.

Error Handling Improvements:

  • New FrameDecodeFailed error variant with specific messaging
  • Reduced retry counts (5→3) and delays (100ms→50ms) for faster failure detection
  • Enhanced logging for decoder fallbacks and retry attempts

Testing:
Updated benchmark and long video export tests to explicitly set force_ffmpeg_decoder: false.

Confidence Score: 3/5

  • Moderate risk - has race condition with global state that could affect concurrent exports
  • The implementation has a P1 race condition issue where concurrent exports can interfere with each other through shared global state. Additionally, there's a P2 issue where panics could leave the flag permanently set. The timeout reduction (P2) may cause false positives on slower systems. The core logic is sound but the global state approach needs architectural improvement for production safety.
  • apps/desktop/src-tauri/src/export.rs (race condition with global state), crates/rendering/src/decoder/mod.rs (timeout reduction may be too aggressive)

Important Files Changed

File Analysis

Filename Score Overview
apps/desktop/src-tauri/src/export.rs 4/5 Adds FFmpeg decoder forcing logic and automatic retry with FFmpeg on frame decode errors. Refactored export logic into do_export helper. Uses global state for force_ffmpeg flag.
apps/desktop/src/routes/editor/ExportPage.tsx 5/5 Adds UI toggle for forcing FFmpeg decoder (macOS only, MP4 only). State is passed to export settings.
apps/desktop/src/utils/tauri.ts 5/5 Auto-generated type bindings updated with force_ffmpeg_decoder field and system diagnostics changes. No manual changes needed.
crates/export/src/mp4.rs 5/5 Added force_ffmpeg_decoder bool field to Mp4ExportSettings with serde default.
crates/export/tests/export_benchmark.rs 5/5 Updated test to set force_ffmpeg_decoder: false for compatibility.
crates/export/tests/long_video_export.rs 5/5 Updated test to set force_ffmpeg_decoder: false for compatibility.
crates/rendering/src/decoder/mod.rs 4/5 Adds global atomic bool for forcing FFmpeg decoder. Implements fallback from AVAssetReader to FFmpeg on macOS. Timeout reduced from 2000ms to 1000ms.
crates/rendering/src/lib.rs 4/5 Exports FFmpeg decoder control functions. Adds FrameDecodeFailed error variant. Reduced MAX_RETRIES from 5 to 3 and adjusted retry delays.
crates/rendering/src/zoom.rs 5/5 Minor formatting change in test assertion message (cosmetic).

Sequence Diagram

sequenceDiagram
    participant User as User (ExportPage.tsx)
    participant Export as export_video (export.rs)
    participant Rendering as cap_rendering
    participant Decoder as spawn_decoder (decoder/mod.rs)
    participant AVAsset as AVAssetReaderDecoder
    participant FFmpeg as FfmpegDecoder

    User->>Export: export_video(settings with force_ffmpeg_decoder)
    Export->>Rendering: set_force_ffmpeg_decoder(force_ffmpeg)
    Export->>Export: do_export()
    Export->>Rendering: render_video_to_channel()
    
    alt Force FFmpeg is true
        Rendering->>Decoder: spawn_decoder()
        Decoder->>FFmpeg: spawn FFmpeg decoder (forced)
        FFmpeg-->>Decoder: return decoder handle
    else Force FFmpeg is false (normal path)
        Rendering->>Decoder: spawn_decoder()
        Decoder->>AVAsset: spawn AVAssetReader
        alt AVAssetReader succeeds
            AVAsset-->>Decoder: return decoder handle
        else AVAssetReader fails
            Decoder->>FFmpeg: spawn FFmpeg decoder (fallback)
            FFmpeg-->>Decoder: return decoder handle with fallback_reason
        end
    end
    
    Rendering-->>Export: rendering result
    
    alt Export succeeds
        Export->>Rendering: set_force_ffmpeg_decoder(false)
        Export-->>User: Ok(path)
    else Export fails with frame decode error AND not forced
        Export->>Rendering: set_force_ffmpeg_decoder(true)
        Export->>Export: do_export() retry
        Export->>Rendering: render with FFmpeg forced
        alt Retry succeeds
            Export->>Rendering: set_force_ffmpeg_decoder(false)
            Export-->>User: Ok(path)
        else Retry fails
            Export->>Rendering: set_force_ffmpeg_decoder(false)
            Export-->>User: Err(retry_e)
        end
    else Export fails with other error
        Export->>Rendering: set_force_ffmpeg_decoder(false)
        Export-->>User: Err(e)
    end
Loading

@cursor
Copy link

cursor bot commented Jan 12, 2026

You have run out of free Bugbot PR reviews for this billing cycle. This will reset on February 8.

To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

3 files reviewed, 3 comments

Edit Code Review Agent Settings | Greptile

@richiemcilroy richiemcilroy merged commit ab4334c into main Jan 12, 2026
14 of 16 checks passed
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