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

onframe Event #210

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

onframe Event #210

dsanders11 opened this issue Jul 11, 2019 · 2 comments

Comments

@dsanders11
Copy link
Contributor

The current Chromium implementation of grabFrame waits until a new frame is available before resolving, which means if frameRate is 1, it may take up to 1 second for the promise to resolve.

Although the spec doesn't have anything to say on the timing of grabFrame, I think this is unexpected behavior, and should probably be changed in Chromium, and clarified in the spec.

However, there is a nice benefit to that behavior that I noticed: it throttles grabFrame for you automatically, so you can nicely use a loop with await on each grabFrame and it will run at the frame rate of the video, without needing to manage this yourself.

So, to retain this useful behavior, how about adding an onframe event to ImageCapture in the spec? It would

This would also be flexible enough to easily create a Promise version which emulates the current Chromium grabFrame behavior:

function waitForFrame (capturer) {
  return new Promise(resolve => {
    capturer.addEventListener('frame', event => {
      resolve(event.frame);
    }, { once: true });
  }
}
@dsanders11
Copy link
Contributor Author

dsanders11 commented Jul 16, 2019

I made a rough implementation of this in Chromium for my own testing purposes, and quickly realized he importance of ensuring the events don't tie up too much memory. Here's a proposed change to the spec to accomplish that:

partial interface ImageCapture {
  attribute EventHandler onframe;
}

[Exposed=Window]
interface ImageCaptureFrameEvent : Event {
   [NewObject] Promise<ImageBitmap> createImageBitmap();
};

I left ImageCaptureFrameEvent barebones for the moment, but there are several potential attributes which might be useful to include, which would make the event also a good passive way to collect info about the MediaStreamTrack.

The promise for createImageBitmap will reject if the browser has already discarded the memory associated with that frame. This makes it efficient to listen to frame events passively without causing lots of garbage collection or high memory usage.

There's a lot of benefit to having a frame event. It ensures that you can be notified of every frame delivered by the MediaStreamTrack, makes it easy to tell when you're getting behind in processing, and provide flexibility in how to handle those frames. The current implementation of grabFrame in Chromium will not resolve until the next frame available, which adds undue latency at low framerates if you just want a current shot of the MediaStreamTrack, and requires unnecessary CPU usage if you want to process every other frame since it requires creating an ImageBitmap for the frames you plan on skipping anyway. Trying to use wall time to accomplish this sort of behavior is fragile and more complicated than it should be.

Below is an example of how to run grabFrame only up to the MediaStreamTrack frame rate, using the proposed frame event. Should highlight the flexibility of this approach,

class ThrottledImageCapture {
  constructor (capturer) {
    this.bufferCount_ = 2;
    this.buffers_ = [];

    this.dropCount_ = 0;
    this.discardCount_ = 0;

    this.pendingGrabFramePromise_ = null;

    capturer.addEventListener('frame', async (event) => {
      if (this.buffers_.length === this.bufferCount_) {
        // We ran out of buffers, so we had to
        // drop a frame due to being too slow
        const frame = this.buffers_.shift();
        frame.close();  // Dispose of memory
        this.dropCount_++;
      }

      try {
        this.buffers_.push(await event.createImageBitmap());
      } catch {
        // JS execution was too slow and the browser
        // discarded the data for the frame before we
        // could get it as an ImageBitmap
        this.discardCount_++;
        return;
      }

      if (this.pendingGrabFramePromise_) {
        // Pending promise, consume the latest frame
        this.pendingGrabFramePromise_.resolve(this.buffers_.shift());
        this.pendingGrabFramePromise_ = null;
      }
    });
  }

  grabFrame () {
    if (this.buffers_.length) {
      // Frame available, so consume it
      return Promise.resolve(this.buffers_.shift());
    }

    // No frame available, wait for next one. Reuse
    // promise if already created
    if (this.pendingGrabFramePromise_) {
      return this.pendingGrabFramePromise_.promise;
    }

    const promise = new Promise((resolve, reject) => {
      this.pendingGrabFramePromise_ = { resolve, reject };
    });

    this.pendingGrabFramePromise_.promise = promise;

    return promise;
  }
}

@dsanders11
Copy link
Contributor Author

dsanders11 commented Jul 18, 2019

Another example of how an onframe event moves flexibility into userland and lets developers deal with issues that they can't currently deal with due to the implementation.

Currently if a next frame never comes, the Chromium implementation will leave the promise from grabFrame unresolved forever. The implementation should reject the promise when the stream ends or is muted, otherwise it leads to the never resolving (or rejecting) promise.

With the above ThrottledImageCapture class, covering this case in pure JS, independent of bugs in implementations is easy, simply add:

class ThrottledImageCapture {
  constructor (capturer) {
    // See above for full constructor, keeping abbreviated

    this.streamEnded_ = false;

    capturer.track.addEventListener('ended', () => {
      this.streamEnded_ = true;
      this.pendingGrabFramePromise_.reject('Stream ended');
    }
  }

  grabFrame () {
    if (this.streamEnded_) {
      return Promise.reject('Stream ended');
    }

    // Rest of implementation
  }

It really seems like an onframe event provides the ability to entirely implement grabFrame in userland, which lets users make the decisions on issues like this, instead of them being left to the spec or implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant