diff --git a/.gitignore b/.gitignore index 61dc9d2fc..33cf7a9d2 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 30a3069d6..1c0f08140 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 42b708910..60f82e9f5 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 8f5787481..e5fbd74c8 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 e20096c4e..89f0b55fe 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/package.json b/package.json index 8d40c9d7e..5ffe596d3 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "openscreen", "private": true, - "version": "1.6.0", + "version": "1.7.0", "type": "module", "packageManager": "npm@10.9.4", "engines": { diff --git a/src/components/launch/LaunchWindow.test.tsx b/src/components/launch/LaunchWindow.test.tsx index 154feef73..0b745c80c 100644 --- a/src/components/launch/LaunchWindow.test.tsx +++ b/src/components/launch/LaunchWindow.test.tsx @@ -460,6 +460,75 @@ describe("LaunchWindow system language prompt", () => { }); }); +describe("LaunchWindow HUD drag", () => { + beforeEach(() => { + platformState.value = "darwin"; + resetLaunchMocks(); + resizeCallbacks.length = 0; + vi.stubGlobal("ResizeObserver", CapturingResizeObserver); + // jsdom doesn't implement the Pointer Capture API; stub it so the drag handlers + // (which call set/has/releasePointerCapture) don't throw. + HTMLElement.prototype.setPointerCapture = vi.fn(); + HTMLElement.prototype.hasPointerCapture = vi.fn(() => true); + HTMLElement.prototype.releasePointerCapture = vi.fn(); + }); + + afterEach(() => { + cleanup(); + vi.unstubAllGlobals(); + }); + + it("suppresses ResizeObserver-driven measurement while dragging, and measures once on release", async () => { + renderLaunchWindow(); + + const dragHandle = await screen.findByTestId("hud-drag-handle"); + + // Give the bar a non-zero, changing size so a resize observation would actually + // trigger a `setHudOverlaySize` call if it weren't suppressed during the drag. + const bar = dragHandle.closest("[data-tray-layout]") as HTMLElement | null; + if (bar) { + vi.spyOn(bar, "getBoundingClientRect").mockReturnValue({ + top: 700, + left: 200, + right: 600, + bottom: 756, + width: 400, + height: 56, + x: 200, + y: 700, + toJSON: () => ({}), + }); + Object.defineProperty(bar, "scrollHeight", { value: 56, configurable: true }); + Object.defineProperty(bar, "scrollWidth", { value: 400, configurable: true }); + } + + const sizeMock = window.electronAPI.setHudOverlaySize as unknown as { + mockClear: () => void; + }; + sizeMock.mockClear(); + + fireEvent.pointerDown(dragHandle, { screenX: 100, screenY: 100 }); + + // Simulate a ResizeObserver firing mid-drag (e.g. transient reflow) -- this must + // NOT reposition/resize the HUD while the user's pointer is still down. + await act(async () => { + for (const callback of resizeCallbacks) { + callback([], {} as ResizeObserver); + } + }); + expect(window.electronAPI.setHudOverlaySize).not.toHaveBeenCalled(); + + fireEvent.pointerMove(dragHandle, { screenX: 140, screenY: 130 }); + fireEvent.pointerUp(dragHandle, { screenX: 140, screenY: 130 }); + + // Content is re-measured once the drag ends, so a real size change made mid-drag + // still gets picked up promptly. + await waitFor(() => { + expect(window.electronAPI.setHudOverlaySize).toHaveBeenCalled(); + }); + }); +}); + describe("LaunchWindow software encoder fallback notice", () => { beforeEach(() => { platformState.value = "darwin"; diff --git a/src/components/launch/LaunchWindow.tsx b/src/components/launch/LaunchWindow.tsx index 1136d8537..3350bb6a8 100644 --- a/src/components/launch/LaunchWindow.tsx +++ b/src/components/launch/LaunchWindow.tsx @@ -321,9 +321,16 @@ export function LaunchWindow() { // and scrolls. Measure from the window's bottom-centre (the anchor the main process // preserves) so fixed bottom/centre offsets keep this stable and it doesn't oscillate. const lastHudSizeRef = useRef({ width: 0, height: 0 }); + const isDraggingHudRef = useRef(false); const measureHudSize = useCallback(() => { const barEl = hudBarRef.current; if (!barEl || !window.electronAPI?.setHudOverlaySize) return; + // While the user is dragging the HUD, ignore content-size measurements. A + // ResizeObserver-driven resize (hud-overlay-set-size) re-centres the window from + // its own bottom-centre anchor, which fights the position "hud-overlay-move-by" is + // actively applying frame-by-frame -- the two IPC channels racing is what produces + // the reported drift. Content size is re-measured once the drag ends instead. + if (isDraggingHudRef.current) return; // Breathing room so the drop shadow isn't clipped. TOP_MARGIN must also exceed the // slack in the bar's `max-h: calc(100vh - 2.5rem)` cap (40px reserved - 20px bottom @@ -634,6 +641,7 @@ export function LaunchWindow() { setHudMouseEventsEnabled(true); event.currentTarget.setPointerCapture(event.pointerId); dragLastPositionRef.current = { x: event.screenX, y: event.screenY }; + isDraggingHudRef.current = true; }; const handleHudDragPointerMove = (event: React.PointerEvent) => { const lastPosition = dragLastPositionRef.current; @@ -650,6 +658,8 @@ export function LaunchWindow() { event.currentTarget.releasePointerCapture(event.pointerId); } setHudMouseEventsEnabled(false); + isDraggingHudRef.current = false; + measureHudSize(); }; return ( @@ -916,6 +926,7 @@ export function LaunchWindow() { > {/* Drag handle */}
0 ? inferredDurationMs / 1000 : 0); setError(null); @@ -415,6 +419,13 @@ export default function VideoEditor() { setWebcamVideoPath(webcamSourcePath ? toFileUrl(webcamSourcePath) : null); setRecordingCursorCaptureMode(projectCursorCaptureMode); setCurrentProjectPath(path ?? null); + // Reset the memoized last-resolved-duration guard so resolution isn't skipped + // just because the real duration happens to match a value already seen from + // before this load (e.g. reloading a project referencing the same video file, + // whose src may not change and so never re-fires `loadedmetadata`). Must run + // after the setDuration placeholder above, or its correction gets clobbered by + // the same-tick placeholder assignment winning the state-batch race. + videoPlaybackRef.current?.resetDurationResolution(); // A loaded project keeps its zooms exactly as saved, so never auto-suggest // over it (even if it has zero zooms because the user deleted them all). @@ -1004,6 +1015,16 @@ export default function VideoEditor() { video.currentTime = time; } + // Reads the live playhead position directly off the