Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Proposal] ImageBitmap.getImageData #4785

Open
dsanders11 opened this issue Jul 17, 2019 · 2 comments

Comments

@dsanders11
Copy link

commented Jul 17, 2019

This is meant to address #3802, and supersedes #4748. More background info can be found on those two issues.

There is currently no high-performance code path for getting ImageData from a MediaStreamTrack or an HTMLVideoElement. The current idiom for doing so involves drawing to an HTMLCanvasElement (or OffscreenCanvas where available) and creating a new ImageData each time. This is particularly low-performance on lower spec devices.

For the purposes of this proposal, high-performance means:

  • Manual control over memory allocation/deallocation instead of leaving it to GC
  • Low enough latency to maintain reasonable (20+ FPS) frame rates

I have a repo with performance results testing the various methods of getting ImageData for webcam frames in the latest Chromium and Firefox, using a Raspberry Pi Model 3B+ as a low-spec, easily reproducible test bed. The low memory (1 GB) is of particular concern for this kind of test, and low disk performance means swapping at that kind of rate will lock up the device. Firefox fails all tests as it immediately runs the system out of memory when creating ImageData for 1080p video, presumably because its garbage collector is overwhelmed. This isn't entirely surprising, as at 30 FPS that's 240 MB/sec of garbage being generated. This is why a memory-efficient way of generating ImageData for webcam frames is crucial.

As opposed to #3802 and #4748, this proposal seeks to remove the need to use a canvas as an intermediary for getting ImageData, which provides a performance boost, and in providing a new API it can have changes which would otherwise be breaking for CanvasRenderingContext2D.getImageData.

The main usefulness in this proposed change is the ability to steal the memory from an existing ArrayBuffer when getting ImageData from an ImageBitmap. Since an ArrayBuffer can be detached/neutered, this allows a clean way to reuse memory without dangling references.

Here's the proposed change in Web IDL. No strong opinion on any naming. The (perhaps poorly named) neuter option would call ImageBitmap.close before resolving the ImageBitmap.getImageData promise, with the idea being to avoid triggering GC as much as possible. Actual testing shows mixed (and possibly negligible) results, so it may be dropped as an option.

dictionary GetImageDataOptions {
  ArrayBuffer? buffer;
  boolean neuter = false;
}

partial interface ImageBitmap {
  [NewObject] Promise<ImageData> getImageData(int sx, int sy, int sw, int sh, optional GetImageDataOptions = {});
}

Benefits of this proposed change:

  • Stable memory footprint - reusing the memory from an existing ArrayBuffer allows developers to know the memory footprint of their code rather than leaving it to the fluctuations of GC, which as seen by the Firefox test results, may be inadequate.
  • Helps avoid memory fragmentation. Similar to the stable memory footprint, reusing memory will help avoid memory fragmentation which slows down performance. Fragmentation is prone to happen with these sort of large allocations of contiguous memory, as once it's freed, a small object may be allocated in the multi-megabyte chunk, preventing it from being reused, and instead requiring a new multi-megabyte chunk to be allocated. Allocation methods which use arenas can suffer from a 'high water mark', holding significant system memory with most of it being unused by the application, but unavailable to the system.
  • Since ImageBitmap is [Transferable], it can be efficiently offloaded to a web worker where the ImageData can be extracted and processing done off the main thread. Firefox currently has no way to perform this extraction of ImageData on a web worker due to not supporting 2d as a context for OffscreenCanvas.
  • Pairs efficiently with methods of getting an ImageBitmap for webcam data or video, such as createImageBitmap(videoElement) and ImageCapture.grabFrame.
  • Avoiding canvas removes potential for captured webcam frames from being written from CPU-to-GPU-to-CPU when accelerated canvases are in-use.
  • Returning a promise allows it to be used on the main thread without blocking, and opens the possibility to use multiple threads for particularly large resolutions.

Open questions:

  • How to deal with concurrency issues due to asynchronous API, when reusing an ArrayBuffer? I believe as long as the original ArrayBuffer is detached before promise is returned, then it should be no different than transferring an ArrayBuffer currently.
  • How to deal with concurrency issues when an async ImageBitmap.getImageData is in-progress and an ImageBitmap.close or transfer occurs? I would assume both issues also exist with using createImageBitmap(otherImageBitmap), although I don't see anything mentioned in the spec for concurrency issues here.

The repo I linked earlier in this post contains patch implementations for both Chromium and Firefox. They're incomplete and were done just for testing the 'go right' path: they don't have error handling, and they're not pretty (apologies to the devs for bastardizing their code). The Chromium patch shows 2-3x the performance of the current best method, achieving 30 FPS at 720p@30 and over 20 FPS at 1080p@30. In the latter case the bottleneck is elsewhere in the code. For Firefox there can be no performance comparison since it can't complete the testing without the patch to begin with.

Overall this seems like a low-effort to implement, with a large positive impact.

@annevk

This comment has been minimized.

Copy link
Member

commented Jul 19, 2019

I don't really understand the proposal (in particular it seems you're suggesting to use a detached ArrayBuffer somehow, but that's not possible). I recommend reading https://whatwg.org/faq#adding-new-features and perhaps starting a thread at https://discourse.wicg.io/ to refine this a bit.

@dsanders11

This comment has been minimized.

Copy link
Author

commented Jul 19, 2019

I don't really understand the proposal (in particular it seems you're suggesting to use a detached ArrayBuffer somehow, but that's not possible).

@annevk, I'm not suggesting using a detached ArrayBuffer, I'm suggesting detaching one.

let imageBitmap;  // Assume this is an ImageBitmap we want ImageData from

const oldImageData = new ImageData(1920, 1080);
const oldArrayBuffer = oldImageData.data.buffer;

const newImageData = await imageBitmap.getImageData(0, 0, 1920, 1080, { buffer: oldArrayBuffer });
const newArrayBuffer = newImageData.data.buffer;

// oldArrayBuffer is now detached - the memory it had now belongs to newArrayBuffer

That's an overly wordy example with the variable names, how it would actually be used with ImageCapture.grabFrame:

let imageBitmap;
let imageData = new ImageData(1920, 1080);

while (true) {
  imageBitmap = await imageCapturer.grabFrame();
  imageData = await imageBitmap.getImageData(0, 0, 1920, 1080, { buffer: imageData.data.buffer });

  // Process ImageData
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants
You can’t perform that action at this time.