Sunshine
Sunshine copied to clipboard
feat(audio): custom surround-params
Description
This PR is made for improving webOS client support. On webOS 5.0 and above, if surround channel configuration is done properly, it can re-encode OPUS stream on-the-fly & send it to surround sound system through eARC/Optical.
Currently, webOS only handles 6-channels, 4 streams & 2 coupled streams configuration, and the channel must be ordered as [0, 1, 4, 5, 2, 3]
which Sunshine doesn't have. It won't accept stream header either, which makes this PR necessary.
With this PR, if surroundParams
is specified (formatted like surround-params=
in RTSP handshake, e.g. 642014523
) in HTTP launch request, it will be applied, and during RTSP handshake, our custom surround-params a=fmtp:97 surround-params=642014523
will be advertised twice first, followed by the original ones.
Screenshot
Not applicable
Issues Fixed or Closed
Type of Change
- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
- [ ] Dependency update (updates to dependencies)
- [ ] Documentation update (changes to documentation)
- [ ] Repository update (changes to repository files, e.g.
.github/...
)
Checklist
- [ ] My code follows the style guidelines of this project
- [ ] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated the in code docstring/documentation-blocks for new or existing methods/components
Branch Updates
LizardByte requires that branches be up-to-date before merging. This means that after any PR is merged, this branch must be updated before it can be merged. You must also Allow edits from maintainers.
- [x] I want maintainers to keep my branch updated
Do we want this to work with 11+ channels eventually? If so, using 0
-9
won't work. Perhaps using a ,
delimiter and then parsing as an int
would scale better?
@kentyman23 In that case, Moonlight core lib needs to changed. And of course webOS needs that support too.
Hi, will ac3 encoding be functional on moonlight-android and moonlight-qt? (provided that development is done I suppose)
@moi952 Hi, this PR won't enable AC3 codec, it only rearranges sound channel mapping. May I ask why you are interested in AC3 codec? As far as I know, all existing clients handles OPUS better.
@moi952 Hi, this PR won't enable AC3 codec, it only rearranges sound channel mapping. May I ask why you are interested in AC3 codec? As far as I know, all existing clients handles OPUS better.
Thank you for your answer. Too bad, I don't know OPUS but I guess it's PCM? In fact with my current TV if I connect my Nvidia Shield or my PC to the TV and I put it in 5.1, my amp (Denon AVC-6500x) remains in stereo, I don't know if the TV doesn't allow it to accept uncompressed 5.1. On the other hand, if I connect my Nvidia Shield directly to my amp I have good sound in 5.1
I tried different sound input/output settings on my TV but it doesn't work.
For example for Windows if I connect my PC to my TV, it does not offer me more than 2 channels in the sound output configuration.
@moi952 That may have something to do with the TV, and yes while Sunshine transfers audio to Moonlight using OPUS, it will eventually become PCM. Let's discuss in the channel you're in on discord.
@mariotaku could you rebase your PR?
Codecov Report
Attention: Patch coverage is 20.83333%
with 38 lines
in your changes missing coverage. Please review.
Project coverage is 7.98%. Comparing base (
7371a20
) to head (4e2d4ff
). Report is 155 commits behind head on master.
Files with missing lines | Patch % | Lines |
---|---|---|
src/rtsp.cpp | 0.00% | 19 Missing :warning: |
src/audio.cpp | 37.03% | 17 Missing :warning: |
src/nvhttp.cpp | 0.00% | 1 Missing :warning: |
src/process.cpp | 0.00% | 1 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## master #2424 +/- ##
=========================================
+ Coverage 7.01% 7.98% +0.97%
=========================================
Files 88 88
Lines 17987 18036 +49
Branches 8579 8596 +17
=========================================
+ Hits 1261 1441 +180
- Misses 14019 15827 +1808
+ Partials 2707 768 -1939
Flag | Coverage Δ | |
---|---|---|
Linux | 6.05% <17.50%> (+0.73%) |
:arrow_up: |
Windows | 3.76% <21.95%> (+1.15%) |
:arrow_up: |
macOS-12 | 9.03% <12.76%> (+1.00%) |
:arrow_up: |
macOS-13 | 8.92% <12.76%> (+1.00%) |
:arrow_up: |
macOS-14 | 9.22% <12.76%> (+1.00%) |
:arrow_up: |
Flags with carried forward coverage won't be shown. Click here to find out more.
Files with missing lines | Coverage Δ | |
---|---|---|
src/audio.h | 0.00% <ø> (ø) |
|
src/rtsp.h | 0.00% <ø> (ø) |
|
src/nvhttp.cpp | 1.34% <0.00%> (-0.15%) |
:arrow_down: |
src/process.cpp | 1.62% <0.00%> (-0.28%) |
:arrow_down: |
src/audio.cpp | 28.57% <37.03%> (+27.02%) |
:arrow_up: |
src/rtsp.cpp | 1.54% <0.00%> (-0.06%) |
:arrow_down: |
@mariotaku are you happy with this PR and tests?