diff --git a/.gitignore b/.gitignore index 61dc9d2fcf..33cf7a9d2f 100644 --- a/.gitignore +++ b/.gitignore @@ -49,6 +49,7 @@ playwright-report/ # Vitest browser mode screenshots __screenshots__/ +.vitest-attachments/ # shell files /shell.sh diff --git a/electron/native/wgc-capture/src/main.cpp b/electron/native/wgc-capture/src/main.cpp index 30a3069d67..1c0f081400 100644 --- a/electron/native/wgc-capture/src/main.cpp +++ b/electron/native/wgc-capture/src/main.cpp @@ -606,9 +606,14 @@ int main(int argc, char* argv[]) { int64_t lastEncodedVideoTimestampHns = -1; while (!control.stopRequested && !encodeFailed) { + Microsoft::WRL::ComPtr videoSample; + Microsoft::WRL::ComPtr webcamSample; + bool hasVideoSample = false; + bool hasWebcamSample = false; + { std::unique_lock lock(mutex); - control.cv.wait(lock, [&] { + control.cv.wait_for(lock, std::chrono::milliseconds(100), [&] { return control.stopRequested.load() || encodeFailed.load() || (!control.paused.load() && latestFrameTexture); @@ -653,29 +658,59 @@ int main(int argc, char* argv[]) { latestWebcamSequence != lastWrittenWebcamSequence) { const int64_t webcamTimestampHns = static_cast( (webcamOutputFrameIndex * 10'000'000ULL) / std::max(1, webcamCapture.fps())); - if (!webcamEncoder.writeBgraFrame(webcamFrame, webcamTimestampHns)) { + hasWebcamSample = webcamEncoder.captureBgraSample(webcamFrame, webcamTimestampHns, webcamSample); + if (!hasWebcamSample) { encodeFailed = true; control.stopRequested = true; control.cv.notify_all(); - return; + break; } lastWrittenWebcamSequence = latestWebcamSequence; webcamOutputFrameIndex += 1; } - if (latestFrameTexture && !encoder.writeFrame( + if (latestFrameTexture) { + // captureVideoSample performs the GPU readback + // (CopyResource/Map) from latestFrameTexture, which must + // stay serialized (via `mutex`) against the WGC + // frame-arrival callback above, which writes new data + // into the same texture on another thread. + hasVideoSample = encoder.captureVideoSample( latestFrameTexture.Get(), frameTimestampHns, - !writeSeparateWebcam && webcamFrame.data ? &webcamFrame : nullptr)) { - encodeFailed = true; - control.stopRequested = true; - control.cv.notify_all(); - return; - } - if (latestFrameTexture) { + !writeSeparateWebcam && webcamFrame.data ? &webcamFrame : nullptr, + videoSample); + if (!hasVideoSample) { + encodeFailed = true; + control.stopRequested = true; + control.cv.notify_all(); + break; + } lastEncodedVideoTimestampHns = frameTimestampHns; } } + // Submit the captured samples to their sink writers OUTSIDE + // `mutex`. IMFSinkWriter::WriteSample runs the H.264 encode + // synchronously and can be slow (especially the software encoder + // fallback used when preferSoftwareEncoder is set). Holding + // `mutex` across it would block the main thread's stop-wait + // (which locks the same mutex to check control.stopRequested) + // for as long as this thread keeps re-acquiring the lock faster + // than the main thread can, hanging the helper indefinitely + // after a stop request (issue #115). + if (hasWebcamSample && !webcamEncoder.submitVideoSample(webcamSample.Get())) { + encodeFailed = true; + control.stopRequested = true; + control.cv.notify_all(); + break; + } + if (hasVideoSample && !encoder.submitVideoSample(videoSample.Get())) { + encodeFailed = true; + control.stopRequested = true; + control.cv.notify_all(); + break; + } + frameIndex += 1; std::this_thread::sleep_for(frameDuration); } diff --git a/electron/native/wgc-capture/src/mf_encoder.cpp b/electron/native/wgc-capture/src/mf_encoder.cpp index 42b7089105..60f82e9f55 100644 --- a/electron/native/wgc-capture/src/mf_encoder.cpp +++ b/electron/native/wgc-capture/src/mf_encoder.cpp @@ -603,23 +603,38 @@ bool MFEncoder::copyBgraFrameToBuffer(const BgraFrameView& frame, BYTE* destinat return true; } -bool MFEncoder::writeFrame(ID3D11Texture2D* texture, int64_t timestampHns, const BgraFrameView* webcamFrame) { - std::scoped_lock writerLock(writerMutex_); - if (!sinkWriter_ || finalized_) { - return false; - } +bool MFEncoder::captureVideoSample( + ID3D11Texture2D* texture, + int64_t timestampHns, + const BgraFrameView* webcamFrame, + Microsoft::WRL::ComPtr& outSample) { + outSample.Reset(); - if (firstTimestampHns_ < 0) { - firstTimestampHns_ = timestampHns; - } + const int64_t sampleDuration = 10'000'000LL / fps_; + int64_t sampleTime = 0; + { + std::scoped_lock writerLock(writerMutex_); + if (!sinkWriter_ || finalized_) { + return false; + } - int64_t sampleTime = timestampHns - firstTimestampHns_; - if (sampleTime <= lastTimestampHns_) { - sampleTime = lastTimestampHns_ + (10'000'000LL / fps_); + if (firstTimestampHns_ < 0) { + firstTimestampHns_ = timestampHns; + } + + sampleTime = timestampHns - firstTimestampHns_; + if (sampleTime <= lastTimestampHns_) { + sampleTime = lastTimestampHns_ + sampleDuration; + } + lastTimestampHns_ = sampleTime; } - const int64_t sampleDuration = 10'000'000LL / fps_; - lastTimestampHns_ = sampleTime; + // The GPU readback below (copyFrameToBuffer -> CopyResource/Map on + // `texture`) is not internally synchronized here. Callers must hold their + // own lock around this call that also serializes against whatever thread + // writes new frame data into `texture` (see main.cpp's writeVideoFrames, + // 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); if (!succeeded(MFCreateMemoryBuffer(frameBytes, &buffer), "MFCreateMemoryBuffer")) { @@ -648,25 +663,34 @@ bool MFEncoder::writeFrame(ID3D11Texture2D* texture, int64_t timestampHns, const sample->SetSampleTime(sampleTime); sample->SetSampleDuration(sampleDuration); - return succeeded(sinkWriter_->WriteSample(videoStreamIndex_, sample.Get()), "WriteSample"); + outSample = sample; + return true; } -bool MFEncoder::writeBgraFrame(const BgraFrameView& frame, int64_t timestampHns) { - std::scoped_lock writerLock(writerMutex_); - if (!sinkWriter_ || finalized_) { - return false; - } +bool MFEncoder::captureBgraSample( + const BgraFrameView& frame, + int64_t timestampHns, + Microsoft::WRL::ComPtr& outSample) { + outSample.Reset(); - if (firstTimestampHns_ < 0) { - firstTimestampHns_ = timestampHns; - } + const int64_t sampleDuration = 10'000'000LL / fps_; + int64_t sampleTime = 0; + { + std::scoped_lock writerLock(writerMutex_); + if (!sinkWriter_ || finalized_) { + return false; + } - int64_t sampleTime = timestampHns - firstTimestampHns_; - if (sampleTime <= lastTimestampHns_) { - sampleTime = lastTimestampHns_ + (10'000'000LL / fps_); + if (firstTimestampHns_ < 0) { + firstTimestampHns_ = timestampHns; + } + + sampleTime = timestampHns - firstTimestampHns_; + if (sampleTime <= lastTimestampHns_) { + sampleTime = lastTimestampHns_ + sampleDuration; + } + lastTimestampHns_ = sampleTime; } - const int64_t sampleDuration = 10'000'000LL / fps_; - lastTimestampHns_ = sampleTime; Microsoft::WRL::ComPtr buffer; const DWORD frameBytes = static_cast(width_ * height_ * 4); @@ -696,7 +720,25 @@ bool MFEncoder::writeBgraFrame(const BgraFrameView& frame, int64_t timestampHns) sample->SetSampleTime(sampleTime); sample->SetSampleDuration(sampleDuration); - return succeeded(sinkWriter_->WriteSample(videoStreamIndex_, sample.Get()), "WriteSample(webcam)"); + outSample = sample; + return true; +} + +bool MFEncoder::submitVideoSample(IMFSample* sample) { + if (!sample) { + return false; + } + + // This is the potentially slow, blocking step (especially with the + // software H.264 encoder fallback): IMFSinkWriter::WriteSample runs the + // encode synchronously on the calling thread. Callers must NOT hold any + // lock shared with a thread that needs to make timely progress (e.g. a + // stop-request check) across this call. + std::scoped_lock writerLock(writerMutex_); + if (!sinkWriter_ || finalized_) { + return false; + } + return succeeded(sinkWriter_->WriteSample(videoStreamIndex_, sample), "WriteSample"); } bool MFEncoder::writeAudio(const BYTE* data, DWORD byteCount, int64_t timestampHns, int64_t durationHns) { diff --git a/electron/native/wgc-capture/src/mf_encoder.h b/electron/native/wgc-capture/src/mf_encoder.h index 8f57874819..e5fbd74c88 100644 --- a/electron/native/wgc-capture/src/mf_encoder.h +++ b/electron/native/wgc-capture/src/mf_encoder.h @@ -53,8 +53,25 @@ class MFEncoder { ID3D11DeviceContext* context, const AudioInputFormat* audioFormat = nullptr, MFEncoderOptions options = {}); - bool writeFrame(ID3D11Texture2D* texture, int64_t timestampHns, const BgraFrameView* webcamFrame = nullptr); - bool writeBgraFrame(const BgraFrameView& frame, int64_t timestampHns); + // Capturing a video/webcam sample (GPU readback + IMFSample creation) is + // split from submitting it to the sink writer (IMFSinkWriter::WriteSample) + // so callers that hold an external lock across the GPU-touching capture + // step (to serialize against a producer thread writing into the same + // texture) are not forced to also hold that lock across the potentially + // slow, blocking WriteSample call. See main.cpp's writeVideoFrames for why + // this split exists: holding the shared frame-state mutex across + // WriteSample let the software H.264 encoder path starve the main + // thread's stop-request check indefinitely (issue #115). + bool captureVideoSample( + ID3D11Texture2D* texture, + int64_t timestampHns, + const BgraFrameView* webcamFrame, + Microsoft::WRL::ComPtr& outSample); + bool captureBgraSample( + const BgraFrameView& frame, + int64_t timestampHns, + Microsoft::WRL::ComPtr& outSample); + bool submitVideoSample(IMFSample* sample); bool writeAudio(const BYTE* data, DWORD byteCount, int64_t timestampHns, int64_t durationHns); bool finalize(); const char* videoEncoderSelection() const; diff --git a/electron/native/wgc-capture/src/wgc_session.cpp b/electron/native/wgc-capture/src/wgc_session.cpp index e20096c4ea..89f0b55fe0 100644 --- a/electron/native/wgc-capture/src/wgc_session.cpp +++ b/electron/native/wgc-capture/src/wgc_session.cpp @@ -32,6 +32,28 @@ int64_t timeSpanToHns(wf::TimeSpan const& value) { return value.count(); } +// H.264 encoding (and the RGB32->NV12 conversion feeding it) requires even +// frame dimensions. Monitor resolutions are always even in practice, so +// CreateForMonitor items never hit this. Windows, however, frequently have +// odd client-area dimensions (arbitrary drag-resize, DPI rounding), and +// GraphicsCaptureItem::Size() reports the window's *actual* size verbatim. +// If we requested a Direct3D11CaptureFramePool sized to that odd value while +// the rest of the pipeline (main.cpp's bitrate calc, MFEncoder) rounds down +// to even, the frame pool's real DXGI textures end up one pixel wider/taller +// than the staging texture the encoder allocates. ID3D11DeviceContext:: +// CopyResource silently no-ops on a size mismatch (it only emits a debug- +// layer warning), so the staging texture never receives pixel data and the +// output is solid black for the entire recording -- or, if the mismatch +// trips up the video MFT's input negotiation, SetInputMediaType fails +// outright. Rounding up to the nearest even size here, and using that +// rounded size (not the raw item size) for both the frame pool and +// `captureWidth()`/`captureHeight()`, keeps every consumer of this session +// looking at the exact same dimensions as the real captured texture. +int roundUpToEven(int value) { + const int clamped = std::max(2, value); + return (clamped % 2 == 0) ? clamped : clamped + 1; +} + } // namespace WgcSession::~WgcSession() { @@ -135,8 +157,8 @@ bool WgcSession::createCaptureItem(HWND window) { item_ = item; const auto size = item_.Size(); - width_ = static_cast(size.Width); - height_ = static_cast(size.Height); + width_ = roundUpToEven(static_cast(size.Width)); + height_ = roundUpToEven(static_cast(size.Height)); return width_ > 0 && height_ > 0; } @@ -199,7 +221,7 @@ bool WgcSession::initialize(HMONITOR monitor, int fps, bool captureCursor) { winrtDevice_, wgdx::DirectXPixelFormat::B8G8R8A8UIntNormalized, 2, - item_.Size()); + winrt::Windows::Graphics::SizeInt32{width_, height_}); session_ = framePool_.CreateCaptureSession(item_); if (!applySessionOptions(captureCursor)) { @@ -223,7 +245,7 @@ bool WgcSession::initialize(HWND window, int fps, bool captureCursor) { winrtDevice_, wgdx::DirectXPixelFormat::B8G8R8A8UIntNormalized, 2, - item_.Size()); + winrt::Windows::Graphics::SizeInt32{width_, height_}); session_ = framePool_.CreateCaptureSession(item_); if (!applySessionOptions(captureCursor)) { diff --git a/src/components/video-editor/VideoEditor.tsx b/src/components/video-editor/VideoEditor.tsx index b49197da4c..f0bfbd3fed 100644 --- a/src/components/video-editor/VideoEditor.tsx +++ b/src/components/video-editor/VideoEditor.tsx @@ -1004,6 +1004,16 @@ export default function VideoEditor() { video.currentTime = time; } + // Reads the live playhead position directly off the