From 03b1f45ebc3a3c98d9fe78b08c1bd35c34455aee Mon Sep 17 00:00:00 2001 From: EtienneLescot Date: Wed, 30 Sep 2026 23:49:33 +0200 Subject: [PATCH 1/3] fix(windows): encode H.264 High with BT.709 colour the compositor expects Every Windows recording was written in BT.601 and untagged. The helper fed the encoder RGB32, and the colour converter Media Foundation puts in front of it produced BT.601 whatever the media types said: solid red came out at Y 82, Cb 90 from the software and the hardware encoder alike, where BT.709 is 63 and 102. The compositor decodes every recording as BT.709, so colours shifted. The encoders also defaulted to Constrained Baseline. - The helper now converts BGRA to NV12 BT.709 studio range itself (SSE2) and feeds NV12 on every path. A 1080p frame costs 1.6 ms to hand over, against about 2 ms for the old copy plus converter. - Range, matrix, primaries and transfer are tagged BT.709 on both tracks. - H.264 High profile, with B-frames off through the encoding parameters (the software encoder added them in High). - The cpuInputIsNv12 option is gone: every system-memory path is NV12. mf_encoder_color_test drives the real encoder, software and hardware, and reads the file back: High, no B-frames, bt709 tags, red/green/blue exact to the code value, and the converter within one code value of BT.709 on noise. It runs from npm run build:native:win. Fixes #922 Fixes #923 --- electron/native/wgc-capture/CMakeLists.txt | 27 ++ electron/native/wgc-capture/src/main.cpp | 1 - .../native/wgc-capture/src/mf_encoder.cpp | 212 +++++++++---- electron/native/wgc-capture/src/mf_encoder.h | 17 +- .../wgc-capture/src/mf_encoder_color_test.cpp | 295 ++++++++++++++++++ scripts/build-windows-wgc-helper.mjs | 11 + .../testing/manual-e2e-checklist.md | 1 + 7 files changed, 495 insertions(+), 69 deletions(-) create mode 100644 electron/native/wgc-capture/src/mf_encoder_color_test.cpp diff --git a/electron/native/wgc-capture/CMakeLists.txt b/electron/native/wgc-capture/CMakeLists.txt index c242db712..f5dfeb71b 100644 --- a/electron/native/wgc-capture/CMakeLists.txt +++ b/electron/native/wgc-capture/CMakeLists.txt @@ -170,3 +170,30 @@ target_compile_definitions(webcam_snapshot_test PRIVATE ) target_compile_options(webcam_snapshot_test PRIVATE /EHsc /W4 /utf-8) + +add_executable(mf_encoder_color_test + src/audio_sample_utils.cpp + src/audio_sample_utils.h + src/mf_encoder.cpp + src/mf_encoder.h + src/mf_encoder_color_test.cpp +) + +target_compile_definitions(mf_encoder_color_test PRIVATE + NOMINMAX + WIN32_LEAN_AND_MEAN + _WIN32_WINNT=0x0A00 +) + +target_compile_options(mf_encoder_color_test PRIVATE /EHsc /W4 /utf-8) + +target_link_libraries(mf_encoder_color_test PRIVATE + d3d11 + dxgi + mf + mfplat + mfreadwrite + mfuuid + ole32 + propsys +) diff --git a/electron/native/wgc-capture/src/main.cpp b/electron/native/wgc-capture/src/main.cpp index 11b82f4b4..a6bba7c76 100644 --- a/electron/native/wgc-capture/src/main.cpp +++ b/electron/native/wgc-capture/src/main.cpp @@ -971,7 +971,6 @@ int wmain(int argc, wchar_t* argv[]) { MFEncoderOptions webcamEncoderOptions = encoderOptions; webcamEncoderOptions.injectDefaultSinkWriterFailureOnce = false; webcamEncoderOptions.useDxgiInput = false; - webcamEncoderOptions.cpuInputIsNv12 = webcamCapture.deliversNv12(); // The two-step ladder this replaces topped out at 8 Mbit/s for anything // 720p or larger. That was sized for a camera nobody had configured // above 640x480; now that the capture runs at the camera's real diff --git a/electron/native/wgc-capture/src/mf_encoder.cpp b/electron/native/wgc-capture/src/mf_encoder.cpp index 786881e00..a1af5af79 100644 --- a/electron/native/wgc-capture/src/mf_encoder.cpp +++ b/electron/native/wgc-capture/src/mf_encoder.cpp @@ -10,6 +10,8 @@ #include #include +#include + #include #include #include @@ -592,6 +594,94 @@ struct StageGuard { } // namespace +// BGRA to NV12, BT.709 studio range, which is what the compositor decodes every +// recording as (getopenscreen/openscreen#923). Media Foundation's own colour +// converter, which the sink writer used to insert in front of the encoder for +// RGB32 input, produced BT.601 whatever the media types said. Chroma is the +// average of each 2x2 block, and the width must be even, as H.264 needs it anyway +// (main.cpp rounds the capture down); an odd last row repeats its neighbour. +// Fixed point: the coefficients are BT.709's scaled by 2^15 (luma) and 2^16 +// (chroma), so a solid primary lands within one code value of the table. +void convertBgraToNv12Bt709(const BYTE* bgra, int stride, int width, int height, BYTE* nv12) { + BYTE* luma = nv12; + BYTE* chroma = nv12 + static_cast(width) * height; + // Luma four pixels at a time in SSE2, which every x64 CPU has: this runs on + // the video writer's thread for every frame, and the scalar loop alone cost + // 2.5 ms a 1080p frame. 15-bit coefficients so they fit madd's int16 lanes. + const __m128i zero = _mm_setzero_si128(); + const __m128i lumaCoefficients = _mm_setr_epi16(2032, 20127, 5983, 0, 2032, 20127, 5983, 0); + const __m128i lumaRounding = _mm_set1_epi32(16384); + const __m128i lumaOffset = _mm_set1_epi32(16); + const auto lumaOfTwo = [&](__m128i pixels) { + // [b0*cb + g0*cg, r0*cr, b1*cb + g1*cg, r1*cr] -> each pixel's sum in lanes 0 and 2. + const __m128i products = _mm_madd_epi16(pixels, lumaCoefficients); + return _mm_add_epi32(products, _mm_shuffle_epi32(products, _MM_SHUFFLE(2, 3, 0, 1))); + }; + for (int y = 0; y < height; y += 1) { + const BYTE* row = bgra + static_cast(y) * stride; + BYTE* out = luma + static_cast(y) * width; + int x = 0; + for (; x + 4 <= width; x += 4) { + const __m128i pixels = _mm_loadu_si128(reinterpret_cast(row + x * 4)); + const __m128i first = lumaOfTwo(_mm_unpacklo_epi8(pixels, zero)); + const __m128i second = lumaOfTwo(_mm_unpackhi_epi8(pixels, zero)); + __m128i sums = _mm_castps_si128(_mm_shuffle_ps( + _mm_castsi128_ps(first), _mm_castsi128_ps(second), _MM_SHUFFLE(2, 0, 2, 0))); + sums = _mm_add_epi32(_mm_srai_epi32(_mm_add_epi32(sums, lumaRounding), 15), lumaOffset); + const __m128i bytes = _mm_packus_epi16(_mm_packs_epi32(sums, zero), zero); + const int packed = _mm_cvtsi128_si32(bytes); + std::memcpy(out + x, &packed, 4); + } + for (; x < width; x += 1) { + const int b = row[x * 4]; + const int g = row[x * 4 + 1]; + const int r = row[x * 4 + 2]; + out[x] = static_cast(((5983 * r + 20127 * g + 2032 * b + 16384) >> 15) + 16); + } + } + // Chroma the same way, two 2x2 blocks at a time: the four pixels of each + // block summed in int16 lanes (at most 1020), then one madd per channel. + const __m128i cbCoefficients = _mm_setr_epi16(28784, -22189, -6596, 0, 28784, -22189, -6596, 0); + const __m128i crCoefficients = _mm_setr_epi16(-2642, -26142, 28784, 0, -2642, -26142, 28784, 0); + const __m128i chromaRounding = _mm_set1_epi32(131072); + const __m128i chromaOffset = _mm_set1_epi32(128); + const auto chromaOfTwo = [&](__m128i blocks, __m128i coefficients) { + const __m128i products = _mm_madd_epi16(blocks, coefficients); + const __m128i sums = _mm_add_epi32(products, _mm_shuffle_epi32(products, _MM_SHUFFLE(2, 3, 0, 1))); + return _mm_shuffle_epi32(sums, _MM_SHUFFLE(2, 0, 2, 0)); + }; + for (int y = 0; y < height; y += 2) { + const BYTE* top = bgra + static_cast(y) * stride; + const BYTE* bottom = y + 1 < height ? top + stride : top; + BYTE* out = chroma + static_cast(y / 2) * width; + int x = 0; + for (; x + 4 <= width; x += 4) { + const __m128i upper = _mm_loadu_si128(reinterpret_cast(top + x * 4)); + const __m128i lower = _mm_loadu_si128(reinterpret_cast(bottom + x * 4)); + // [pixel 0 + pixel 1 | pixel 2 + pixel 3] of both rows, as int16 B, G, R, A. + const __m128i left = _mm_add_epi16(_mm_unpacklo_epi8(upper, zero), _mm_unpacklo_epi8(lower, zero)); + const __m128i right = _mm_add_epi16(_mm_unpackhi_epi8(upper, zero), _mm_unpackhi_epi8(lower, zero)); + const __m128i blocks = _mm_unpacklo_epi64( + _mm_add_epi16(left, _mm_srli_si128(left, 8)), _mm_add_epi16(right, _mm_srli_si128(right, 8))); + __m128i values = _mm_unpacklo_epi32( + chromaOfTwo(blocks, cbCoefficients), chromaOfTwo(blocks, crCoefficients)); + values = _mm_add_epi32(_mm_srai_epi32(_mm_add_epi32(values, chromaRounding), 18), chromaOffset); + const int packed = _mm_cvtsi128_si32(_mm_packus_epi16(_mm_packs_epi32(values, zero), zero)); + std::memcpy(out + x, &packed, 4); + } + for (; x < width; x += 2) { + const int right = (x + 1 < width ? x + 1 : x) * 4; + const int left = x * 4; + const int b = top[left] + top[right] + bottom[left] + bottom[right]; + const int g = top[left + 1] + top[right + 1] + bottom[left + 1] + bottom[right + 1]; + const int r = top[left + 2] + top[right + 2] + bottom[left + 2] + bottom[right + 2]; + // Sums of four, so the shift is two bits wider. + out[x] = static_cast(((-6596 * r - 22189 * g + 28784 * b + 131072) >> 18) + 128); + out[x + 1] = static_cast(((28784 * r - 26142 * g - 2642 * b + 131072) >> 18) + 128); + } + } +} + MFEncoder::~MFEncoder() { finalize(); } @@ -690,7 +780,6 @@ bool MFEncoder::initialize( // attempt would eat the injection and the run would land on the plain CPU // encoder, never reaching the software encoder the knob is aimed at. useDxgiInput_ = options.useDxgiInput && !options.injectDefaultSinkWriterFailureOnce; - cpuInputIsNv12_ = options.cpuInputIsNv12; videoEncoderSelection_ = kVideoEncoderSelectionDefault; videoEncoderRuntime_ = kVideoEncoderRuntimeUnknown; @@ -716,6 +805,21 @@ bool MFEncoder::initialize( setFrameSize(outputType.Get(), static_cast(width_), static_cast(height_)); setFrameRate(outputType.Get(), static_cast(fps_)); setPixelAspectRatio(outputType.Get()); + // Left unset, every encoder on the machine chose Constrained Baseline: one + // reference frame, CAVLC. Re-encoding real takes through the same NVENC MFT + // at an equal bitrate, High gained 1.7 dB PSNR on the screen and 1.9 VMAF on + // the webcam (getopenscreen/openscreen#922). Every hardware H.264 encoder and + // the Microsoft software one support it. No B-frames: the encoders add none + // unless asked, and they broke the fragmented writer on macOS. + outputType->SetUINT32(MF_MT_MPEG2_PROFILE, eAVEncH264VProfile_High); + // Carried on the H.264 type as well so the MP4 sink writes the matching + // colour tags instead of leaving players to guess from the frame size. + // Primaries and transfer too: without them the webcam track read + // `reserved`, and the screen track, untagged, read `unknown` throughout. + outputType->SetUINT32(MF_MT_VIDEO_NOMINAL_RANGE, MFNominalRange_16_235); + outputType->SetUINT32(MF_MT_YUV_MATRIX, MFVideoTransferMatrix_BT709); + outputType->SetUINT32(MF_MT_VIDEO_PRIMARIES, MFVideoPrimaries_BT709); + outputType->SetUINT32(MF_MT_TRANSFER_FUNCTION, MFVideoTransFunc_709); Microsoft::WRL::ComPtr inputType; if (!succeeded(MFCreateMediaType(&inputType), "MFCreateMediaType(input)")) { @@ -726,13 +830,15 @@ bool MFEncoder::initialize( // type the RGB32 path would have produced from scratch. Every attribute // one mode sets is deleted by the other; nothing carries over. auto configureVideoInputType = [&](bool dxgi) { - // Three input shapes, not two: GPU NV12, system-memory NV12 (the - // webcam, whose camera hands us NV12 already) and system-memory RGB32. - const bool nv12 = dxgi || cpuInputIsNv12_; + // NV12 on every path: GPU NV12, and system-memory NV12 -- from the + // camera as it came (the webcam), or converted here from BGRA + // (`convertBgraToNv12Bt709`). BGRA used to go in as RGB32, and the + // colour converter the sink writer put in front of the encoder turned it + // into BT.601, whatever the types said (#923). inputType->SetGUID(MF_MT_MAJOR_TYPE, MFMediaType_Video); - inputType->SetGUID(MF_MT_SUBTYPE, nv12 ? MFVideoFormat_NV12 : MFVideoFormat_RGB32); + inputType->SetGUID(MF_MT_SUBTYPE, MFVideoFormat_NV12); inputType->SetUINT32(MF_MT_INTERLACE_MODE, MFVideoInterlace_Progressive); - if (!dxgi && cpuInputIsNv12_) { + if (!dxgi) { // NV12's declared stride is the Y plane's, which is one byte per // pixel -- not the four an RGB32 row needs. inputType->SetUINT32(MF_MT_DEFAULT_STRIDE, static_cast(width_)); @@ -746,49 +852,28 @@ bool MFEncoder::initialize( setPixelAspectRatio(inputType.Get()); return; } - if (dxgi) { - inputType->DeleteItem(MF_MT_DEFAULT_STRIDE); - // The video processor below converts full-range BGRA into - // studio-range BT.709, so say so. Left untagged, the encoder and - // the player each pick their own default (BT.601 is the common - // one) and the recording comes back with shifted colours the CPU - // path does not have. - inputType->SetUINT32(MF_MT_VIDEO_NOMINAL_RANGE, MFNominalRange_16_235); - inputType->SetUINT32(MF_MT_YUV_MATRIX, MFVideoTransferMatrix_BT709); - } else { - inputType->SetUINT32(MF_MT_DEFAULT_STRIDE, static_cast(width_ * 4)); - inputType->DeleteItem(MF_MT_VIDEO_NOMINAL_RANGE); - inputType->DeleteItem(MF_MT_YUV_MATRIX); - } + inputType->DeleteItem(MF_MT_DEFAULT_STRIDE); + // The video processor below converts full-range BGRA into + // studio-range BT.709, so say so. Left untagged, the encoder and + // the player each pick their own default (BT.601 is the common + // one) and the recording comes back with shifted colours. + inputType->SetUINT32(MF_MT_VIDEO_NOMINAL_RANGE, MFNominalRange_16_235); + inputType->SetUINT32(MF_MT_YUV_MATRIX, MFVideoTransferMatrix_BT709); setFrameSize(inputType.Get(), static_cast(width_), static_cast(height_)); setFrameRate(inputType.Get(), static_cast(fps_)); setPixelAspectRatio(inputType.Get()); }; - // Carried on the H.264 type as well so the MP4 sink writes the matching - // colour tags instead of leaving players to guess from the frame size. - auto configureOutputColorTags = [&](bool dxgi) { - if (dxgi || cpuInputIsNv12_) { - outputType->SetUINT32(MF_MT_VIDEO_NOMINAL_RANGE, MFNominalRange_16_235); - outputType->SetUINT32(MF_MT_YUV_MATRIX, MFVideoTransferMatrix_BT709); - } else { - outputType->DeleteItem(MF_MT_VIDEO_NOMINAL_RANGE); - outputType->DeleteItem(MF_MT_YUV_MATRIX); - } - }; - // The allocator is the last thing that can refuse the GPU path, and it can // only be built once the NV12 type exists. Falling back here costs nothing // but the type rewrite, because no sink writer has been created yet. configureVideoInputType(useDxgiInput_); - configureOutputColorTags(useDxgiInput_); if (useDxgiInput_ && !initializeSampleAllocator(inputType.Get())) { std::cerr << "WARNING: The DXGI sample allocator is unavailable on this machine; " << "using the CPU readback path." << std::endl; releaseDxgiPipeline(); useDxgiInput_ = false; configureVideoInputType(false); - configureOutputColorTags(false); } bool injectedDefaultSinkWriterFailure = false; @@ -887,8 +972,18 @@ bool MFEncoder::initialize( // track: an H.264 input type against an AAC stream sink has no encoder // that can bridge it, so a bad mapping fails loudly here instead of // quietly writing video samples into the audio track. - if (!succeeded(sinkWriter_->SetInputMediaType(videoStreamIndex_, inputType.Get(), nullptr), - "SetInputMediaType")) { + // No B-frames. High allows them and the Microsoft software encoder uses + // them unless told otherwise (has_b_frames=1, measured); their negative + // composition offsets are what broke the fragmented writer on macOS. + // Passed as encoding parameters because the encoder only reads this + // before its types are set: ICodecAPI::SetValue afterwards is refused. + Microsoft::WRL::ComPtr encodingParameters; + if (SUCCEEDED(MFCreateAttributes(&encodingParameters, 1))) { + encodingParameters->SetUINT32(CODECAPI_AVEncMPVDefaultBPictureCount, 0); + } + if (!succeeded( + sinkWriter_->SetInputMediaType(videoStreamIndex_, inputType.Get(), encodingParameters.Get()), + "SetInputMediaType")) { return false; } if (!forceSoftwareEncoder) { @@ -946,7 +1041,6 @@ bool MFEncoder::initialize( releaseDxgiPipeline(); useDxgiInput_ = false; configureVideoInputType(false); - configureOutputColorTags(false); if (configureSinkWriterAttempt(false, kVideoEncoderSelectionDefault, false, true)) { return true; } @@ -1096,19 +1190,24 @@ bool MFEncoder::copyFrameToBuffer( } const DWORD rowBytes = static_cast(width_ * 4); - const DWORD requiredBytes = rowBytes * static_cast(height_); - if (destinationSize < requiredBytes) { + if (destinationSize < static_cast(width_ * height_ * 3 / 2)) { context_->Unmap(stagingTexture_.Get(), 0); std::cerr << "ERROR: Media Foundation buffer is too small" << std::endl; return false; } auto* source = static_cast(mapped.pData); - for (int y = 0; y < height_; y += 1) { - std::memcpy(destination + rowBytes * y, source + mapped.RowPitch * y, rowBytes); - } if (webcamFrame) { - compositeWebcam(destination, width_, height_, *webcamFrame); + // The picture-in-picture is drawn in BGRA, so the frame goes through a + // copy it can be drawn on before the conversion. + bgraScratch_.resize(static_cast(rowBytes) * height_); + for (int y = 0; y < height_; y += 1) { + std::memcpy(bgraScratch_.data() + rowBytes * y, source + mapped.RowPitch * y, rowBytes); + } + compositeWebcam(bgraScratch_.data(), width_, height_, *webcamFrame); + convertBgraToNv12Bt709(bgraScratch_.data(), static_cast(rowBytes), width_, height_, destination); + } else { + convertBgraToNv12Bt709(source, static_cast(mapped.RowPitch), width_, height_, destination); } context_->Unmap(stagingTexture_.Get(), 0); @@ -1121,29 +1220,24 @@ bool MFEncoder::copyBgraFrameToBuffer(const BgraFrameView& frame, BYTE* destinat } const DWORD rowBytes = static_cast(width_ * 4); - const DWORD requiredBytes = rowBytes * static_cast(height_); - if (destinationSize < requiredBytes) { + if (destinationSize < static_cast(width_ * height_ * 3 / 2)) { std::cerr << "ERROR: Media Foundation webcam buffer is too small" << std::endl; return false; } if (frame.width == width_ && frame.height == height_) { - // One memcpy, not a per-pixel loop forcing alpha to 255. - // - // The loop this replaces ran once per BYTE: at 3840x2160 that is 8.3 - // million iterations per frame, which measured out at ~12 fps of real - // camera motion inside a file whose container claimed 30 -- the encoder - // padded the gap with duplicates. The alpha it was writing is dead - // weight anyway: this buffer feeds an H.264 encoder through - // MFVideoFormat_RGB32, and RGB-to-YUV conversion ignores the alpha - // channel entirely. - std::memcpy(destination, frame.data, requiredBytes); + // Straight from the camera's frame, never through a per-byte loop: one + // that forced alpha to 255 measured out at ~12 fps of real camera + // motion at 3840x2160, inside a file whose container claimed 30. The + // conversion ignores alpha anyway. + convertBgraToNv12Bt709(frame.data, static_cast(rowBytes), width_, height_, destination); return true; } + bgraScratch_.resize(static_cast(rowBytes) * height_); for (int y = 0; y < height_; y += 1) { const int sourceY = static_cast((static_cast(y) * frame.height) / height_); - BYTE* destinationRow = destination + rowBytes * y; + BYTE* destinationRow = bgraScratch_.data() + rowBytes * y; for (int x = 0; x < width_; x += 1) { const int sourceX = static_cast((static_cast(x) * frame.width) / width_); const BYTE* source = frame.data + (sourceY * frame.width + sourceX) * 4; @@ -1154,7 +1248,7 @@ bool MFEncoder::copyBgraFrameToBuffer(const BgraFrameView& frame, BYTE* destinat target[3] = 255; } } - + convertBgraToNv12Bt709(bgraScratch_.data(), static_cast(rowBytes), width_, height_, destination); return true; } @@ -1640,7 +1734,7 @@ bool MFEncoder::captureVideoSample( // which holds the shared frame-state mutex across this call but not // across submitVideoSample). Microsoft::WRL::ComPtr buffer; - const DWORD frameBytes = static_cast(width_ * height_ * 4); + const DWORD frameBytes = static_cast(width_ * height_ * 3 / 2); if (!succeeded(MFCreateMemoryBuffer(frameBytes, &buffer), "MFCreateMemoryBuffer")) { return false; } @@ -1731,7 +1825,7 @@ bool MFEncoder::captureBgraSample( const int64_t sampleTime = nextSampleTime(timestampHns, sampleDuration); Microsoft::WRL::ComPtr buffer; - const DWORD frameBytes = static_cast(width_ * height_ * 4); + const DWORD frameBytes = static_cast(width_ * height_ * 3 / 2); if (!succeeded(MFCreateMemoryBuffer(frameBytes, &buffer), "MFCreateMemoryBuffer(webcam)")) { return false; } diff --git a/electron/native/wgc-capture/src/mf_encoder.h b/electron/native/wgc-capture/src/mf_encoder.h index 148693408..468ca23e3 100644 --- a/electron/native/wgc-capture/src/mf_encoder.h +++ b/electron/native/wgc-capture/src/mf_encoder.h @@ -11,6 +11,7 @@ #include #include #include +#include struct BgraFrameView { const BYTE* data = nullptr; @@ -33,6 +34,10 @@ struct Nv12FrameView { int height = 0; }; +// BGRA to NV12, BT.709 studio range, for every frame the encoder gets from +// system memory. `width` must be even. Exposed for mf_encoder_color_test. +void convertBgraToNv12Bt709(const BYTE* bgra, int stride, int width, int height, BYTE* nv12); + struct AudioInputFormat { GUID subtype = MFAudioFormat_PCM; UINT32 sampleRate = 0; @@ -62,14 +67,6 @@ struct MFEncoderOptions { // driver that refuses shared keyed-mutex textures records exactly as it did // before the path existed. Ask usesDxgiInput() for what actually happened. bool useDxgiInput = false; - /** - * Feed this encoder NV12 from system memory instead of RGB32. - * - * Only meaningful when `useDxgiInput` is false. The webcam encoder sets it - * when the camera itself delivers NV12; the screen encoder's CPU path - * still produces BGRA and leaves it alone. - */ - bool cpuInputIsNv12 = false; }; constexpr const char* kVideoEncoderSelectionDefault = "default"; @@ -257,6 +254,9 @@ class MFEncoder { DWORD videoStreamIndex_ = 0; DWORD audioStreamIndex_ = 0; bool hasAudioStream_ = false; + // The BGRA frame when it has to be drawn on or rescaled before the NV12 + // conversion; reused so a frame does not allocate one. + std::vector bgraScratch_; int width_ = 0; int height_ = 0; int fps_ = 60; @@ -264,7 +264,6 @@ class MFEncoder { int64_t lastTimestampHns_ = -1; bool finalized_ = false; bool useDxgiInput_ = false; - bool cpuInputIsNv12_ = false; const char* videoEncoderSelection_ = kVideoEncoderSelectionDefault; const char* videoEncoderRuntime_ = kVideoEncoderRuntimeUnknown; const char* containerFormat_ = kContainerFormatMp4; diff --git a/electron/native/wgc-capture/src/mf_encoder_color_test.cpp b/electron/native/wgc-capture/src/mf_encoder_color_test.cpp new file mode 100644 index 000000000..9eca7dec0 --- /dev/null +++ b/electron/native/wgc-capture/src/mf_encoder_color_test.cpp @@ -0,0 +1,295 @@ +// What the H.264 track actually says about itself (getopenscreen/openscreen#922, +// #923). Drives the real MFEncoder with synthetic BGRA frames -- three solid +// bands, red, green and blue -- into a temporary MP4, then reads the file back +// with ffprobe and ffmpeg: the profile, the colour tags, and the YUV values the +// encoder really produced for each band. The compositor decodes every +// recording as BT.709 limited range, so that is what the file has to be and +// has to say. +// +// Needs ffprobe and ffmpeg on PATH; without them the checks are skipped, not +// failed. The hardware encoder is covered when the host has one. + +#include "mf_encoder.h" + +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include + +namespace { + +int g_ran = 0; +int g_failed = 0; + +void expect(const std::string& name, bool ok, const std::string& detail) { + g_ran += 1; + std::cout << (ok ? "PASS " : "FAIL ") << name << (ok ? "" : " " + detail) << "\n"; + if (!ok) { + g_failed += 1; + } +} + +void skip(const std::string& name, const std::string& reason) { + std::cout << "SKIP " << name << " " << reason << "\n"; +} + +std::string run(const std::string& command) { + std::string output; + FILE* pipe = _popen(command.c_str(), "rb"); + if (!pipe) { + return output; + } + char buffer[65536]; + size_t read = 0; + while ((read = fread(buffer, 1, sizeof(buffer), pipe)) > 0) { + output.append(buffer, read); + } + _pclose(pipe); + return output; +} + +bool toolsAvailable() { + return run("ffprobe -version 2>NUL").find("ffprobe") != std::string::npos && + run("ffmpeg -version 2>NUL").find("ffmpeg") != std::string::npos; +} + +std::string field(const std::string& probe, const std::string& key) { + const auto at = probe.find(key + "="); + if (at == std::string::npos) { + return ""; + } + const auto start = at + key.size() + 1; + const auto end = probe.find_first_of("\r\n", start); + return probe.substr(start, end - start); +} + +constexpr int kWidth = 480; +constexpr int kHeight = 272; +constexpr int kFrames = 30; + +struct Expected { + const char* name; + int y, cb, cr; +}; +// BT.709, studio range. BT.601 would put red at Y 81, Cb 90. +constexpr Expected kBands[] = {{"red", 63, 102, 240}, {"green", 173, 42, 26}, {"blue", 32, 240, 118}}; + +void checkEncoder(ID3D11Device* device, ID3D11DeviceContext* context, bool software) { + const std::string label = software ? "software" : "default"; + char tempDir[MAX_PATH]{}; + GetTempPathA(MAX_PATH, tempDir); + const std::string path = std::string(tempDir) + "openscreen-mf-encoder-color-" + label + ".mp4"; + const std::wstring widePath(path.begin(), path.end()); + DeleteFileA(path.c_str()); + + std::vector bgra(static_cast(kWidth) * kHeight * 4); + for (int y = 0; y < kHeight; y += 1) { + for (int x = 0; x < kWidth; x += 1) { + BYTE* pixel = &bgra[(static_cast(y) * kWidth + x) * 4]; + const int band = x * 3 / kWidth; + pixel[0] = band == 2 ? 255 : 0; // B + pixel[1] = band == 1 ? 255 : 0; // G + pixel[2] = band == 0 ? 255 : 0; // R + pixel[3] = 255; + } + } + + { + MFEncoder encoder; + MFEncoderOptions options; + options.preferSoftwareEncoder = software; + if (!encoder.initialize(widePath, kWidth, kHeight, 30, 2'000'000, device, context, nullptr, options)) { + expect("encoder-initialize-" + label, false, "initialize failed"); + return; + } + const std::string runtime = encoder.videoEncoderRuntime(); + std::cout << "COLOR_RAW " << label << " runtime=" << runtime << "\n"; + if (!software && runtime != kVideoEncoderRuntimeHardware) { + skip("encoder-" + label, "no hardware H.264 encoder on this host (" + runtime + ")"); + encoder.finalize(); + return; + } + const BgraFrameView frame{bgra.data(), kWidth, kHeight}; + bool wrote = true; + for (int i = 0; i < kFrames && wrote; i += 1) { + Microsoft::WRL::ComPtr sample; + wrote = encoder.captureBgraSample(frame, static_cast(i) * 333'333, sample) && + encoder.submitVideoSample(sample.Get()); + } + expect("encoder-writes-" + label, wrote && encoder.finalize(), "write or finalize failed"); + } + + const std::string probe = run( + "ffprobe -v error -select_streams v:0 -show_entries " + "stream=profile,has_b_frames,color_range,color_space,color_primaries,color_transfer -of default=nw=1 \"" + + path + "\""); + std::cout << "COLOR_RAW " << label << " profile=" << field(probe, "profile") + << " range=" << field(probe, "color_range") << " space=" << field(probe, "color_space") + << " primaries=" << field(probe, "color_primaries") + << " transfer=" << field(probe, "color_transfer") << "\n"; + expect("profile-high-" + label, field(probe, "profile") == "High", probe); + // B-frames broke the fragmented writer on macOS; the profile must not bring them. + expect("no-b-frames-" + label, field(probe, "has_b_frames") == "0", probe); + expect( + "colour-tags-bt709-limited-" + label, + field(probe, "color_range") == "tv" && field(probe, "color_space") == "bt709" && + field(probe, "color_primaries") == "bt709" && field(probe, "color_transfer") == "bt709", + probe); + + // The last frame, as the encoder wrote its samples: yuv420p out of a yuv420p + // stream is a straight copy, no range or matrix conversion on the way. + const std::string yuv = run( + "ffmpeg -v error -sseof -0.2 -i \"" + path + "\" -frames:v 1 -f rawvideo -pix_fmt yuv420p -"); + const size_t lumaSize = static_cast(kWidth) * kHeight; + if (yuv.size() < lumaSize * 3 / 2) { + expect("yuv-readback-" + label, false, "ffmpeg returned " + std::to_string(yuv.size()) + " bytes"); + return; + } + for (int band = 0; band < 3; band += 1) { + const int x = kWidth * (2 * band + 1) / 6; + const int y = kHeight / 2; + const int luma = static_cast(yuv[static_cast(y) * kWidth + x]); + const size_t chroma = static_cast(y / 2) * (kWidth / 2) + x / 2; + const int cb = static_cast(yuv[lumaSize + chroma]); + const int cr = static_cast(yuv[lumaSize + lumaSize / 4 + chroma]); + const Expected& want = kBands[band]; + char detail[128]{}; + sprintf_s( + detail, "%s Y=%d Cb=%d Cr=%d, BT.709 limited wants %d/%d/%d", want.name, luma, cb, cr, want.y, + want.cb, want.cr); + std::cout << "COLOR_RAW " << label << " " << detail << "\n"; + expect( + std::string("yuv-bt709-") + want.name + "-" + label, + std::abs(luma - want.y) <= 3 && std::abs(cb - want.cb) <= 3 && std::abs(cr - want.cr) <= 3, + detail); + } + DeleteFileA(path.c_str()); +} + +// The converter against BT.709 in double precision, on noise: solid bands +// cannot tell a 2x2 average from a wrong pairing of pixels, and 38 wide with a +// padded stride runs the SIMD body, its scalar tail and the row pitch. +void checkConverterAgainstReference() { + constexpr int width = 38; + constexpr int height = 22; + constexpr int stride = width * 4 + 24; + std::vector bgra(static_cast(stride) * height); + uint32_t seed = 12345; + for (BYTE& value : bgra) { + seed = seed * 1664525u + 1013904223u; + value = static_cast(seed >> 24); + } + std::vector nv12(static_cast(width) * height * 3 / 2); + convertBgraToNv12Bt709(bgra.data(), stride, width, height, nv12.data()); + + const double kr = 0.2126; + const double kb = 0.0722; + const auto channel = [&](int x, int y, int c) { return bgra[static_cast(y) * stride + x * 4 + c] / 255.0; }; + int worst = 0; + for (int y = 0; y < height; y += 1) { + for (int x = 0; x < width; x += 1) { + const double luma = kr * channel(x, y, 2) + (1 - kr - kb) * channel(x, y, 1) + kb * channel(x, y, 0); + const int want = static_cast(std::lround(16 + 219 * luma)); + worst = std::max(worst, std::abs(want - nv12[static_cast(y) * width + x])); + } + } + for (int y = 0; y < height; y += 2) { + for (int x = 0; x < width; x += 2) { + double r = 0, g = 0, b = 0; + for (int dy = 0; dy < 2; dy += 1) { + for (int dx = 0; dx < 2; dx += 1) { + r += channel(x + dx, y + dy, 2) / 4; + g += channel(x + dx, y + dy, 1) / 4; + b += channel(x + dx, y + dy, 0) / 4; + } + } + const double luma = kr * r + (1 - kr - kb) * g + kb * b; + const int cb = static_cast(std::lround(128 + 224 * (b - luma) / (2 * (1 - kb)))); + const int cr = static_cast(std::lround(128 + 224 * (r - luma) / (2 * (1 - kr)))); + const size_t at = static_cast(width) * height + static_cast(y / 2) * width + x; + worst = std::max({worst, std::abs(cb - nv12[at]), std::abs(cr - nv12[at + 1])}); + } + } + std::cout << "COLOR_RAW converter worst deviation from BT.709 = " << worst << " code values" << std::endl; + expect("converter-matches-bt709-reference", worst <= 1, "worst=" + std::to_string(worst)); +} + +// Not a pass/fail: what one 1080p frame costs to hand over and encode, for a +// before/after comparison of the conversion's cost. +void timeFullHd(ID3D11Device* device, ID3D11DeviceContext* context) { + constexpr int width = 1920; + constexpr int height = 1080; + constexpr int frames = 120; + char tempDir[MAX_PATH]{}; + GetTempPathA(MAX_PATH, tempDir); + const std::string path = std::string(tempDir) + "openscreen-mf-encoder-timing.mp4"; + const std::wstring widePath(path.begin(), path.end()); + std::vector bgra(static_cast(width) * height * 4); + for (size_t i = 0; i < bgra.size(); i += 1) { + bgra[i] = static_cast((i * 2654435761u) >> 24); + } + MFEncoder encoder; + if (!encoder.initialize(widePath, width, height, 60, 18'000'000, device, context, nullptr, {})) { + return; + } + const BgraFrameView frame{bgra.data(), width, height}; + double captureMs = 0.0; + double submitMs = 0.0; + for (int i = 0; i < frames; i += 1) { + Microsoft::WRL::ComPtr sample; + const auto start = std::chrono::steady_clock::now(); + encoder.captureBgraSample(frame, static_cast(i) * 166'667, sample); + const auto captured = std::chrono::steady_clock::now(); + encoder.submitVideoSample(sample.Get()); + const auto submitted = std::chrono::steady_clock::now(); + captureMs += std::chrono::duration(captured - start).count(); + submitMs += std::chrono::duration(submitted - captured).count(); + } + encoder.finalize(); + DeleteFileA(path.c_str()); + std::cout << "COLOR_RAW timing 1080p " << encoder.videoEncoderRuntime() << ": capture " + << captureMs / frames << " ms, submit " << submitMs / frames << " ms per frame" << std::endl; +} + +} // namespace + +int main() { + checkConverterAgainstReference(); + if (!toolsAvailable()) { + skip("mf-encoder-color", "ffprobe/ffmpeg not on PATH"); + return g_failed == 0 ? 0 : 1; + } + if (FAILED(CoInitializeEx(nullptr, COINIT_MULTITHREADED))) { + skip("mf-encoder-color", "CoInitializeEx failed"); + return 0; + } + Microsoft::WRL::ComPtr device; + Microsoft::WRL::ComPtr context; + const UINT flags = D3D11_CREATE_DEVICE_BGRA_SUPPORT | D3D11_CREATE_DEVICE_VIDEO_SUPPORT; + if (FAILED(D3D11CreateDevice( + nullptr, D3D_DRIVER_TYPE_HARDWARE, nullptr, flags, nullptr, 0, D3D11_SDK_VERSION, &device, + nullptr, &context)) && + FAILED(D3D11CreateDevice( + nullptr, D3D_DRIVER_TYPE_WARP, nullptr, D3D11_CREATE_DEVICE_BGRA_SUPPORT, nullptr, 0, + D3D11_SDK_VERSION, &device, nullptr, &context))) { + skip("mf-encoder-color", "no D3D11 device"); + return 0; + } + checkEncoder(device.Get(), context.Get(), true); + checkEncoder(device.Get(), context.Get(), false); + timeFullHd(device.Get(), context.Get()); + + std::cout << "ran " << g_ran << " tests\n"; + if (g_failed != 0) { + std::cout << g_failed << " failed\n"; + return 1; + } + return 0; +} diff --git a/scripts/build-windows-wgc-helper.mjs b/scripts/build-windows-wgc-helper.mjs index 42594fe27..ab4b26c00 100644 --- a/scripts/build-windows-wgc-helper.mjs +++ b/scripts/build-windows-wgc-helper.mjs @@ -134,3 +134,14 @@ if (!fs.existsSync(webcamSnapshotTestPath)) { // the camera's, so a frame it already holds must not be copied again. await run(webcamSnapshotTestPath, [], { cwd: BUILD_DIR }); console.log(`Passed ${webcamSnapshotTestPath}`); + +const encoderColorTestPath = path.join(BUILD_DIR, "mf_encoder_color_test.exe"); +if (!fs.existsSync(encoderColorTestPath)) { + throw new Error(`WGC helper build completed but ${encoderColorTestPath} was not found.`); +} +// Guards what the H.264 track is and says: High profile, BT.709 studio range +// in the samples and in the tags, which is what the compositor decodes. Media +// Foundation's own colour converter wrote BT.601. Skips its file checks when +// ffprobe/ffmpeg are not on PATH. +await run(encoderColorTestPath, [], { cwd: BUILD_DIR }); +console.log(`Passed ${encoderColorTestPath}`); diff --git a/technical-documentation/testing/manual-e2e-checklist.md b/technical-documentation/testing/manual-e2e-checklist.md index 3017b8152..10e09ab9e 100644 --- a/technical-documentation/testing/manual-e2e-checklist.md +++ b/technical-documentation/testing/manual-e2e-checklist.md @@ -735,6 +735,7 @@ Edits are saved as they land. The dot after the project name reads "Unsaved" (ho - [ ] **post-1.10.0** — Confirm forcing the software encoder still reports `"software"`, so the default is a default and not a hard-wire. - [ ] **post-1.10.0** — Record with microphone and system audio and confirm the resulting MP4 carries a valid AAC track at a legal rate (48 kHz). - [ ] Speak continuously for 30 s with the microphone on, then play the file in a neutral player (VLC, ffplay) and confirm there is no crackle. Include the moment you reach for the HUD to stop: that is where the holes of #911 clustered. In a waveform view, the defect looks like drops to digital silence of up to 10 ms in the middle of words. +- [ ] Record with the webcam and confirm `ffprobe` reads `profile=High`, `has_b_frames=0` and `tv / bt709 / bt709 / bt709` colour tags on the screen and webcam tracks (#922, #923). Then export: solid colours must match the source within ±2 in RGB. - [ ] **post-1.10.0** — On a device whose native rate AAC cannot take (96 kHz), confirm the recording still succeeds with the rate snapped to 48 kHz rather than failing at `SetInputMediaType`. The helper's own `audio_sample_utils_test` covers the accept/reject probes at build time; this check is the end-to-end half. - [ ] **post-1.10.0** — Confirm a long recording's audio stays in sync, so the downsample remainder is carried across packets rather than drifting. - [ ] **post-1.10.0** — On a device that can be set to 96 kHz, record system audio while a 36 kHz tone plays and confirm the recording carries **no** 12 kHz component. That fold is what an inadequate anti-alias filter produces, and neither of the two checks above would catch it: the rate-snap check only asks that the recording succeeds, and the sync check only asks that frame counts stay aligned. Probe tones must sit well inside what AAC keeps — 12 kHz is fine; a 20 kHz probe was absent from the app's recording, and a 192 kbps AAC encode alone (tested with ffmpeg) removes it too, so it cannot be measured. Needs an endpoint whose **shared-mode** format is above 48 kHz and an integer multiple of it — check the Advanced tab's format list, and check every endpoint, not just the current format of the default one; a USB DAC is one way to get such an endpoint. Exclusive-mode support is not enough, because loopback reports the shared-mode format. If every endpoint really is 48 kHz, the check cannot run at all: forcing a lower encoder target instead does not work, since every AAC rate that would divide 48 kHz is rejected by the Media Foundation encoder on the host tested. From c3a1a9c613974b183365ece13774ef07db90a16d Mon Sep 17 00:00:00 2001 From: EtienneLescot Date: Thu, 1 Oct 2026 00:22:22 +0200 Subject: [PATCH 2/3] test(windows): widen the temp path through the ANSI code page --- .../wgc-capture/src/mf_encoder_color_test.cpp | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/electron/native/wgc-capture/src/mf_encoder_color_test.cpp b/electron/native/wgc-capture/src/mf_encoder_color_test.cpp index 9eca7dec0..1faf61ff0 100644 --- a/electron/native/wgc-capture/src/mf_encoder_color_test.cpp +++ b/electron/native/wgc-capture/src/mf_encoder_color_test.cpp @@ -55,6 +55,15 @@ std::string run(const std::string& command) { return output; } +// GetTempPathA answers in the ANSI code page, which _popen also speaks; the +// encoder takes a wide path, so widen it through that code page, not per byte. +std::wstring widen(const std::string& ansi) { + const int size = MultiByteToWideChar(CP_ACP, 0, ansi.data(), static_cast(ansi.size()), nullptr, 0); + std::wstring result(size, L'\0'); + MultiByteToWideChar(CP_ACP, 0, ansi.data(), static_cast(ansi.size()), result.data(), size); + return result; +} + bool toolsAvailable() { return run("ffprobe -version 2>NUL").find("ffprobe") != std::string::npos && run("ffmpeg -version 2>NUL").find("ffmpeg") != std::string::npos; @@ -86,7 +95,7 @@ void checkEncoder(ID3D11Device* device, ID3D11DeviceContext* context, bool softw char tempDir[MAX_PATH]{}; GetTempPathA(MAX_PATH, tempDir); const std::string path = std::string(tempDir) + "openscreen-mf-encoder-color-" + label + ".mp4"; - const std::wstring widePath(path.begin(), path.end()); + const std::wstring widePath = widen(path); DeleteFileA(path.c_str()); std::vector bgra(static_cast(kWidth) * kHeight * 4); @@ -230,7 +239,7 @@ void timeFullHd(ID3D11Device* device, ID3D11DeviceContext* context) { char tempDir[MAX_PATH]{}; GetTempPathA(MAX_PATH, tempDir); const std::string path = std::string(tempDir) + "openscreen-mf-encoder-timing.mp4"; - const std::wstring widePath(path.begin(), path.end()); + const std::wstring widePath = widen(path); std::vector bgra(static_cast(width) * height * 4); for (size_t i = 0; i < bgra.size(); i += 1) { bgra[i] = static_cast((i * 2654435761u) >> 24); From e4848d2431dacfd89996c8888fe322e96b6a5a21 Mon Sep 17 00:00:00 2001 From: EtienneLescot Date: Thu, 1 Oct 2026 00:54:26 +0200 Subject: [PATCH 3/3] build(windows): link avrt into the encoder colour test, which compiles the MMCSS mixer --- electron/native/wgc-capture/CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/electron/native/wgc-capture/CMakeLists.txt b/electron/native/wgc-capture/CMakeLists.txt index f5dfeb71b..50466d0d0 100644 --- a/electron/native/wgc-capture/CMakeLists.txt +++ b/electron/native/wgc-capture/CMakeLists.txt @@ -188,6 +188,7 @@ target_compile_definitions(mf_encoder_color_test PRIVATE target_compile_options(mf_encoder_color_test PRIVATE /EHsc /W4 /utf-8) target_link_libraries(mf_encoder_color_test PRIVATE + avrt d3d11 dxgi mf