[web] feature-detect and use ImageDecoder for all image decoding #29419
Conversation
flutter-dashboard bot added the platform-web 
| ); | ||
| } | ||
| | ||
| if (!await _isImageTypeSupported(contentType)) { |
There was a problem hiding this comment.
If you don't already have an async step to hide this under, you can just create the image decoder and get the exception there instead of adding any async delay here.
| // color space, typically SRGB, which is what we want. | ||
| colorSpaceConversion: 'default', | ||
| | ||
| // Flutter doesn't give the developer to customize this, so if this is an |
| // wait until the image is fully decoded. | ||
| // package:js bindings don't work with getters that return a Promise, which | ||
| // is why js_util is used instead. | ||
| await js_util.promiseToFuture<void>(js_util.getProperty(webDecoder, 'completed')); |
There was a problem hiding this comment.
completed is for loading, not decoding. No decoding starts until decode() is called. This will ensure the frame count is accurate though.
| preferAnimation: true, | ||
| )); | ||
| | ||
| await js_util.promiseToFuture<void>(webDecoder.tracks.ready); |
There was a problem hiding this comment.
This is all that's needed unless you want to ensure frame count is fully known.
There was a problem hiding this comment.
For some reason, until this morning (when I was forced to update my Chrome), if I didn't also await ImageDecoder.completed I would get different values for VideoFrame.format. It would flip between BGRX and I420. I assumed there was some race between some internal work that ImageDecoder does and the call to decode() (e.g. progressive loading of images).
With the latest version of Chrome I cannot reproduce this any more, but I'm not sure what behavior to expect. Since I changed the code to load the VideoFrame straight to texImage2D, it probably doesn't matter. We should be able to handle both. The only thing that might be surprising to the user (and perhaps to our goldens test infrastructure) is if the image is sometimes decoded at full resolution and sometimes partially.
There was a problem hiding this comment.
Ah, Chrome will opportunistically use YUV decoding, but it depends on if all the frame data is available. So waiting for completed will ensure it's YUV if the image format allows it and Chrome thinks it can decode to it.
In cases where a YUV capable image is decoded to BGRX it's still full resolution, it shouldn't muddle any pixel tests, but I'm not sure off the top of my head.
| // surface, if necessary, or reuse the existing one. | ||
| final CkSurface ckSurface = SurfaceFactory.instance.baseSurface.createOrUpdateSurface(ui.window.physicalSize); | ||
| final SkSurface skSurface = ckSurface.surface; | ||
| final SkImage? skImage = skSurface.makeImageFromTextureSource( |
There was a problem hiding this comment.
You can also call copyTo to get the raw pixel bytes. But if your destination is the gpu drawImage/texImage straight from the VideoFrame is definitely better.
There was a problem hiding this comment.
Yes, preferably it should go straight to the GPU. However, in a follow-up PR I'll add support for target size, and since not all codecs support resizing, we'll need the pixels in the CPU memory, which is when copyTo will come handy.
| /// type. It should therefore contain the most popular file formats at the | ||
| /// top, and less popular towards the bottom. | ||
| static const List<ImageFileFormat> values = <ImageFileFormat>[ | ||
| // PNG |
There was a problem hiding this comment.
AVIF is also supported by Chrome if you want.
There was a problem hiding this comment.
I do, but I don't know how to detect it :)
There was a problem hiding this comment.
It's slightly complicated if you want to do precise detection, but 'ftyp' in the first 16 bytes is good enough to detect an ISO-BMFF image type -- you'll fail later during configure if it's not avif and instead heic or a video.
There was a problem hiding this comment.
Fantastic! Thanks for the link. I'll add it.
yjbanov force-pushed the web-codecs branch 3 times, most recently from bdb0a5f to 1c6be9a Compare yjbanov force-pushed the web-codecs branch 2 times, most recently from b5d373a to 2ebde7c Compare 
| @@ -0,0 +1,436 @@ | |||
| // Copyright 2013 The Flutter Authors. All rights reserved. | |||
There was a problem hiding this comment.
Our project uses 2013 everywhere. Using a future date is problematic, but using an older date is fine.
| ); | ||
| } | ||
| | ||
| final _ImageDecoder webDecoder = _ImageDecoder(_ImageDecoderOptions( |
There was a problem hiding this comment.
I think you want this in the try/catch below.
| frame.displayWidth, | ||
| frame.displayHeight, | ||
| ); | ||
| |
There was a problem hiding this comment.
If you're done with frame at this point it's good practice to call frame.close() to free up memory.
| Update: found a bug where the image is lost when moved across GL contexts. Waiting for a CanvasKit-side fix before merging this. |
fluttergithubbot force-pushed the master branch from b1a8771 to 2962099 Compare yjbanov force-pushed the web-codecs branch 4 times, most recently from 6987dd1 to 0875758 Compare | CanvasKit fix landed, so I will merge this as soon as it goes green. |
| Gold has detected about 1 new digest(s) on patchset 1. |
| Gold has detected about 1 new digest(s) on patchset 1. |
| The luci-engine status seems to be incorrect again. |
5 participants
felt test --debug, auto-open Chrome DevTools and maximize the window (it is annoying to have to do this manually every time).The benefit of
ImageDecoderis that it decodes asynchronously on a separate thread, so it doesn't block the frames while decoding. Once this API is supported in all browsers, we can remove the WASM codecs and reduce the size of CanvasKit by a few 100s of KB.Fixes flutter/flutter#63397