Skip to content

[macOS/iOS] FlutterTextureRegistry accepts only a few CVPixelBuffer formats. It should be documented. #147242

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

Closed
bc-lee opened this issue Apr 23, 2024 · 1 comment · Fixed by flutter/engine#52326
Labels
d: api docs Issues with https://api.flutter.dev/ engine flutter/engine repository. See also e: labels. platform-ios iOS applications specifically platform-mac Building on or for macOS specifically team-engine Owned by Engine team

Comments

@bc-lee
Copy link
Contributor

bc-lee commented Apr 23, 2024

Steps to reproduce

  1. In a macOS/iOS project, create a texture using FlutterTextureRegistry.
  2. In the copyPixelBuffer method, pass a CVPixelBuffer with a type of kCVPixelFormatType_420YpCbCr8PlanarFullRange.

Expected results

The CVPixelBuffer should be rendered on Flutter's Texture widget.

Actual results

Glitches and incorrect colors.

Code sample

Unfortunately, the minimal working example is quite large. I will only attempt to upload a minimal example if it is necessary.

Analysis

According to the Flutter engine source code, the root cause of this issue is quite clear.

https://github.com/flutter/engine/blob/79f49954cce802a841be46a9a466ff4e50b8fa47/shell/platform/darwin/graphics/FlutterDarwinExternalTextureMetal.mm#80

- (void)onNeedsUpdatedTexture:(flutter::Texture::PaintContext&)context {
  CVPixelBufferRef pixelBuffer = [_externalTexture copyPixelBuffer];
  if (pixelBuffer) {
    CVPixelBufferRelease(_lastPixelBuffer);
    _lastPixelBuffer = pixelBuffer;
    _pixelFormat = CVPixelBufferGetPixelFormatType(_lastPixelBuffer);
  }

  ...
  sk_sp<flutter::DlImage> image = [self wrapExternalPixelBuffer:_lastPixelBuffer context:context];
  ...
}
...

- (sk_sp<flutter::DlImage>)wrapExternalPixelBuffer:(CVPixelBufferRef)pixelBuffer
                                           context:(flutter::Texture::PaintContext&)context {
  if (!pixelBuffer) {
    return nullptr;
  }

  sk_sp<flutter::DlImage> image = nullptr;
  if (_pixelFormat == kCVPixelFormatType_420YpCbCr8BiPlanarVideoRange ||
      _pixelFormat == kCVPixelFormatType_420YpCbCr8BiPlanarFullRange) {
    image = [self wrapNV12ExternalPixelBuffer:pixelBuffer context:context];
  } else {
    image = [self wrapRGBAExternalPixelBuffer:pixelBuffer context:context];
  }

  if (!image) {
    FML_DLOG(ERROR) << "Could not wrap Metal texture as a display list image.";
  }

  return image;
}

In short, the Flutter engine checks if kCVPixelFormatType_420YpCbCr8BiPlanarVideoRange and kCVPixelFormatType_420YpCbCr8BiPlanarFullRange (a.k.a NV12) are used, and then proceeds to the wrapNV12ExternalPixelBuffer method. Otherwise, it moves on to the wrapRGBAExternalPixelBuffer method, which assumes that the pixel format is kCVPixelFormatType_32BGRA, I guess. However, these source code lines ignore other pixel formats like kCVPixelFormatType_420YpCbCr8PlanarFullRange (a.k.a I420) and treat them as RGBA. That is why the colors are incorrect, and the images repeat four times.

Suggestion

I understand that the preferred formats in Apple's ecosystem are kCVPixelFormatType_32BGRA, kCVPixelFormatType_420YpCbCr8BiPlanarVideoRange, and kCVPixelFormatType_420YpCbCr8BiPlanarFullRange. Other formats are not well-supported even in Apple's own APIs. However, when it comes to APIs, it's better to log an error message stating that the pixel format is not supported, rather than rendering incorrect images. Additionally, it would be helpful to document this limitation in Flutter's official documentation.

Flutter Doctor output

Doctor output
$ flutter doctor -v
[✓] Flutter (Channel stable, 3.19.6, on macOS 14.4.1 23E224 darwin-arm64, locale en-KR)
    • Flutter version 3.19.6 on channel stable at /Users/leebc/flutter
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision 54e66469a9 (6 days ago), 2024-04-17 13:08:03 -0700
    • Engine revision c4cd48e186
    • Dart version 3.3.4
    • DevTools version 2.31.1

[✓] Android toolchain - develop for Android devices (Android SDK version 34.0.0)
    • Android SDK at /Users/leebc/android/sdk
    • Platform android-34, build-tools 34.0.0
    • ANDROID_HOME = /Users/leebc/android/sdk
    • Java binary at: /Users/leebc/Applications/Android Studio.app/Contents/jbr/Contents/Home/bin/java
    • Java version OpenJDK Runtime Environment (build 17.0.10+0-17.0.10b1087.21-11609105)
    • All Android licenses accepted.

[✓] Xcode - develop for iOS and macOS (Xcode 15.2)
    • Xcode at /Applications/Xcode15.2.app/Contents/Developer
    • Build 15C500b
    • CocoaPods version 1.15.2

[✓] Chrome - develop for the web
    • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[!] Android Studio (version unknown)
    • Android Studio at /Users/leebc/Applications/Android Studio.app/Contents
    • Flutter plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/6351-dart
    ✗ Unable to determine Android Studio version.
    • android-studio-dir = /Users/leebc/Applications/Android Studio.app
    • Java version OpenJDK Runtime Environment (build 17.0.10+0-17.0.10b1087.21-11609105)

[✓] IntelliJ IDEA Ultimate Edition (version 2024.1)
    • IntelliJ at /Users/leebc/Applications/IntelliJ IDEA Ultimate.app
    • Flutter plugin installed
    • Dart plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/6351-dart

[✓] VS Code (version unknown)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension version 3.86.0
    ✗ Unable to determine VS Code version.

[✓] Connected device (2 available)
    • macOS (desktop) • macos  • darwin-arm64   • macOS 14.4.1 23E224 darwin-arm64
    • Chrome (web)    • chrome • web-javascript • Google Chrome 124.0.6367.62

[✓] Network resources
    • All expected network resources are available.
@darshankawar darshankawar added in triage Presently being triaged by the triage team platform-ios iOS applications specifically engine flutter/engine repository. See also e: labels. platform-mac Building on or for macOS specifically d: api docs Issues with https://api.flutter.dev/ team-engine Owned by Engine team and removed in triage Presently being triaged by the triage team labels Apr 24, 2024
auto-submit bot pushed a commit to flutter/engine that referenced this issue Apr 24, 2024
CVPixelBuffers from the application can have arbitrary pixel formats. However, current implementation doesn't support all pixel formats. To address this, add a comment to the FlutterTexture class to clarify which pixel formats are supported. Also, reject unsupported pixel formats so that the application can handle them properly.

Fixes [#147242](flutter/flutter#147242).

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Copy link

github-actions bot commented May 8, 2024

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 8, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
d: api docs Issues with https://api.flutter.dev/ engine flutter/engine repository. See also e: labels. platform-ios iOS applications specifically platform-mac Building on or for macOS specifically team-engine Owned by Engine team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants