Skip to content

feat(audio): add optional Dolby Digital (AC-3 and E-AC-3) output to the Sunshine offerts - #5917

Open
fidoboy wants to merge 30 commits into
LizardByte:masterfrom
fidoboy:pr/dolby-ac3-eac3
Open

fidoboy wants to merge 30 commits into
LizardByte:masterfrom
fidoboy:pr/dolby-ac3-eac3

Conversation

@fidoboy

@fidoboy fidoboy commented Oct 13, 2026 •

Copy link
Copy Markdown

Description

This PR adds optional Dolby Digital (AC-3) and Dolby Digital Plus (E-AC-3) encoded audio output to Sunshine on platforms whose linked FFmpeg build contains the corresponding encoders. Compatible Moonlight clients can negotiate these formats as an alternative to Opus when they need a compressed Dolby bitstream for native decoding or passthrough.

The main use case is Moonlight running directly on a Smart TV connected to a receiver or sound system through optical S/PDIF. AC-3 can provide Dolby Digital stereo or 5.1 output over compatible TV audio paths. Actual output depends on the TV model, its audio settings, the receiver, and client support; E-AC-3 passthrough over optical output is model-dependent.

Related client implementation

The companion Moonlight Tizen with AC-3 support (see details in its website) implements the client-side Dolby offer/accept negotiation and passthrough through Tizen's native EMSS audio renderer. It is an independently maintained client and is linked here as an example of a client that uses this new host feature. This PR changes Sunshine only; other clients need their own support for the optional negotiation and playback/passthrough path.

Implementation

  • AC-3 and E-AC-3 encoding through Sunshine's existing linked FFmpeg libraries; no separate FFmpeg runtime distribution or external DLLs are required.
  • Encoder availability detection as part of host-side codec negotiation.
  • Audio format settings for Opus (default), AC-3, and E-AC-3 on supported platforms. If the active FFmpeg build lacks the selected encoder, Sunshine falls back to Opus.
  • RTSP offer/accept negotiation: Sunshine selects Dolby only when the client explicitly accepts the exact codec offered; otherwise it uses Opus.
  • Stereo and 5.1 channel support for both Dolby formats.
  • A High quality audio switch, disabled by default, for higher Dolby bitrates at the cost of increased network bandwidth. It has no effect on Opus and is disabled in the UI while Opus is selected.
  • 32 ms Dolby access units and an increased audio packet buffer size for Dolby frames.
  • Configuration documentation and English UI strings.

The existing Opus encoder and its normal negotiation path are intended to remain behaviorally unchanged. This is a host-side feature, not a Tizen-only or Windows-only server feature.

Testing

  • Added native regression tests for channel-aware bitrates, exact AC-3/E-AC-3 acceptance, missing or mismatched acceptance, unavailable encoders, unsupported channel counts, packet duration, quality flags, codec labels, and encoding without capture hardware.
  • Added Web UI regression tests for the Dolby selector across Windows, Linux, macOS, and FreeBSD, including preservation of a saved Dolby selection without capability metadata.
  • Manually tested end-to-end AC-3 and E-AC-3 playback with a Windows Sunshine host and the companion Moonlight Tizen client, including stereo/5.1 and normal/high-quality bitrate settings.
  • Windows AMD64 and ARM64 build and native tests passed on the merged feature tree in validation run 38719776826.
  • SonarCloud Quality Gate passed with zero new issues.
  • The primary sunshinestream Read the Docs build passed (build 35163518).
  • Merged current upstream master into the feature branch in commit e7e57df; GitHub reports no merge conflicts.

Type of Change

  • feat: New feature (non-breaking change which adds functionality)
  • fix: Bug fix
  • docs: Documentation only changes
  • style: Changes that do not affect the meaning of the code
  • refactor: Code change that does not fix a bug or add a feature
  • perf: Code change that improves performance
  • build: Changes that affect the build system or external dependencies
  • ci: Changes to CI configuration files and scripts
  • chore: Other changes that do not modify src or test files
  • revert: Reverts a previous commit

Checklist

  • Code follows the project's style guidelines (the repository's lb-update-clang-format helper has been run and a second pass produced no changes; other lint checks have not been run on the latest revision).
  • Code has been self-reviewed.
  • Code has comments in key parts of the implementation.
  • Documentation for the new configuration options has been added.
  • Tests have been added or updated, and the latest Windows build/test validation is complete.

AI Usage

  • AI tools were used while preparing this PR.

The contributor remains responsible for reviewing the implementation, verifying test results, and addressing review feedback.

@fidoboy
fidoboy marked this pull request as ready for review October 13, 2026 21:25

fidoboy commented Oct 14, 2026 •

Copy link
Copy Markdown
Author

Update: I retrieved the raw log for the sunshinestream Read the Docs build. The failure was caused by missing Doxygen documentation for launch_session_t::audio_negotiation_t and its fields, audio::dolby_bitrate_kbps parameters/return value, and the RTSP helper/handler functions cmd_option_impl, cmd_describe_impl, cmd_setup, and cmd_play_impl.

I added the missing comments in c9ad363. The new sunshinestream Read the Docs check is now successful for that commit. The PR's status links to the rendered preview: https://lizardbyte--5917.org.readthedocs.build/projects/sunshine/5917/.

Per your guidance, I have not started the GitHub Actions workflows. I have also left the separate lizardbyte-gh-pages-sunshine check alone.

@ReenigneArcher

Copy link
Copy Markdown
Member

Could a maintainer approve/enable these workflow runs for this PR?

We only get 20 runners from GitHub and these are basically running all day long, so I'll run it when I'm ready to merge the code.

Docs failures are here: https://app.readthedocs.org/projects/sunshinestream/builds/35131897/

Don't worry about the other already reported failure.

Your agent can lint and build the docs if it follows the AGENTS.md file.

@ReenigneArcher ReenigneArcher left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Requesting changes on c9ad3637b48907316f3957f72ac46cb9c7155023: the settings page cannot see the advertised encoder capabilities, and the new backend/negotiation behavior has no added regression tests.

On FFmpeg packaging, I recommend using Sunshine's existing linked FFmpeg libraries for this feature. The pinned build-deps configuration enables the encoders, and its published macOS archive already contains both native encoders. I linked the PR's extracted encoder class to that archive and successfully encoded all eight combinations of AC-3/E-AC-3, stereo/5.1, and normal/high quality. This establishes that a second runtime FFmpeg distribution is unnecessary for those encoding operations; it is not a Windows DLL-loader test.

The native AC-3 and E-AC-3 implementations are LGPL-2.1-or-later, compatible with Sunshine's GPL-3.0 licensing. FFmpeg's legal page distinguishes copyright licensing from codec patents. I found no identified licensing requirement in this PR that mandates user-supplied DLLs. Dolby's 2007 filing listed original Dolby Digital patent expirations through 2017 and Dolby Digital Plus through 2022; that historical disclosure is not comprehensive present-day patent or trademark clearance.

Validation: 28 existing focused Vue tests passed, along with two temporary review reproductions showing the settings bug and verifying the proposed endpoint correction; all eight isolated encoding cases passed; whitespace checks passed for all 15 changed files. The reproductions and probes live outside the working checkout and are not tests contributed by this PR.

GitHub reports the head as mergeable and behind master (current branch tip a178f465e42f8e77102002e9156b8becd7b13628). The Sunshine documentation check passed. Full Windows builds, Windows DLL loading, client passthrough, and protocol interoperability were not verified locally; GitHub Actions await maintainer approval. The reported Sonar issue points to the unchanged permission-status test override, so it is not a new-code finding.

Review completed by Codex | Model: gpt-6.1-sol | Thinking: xhigh

Comment thread src/confighttp.cpp Resolved
Comment thread src/audio.cpp Resolved

fidoboy commented Oct 15, 2026

Copy link
Copy Markdown
Author

Thanks for the detailed review. I pushed a revision to address the requested changes:

  • Moved ac3_available and eac3_available into /api/config, which is the endpoint consumed by Config.vue.
  • Replaced the extra runtime FFmpeg DLL loader with calls to Sunshine's existing linked FFmpeg libraries and updated the configuration documentation and UI text accordingly.
  • Extracted the exact offer/accept and fallback decision into a helper used by RTSP and the native unit tests. Added coverage for bitrate selection, matching/mismatched or missing acceptance, unavailable encoders, unsupported channel counts, codec labels, and Windows x64 encoding without capture hardware.
  • Added Web UI regression tests for full, partial, and no Dolby encoder availability.

The code and tests are committed, but I have not run the revised Windows build or tests locally. I have updated the PR description to reflect that CI and playback re-validation of the linked-FFmpeg change are still pending. I have not attempted to start the Actions workflows, per your note about the runner pool.

fidoboy commented Oct 15, 2026

Copy link
Copy Markdown
Author

I extended the revision beyond the review fixes to remove the remaining Windows/runtime-DLL assumptions:

  • The Dolby encoder and FFmpeg headers are now compiled through Sunshine's existing FFmpeg linkage without a Windows x64 guard.
  • The audio-format selector and High quality audio control are visible on Windows, Linux, macOS, and FreeBSD. Capability metadata is no longer exposed just to gate the form; a saved Dolby selection is no longer rewritten to Opus while loading the page.
  • Runtime encoder detection and the RTSP offer/accept path still retain Opus as the safe streaming fallback if the linked build lacks an encoder or the client does not accept the offer.
  • Native encoder tests no longer have a Windows-only guard, and the Web UI test now checks the selector across the four platforms.
  • I updated the documentation and removed the obsolete external-DLL warning text. I also removed the old UI screenshots from the description because they showed the previous availability-gated selector.

The current branch head contains these changes. I have not run the revised build/tests or verified Dolby playback on other platforms; per the earlier request, I am leaving Actions for maintainer scheduling.

fidoboy commented Oct 15, 2026

Copy link
Copy Markdown
Author

@ReenigneArcher I've addressed the four SonarCloud findings in the latest revision (4b10f9a): the FFmpeg encoder class now explicitly deletes copy/move operations, channel_layout is scoped in the if init-statement, and the two enum-verbosity findings use C++20 using enum. SonarCloud's Quality Gate is now green, and the latest Windows validation passed both build and tests on AMD64 and ARM64 (run 38719776826). The sunshinestream Read the Docs build also passes. Could you please review the updated PR again? Thank you.

fidoboy commented Oct 19, 2026

Copy link
Copy Markdown
Author

Hi @ReenigneArcher, just following up on this PR. I've addressed the requested changes and tested the Windows build locally; it's working correctly with Opus, AC-3, and E-AC-3. When you have a chance, could you please take another look and approve it if everything looks good? Thanks!

@fidoboy fidoboy changed the title feat(audio): add optional AC-3 and E-AC-3 output feat(audio): add optional Dolby Digital (AC-3 and E-AC-3) output to the Sunshine offerts Oct 29, 2026
@fidoboy

fidoboy commented Nov 3, 2026

Copy link
Copy Markdown
Author

There is still something I'm missing? Do I need to make any other change here @ReenigneArcher ?

@ReenigneArcher ReenigneArcher left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could you fix the sonar issues before I review this again?

@fidoboy

fidoboy commented Nov 4, 2026 •

Copy link
Copy Markdown
Author

Could you fix the sonar issues before I review this again?

Thanks for the feedback. I've gone through all 30 SonarCloud issues listed in the report and compared their locations against the actual PR diff.

None of the reported issues appears to be located in a line modified by this PR. Although some findings are in files touched by the PR, they refer to pre-existing code outside the changed sections. Several are also reported as having been present for years.

For example, the numerous #undef findings in src/config.cpp refer to code that predates this PR by several years. My changes in that file are limited to adding the two audio configuration settings elsewhere.

I agree that genuine Sonar issues introduced by this PR should be addressed. However, I don't think it would be appropriate to modify unrelated upstream code as part of this feature, particularly when none of these 30 findings appears to originate from my changes.

Could you please check why SonarCloud is classifying these findings as new issues for this PR? If there are specific findings located in code actually modified by this PR that I've overlooked, please point me to them and I'll address them.

@ReenigneArcher

Copy link
Copy Markdown
Member

None of the reported issues appears to be located in a line modified by this PR.

Okay, thank you for double checking. Sonar will report issues that are close to changed code, so those are likely acceptable.

@fidoboy

fidoboy commented Nov 4, 2026

Copy link
Copy Markdown
Author

None of the reported issues appears to be located in a line modified by this PR.

Okay, thank you for double checking. Sonar will report issues that are close to changed code, so those are likely acceptable.

You can merge the PR then. All is ok :)

@fidoboy

fidoboy commented Nov 6, 2026

Copy link
Copy Markdown
Author

@ReenigneArcher I can also open a new PR from a clean LizardByte/Sunshine branch or directly from master, but if it isn't reviewed and approved soon, the same thing will happen again. So I'm not sure what the best course of action is at this point. Could you approve the current PR, or would you prefer me to create a new one?

@ReenigneArcher

Copy link
Copy Markdown
Member

I'm going as fast as I can

@sonarqubecloud

sonarqubecloud Bot commented Nov 8, 2026

Copy link
Copy Markdown

This branch has not been deployed

No deployments
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