Support MediaMetadataRetriever.BitmapParams on API 28 (+)

[mhelder]

Is your feature request related to a problem? Please describe.
While working on an app that uses VideoFrameDecoder to render a still preview of videos, I noticed the resulting image has quite some visual banding (see 1st image below). I traced this back to where VideoFrameDecoder uses the MediaMetadataRetriever to decode a frame:

https://github.com/coil-kt/coil/blob/6ac35963bb3b1c06caca47ee43dd8370e9336f49/coil-video/src/main/java/coil/decode/VideoFrameDecoder.kt#L82;L89

The rawBitmap (as decoded by MediaMetadataRetriever) already displays the banding, and that's the result of the decoded bitmap config being RGB_565. Although Coil isn't directly to blame for this because it didn't decode or transform the bitmap, it also doesn't make use of the MediaMetadataRetriever.BitmapParams that were added in API 28. By providing these, we could avoid the banding. (see 2nd image below).

Describe the solution you'd like
It'd be great it Coil could provide MediaMetadataRetriever.BitmapParams to the relevant MediaMetadataRetriever functions on platforms that support it. These BitmapParams should probably be based on the configuration of the ImageRequest options, to control the bitmap config used for decoding. Specifically, allowRgb565 and bitmapConfig would be relevant (perhaps more options?).

I also noted that the docs for e.g. MediaMetadataRetriever.getScaledFrameAtTime mention that the current behaviour of supplying null for the BitmapParams result in "that the device will choose the actual Bitmap.Config to use". So the current behaviour of decoding video frames may differ from device to device and vendor to vendor. So far, I've been able to confirm that at least the Google Pixel 6 (Android 13) and Samsung S21 5G (Android 12) select RGB_565 by default.

I'm happy to take a crack at implementing and submitting a PR for this, but could use some guidance on some details:

• How do the allowRgb565 and bitmapConfig options relate to each other? Am I correct if I state that the bitmap config is merely a preferred config, and allowRgb565 (and some other scenarios) may override this?
• Do you think we should always provide BitmapParams on supported platforms? It'll then effectively default to ARGB_8888. Or should we keep it null in case no explicit bitmapConfig option is set in the image request (for backwards compatible behaviour, so the device will still "choose the actual config")? (I'm leaning towards the latter.)

Additional context
RGB_565 bitmap with banding:

ARGB_8888 bitmap without banding

I've also pushed a sample video in a fork if you want to play around with this yourself. Run coil-sample-view from:
https://github.com/mhelder/coil/tree/bug/videoframedecoder-banding

[colinrtwhite]

Thanks for the detailed bug report! I took a quick stab at it here. Unfortunately, we can't use BitmapParams until API 30 since Android didn't add getFrameAtTime and getScaledFrameAtTime overloads with both an option and params until API 30.

[mhelder]

That was quick! Nice catch and also a bummer that getFrameAtTime and getScaledFrameAtTime are only available on API 30 and above - I didn't look beyond the BitmapParams class when I opened the issue 😅. Thanks for fixing this!