Skip to content

Conversation

@drondeseries
Copy link
Contributor

Fix: Ensure Robust Failover for Prematurely Ending Streams

This PR addresses an issue where the channel streaming failover mechanism would not correctly engage if the primary stream source failed prematurely after HTTP headers had already been sent to the client. Previously, FFmpeg processes exiting with a status code of 0 (even with errors like "Stream ends prematurely" or "I/O error" in stderr) were treated as successful, halting any further failover attempts for that channel request.

Changes Implemented:

  1. Enhanced Error Detection in startStream:

    • The StreamController::startStream() method now captures FFmpeg's stderr output.
    • It explicitly checks for known error patterns (e.g., "Stream ends prematurely", "Error during demuxing: I/O error") in the stderr.
    • If such patterns are detected, a SourceNotResponding exception is thrown, irrespective of FFmpeg's exit code. This ensures these conditions are treated as failures.
  2. Improved Exception Propagation in startStream:

    • The generic catch (Exception $e) block within startStream has been modified to re-throw exceptions (unless they are simple "Connection aborted by client" messages). This allows critical exceptions, like the SourceNotResponding thrown for premature ends, to properly propagate to the calling __invoke method for appropriate handling.
  3. Refined Failover Logic in __invoke:

    • The catch blocks within StreamController::__invoke() have been updated to more intelligently handle failures occurring after headers have been sent.
    • If an exception is caught and headers were already sent:
      • A check is now performed to see if the failing stream was the primary source for the channel.
      • If it was the primary source, the failover process is now allowed to continue to the next configured backup source for that channel. This is the key change enabling failover in the problematic scenario.
      • If it was a subsequent failover stream that failed after headers were initially sent (by the primary), the request will terminate as before, preventing potentially malformed responses to the client.

Impact:

With these changes, the system is now significantly more resilient. If a primary stream starts (sending headers to the client) but then quickly fails due to issues like "Stream ends prematurely," the application will now correctly attempt to switch to the next configured failover source for that channel, improving the continuity of service for the end-user.

Testing:
Verified through log analysis across multiple iterations. The latest logs confirm:

  • Premature stream errors are correctly identified and trigger a SourceNotResponding exception.
  • This exception correctly propagates from startStream to __invoke.
  • __invoke's logic now successfully initiates a failover attempt to the next available source for the originally requested channel, even if the primary stream had already sent headers before failing.

google-labs-jules bot and others added 22 commits June 22, 2025 18:22
When an ffmpeg stream fails mid-stream after exhausting all its internal retries,
the system will now attempt to switch to the next available failover stream
for the channel.

This is achieved by:
1. Introducing a `MaxRetriesReachedException`.
2. Modifying `StreamController::startStream` to throw this exception when
   ffmpeg retries are exhausted for a specific stream URL.
3. Updating `StreamController::__invoke` to catch this new exception,
   log the event, update stream counts, mark the problematic URL as
   bad in cache, and then continue to the next failover stream in the
   channel's list.
When a stream fails due to MaxRetriesReachedException being thrown from
the StreamedResponse callback, Laravel's default error handling would
attempt to render an HTML error page, leading to a 'headers already sent'
fatal error because the stream had already started sending data.

This commit modifies the global exception handling in `bootstrap/app.php`
to specifically catch MaxRetriesReachedException. When caught, it now:
1. Logs the detailed error.
2. Returns a plain text 503 response directly.

This bypasses the default HTML error rendering for this specific case,
preventing the fatal error and making the stream termination cleaner.
This does not implement mid-stream failover to a new URL but resolves
the immediate fatal error condition.
Refines the handling of MaxRetriesReachedException during streaming.
When this exception occurs (indicating a stream URL has failed definitively
after multiple ffmpeg retries), the custom exception handler in
`bootstrap/app.php` now:

1. Logs the error.
2. Checks `headers_sent()`.
3. If headers were not already sent, it attempts to send a minimal
   plain text 503 error response.
4. Calls `exit;` to immediately terminate script execution.

This `exit;` call is crucial as it prevents Laravel's default error
handler from attempting to render an HTML error page, which was the
primary cause of the 'headers already sent' fatal error when the
StreamedResponse had already begun sending output.

This makes the failure of a single stream URL cleaner and more stable,
though it does not implement mid-stream failover to a new URL.
This significant refactor changes how streams are handled in `StreamController`
to enable more robust failover, particularly when a stream URL fails.

Key changes:

1.  **Removed `StreamedResponse` from `startStream`:**
    *   `startStream` now handles the `ffmpeg` process execution and output
        piping directly, rather than returning a `StreamedResponse` object.
    *   It manages PHP `ini_set` for direct output, `ffprobe` pre-checks,
        the `ffmpeg` retry loop, and direct `echo`/`flush` of video data.
    *   If `ffmpeg` fails definitively for a URL after retries, `startStream`
        now throws `MaxRetriesReachedException` synchronously.

2.  **Centralized Header Management:**
    *   A new private method `sendStreamingHeaders` is introduced to send all
        initial HTTP streaming headers (200 OK, Content-Type, Cache-Control, etc.).
    *   A `$headersSentInfo` flag (passed by reference as an array `['value' => false]`)
        is used between `__invoke` (or `episode`) and `startStream` to ensure
        headers are sent only once for the entire client request, by the first
        successful stream attempt that starts outputting data.

3.  **Enhanced `__invoke` (Channel Streaming):**
    *   The `foreach` loop in `__invoke` now correctly iterates through primary
        and failover stream URLs.
    *   It calls the refactored `startStream`.
    *   If `startStream` throws `SourceNotResponding` (e.g. ffprobe fails) or
        `MaxRetriesReachedException` (ffmpeg fails mid-stream for that URL),
        `__invoke` catches these, logs, decrements active stream counts,
        caches the bad URL, and `continue`s to the next failover URL.
    *   If a stream starts successfully (headers sent) and then fails
        (e.g. `MaxRetriesReachedException`), further failover attempts for that
        *specific client request* are gracefully terminated because HTTP headers
        cannot be resent. The client would experience a broken stream.
    *   If all stream URLs fail before any data is sent, a 503 error is aborted.

4.  **Updated `episode` Method:**
    *   Refactored for consistency to use the new `startStream` and header
        management logic. Failures typically result in a 503 error as episodes
        don't have failover URLs.

5.  **Global Exception Handler Modification:**
    *   The global handler for `MaxRetriesReachedException` in `bootstrap/app.php`
        (that called `exit;`) has been temporarily commented out to allow
        `StreamController` to catch and handle this exception for its failover logic.
        A TODO has been left to review this.

This change allows the system to try subsequent failover URLs if an initial stream
attempt fails before HTTP response headers and video data are sent to the client.
It also makes the server-side stream handling more direct within the controller.
This commit further refines stream handling in `StreamController` to prevent
'headers already sent' errors by ensuring script termination via `exit;`
after HTTP headers and stream data have begun sending.

Changes:

1.  `StreamController::startStream`:
    *   If this method was responsible for sending headers (i.e., the
        `$headersHaveBeenSent['value']` flag is true), it now calls `exit;`
        at the end of its execution (after the ffmpeg loop completes or is
        aborted). This ensures that if `startStream` handled the output, it
        also handles the script termination, preventing Laravel from trying
        to send further headers.

2.  `StreamController::__invoke` (Channels):
    *   If `startStream` returns (indicating client disconnect or successful
        finite stream) and headers had been sent, `__invoke` now also calls
        `exit;` as a safeguard.
    *   In `catch` blocks for `MaxRetriesReachedException` and generic `Exception`,
        if the exception occurred *after* headers for that stream attempt were
        sent (`$headersSentInfo['value'] == true`), the code now logs the issue
        and calls `exit;`. This stops attempts to failover to a new URL on an
        already committed HTTP response, which would be invalid.
    *   If an exception is caught and headers were *not* sent for that attempt,
        it continues to the next failover URL as intended.
    *   Added an `exit;` at the end of `__invoke` if the stream loop completes
        but headers had been sent by one of an earlier attempt.

3.  `StreamController::episode`:
    *   Applied similar `exit;` logic to ensure clean termination if headers
        were sent before an error or before the stream completed.

These changes make the conditions for script termination explicit when direct
output has occurred, significantly improving robustness against 'headers already
sent' errors. Failover to a new URL remains conditional on the previous stream
attempt failing *before* it sends HTTP data headers.
This commit resolves two issues:
1. Modifies FFmpeg command generation to use `libopus` instead of
   `opus` when 'opus' is specified as the audio codec. This avoids
   errors related to 'opus' being an experimental codec that requires
   the `-strict -2` flag, thereby preventing `Conversion failed!` errors.

2. Ensures that the previously implemented failover logic for prematurely
   ending streams (where FFmpeg might exit with code 0 but output
   errors to stderr) functions correctly by ensuring exceptions from
   `startStream` propagate to `__invoke` for proper handling.
   This includes allowing failover for a primary stream that fails
   after sending headers, while correctly terminating if a subsequent
   failover stream also fails after headers were sent.
This commit provides a comprehensive fix for FFmpeg streaming issues:

1.  **libopus Encoder Parameters:**
    - Modifies `StreamController::buildCmd()` to automatically append
      `-b:a 128k` when `libopus` is used as the audio encoder (either
      directly or when `opus` is switched to `libopus`).
    - This resolves FFmpeg errors related to `libopus` requiring an explicit
      bitrate (e.g., "No bit rate set", "Quality-based encoding not supported").

2.  **Premature Stream End Failover (Consolidated from previous steps):**
    - `StreamController::startStream()` now detects specific FFmpeg `stderr`
      messages (e.g., "Stream ends prematurely") and throws a
      `SourceNotResponding` exception, even if FFmpeg exits with code 0.
    - The generic `catch (Exception $e)` block in `startStream` correctly
      re-throws such critical exceptions to ensure they are handled by `__invoke`.
    - `StreamController::__invoke()` catch blocks are updated to allow failover
      to continue if a *primary* stream fails after sending headers, while still
      terminating if a *subsequent failover stream* fails under similar
      circumstances.

These combined changes ensure more robust stream processing by correctly handling
premature stream endings and providing necessary parameters for the libopus
encoder, leading to more reliable failover behavior.
This commit includes several improvements:
1. Core failover logic for 'Stream ends prematurely' is fixed and verified.
   - `startStream` now detects these errors in stderr and throws SourceNotResponding.
   - `startStream`'s generic catch block re-throws these exceptions.
   - `__invoke`'s catch blocks allow failover for primary stream failures even after headers are sent.

2. Opus audio codec handling:
   - `buildCmd` now switches 'opus' to 'libopus'.
   - A default `-b:a 128k` is added when `libopus` is used for transcoding in the non-QSV path and in the template path.

Note: Logs indicate a remaining issue where `-b:a 128k` might not be correctly applied in QSV transcoding paths when `libopus` is the audio codec, despite code intending to do so. This specific QSV+libopus interaction may require further investigation if Opus transcoding with QSV is a primary use case. The main failover and non-QSV libopus fixes are effective.
This commit specifically addresses an issue where FFmpeg QSV transcodes
targeting `libopus` for audio would fail due to missing bitrate parameters.

Modifications in `StreamController::buildCmd()`:
- Within the QSV-enabled command construction path, the audio arguments
  are now explicitly built.
- If `libopus` is the audio codec (either directly specified or switched
  from `opus`), a default `-b:a 128k` is added unless an audio bitrate
  is already detected in user-provided QSV-specific arguments
  (`ffmpeg_qsv_encoder_options` or `ffmpeg_qsv_additional_args`).
- This prevents `libopus` errors like "No bit rate set" and "Quality-based
  encoding not supported" specifically for QSV transcodes.

This change is intended to work in conjunction with previous fixes for
general stream failover and `opus` to `libopus` codec switching.
This commit addresses FFmpeg errors related to Opus and Vorbis audio encoding.

For Opus encoding (libopus):
- Ensures `-vbr on` is included with `-b:a 128k` when a bitrate is not already specified by other configurations. This fixes the "Quality-based encoding not supported" error.

For Vorbis encoding (vorbis/libvorbis):
- Adds the `-strict -2` flag to enable the experimental Vorbis encoder. This fixes the "experimental codecs are not enabled" error.

These changes were applied to the FFmpeg command generation logic in both StreamController.php and HlsStreamService.php.
This commit addresses FFmpeg errors related to Opus and Vorbis audio encoding.

For Opus encoding (libopus):
- Ensures `-vbr 1` (changed from `-vbr on`) is included with `-b:a 128k` when a bitrate is not already specified by other configurations. This aims to fix the "Quality-based encoding not supported" error.

For Vorbis encoding (vorbis/libvorbis):
- Adds the `-strict -2` flag to enable the experimental Vorbis encoder. This fixes the "experimental codecs are not enabled" error.

These changes were applied to the FFmpeg command generation logic in both StreamController.php and HlsStreamService.php.
The `startStream` method's `finally` block could throw an exception during
`process->stop()` if a client had already aborted. This exception would
propagate to `__invoke`, causing misleading logs and incorrect failover
behavior, and preventing `startStream` from its normal exit path.

This commit wraps `process->stop(0)` in `startStream`'s `finally` block
with a `try...catch`. Exceptions from `stop()` are now logged separately
and not re-thrown, allowing `startStream` to correctly handle client aborts,
log its concluding messages, and call `exit`. This prevents `__invoke` from
erroneously triggering a failover due to a client-initiated disconnection
after the stream had started.
When streaming to MP4 format and copying the AAC audio stream,
FFmpeg can encounter issues with malformed AAC bitstreams.
This change applies the `aac_adtstoasc` bitstream filter
as recommended by FFmpeg to resolve this issue.

The filter is only applied when the output format is 'mp4'
and the audio codec is set to 'copy'.
@sparkison sparkison merged commit 9840412 into sparkison:xteve Jun 26, 2025
@drondeseries drondeseries deleted the dev branch July 1, 2025 22:04
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