feat: Isolate MJPEG helpers in the driver - #1028
Conversation
There was a problem hiding this comment.
Pull request overview
This PR moves MJPEG stream handling into this driver by introducing a local MJpegStream implementation (plus unit tests), and wiring the driver to use it instead of the upstream appium/support MJPEG helper.
Changes:
- Added
lib/utils/mjpeg.tsimplementing MJPEG frame parsing and aMJpegStreamclass. - Updated the driver to instantiate the new local
MJpegStream. - Added unit tests for
MJpegStreambehavior and addedsharpas an optional dependency.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/unit/utils/mjpeg-specs.ts | Adds unit coverage for MJPEG frame capture, conversion, and failure modes |
| package.json | Adds sharp as an optional dependency for MJPEG PNG conversion support |
| lib/utils/mjpeg.ts | Introduces the new MJPEG parser and stream wrapper used by the driver |
| lib/utils/index.ts | Exposes MJpegStream from the utils barrel |
| lib/driver.ts | Switches from appium/support MJPEG helper to the driver-local MJpegStream |
Comments suppressed due to low confidence (1)
lib/utils/mjpeg.ts:236
- If startPromise rejects (e.g., no frame arrives before the timeout), start() returns with responseStream still open and the piping still active, relying on callers to remember to stop(). This can leak an open HTTP connection and keep the process busy even though start() failed.
this.registerStartFailure = reject;
timeoutId = setTimeout(
() =>
reject(new Error(`Waited ${serverTimeout}ms but the MJPEG server never sent any images`)),
serverTimeout,
);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const onClose = () => { | ||
| log.debug(`The connection to MJPEG server at ${url} has been closed`); | ||
| this.lastChunk = null; | ||
| }; |
There was a problem hiding this comment.
Fixed in 62bda99. onClose() now rejects the pending registerStartFailure (a no-op if start() already resolved), so a connection that closes before any frame arrives fails immediately instead of waiting out the full timeout. Also addressed the related "suppressed due to low confidence" comment about leaking the connection: start() now calls this.stop() in a catch before rethrowing, so pipes/the response stream are torn down on any failure path (timeout or early close). Added tests covering both the fast-rejection timing and that responseStream is cleared after a failed start().
Fixes two issues flagged on PR appium#1028: - MjpegFrameParser.emitFrame() pushed the full pre-allocated buffer even when fewer bytes were actually copied, leaking trailing zero bytes, and never cleared its frame state, so a later stray chunk could be appended onto an already-emitted, downstream buffer. - MJpegStream.start() waited for the full timeout instead of failing fast when the underlying connection closed before any frame arrived, and left the connection/pipes open on failure. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ser.appendChunk() appendChunk() continues a frame that is already open, so the whole chunk belongs to it and must be copied from offset 0. Any SOI marker found in that chunk necessarily belongs to the *next* frame (it can only appear after the current frame's own EOI), so using it as the copy start skipped the current frame's tail bytes and could truncate it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
## [8.2.0](v8.1.2...v8.2.0) (2026-07-27) ### Features * Isolate MJPEG helpers in the driver ([#1028](#1028)) ([f32c5b7](f32c5b7))
|
🎉 This PR is included in version 8.2.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Similar to appium/appium-xcuitest-driver#2915