From 9a368cd0e51efa17698a68e073f3b2a2c636e359 Mon Sep 17 00:00:00 2001 From: Etienne Lescot Date: Tue, 11 Aug 2026 11:19:23 +0200 Subject: [PATCH] fix(recording): write fragmented MP4 so a killed helper keeps its file A frozen recording on Windows costs the user the whole file, and the freeze is not why. A plain MP4 has no index until IMFSinkWriter::Finalize() writes moov at the very end, so when the shutdown watchdog force-exits a wedged helper the .mp4 on disk holds every frame and no way to read them. Minutes of capture thrown away for about eight kilobytes of table. That is issues #252 / #292 / #327. A fragmented MP4 writes its index up front and its samples in self-describing moof+mdat pairs, so the same kill leaves a file that plays up to the last complete fragment. This does not fix the freeze -- the recording still stops on its own -- it stops the freeze from destroying the recording, and it does so without anyone knowing which call is stuck. MFCreateFMPEG4MediaSink wants both output media types at construction, which inverts the order the encoder was written in: it created the sink writer and described the streams afterwards with AddStream. So the AAC output type moves out of configureAudioStream into buildAacOutputType, the format guard moves with it, and createSinkWriter grows a `fragmented` switch instead of a second copy of the software-MFT registration, the hardware-transform flag and the D3D manager. The stream positions are read back off the sink by major type rather than assumed: a wrong index would aim audio writes at the video track. Two things this had to not break. The first is the fallback chain that produced the #336 regression -- all four paths stay fragmented, and a fifth attempt with the plain container runs only after every fragmented one has failed, so a machine the fragmented sink does not fit records exactly as it did before this commit rather than not recording. `container` in the encoder-selection event says which one was used, because a bug report that cannot tell them apart cannot say whether a truncated file was supposed to survive its kill. The second is ownership: MFCreateSinkWriterFromMediaSink does not take it, so releasing the writer left the sink live and the output file open, and the next attempt's MFCreateFile(DELETE_IF_EXIST) would have raced the previous attempt's own handle. releaseSinkWriter() shuts the sink down and closes the byte stream, between attempts and at finalize(). macOS gets the same property from one AVAssetWriter.movieFragmentInterval; finishWriting() still writes a normal moov on a clean stop. Linux stays plain: empty_moov makes the output permanently non-seekable and the native Linux path has no re-index step, so the editor's scrub cost has to be measured first. The helper test truncates its own output to 60% and reprobes it. That is a proxy for the kill, not a replacement -- it proves the container survives losing its tail, not that the helper flushed anything before dying. --- electron/native/README.md | 8 +- .../ScreenCaptureRecorder.swift | 5 + electron/native/wgc-capture/src/main.cpp | 5 + .../native/wgc-capture/src/mf_encoder.cpp | 323 +++++++++++++++--- electron/native/wgc-capture/src/mf_encoder.h | 28 ++ scripts/test-windows-wgc-helper.mjs | 54 ++- .../architecture/recording.md | 2 + 7 files changed, 365 insertions(+), 60 deletions(-) diff --git a/electron/native/README.md b/electron/native/README.md index 1035aee9..438e8d75 100644 --- a/electron/native/README.md +++ b/electron/native/README.md @@ -25,7 +25,7 @@ npm run build:native:mac On non-macOS hosts this command exits successfully and does not affect Windows/Linux development. On macOS it builds the Swift package at `electron/native/screencapturekit`, writes the development binaries to `electron/native/screencapturekit/build`, and copies redistributable binaries to `electron/native/bin/darwin-${arch}`. -The current helper implementation supports display/window ScreenCaptureKit video capture, cursor exclusion through `SCStreamConfiguration.showsCursor`, H.264 encoding, MP4 muxing, and ScreenCaptureKit system audio. It also attempts native ScreenCaptureKit microphone capture when the running macOS version exposes that capability. Webcam recording currently stays as an Electron sidecar and is attached to the same recording session after the native screen capture stops. +The current helper implementation supports display/window ScreenCaptureKit video capture, cursor exclusion through `SCStreamConfiguration.showsCursor`, H.264 encoding, MP4 muxing (with `AVAssetWriter.movieFragmentInterval` at 1s, so a helper that dies before `finishWriting()` still leaves a readable file — same reasoning as the Windows fragmented sink below), and ScreenCaptureKit system audio. It also attempts native ScreenCaptureKit microphone capture when the running macOS version exposes that capability. Webcam recording currently stays as an Electron sidecar and is attached to the same recording session after the native screen capture stops. Electron exposes `is-native-mac-capture-available` for capability probing. It resolves the same helper locations listed above and reports `missing-helper` until a Swift helper binary is present. When available, macOS recording routes screen/window capture through the native helper so editable cursor recordings do not bake the system cursor into the video. Cursor positions are sampled in Electron; when the cursor helper is available and Accessibility is granted, samples are also tagged with link/text cursor hints such as `pointer`. @@ -83,13 +83,15 @@ Current V2 JSON shape: The current helper implementation supports display/window video capture, system audio loopback, selected-microphone capture, Media Foundation webcam capture, and a DirectShow webcam fallback for virtual cameras that are not exposed through Media Foundation. Webcam frames are currently composed into the primary MP4 as a bottom-right picture-in-picture overlay. Browser `deviceId` values do not always map to Media Foundation symbolic links or WASAPI endpoint IDs, so the renderer passes both browser IDs and user-visible device names. For microphones, the helper tries the requested WASAPI endpoint ID first, then resolves an active capture endpoint by `microphoneDeviceName`, then falls back to the default endpoint. For webcams, Electron resolves a matching DirectShow filter CLSID for the selected label; the helper uses Media Foundation first, then that exact DirectShow filter when the requested camera is absent from Media Foundation. +Container: recordings are written as fragmented MP4 (`MFCreateFMPEG4MediaSink` + `MFCreateSinkWriterFromMediaSink`, `MF_MPEG4SINK_MIN_FRAGMENT_DURATION` = 1s) rather than plain MP4. A plain MP4 has no index until `IMFSinkWriter::Finalize()` writes `moov` at the very end, so when the shutdown watchdog force-exits a wedged helper the file on disk holds every frame and no way to read them — that is why issues #252 / #292 / #327 cost the whole recording rather than the frozen tail of it. A fragmented MP4 writes its index up front and its samples in self-describing `moof`+`mdat` pairs, so the same kill leaves a file that plays up to the last complete fragment. This does not fix the freeze; it removes the data loss the freeze causes. Because the fragmented sink needs both output media types at construction, the sink writer is built from a media sink instead of from a URL, and the helper reads the video/audio stream positions back off the sink rather than assuming them. If any of that is unavailable on a machine, the helper retries with the plain container and says so — `container` in the `encoder-selection` event is `fragmented-mp4` or `mp4`, and it reports what was used, not what was asked for. + Encoder selection: by default the helper keeps the existing sink-writer path first. If that path fails while setting up H.264, it retries with the Microsoft software H.264 encoder (`mfh264enc.dll`). The key of this retry is registering that encoder locally in the helper process via `MFTRegisterLocalByCLSID`, which makes a software H.264 encoder available even when the machine's hardware encoders are missing or broken; hardware transforms are disabled for the retry only as a secondary guard so the sink writer prefers the locally registered software encoder, not as the fallback mechanism itself. Set `preferSoftwareEncoder: true` in the helper JSON, or set `OPENSCREEN_WGC_PREFER_SOFTWARE_ENCODER=true` before launching Electron, to force the software path from the first attempt. Frame input path: the helper feeds the encoder from the GPU when it can. On that path it copies the WGC frame across a keyed-mutex bridge to a second D3D11 device, converts BGRA to NV12 with the D3D11 video processor, and submits an allocator-owned DXGI sample to the hardware H.264 encoder, so no frame ever passes through system memory. The alternative is the original path: a staging texture, `Map(D3D11_MAP_READ)`, and a row-by-row copy into an `IMFMediaBuffer` — which is where a driver stall costs a recording (issue #252). The GPU path is a preference, never a requirement: it is skipped outright for `preferSoftwareEncoder` and for inline webcam PiP (both need the frame in system memory), and it degrades to the CPU path on its own if the encoding device, the NV12 video processor, the shared bridge texture, the DXGI sample allocator, or the hardware sink writer is unavailable. The GPU path is OFF by default: it fixed #252 on the machine that reproduces it and broke recording outright in #336, and its fallbacks only cover failures during `initialize()`, not one that appears once frames are flowing. Set `OPENSCREEN_WGC_ENABLE_DXGI_INPUT=1` to turn it on. Because the two paths land on different encoders and hardware MFTs default to constant bitrate, the GPU path asks for VBR through `ICodecAPI`; without it a static screen spends the full configured budget (measured 16.9 Mbps against 1.95 for the same desktop). -The helper reports the outcome through the `encoder-selection` stdout event (`video` is `default`, `software-preferred`, or `software-fallback`; `videoInput` is `dxgi-nv12` or `cpu-rgb32`, and reports what the encoder settled on rather than what was asked for). On the GPU path the helper also prints one `[frame-drops] gpu_bridge_contended=` line to stderr at stop: a frame the bridge was too busy to take is skipped rather than failing the recording, and a large count there is the first thing to look at in a report about missing frames. When the app sees `software-fallback` — the default encoder failed and the helper switched on its own — it shows a small dismissible notice in the recording HUD with a "Don't show again" option, because software encoding can raise CPU usage. An explicit `software-preferred` selection shows no notice, and the event stays available for diagnostics either way. +The helper reports the outcome through the `encoder-selection` stdout event (`video` is `default`, `software-preferred`, or `software-fallback`; `videoInput` is `dxgi-nv12` or `cpu-rgb32`; `container` is `fragmented-mp4` or `mp4`; all three report what the encoder settled on rather than what was asked for). On the GPU path the helper also prints one `[frame-drops] gpu_bridge_contended=` line to stderr at stop: a frame the bridge was too busy to take is skipped rather than failing the recording, and a large count there is the first thing to look at in a report about missing frames. When the app sees `software-fallback` — the default encoder failed and the helper switched on its own — it shows a small dismissible notice in the recording HUD with a "Don't show again" option, because software encoding can raise CPU usage. An explicit `software-preferred` selection shows no notice, and the event stays available for diagnostics either way. -Encoder diagnostic on final sink-writer failure: when the final `MFCreateSinkWriterFromURL` attempt fails, the helper logs the registered H.264 video encoder MFT count (via `MFTEnumEx`), the registered AAC encoder count when audio was requested, and the hex HRESULT. If no H.264 encoder is registered, it additionally emits the four-bullet actionable error (missing Media Feature Pack / GPU driver registration / empty `HKLM:\SOFTWARE\Microsoft\Windows Media Foundation\Transforms` / reboot). If an H.264 encoder IS registered but the sink writer still failed, it logs a hint pointing at invalid output path, missing MP4 mux, or GPU driver incompatibility. There is still no fail-fast pre-flight gate because `MFTEnumEx` and `MFCreateSinkWriterFromURL` can disagree about which H.264 encoders are available in non-interactive / Session 0 contexts. +Encoder diagnostic on final sink-writer failure: when the final sink-writer attempt fails (`MFCreateSinkWriterFromMediaSink` on the fragmented container, `MFCreateSinkWriterFromURL` on the plain one; the message names which), the helper logs the registered H.264 video encoder MFT count (via `MFTEnumEx`), the registered AAC encoder count when audio was requested, and the hex HRESULT. If no H.264 encoder is registered, it additionally emits the four-bullet actionable error (missing Media Feature Pack / GPU driver registration / empty `HKLM:\SOFTWARE\Microsoft\Windows Media Foundation\Transforms` / reboot). If an H.264 encoder IS registered but the sink writer still failed, it logs a hint pointing at invalid output path, missing MP4 mux, or GPU driver incompatibility. There is still no fail-fast pre-flight gate because `MFTEnumEx` and the sink writer can disagree about which H.264 encoders are available in non-interactive / Session 0 contexts. Smoke-test the helper with: diff --git a/electron/native/screencapturekit/Sources/OpenScreenScreenCaptureKitHelper/ScreenCaptureRecorder.swift b/electron/native/screencapturekit/Sources/OpenScreenScreenCaptureKitHelper/ScreenCaptureRecorder.swift index 786362cd..42e764e3 100644 --- a/electron/native/screencapturekit/Sources/OpenScreenScreenCaptureKitHelper/ScreenCaptureRecorder.swift +++ b/electron/native/screencapturekit/Sources/OpenScreenScreenCaptureKitHelper/ScreenCaptureRecorder.swift @@ -444,6 +444,11 @@ final class ScreenCaptureRecorder: NSObject, SCStreamOutput, SCStreamDelegate { ) let writer = try AVAssetWriter(outputURL: outputUrl, fileType: .mp4) + // Costs nothing on a clean stop -- finishWriting() still writes a normal + // moov -- and is the difference between a readable file and a total loss + // when the helper dies before reaching it. The Windows helper gets the + // same property from MFCreateFMPEG4MediaSink; see issues #252/#292/#327. + writer.movieFragmentInterval = CMTime(seconds: 1, preferredTimescale: 600) let settings: [String: Any] = [ AVVideoCodecKey: AVVideoCodecType.h264, AVVideoWidthKey: outputWidth, diff --git a/electron/native/wgc-capture/src/main.cpp b/electron/native/wgc-capture/src/main.cpp index 5d55592a..8afbef7a 100644 --- a/electron/native/wgc-capture/src/main.cpp +++ b/electron/native/wgc-capture/src/main.cpp @@ -757,6 +757,11 @@ int main(int argc, char* argv[]) { std::cout << "{\"event\":\"encoder-selection\",\"schemaVersion\":2,\"video\":\"" << encoder.videoEncoderSelection() << "\",\"videoInput\":\"" << (usesDxgiInput ? "dxgi-nv12" : "cpu-rgb32") + // Reported for the same reason `videoInput` is: the encoder falls + // back to the plain container rather than failing a recording, and + // "was this file supposed to survive a kill?" is unanswerable from + // a bug report that cannot tell the two apart. + << "\",\"container\":\"" << encoder.containerFormat() << "\",\"preferSoftwareEncoder\":" << (config.preferSoftwareEncoder ? "true" : "false") << "}" << std::endl; diff --git a/electron/native/wgc-capture/src/mf_encoder.cpp b/electron/native/wgc-capture/src/mf_encoder.cpp index 9fee5a7c..4058ca24 100644 --- a/electron/native/wgc-capture/src/mf_encoder.cpp +++ b/electron/native/wgc-capture/src/mf_encoder.cpp @@ -127,21 +127,32 @@ void logMissingH264EncoderError() { << std::endl; } -// Which step of createSinkWriterFromUrl produced a failing HRESULT. Only a -// CreateSinkWriter failure means MFCreateSinkWriterFromURL itself failed and +// Which step of createSinkWriter produced a failing HRESULT. Only a +// CreateSinkWriter failure means the sink-writer creation itself failed and // warrants the encoder-enumeration diagnostics in logSinkWriterCreateFailure. -// The earlier software-path setup steps each log their own specific error, so -// routing them through the sink-writer diagnostics would misattribute the -// failure (e.g. reporting a local MFT registration failure as a sink-writer -// failure on the VM / headless / broken-driver systems this path targets). +// The earlier setup steps each log their own specific error, so routing them +// through the sink-writer diagnostics would misattribute the failure (e.g. +// reporting a local MFT registration failure as a sink-writer failure on the +// VM / headless / broken-driver systems this path targets). CreateFile and +// CreateFragmentedMediaSink are container problems rather than encoder ones and +// are excluded for the same reason. enum class SinkWriterCreateStage { SoftwareEncoderRegistration, CreateAttributes, DisableHardwareTransforms, ConfigureDxgiManager, + CreateFile, + CreateFragmentedMediaSink, CreateSinkWriter, }; +// How often the fragmented MP4 sink is allowed to close a moof+mdat pair. It is +// a floor, not a period: the sink still waits for the encoder's next key frame. +// One second is the trade the whole change rests on -- a killed helper loses at +// most the fragment in flight, while the per-fragment box overhead stays +// negligible against a multi-megabit H.264 payload. +constexpr UINT64 kFragmentDurationHns = 10'000'000ULL; + HRESULT ensureSoftwareH264EncoderRegisteredForProcess() { static std::mutex registrationMutex; static bool attempted = false; @@ -181,12 +192,25 @@ HRESULT ensureSoftwareH264EncoderRegisteredForProcess() { return result; } -HRESULT createSinkWriterFromUrl( +// `fragmented` picks between the two ways to reach a sink writer, and it is the +// only reason this function grew output types. MFCreateSinkWriterFromURL builds +// the MP4 sink itself and lets AddStream describe the streams afterwards; +// MFCreateFMPEG4MediaSink demands both output types up front because the +// fragmented sink writes its stream table before the first sample. Everything +// before that last step -- the local software MFT registration, the +// hardware-transform flag, the D3D manager -- is identical either way and stays +// on one path rather than being duplicated per container. +HRESULT createSinkWriter( const std::wstring& outputPath, + bool fragmented, + IMFMediaType* videoOutputType, + IMFMediaType* audioOutputType, bool forceSoftwareEncoder, IMFDXGIDeviceManager* dxgiDeviceManager, bool injectDefaultSinkWriterFailureOnce, bool& injectedDefaultSinkWriterFailure, + Microsoft::WRL::ComPtr& byteStream, + Microsoft::WRL::ComPtr& mediaSink, Microsoft::WRL::ComPtr& sinkWriter, SinkWriterCreateStage& failedStage) { // Default to the sink-writer creation step; the software-path steps below @@ -249,20 +273,68 @@ HRESULT createSinkWriterFromUrl( } failedStage = SinkWriterCreateStage::CreateSinkWriter; + // Ahead of the byte stream on purpose: an injection that fired after + // MFCreateFile would leave the output file open on the very path whose job + // is to prove the next attempt can still create it. if ( !forceSoftwareEncoder && injectDefaultSinkWriterFailureOnce && !injectedDefaultSinkWriterFailure) { injectedDefaultSinkWriterFailure = true; std::cerr - << "TEST-ONLY: Injected default MFCreateSinkWriterFromURL failure " + << "TEST-ONLY: Injected default sink-writer creation failure " << "(hr=0x80070003); injection consumed exactly once." << std::endl; return HRESULT_FROM_WIN32(ERROR_PATH_NOT_FOUND); } - const HRESULT sinkWriterHr = MFCreateSinkWriterFromURL( - outputPath.c_str(), nullptr, attributes.Get(), &sinkWriter); + if (!fragmented) { + const HRESULT sinkWriterHr = MFCreateSinkWriterFromURL( + outputPath.c_str(), nullptr, attributes.Get(), &sinkWriter); + if (SUCCEEDED(sinkWriterHr) && forceSoftwareEncoder) { + std::cerr << "INFO: Created the real software H.264 sink writer successfully." + << std::endl; + } + return sinkWriterHr; + } + + failedStage = SinkWriterCreateStage::CreateFile; + HRESULT hr = MFCreateFile( + MF_ACCESSMODE_WRITE, + MF_OPENMODE_DELETE_IF_EXIST, + MF_FILEFLAGS_NONE, + outputPath.c_str(), + &byteStream); + if (FAILED(hr)) { + std::cerr << "ERROR: MFCreateFile(recording output) failed (hr=0x" + << std::hex << hr << std::dec << ")" << std::endl; + return hr; + } + + failedStage = SinkWriterCreateStage::CreateFragmentedMediaSink; + hr = MFCreateFMPEG4MediaSink(byteStream.Get(), videoOutputType, audioOutputType, &mediaSink); + if (FAILED(hr)) { + std::cerr << "ERROR: MFCreateFMPEG4MediaSink failed (hr=0x" + << std::hex << hr << std::dec << "); the recording falls back to a " + << "plain MP4, which is unreadable if the helper is killed." << std::endl; + return hr; + } + + // Best effort. A sink that will not take the attribute still fragments, on + // whatever interval it picked for itself, and that is already the whole + // benefit; refusing the recording over the interval would trade the bug for + // a worse one. + Microsoft::WRL::ComPtr sinkAttributes; + if ( + FAILED(mediaSink.As(&sinkAttributes)) || + FAILED(sinkAttributes->SetUINT64(MF_MPEG4SINK_MIN_FRAGMENT_DURATION, kFragmentDurationHns))) { + std::cerr << "WARNING: Could not set the fragmented MP4 fragment duration; " + << "the sink's own interval applies." << std::endl; + } + + failedStage = SinkWriterCreateStage::CreateSinkWriter; + const HRESULT sinkWriterHr = MFCreateSinkWriterFromMediaSink( + mediaSink.Get(), attributes.Get(), &sinkWriter); if (SUCCEEDED(sinkWriterHr) && forceSoftwareEncoder) { std::cerr << "INFO: Created the real software H.264 sink writer successfully." << std::endl; @@ -270,12 +342,55 @@ HRESULT createSinkWriterFromUrl( return sinkWriterHr; } -void logSinkWriterCreateFailure(HRESULT sinkWriterHr, const AudioInputFormat* audioFormat) { +// The sink writer addresses a media sink's streams by their position in the +// sink. Nothing promises video is position 0 -- MFCreateFMPEG4MediaSink is +// handed two media types and decides for itself -- and getting it wrong would +// aim the audio writes at the video track. So the position is read back from +// the sink by major type instead of assumed, and logged with the stream +// identifier beside it, which is a different number and the one an MP4 dump +// shows. +bool resolveStreamSinkIndex(IMFMediaSink* mediaSink, const GUID& majorType, DWORD& streamIndex) { + const char* const label = (majorType == MFMediaType_Video) ? "video" : "audio"; + DWORD streamSinkCount = 0; + if (!succeeded(mediaSink->GetStreamSinkCount(&streamSinkCount), "GetStreamSinkCount")) { + return false; + } + + for (DWORD index = 0; index < streamSinkCount; index += 1) { + Microsoft::WRL::ComPtr streamSink; + if (FAILED(mediaSink->GetStreamSinkByIndex(index, &streamSink))) { + continue; + } + Microsoft::WRL::ComPtr typeHandler; + if (FAILED(streamSink->GetMediaTypeHandler(&typeHandler))) { + continue; + } + GUID streamMajorType{}; + if (FAILED(typeHandler->GetMajorType(&streamMajorType)) || streamMajorType != majorType) { + continue; + } + DWORD identifier = 0; + streamSink->GetIdentifier(&identifier); + std::cerr << "INFO: Fragmented MP4 sink carries " << label << " on stream index " + << index << " (identifier " << identifier << ")." << std::endl; + streamIndex = index; + return true; + } + + std::cerr << "ERROR: The fragmented MP4 sink exposes no " << label << " stream sink (" + << streamSinkCount << " stream sinks)." << std::endl; + return false; +} + +void logSinkWriterCreateFailure( + HRESULT sinkWriterHr, + const char* createCall, + const AudioInputFormat* audioFormat) { const UINT32 h264EncoderCount = countRegisteredH264VideoEncoders(); const UINT32 aacEncoderCount = (audioFormat != nullptr) ? countRegisteredAacAudioEncoders() : 0; - std::cerr << "ERROR: MFCreateSinkWriterFromURL failed (hr=0x" + std::cerr << "ERROR: " << createCall << " failed (hr=0x" << std::hex << sinkWriterHr << std::dec << ")" << std::endl; std::cerr << " Registered H.264 video encoder MFTs: " << h264EncoderCount << std::endl; @@ -313,6 +428,32 @@ void setAudioFormat(IMFMediaType* type, UINT32 channels, UINT32 sampleRate, UINT type->SetUINT32(MF_MT_AUDIO_BITS_PER_SAMPLE, bitsPerSample); } +// Lifted out of configureAudioStream unchanged, including the format guard, +// because the fragmented sink needs this type before a sink writer exists at +// all. The guard has to come with it: an invalid format must be refused before +// anything is built from it, not after. +bool buildAacOutputType( + const AudioInputFormat& audioFormat, + Microsoft::WRL::ComPtr& outputType) { + if (audioFormat.sampleRate == 0 || audioFormat.channels == 0 || audioFormat.blockAlign == 0) { + std::cerr << "ERROR: Invalid audio input format" << std::endl; + return false; + } + + const AudioInputFormat encoderFormat = makeAacCompatibleAudioFormat(audioFormat); + const UINT32 aacBytesPerSecond = 24'000; + + if (!succeeded(MFCreateMediaType(&outputType), "MFCreateMediaType(audio output)")) { + return false; + } + outputType->SetGUID(MF_MT_MAJOR_TYPE, MFMediaType_Audio); + outputType->SetGUID(MF_MT_SUBTYPE, MFAudioFormat_AAC); + setAudioFormat(outputType.Get(), encoderFormat.channels, encoderFormat.sampleRate, 16); + outputType->SetUINT32(MF_MT_AUDIO_AVG_BYTES_PER_SECOND, aacBytesPerSecond); + outputType->SetUINT32(MF_MT_AAC_PAYLOAD_TYPE, 0); + return true; +} + void compositeWebcam(BYTE* destination, int width, int height, const BgraFrameView& webcamFrame) { if (!webcamFrame.data || webcamFrame.width <= 0 || webcamFrame.height <= 0 || width <= 0 || height <= 0) { return; @@ -372,6 +513,29 @@ const char* MFEncoder::videoEncoderSelection() const { return videoEncoderSelection_; } +const char* MFEncoder::containerFormat() const { + return containerFormat_; +} + +// Releasing the sink writer is enough only when the sink writer built the sink +// itself. MFCreateSinkWriterFromMediaSink does not take ownership: dropping the +// writer leaves the fragmented sink live and the output file open underneath +// it. Between two attempts that meant the next MFCreateFile(DELETE_IF_EXIST) +// racing the previous attempt's own handle -- a broken fallback chain, which is +// the one thing this change is not allowed to produce. At finalize() it also +// closes the byte stream, so nothing is left sitting in a buffer. +void MFEncoder::releaseSinkWriter() { + sinkWriter_.Reset(); + if (mediaSink_) { + mediaSink_->Shutdown(); + mediaSink_.Reset(); + } + if (byteStream_) { + byteStream_->Close(); + byteStream_.Reset(); + } +} + bool MFEncoder::usesDxgiInput() const { return useDxgiInput_; } @@ -520,53 +684,98 @@ bool MFEncoder::initialize( bool injectedDefaultSinkWriterFailure = false; auto resetSinkWriterAttempt = [&]() { - sinkWriter_.Reset(); + releaseSinkWriter(); videoStreamIndex_ = 0; audioStreamIndex_ = 0; hasAudioStream_ = false; videoEncoderSelection_ = kVideoEncoderSelectionDefault; + containerFormat_ = kContainerFormatMp4; }; auto configureSinkWriterAttempt = [&, audioFormat]( bool forceSoftwareEncoder, const char* selection, - bool logCreateFailure) { + bool logCreateFailure, + bool fragmented) { resetSinkWriterAttempt(); + // Before the sink writer, not inside configureAudioStream where this + // used to live: MFCreateFMPEG4MediaSink takes the audio output type as + // a construction argument. Null when the recording has no audio, which + // is both the common case and a documented one for that call. + Microsoft::WRL::ComPtr audioOutputType; + if (audioFormat && !buildAacOutputType(*audioFormat, audioOutputType)) { + return false; + } + SinkWriterCreateStage failedStage = SinkWriterCreateStage::CreateSinkWriter; - const HRESULT sinkWriterHr = createSinkWriterFromUrl( + const HRESULT sinkWriterHr = createSinkWriter( outputPath, + fragmented, + outputType.Get(), + audioOutputType.Get(), forceSoftwareEncoder, forceSoftwareEncoder ? nullptr : dxgiDeviceManager_.Get(), options.injectDefaultSinkWriterFailureOnce, injectedDefaultSinkWriterFailure, + byteStream_, + mediaSink_, sinkWriter_, failedStage); if (FAILED(sinkWriterHr)) { - // Only a genuine MFCreateSinkWriterFromURL failure gets the - // sink-writer / encoder-enumeration diagnostics. The earlier - // software-path steps (local MFT registration, attribute creation, - // hardware-transform flag) already logged their own specific error - // inside createSinkWriterFromUrl, so logging here as well would - // misattribute those failures to MFCreateSinkWriterFromURL. + // Only a genuine sink-writer creation failure gets the sink-writer / + // encoder-enumeration diagnostics. The earlier steps (local MFT + // registration, attribute creation, hardware-transform flag, and on + // the fragmented path the byte stream and the media sink) already + // logged their own specific error inside createSinkWriter, so + // logging here as well would misattribute those failures to the + // sink writer. if (failedStage == SinkWriterCreateStage::CreateSinkWriter) { if (logCreateFailure) { - logSinkWriterCreateFailure(sinkWriterHr, audioFormat); + logSinkWriterCreateFailure( + sinkWriterHr, + fragmented ? "MFCreateSinkWriterFromMediaSink" : "MFCreateSinkWriterFromURL", + audioFormat); } else { - std::cerr << "WARNING: Default MFCreateSinkWriterFromURL failed (hr=0x" + std::cerr << "WARNING: Sink-writer creation failed (hr=0x" << std::hex << sinkWriterHr << std::dec << ")" << std::endl; } } return false; } - if (!succeeded(sinkWriter_->AddStream(outputType.Get(), &videoStreamIndex_), "AddStream")) { - return false; + + if (fragmented) { + // The streams already exist -- the sink was built from the output + // types -- so there is nothing to add, only positions to find. + if (!resolveStreamSinkIndex(mediaSink_.Get(), MFMediaType_Video, videoStreamIndex_)) { + return false; + } + if (audioOutputType && + !resolveStreamSinkIndex(mediaSink_.Get(), MFMediaType_Audio, audioStreamIndex_)) { + return false; + } + } else { + if (!succeeded( + sinkWriter_->AddStream(outputType.Get(), &videoStreamIndex_), + "AddStream")) { + return false; + } + if (audioOutputType && + !succeeded( + sinkWriter_->AddStream(audioOutputType.Get(), &audioStreamIndex_), + "AddStream(audio)")) { + return false; + } } if (audioFormat && !configureAudioStream(*audioFormat)) { return false; } + // Also the check that catches a stream index resolved onto the wrong + // 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")) { return false; @@ -579,17 +788,36 @@ bool MFEncoder::initialize( } videoEncoderSelection_ = selection; + containerFormat_ = fragmented ? kContainerFormatFragmentedMp4 : kContainerFormatMp4; return true; }; + // The last resort, tried only once every fragmented attempt has failed. A + // machine where anything about MFCreateFMPEG4MediaSink does not work -- + // an output type the fragmented sink refuses, a stream layout that does not + // resolve, a platform build without it -- records exactly as it did before + // this change instead of not recording at all. The container is the point + // of the change, and it is still not worth a recording. + auto configureUnfragmentedFallback = [&](bool forceSoftwareEncoder, const char* selection) { + std::cerr + << "WARNING: Fragmented MP4 setup failed; retrying with the plain MP4 container. " + << "The recording will be unreadable if the helper has to be killed." + << std::endl; + return configureSinkWriterAttempt(forceSoftwareEncoder, selection, true, false); + }; + if (options.preferSoftwareEncoder) { - return configureSinkWriterAttempt( - true, - kVideoEncoderSelectionSoftwarePreferred, - true); + if (configureSinkWriterAttempt( + true, + kVideoEncoderSelectionSoftwarePreferred, + false, + true)) { + return true; + } + return configureUnfragmentedFallback(true, kVideoEncoderSelectionSoftwarePreferred); } - if (configureSinkWriterAttempt(false, kVideoEncoderSelectionDefault, false)) { + if (configureSinkWriterAttempt(false, kVideoEncoderSelectionDefault, false, true)) { return true; } @@ -607,7 +835,7 @@ bool MFEncoder::initialize( useDxgiInput_ = false; configureVideoInputType(false); configureOutputColorTags(false); - if (configureSinkWriterAttempt(false, kVideoEncoderSelectionDefault, false)) { + if (configureSinkWriterAttempt(false, kVideoEncoderSelectionDefault, false, true)) { return true; } } @@ -616,37 +844,22 @@ bool MFEncoder::initialize( << "WARNING: Default Media Foundation H.264 encoder setup failed; " << "retrying with the Microsoft software H.264 encoder." << std::endl; - return configureSinkWriterAttempt( - true, - kVideoEncoderSelectionSoftwareFallback, - true); + if (configureSinkWriterAttempt(true, kVideoEncoderSelectionSoftwareFallback, false, true)) { + return true; + } + return configureUnfragmentedFallback(true, kVideoEncoderSelectionSoftwareFallback); } +// The output half -- the AAC type and, on the plain container, the AddStream +// that used to sit between the two -- now happens before the sink writer +// exists. What is left is the input type, which is the same on both containers +// and is set on a stream index the caller has already resolved. bool MFEncoder::configureAudioStream(const AudioInputFormat& audioFormat) { if (!sinkWriter_) { return false; } - if (audioFormat.sampleRate == 0 || audioFormat.channels == 0 || audioFormat.blockAlign == 0) { - std::cerr << "ERROR: Invalid audio input format" << std::endl; - return false; - } const AudioInputFormat encoderFormat = makeAacCompatibleAudioFormat(audioFormat); - const UINT32 aacBytesPerSecond = 24'000; - - Microsoft::WRL::ComPtr outputType; - if (!succeeded(MFCreateMediaType(&outputType), "MFCreateMediaType(audio output)")) { - return false; - } - outputType->SetGUID(MF_MT_MAJOR_TYPE, MFMediaType_Audio); - outputType->SetGUID(MF_MT_SUBTYPE, MFAudioFormat_AAC); - setAudioFormat(outputType.Get(), encoderFormat.channels, encoderFormat.sampleRate, 16); - outputType->SetUINT32(MF_MT_AUDIO_AVG_BYTES_PER_SECOND, aacBytesPerSecond); - outputType->SetUINT32(MF_MT_AAC_PAYLOAD_TYPE, 0); - - if (!succeeded(sinkWriter_->AddStream(outputType.Get(), &audioStreamIndex_), "AddStream(audio)")) { - return false; - } Microsoft::WRL::ComPtr inputType; if (!succeeded(MFCreateMediaType(&inputType), "MFCreateMediaType(audio input)")) { @@ -1407,8 +1620,8 @@ bool MFEncoder::finalize() { bool ok = true; if (sinkWriter_) { ok = succeeded(sinkWriter_->Finalize(), "SinkWriter::Finalize"); - sinkWriter_.Reset(); } + releaseSinkWriter(); stagingTexture_.Reset(); // Before MFShutdown(), not left to the destructor. Two of the objects // releaseDxgiPipeline() drops -- videoSampleAllocator_ and diff --git a/electron/native/wgc-capture/src/mf_encoder.h b/electron/native/wgc-capture/src/mf_encoder.h index 8ef6bea1..f8370874 100644 --- a/electron/native/wgc-capture/src/mf_encoder.h +++ b/electron/native/wgc-capture/src/mf_encoder.h @@ -42,6 +42,17 @@ constexpr const char* kVideoEncoderSelectionDefault = "default"; constexpr const char* kVideoEncoderSelectionSoftwarePreferred = "software-preferred"; constexpr const char* kVideoEncoderSelectionSoftwareFallback = "software-fallback"; +// Which MP4 flavour the recording was actually written in. The fragmented sink +// writes a self-describing moof+mdat pair roughly every second, so a helper the +// shutdown watchdog force-exits leaves a file that plays up to the last +// complete fragment. The plain sink writes its only index in Finalize(), which +// is the very call a TerminateProcess pre-empts -- that is how issues #252 / +// #292 / #327 turned a frozen recording into a total loss. Recording falls back +// to the plain container if anything about the fragmented one is unavailable, +// so this is reported rather than assumed. +constexpr const char* kContainerFormatFragmentedMp4 = "fragmented-mp4"; +constexpr const char* kContainerFormatMp4 = "mp4"; + class MFEncoder { public: MFEncoder() = default; @@ -86,6 +97,11 @@ class MFEncoder { bool writeAudio(const BYTE* data, DWORD byteCount, int64_t timestampHns, int64_t durationHns); bool finalize(); const char* videoEncoderSelection() const; + // Which container initialize() settled on, which is not necessarily the one + // it asked for: the fragmented sink degrades to the plain one rather than + // failing a recording. A bug report that cannot tell the two apart cannot + // say whether a truncated file was supposed to survive its kill. + const char* containerFormat() const; // Which video input path initialize() actually settled on, which is not // necessarily the one that was asked for. Callers must read this rather // than their own MFEncoderOptions to decide which capture entry point to @@ -130,9 +146,20 @@ class MFEncoder { DWORD destinationSize, const BgraFrameView* webcamFrame); bool copyBgraFrameToBuffer(const BgraFrameView& frame, BYTE* destination, DWORD destinationSize); + // Only the audio *input* type and SetInputMediaType. The AAC output type is + // built before the sink writer exists (buildAacOutputType in the .cpp), + // because MFCreateFMPEG4MediaSink takes both output types at construction: + // a fragmented sink has all its streams before anything can be added to it. bool configureAudioStream(const AudioInputFormat& audioFormat); + void releaseSinkWriter(); Microsoft::WRL::ComPtr sinkWriter_; + // Held only on the fragmented path, and held because a sink writer built on + // a media sink does not own either of them: releasing the writer leaves the + // sink live and the output file open. releaseSinkWriter() is what closes + // them, both between failed attempts and at finalize(). + Microsoft::WRL::ComPtr mediaSink_; + Microsoft::WRL::ComPtr byteStream_; Microsoft::WRL::ComPtr device_; Microsoft::WRL::ComPtr context_; Microsoft::WRL::ComPtr captureDevice_; @@ -175,4 +202,5 @@ class MFEncoder { bool finalized_ = false; bool useDxgiInput_ = false; const char* videoEncoderSelection_ = kVideoEncoderSelectionDefault; + const char* containerFormat_ = kContainerFormatMp4; }; diff --git a/scripts/test-windows-wgc-helper.mjs b/scripts/test-windows-wgc-helper.mjs index 849bbd6e..ba7bb325 100644 --- a/scripts/test-windows-wgc-helper.mjs +++ b/scripts/test-windows-wgc-helper.mjs @@ -34,7 +34,7 @@ const WITH_SOFTWARE_FALLBACK = process.argv.includes("--software-fallback"); const INJECT_DEFAULT_SINK_WRITER_FAILURE_ENV = "OPENSCREEN_WGC_TEST_INJECT_DEFAULT_SINK_WRITER_FAILURE_ONCE"; -const INJECTION_MARKER = "TEST-ONLY: Injected default MFCreateSinkWriterFromURL failure"; +const INJECTION_MARKER = "TEST-ONLY: Injected default sink-writer creation failure"; const STALL_READBACK_ENV = "OPENSCREEN_WGC_TEST_STALL_READBACK_MS"; /** * Reproduces issue #252 on ordinary hardware: holds the frame lock across a @@ -286,6 +286,43 @@ function probeStreams(outputPath) { return JSON.parse(ffprobe.stdout).streams ?? []; } +/** + * The property the fragmented container exists for, checked without having to + * kill anything: a fragmented MP4 carries its index up front and its samples in + * self-describing `moof`+`mdat` pairs, so a prefix of the file still decodes. A + * plain MP4 only becomes readable when `Finalize()` writes `moov` at the end, + * which is exactly the call the shutdown watchdog's `TerminateProcess` + * pre-empts in issues #252 / #292 / #327. + * + * Truncating a copy is a proxy for that kill, not a replacement: it proves the + * container survives losing its tail. It does not prove the helper flushed + * anything before dying, which only the real kill test can. + */ +function assertPrefixIsReadable(outputPath) { + const truncatedPath = `${outputPath}.truncated.mp4`; + const source = fs.readFileSync(outputPath); + fs.writeFileSync(truncatedPath, source.subarray(0, Math.floor(source.length * 0.6))); + try { + // A plain MP4 does not merely lose its tail here, it fails to open at + // all ("moov atom not found"), so the throw and the empty result are the + // same finding and get the same message. + let truncatedStreams = []; + try { + truncatedStreams = probeStreams(truncatedPath); + } catch { + truncatedStreams = []; + } + if (!truncatedStreams.some((stream) => stream.codec_name === "h264")) { + throw new Error( + `A 60% prefix of ${outputPath} has no readable H.264 stream, so the recording is ` + + "still all-or-nothing: the container is not fragmented.", + ); + } + } finally { + fs.rmSync(truncatedPath, { force: true }); + } +} + function measureFirstFrameLuma(outputPath) { const ffmpeg = spawnSync( "ffmpeg", @@ -517,13 +554,26 @@ if ( `WGC helper encoder selection was ${JSON.stringify(encoderSelection)}, expected ${expectedEncoderSelection} with preferSoftwareEncoder=${WITH_SOFTWARE_ENCODER}: ${result.stdout}`, ); } +// Every fallback path has to stay fragmented, not just the nominal one. The +// helper degrades to the plain container rather than failing a recording, so +// without this the fix could quietly stop applying and every other assertion +// here would still pass. +if (encoderSelection.container !== "fragmented-mp4") { + throw new Error( + `WGC helper wrote a ${encoderSelection.container} container, expected fragmented-mp4: ${result.stdout}`, + ); +} +assertPrefixIsReadable(outputPath); +if (webcamOutputPath && fs.existsSync(webcamOutputPath)) { + assertPrefixIsReadable(webcamOutputPath); +} const combinedHelperOutput = `${result.stdout}\n${result.stderr}`; const helperDiagnosticLines = combinedHelperOutput.split(/\r?\n/).filter(Boolean); const injectionLines = helperDiagnosticLines.filter((line) => line.includes(INJECTION_MARKER)); const fallbackDiagnosticPatterns = [ INJECTION_MARKER, - "WARNING: Default MFCreateSinkWriterFromURL failed (hr=0x80070003)", + "WARNING: Sink-writer creation failed (hr=0x80070003)", "retrying with the Microsoft software H.264 encoder.", "INFO: Registered the Microsoft software H.264 MFT locally for this helper process.", "INFO: Created the real software H.264 sink writer successfully.", diff --git a/technical-documentation/architecture/recording.md b/technical-documentation/architecture/recording.md index 2a0e1af0..7d766fd3 100644 --- a/technical-documentation/architecture/recording.md +++ b/technical-documentation/architecture/recording.md @@ -61,6 +61,8 @@ Electron resolves selected sources, devices, and paths before launching the help ## Output files and sidecars +Windows and macOS both write their screen video as a fragmented MP4 — `MFCreateFMPEG4MediaSink` with a one-second `MF_MPEG4SINK_MIN_FRAGMENT_DURATION`, and `AVAssetWriter.movieFragmentInterval` respectively. A plain MP4 has no index until the writer's final call emits `moov`, so a helper that is force-exited before that leaves every captured frame on disk and no way to read them; that is why a frozen recording used to cost the whole file rather than its tail (issues #252 / #292 / #327). Fragmenting does not stop the freeze, it stops the freeze from destroying the recording. Windows falls back to the plain container if the fragmented sink is unavailable and reports which one it used in the `encoder-selection` event. Linux still writes a plain MP4: `frag_keyframe+empty_moov` would make the output permanently non-seekable and the native Linux path has no re-index step, so the editor's scrub cost has to be measured first. + A session writes a screen video and a `.session.json` manifest. Windows normally muxes the webcam into that MP4; when `webcamPath` is supplied, it writes a separate webcam video. macOS currently writes the webcam as a separate Electron sidecar (`webcamVideoPath`) because native webcam composition is not part of the helper. Linux follows the Electron recorder's separate media-path convention. Audio that the selected backend captures is encoded into its screen output. Cursor samples are persisted as cursor telemetry rather than baked into editable-overlay recordings. The loader resolves the sidecar at `.cursor.json` or through the recording links; see [cursor.md](cursor.md) for the telemetry format and rendering path.