flutter-tizen / engine

The Flutter engine
https://flutter.dev
BSD 3-Clause "New" or "Revised" License
6 stars 19 forks source link

Release buffer after creating egl image #259

Closed xiaowei-guan closed 2 years ago

xiaowei-guan commented 2 years ago
swift-kim commented 2 years ago

The release_callback interface has been added to the upstream's FlutterDesktopPixelBuffer (https://github.com/flutter/engine/pull/28298).

Should we update our implementation (FlutterDesktopGPUBufferTextureConfig and FlutterDesktopGpuBuffer) accordingly? It will be a breaking API change and the video_player_tizen plugin will need to be updated as well.

(Please also take a look at Stuart's comment in https://github.com/flutter/engine/pull/28298#discussion_r768773234 if you're to make the change.)

p.s. FlutterDesktopGPUBufferTextureConfig (capitalized GPU) should also be renamed to FlutterDesktopGpuBufferTextureConfig for consistency.

xiaowei-guan commented 2 years ago

The release_callback interface has been added to the upstream's FlutterDesktopPixelBuffer (flutter#28298).

Yes, For the release callback, we can refer to the implementation of pixel buffer. Thanks for your suggestion. I will apply it.

xiaowei-guan commented 2 years ago

Thank you for taking my suggestion into account and the change generally looks good to me.

Will this change require any client side (i.e. camera_tizen and video_player_tizen) changes?

Yes, the plugin which uses texture api, need update.

bwikbs commented 2 years ago

@xiaowei-guan I have a question (Plz understand that I have never seen driver behavior or egl extension implementations before..). I'm not sure because I'm not in the office right now.. so I can't test this with my target... According to my foggy memory, the reason that destroying tbm is performed in destruciton callback is because this caused a problem in using multiple webviews (like animation or map). And by some inaccurate experiments, I thought that contents of tbm were not copied when creating the egl image.

So..can you explain a little bit more about whether it is safe to create an egl image and delete the tbm right away? 😄

xiaowei-guan commented 2 years ago

@xiaowei-guan I have a question (Plz understand that I have never seen driver behavior or egl extension implementations before..). I'm not sure because I'm not in the office right now.. so I can't test this with my target... According to my foggy memory, the reason that destroying tbm is performed in destruciton callback is because this caused a problem in using multiple webviews (like animation or map). And by some inaccurate experiments, I thought that contents of tbm were not copied when creating the egl image.

So..can you explain a little bit more about whether it is safe to create an egl image and delete the tbm right away? 😄

I just refer to eglCreateImageKHR API guide, I didn't find need wait for destruction callback, and I have tested with video player plugin. But I didn't test others plugin which use texture. I start to check the soruce code of eglCreateImageKHR,I will reply it later.

xiaowei-guan commented 2 years ago

I have a question (Plz understand that I have never seen driver behavior or egl extension implementations before..). I'm not sure because I'm not in the office right now.. so I can't test this with my target...

@bwikbs I have downloaded TV GPU driver source code, and add some log. I have tested Video player plugin,there is no problem. Could you please guide me how to test multiple webviews?

bwikbs commented 2 years ago

@xiaowei-guan

Could you please guide me how to test multiple webviews?

Here is sample Thanks :smile: I'll check it too!


update) In my case, It works ok at TV. :+1: I guess there is some misunderstanding. Plz, double check it your side.

xiaowei-guan commented 2 years ago

I have run webview plugin sample on TV device,also no problem.

swift-kim commented 2 years ago

The client side (camera_tizen and video_player_tizen) changes should be prepared first before we can publish this change.

xiaowei-guan commented 2 years ago

Video player plugin