Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 67 additions & 20 deletions Streaming/VideoRenderer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -159,30 +159,53 @@ bool VideoRenderer::Render(AVFrame *frame) {
ID3D11ShaderResourceView* nullSrvs[2] = {};
ctx->PSSetShaderResources(0, 2, nullSrvs);

if (frame->color_trc != m_LastColorTrc) {
DXGI_COLOR_SPACE_TYPE colorspace = {};

if (frame->color_trc == AVCOL_TRC_SMPTE2084) {
// Switch to Rec 2020 PQ (SMPTE ST 2084) colorspace for HDR10 rendering
colorspace = DXGI_COLOR_SPACE_RGB_FULL_G2084_NONE_P2020;
} else {
// Restore default sRGB colorspace
colorspace = DXGI_COLOR_SPACE_RGB_FULL_G22_NONE_P709;
}
applySwapChainColorSpace(frame);

UINT colorSpaceSupport = 0;
if (colorspace && SUCCEEDED(m_deviceResources->GetSwapChain()->CheckColorSpaceSupport(colorspace, &colorSpaceSupport)) && (colorSpaceSupport & DXGI_SWAP_CHAIN_COLOR_SPACE_SUPPORT_FLAG_PRESENT)) {
DX::ThrowIfFailed(m_deviceResources->GetSwapChain()->SetColorSpace1(colorspace));
Utils::Logf("Colorspace changed to %s\n",
colorspace == DXGI_COLOR_SPACE_RGB_FULL_G2084_NONE_P2020
? "DXGI_COLOR_SPACE_RGB_FULL_G2084_NONE_P2020"
: "DXGI_COLOR_SPACE_RGB_FULL_G22_NONE_P709");
}
return true;
}

m_LastColorTrc = frame->color_trc;
bool VideoRenderer::frameUsesHdrColorSpace(const AVFrame* frame) const {
if (frame->color_trc == AVCOL_TRC_SMPTE2084) {
return true;
}

return true;
if (!configuration->enableHDR && !client->IsHDR()) {
return false;
}

// HDR stream or display: HEVC Main10 often omits TRC or reports BT.709 despite PQ content
return frame->color_trc == AVCOL_TRC_UNSPECIFIED || frame->color_trc == AVCOL_TRC_BT709;
}

void VideoRenderer::applySwapChainColorSpace(const AVFrame* frame) {
bool useHdr = frameUsesHdrColorSpace(frame);
if (useHdr == m_SwapChainHdrColorSpace && frame->color_trc == m_LastColorTrc) {
return;
}

auto* swapChain = m_deviceResources->GetSwapChain();
if (!swapChain) {
return;
}

DXGI_COLOR_SPACE_TYPE colorspace = useHdr
? DXGI_COLOR_SPACE_RGB_FULL_G2084_NONE_P2020
: DXGI_COLOR_SPACE_RGB_FULL_G22_NONE_P709;

HRESULT hr = swapChain->SetColorSpace1(colorspace);
if (SUCCEEDED(hr)) {
Utils::Logf("Colorspace changed to %s (color_trc=%d, hdrStream=%d, displayHdr=%d)\n",
useHdr ? "DXGI_COLOR_SPACE_RGB_FULL_G2084_NONE_P2020"
: "DXGI_COLOR_SPACE_RGB_FULL_G22_NONE_P709",
frame->color_trc, configuration->enableHDR, client->IsHDR());
m_SwapChainHdrColorSpace = useHdr;
m_LastColorTrc = frame->color_trc;
} else {
Utils::Logf("SetColorSpace1(%s) failed: 0x%08X (color_trc=%d)\n",
useHdr ? "DXGI_COLOR_SPACE_RGB_FULL_G2084_NONE_P2020"
: "DXGI_COLOR_SPACE_RGB_FULL_G22_NONE_P709",
hr, frame->color_trc);
}
}

void VideoRenderer::CreateDeviceDependentResources()
Expand Down Expand Up @@ -681,6 +704,30 @@ void VideoRenderer::SetHDR(bool enabled)
// toggle the display to the correct state
client->SetDisplayHDR(false, SS_HDR_METADATA{});
}

// Keep HDMI and swap chain color spaces in sync; force re-apply on next frame if this fails
m_LastColorTrc = AVCOL_TRC_UNSPECIFIED;
m_SwapChainHdrColorSpace = !enabled;

auto* swapChain = m_deviceResources->GetSwapChain();
if (!swapChain) {
return;
}

DXGI_COLOR_SPACE_TYPE colorspace = enabled
? DXGI_COLOR_SPACE_RGB_FULL_G2084_NONE_P2020
: DXGI_COLOR_SPACE_RGB_FULL_G22_NONE_P709;

HRESULT hr = swapChain->SetColorSpace1(colorspace);
if (SUCCEEDED(hr)) {
m_SwapChainHdrColorSpace = enabled;
Utils::Logf("SetHDR(%s): swap chain colorspace set to %s\n",
enabled ? "true" : "false",
enabled ? "DXGI_COLOR_SPACE_RGB_FULL_G2084_NONE_P2020"
: "DXGI_COLOR_SPACE_RGB_FULL_G22_NONE_P709");
} else {
Utils::Logf("SetHDR(%s): SetColorSpace1 failed: 0x%08X\n", enabled ? "true" : "false", hr);
}
Comment on lines +707 to +730

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Trace which thread invokes SetHDR to confirm cross-thread swap-chain access.
rg -nP -C4 '\bSetHDR\s*\(' --type=cpp

Repository: TheElixZammuto/moonlight-xbox

Length of output: 167


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- SetHDR / applySwapChainColorSpace symbols ---'
rg -n --hidden --glob '!**/node_modules/**' --glob '!**/build/**' --glob '!**/dist/**' \
  '\b(SetHDR|applySwapChainColorSpace|m_SwapChainHdrColorSpace|m_LastColorTrc)\b' .

printf '\n%s\n' '--- likely thread/callback dispatch references ---'
rg -n --hidden --glob '!**/node_modules/**' --glob '!**/build/**' --glob '!**/dist/**' \
  '\b(callback|thread|render thread|dispatch|invoke|post|queue|task|async)\b' State Streaming | head -n 300

Repository: TheElixZammuto/moonlight-xbox

Length of output: 7322


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- file list ---'
git ls-files | sed -n '1,200p'

Repository: TheElixZammuto/moonlight-xbox

Length of output: 5492


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- Streaming/moonlight_xbox_dxMain.cpp (around SetHDR assignment) ---'
sed -n '120,150p' Streaming/moonlight_xbox_dxMain.cpp

printf '\n%s\n' '--- Streaming/VideoRenderer.h ---'
sed -n '1,130p' Streaming/VideoRenderer.h

printf '\n%s\n' '--- Streaming/VideoRenderer.cpp (around SetHDR and applySwapChainColorSpace) ---'
sed -n '150,240p' Streaming/VideoRenderer.cpp
printf '\n%s\n' '--- Streaming/VideoRenderer.cpp (around SetHDR) ---'
sed -n '680,740p' Streaming/VideoRenderer.cpp

Repository: TheElixZammuto/moonlight-xbox

Length of output: 10272


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- Streaming/moonlight_xbox_dxMain.cpp (render task / thread comments) ---'
sed -n '200,260p' Streaming/moonlight_xbox_dxMain.cpp
printf '\n%s\n' '--- Streaming/moonlight_xbox_dxMain.cpp (stream start / task creation) ---'
sed -n '340,390p' Streaming/moonlight_xbox_dxMain.cpp

Repository: TheElixZammuto/moonlight-xbox

Length of output: 4116


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- Streaming/moonlight_xbox_dxMain.cpp (120-150) ---'
sed -n '120,150p' Streaming/moonlight_xbox_dxMain.cpp

printf '\n%s\n' '--- Streaming/moonlight_xbox_dxMain.cpp (200-260) ---'
sed -n '200,260p' Streaming/moonlight_xbox_dxMain.cpp

printf '\n%s\n' '--- Streaming/moonlight_xbox_dxMain.cpp (340-390) ---'
sed -n '340,390p' Streaming/moonlight_xbox_dxMain.cpp

printf '\n%s\n' '--- State/MoonlightClient.cpp (372-386) ---'
sed -n '372,386p' State/MoonlightClient.cpp

printf '\n%s\n' '--- Streaming/VideoRenderer.cpp (170-210) ---'
sed -n '170,210p' Streaming/VideoRenderer.cpp

printf '\n%s\n' '--- Streaming/VideoRenderer.cpp (690-730) ---'
sed -n '690,730p' Streaming/VideoRenderer.cpp

Repository: TheElixZammuto/moonlight-xbox

Length of output: 8626


Marshal SetHDR onto the render thread

client->SetHDR is invoked from the connection callback path, but VideoRenderer::SetHDR now mutates m_LastColorTrc/m_SwapChainHdrColorSpace and calls SetColorSpace1 directly. That bypasses the render-loop synchronization and can race with Render()/applySwapChainColorSpace(). Move the swap-chain update onto the render thread or guard the shared state and DXGI access.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Streaming/VideoRenderer.cpp` around lines 707 - 730, VideoRenderer::SetHDR is
mutating shared color-space state and calling swap-chain APIs from the
connection callback path, which can race with Render() and
applySwapChainColorSpace(). Update SetHDR so the actual
m_LastColorTrc/m_SwapChainHdrColorSpace changes and swap-chain SetColorSpace1
call are marshaled onto the render thread, or protected with the same
synchronization used by the render loop. Keep the logic anchored around
VideoRenderer::SetHDR, Render(), and applySwapChainColorSpace() so all DXGI
color-space updates happen on one thread.

}

void VideoRenderer::Stop() {
Expand Down
3 changes: 3 additions & 0 deletions Streaming/VideoRenderer.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ namespace moonlight_xbox_dx
void getFramePremultipliedCscConstants(const AVFrame* frame, std::array<float, 9> &cscMatrix, std::array<float, 3> &offsets);
void getFrameChromaCositingOffsets(const AVFrame* frame, std::array<float, 2> &chromaOffsets);
bool hasFrameFormatChanged(const AVFrame* frame);
bool frameUsesHdrColorSpace(const AVFrame* frame) const;
void applySwapChainColorSpace(const AVFrame* frame);

// Cached pointer to device resources.
std::shared_ptr<DX::DeviceResources> m_deviceResources;
Expand Down Expand Up @@ -97,6 +99,7 @@ namespace moonlight_xbox_dx
AVColorTransferCharacteristic m_LastColorTrc = AVCOL_TRC_UNSPECIFIED;
AVColorSpace m_LastColorSpace = AVCOL_SPC_UNSPECIFIED;
AVChromaLocation m_LastChromaLocation = AVCHROMA_LOC_UNSPECIFIED;
bool m_SwapChainHdrColorSpace = false;
};
}