Texture support for glfw. #9822

cloudwebrtc · 2019-07-13T14:30:58Z

PR for flutter/flutter#30718.
II tested RGB rendering under windows, tried window scaling, and the video showed everything is fine. But it seems that we need to add glad support to third_party/glfw/BULLD.gn.

Screenshot (Windows 10/ Ubuntu 16.04)

Merge pull request from flutter/engine

- Add Success and Error handler for EventSink. - Send Event message or error using MethodCodec.

- Add GLFWPixelBuffer to manage Texture buffer copy. - Remove unnecessary offscreen context in ExternalTextureGL. - Create GL_TEXTURE_2D Texture using the height and width of GLFWPixelBuffer.

cloudwebrtc · 2019-07-13T14:51:02Z

@chinmaygarde @stuartmorgan
It seems that we need to add glad_gl.c to the third_party/glfw/BULLD.gn of glfw, because texture rendering calls the function in glad.
https://fuchsia.googlesource.com/third_party/glfw/+/refs/heads/master/BUILD.gn#29

stuartmorgan-g

A few initial high-level comments.

stuartmorgan-g · 2019-07-14T01:14:07Z

shell/platform/common/cpp/client_wrapper/include/flutter/event_channel.h

@@ -0,0 +1,116 @@
+// Copyright 2013 The Flutter Authors. All rights reserved.


Please split event channel support out into a separate precursor PR to keep the scope smaller.

Ok, I will submit event_channel.h in another PR.

stuartmorgan-g · 2019-07-14T01:15:14Z

shell/platform/common/cpp/client_wrapper/include/flutter/event_channel.h

+
+namespace flutter {
+
+template <typename T>


Most of the things in this file need declaration comments, per Google's C++ style guide.

This applies to all of the class declarations, method declarations, and typedefs in this PR, not just this file.

shell/platform/common/cpp/public/flutter_texture_registrar.h

shell/platform/glfw/external_texture_gl.h

- Remove event_channel.h from this PR - Remove the GLFW prefix for PixelBuffer.

stuartmorgan-g · 2019-07-15T17:48:51Z

@chinmaygarde @stuartmorgan
It seems that we need to add glad_gl.c to the third_party/glfw/BULLD.gn of glfw, because texture rendering calls the function in glad.
https://fuchsia.googlesource.com/third_party/glfw/+/refs/heads/master/BUILD.gn#29

I would expect that to be a separate target, rather than part of the GLFW target; I'm not sure why it was done that way in the Fuchsia build.

Also, adding that may require changes to the license script, as I remember that at least parts of deps had issues when I was initially adding GLFW.

cloudwebrtc · 2019-07-15T18:15:37Z

Probably I got it wrong, I see engine/DEPS just checkout glfw source from Fuchsia, but compile it with buildroot/build/secondary/third_party/glfw/BUILD.gn.
It seems that we should add glad.c to https://github.com/flutter/buildroot/blob/c4df4a7b616f6ef1bc81bd99efdd4bff107a407e/build/secondary/third_party/glfw/BUILD.gn.
So maybe I should add a patch to flutter/buildroot.

stuartmorgan-g · 2019-07-15T20:00:50Z

So maybe I should add a patch to flutter/buildroot.

That's what would be needed, yes, but my comment was to suggest that we should have a separate target for that, not fold it into the primary GLFW target as was done in the file you linked to.

Please test licenses locally before sending the buildroot patch though, so that it doesn't end up being something that prevents buildroot rolls and needs to be reverted (since the problem won't show up in CI until the PR that tries to roll buildroot forward).

- Add comments to related classes. - Remove std::shared_ptr<> for PixelBuffer. - Optimize texture rendering code.

cloudwebrtc · 2019-07-21T11:53:32Z

Fixed a few bugs and tested under Linux.

lattice0 · 2019-10-19T01:40:44Z

I'm trying to run this pull request. I've been able to render a texture to screen by indirectly triggering MarkTextureFrameAvailable when I click on buttons or resize the screen.

If I call texture_registrar->MarkTextureFrameAvailable(texture_id_); I end up with this:

[FATAL:flutter/shell/common/shell.cc(808)] Check failed: task_runners_.GetPlatformTaskRunner()->RunsTasksOnCurrentThread().

it's worth noticing thattexture_registrar->MarkTextureFrameAvailable(texture_id_); only fails if called from another thread. However I don't see utility in calling it from the main thread, I need to call it fromt he thread that received video.

Is there something I'm missing? Is there a way to call MarkTextureFrameAvailable indirectly from another thread in a way that it's called from the main thread?

Thank you so much

(I'm also doing some modifications on the engine to render with arbitrary opengl calls, I guess it's going to be very useful for the community)

stuartmorgan-g · 2019-10-19T03:29:20Z

it's worth noticing that [it] only fails if called from another thread.

Any embedder.h API that doesn't explicitly say otherwise must be called from the same thread as the engine was created on; that includes this one.

However I don't see utility in calling it from the main thread, I need to call it fromt he thread that received video.

If you have a use case where you are doing things on another thread, it's your responsibility to dispatch the calls to the correct thread.

Is there a way to call MarkTextureFrameAvailable indirectly from another thread in a way that it's called from the main thread?

There are a variety of ways to pass information between threads; that's off-topic for this PR though.

(I'm also doing some modifications on the engine to render with arbitrary opengl calls, I guess it's going to be very useful for the community)

I would encourage you to open a "Feature request" issue with your proposal for discussion before implementing a new feature in the engine.

lattice0 · 2019-10-20T03:27:11Z

it's worth noticing that [it] only fails if called from another thread.

Any embedder.h API that doesn't explicitly say otherwise must be called from the same thread as the engine was created on; that includes this one.

However I don't see utility in calling it from the main thread, I need to call it fromt he thread that received video.

If you have a use case where you are doing things on another thread, it's your responsibility to dispatch the calls to the correct thread.

Is there a way to call MarkTextureFrameAvailable indirectly from another thread in a way that it's called from the main thread?

There are a variety of ways to pass information between threads; that's off-topic for this PR though.

(I'm also doing some modifications on the engine to render with arbitrary opengl calls, I guess it's going to be very useful for the community)

I would encourage you to open a "Feature request" issue with your proposal for discussion before implementing a new feature in the engine.

Thanks for the information. I spent the day thinking on how I could do that and couldn't end in a conclusion.

It seems that my only option is to run RunEventLoopWithTimeout in a while loop with a very small timeout (1 milissecond) and probe for MarkTextureFrameAvailable call from the thread but this seems innefficient. If I have 10 videos with 60fps then half a second could be spent checking for thse calls, I guess.

I also checked @cloudwebrtc's demo of RTC app and there's no custom event loop at all: https://github.com/cloudwebrtc/flutter-webrtc-demo/blob/desktop/linux/flutter_webrtc_demo.cc so I wondered how it's done. I gone and found how MarkTextureFrameAvailable is called: https://github.com/cloudwebrtc/flutter-webrtc/blob/desktop/common/src/flutter_video_renderer.cc#L90 but I couldn't find the thread which calls it, because there's lot of external libraries imvolved that I couldn't find. So, it looks like the plugin's thread is the one calling MarkTextureFrameAvailable, how is this possible?

stuartmorgan-g · 2019-10-20T04:35:01Z

So, it looks like the plugin's thread is the one calling MarkTextureFrameAvailable, how is this possible?

Plugins don't intrinsically have their own threads, so it's not clear what you are asking here. They are constructed on, and receive method calls on, the same thread that is used for all other embedder API calls.

lattice0 · 2019-10-20T05:10:33Z

So, it looks like the plugin's thread is the one calling MarkTextureFrameAvailable, how is this possible?

Plugins don't intrinsically have their own threads, so it's not clear what you are asking here. They are constructed on, and receive method calls on, the same thread that is used for all other embedder API calls.

I was analyzing how @cloudwebrtc's flutter-webrtc-demo does the MarkTextureFrameAvailable call and since it receives video through internet it must use a thread. The call happens here: https://github.com/cloudwebrtc/flutter-webrtc/blob/desktop/common/src/flutter_video_renderer.cc#L90 but I couldn't find which thread calls it so I suppose it's not the main thread. However it can't be possible, because MarkTextureFrameAvailable must be called from the main thread.

Anyways, is my only option to use RunEventLoopWithTimeout to probe for MarkTextureFrameAvailable calls on a queue?

stuartmorgan-g · 2019-10-20T05:28:00Z

Again, how to write multi-threaded code using the GLFW embedding is off-topic for this PR; please find another forum for that conversation. The discussion here should be focused on the review of this PR and the process for getting it landed.

cbracken · 2019-11-04T18:40:08Z

@chinmaygarde if you get a moment, would you mind taking a look?

cloudwebrtc · 2019-12-04T09:12:11Z

@stuartmorgan Updated, please review. :)

stuartmorgan-g · 2019-12-04T13:05:11Z

I'll do a full review of the changes when I get a chance, but this still need a review from @chinmaygarde.

rao-subba-venkata · 2019-12-08T03:10:05Z

Appreciate if I can get hint to play mp4 file for Linux or Windows Desktop.

stuartmorgan-g · 2019-12-08T06:13:32Z

@venkata-subbarao As I said above: "The discussion here should be focused on the review of this PR and the process for getting it landed." This isn't the right place to ask for help using this API; please use a support forum for that.

or Windows

This PR is no longer relevant to Windows.

stuartmorgan-g

Just small things left from me.

Still needs a review from @chinmaygarde on the texture aspects.

stuartmorgan-g · 2019-12-08T22:00:51Z

shell/platform/common/cpp/client_wrapper/include/flutter/texture_registrar.h

+  // |height| and |width| parameters of bounds.
+  // In some cases, we need to scale the texture to the bounds size to reduce
+  // memory usage.
+  virtual const PixelBuffer* CopyPixelBuffer(size_t width, size_t height) = 0;


This still needs to be addressed.

shell/platform/common/cpp/public/flutter_texture_registrar.h

winwisely99 · 2020-01-20T19:23:13Z

@stuartmorgan

Polite ping to have a look again.

It looks like the pixel buffer pointer has been documented by @cloudwebrtc

stuartmorgan-g · 2020-01-21T03:16:34Z

@winwisely99 As I said above, this needs a review from @chinmaygarde. Further review from me before then isn't going to advance this.

andrewmd5 · 2020-05-08T09:06:56Z

Totally sympathetic to crazy times right now, but It would be great if @chinmaygarde was able to provide an update on their review as it seems there has been no immediate action and this pull is becoming stale.

If those conversations are happening elsewhere than I apologize.

peerwaya · 2020-05-28T01:15:17Z

@chinmaygarde Kindly update

stuartmorgan-g · 2020-06-17T22:55:43Z

Now that both Windows and Linux have switched away from GLFW, I'm going to close this; while we still build the GLFW embedding for limited internal use, implementing this would no longer affect any of the embeddings used by the flutter tool.

@cloudwebrtc, if you are still interested in implementing this for GLFW for some reason despite that, please comment and I'll re-open, but the way to move forward to support a project like your plugin would be texture support for the Windows (Win32) and Linux (GTK) embeddings.

lattice0 · 2020-06-17T23:00:01Z

@stuartmorgan I don't get it. Wasn't the goal to be independent from the GUI API? So both Qt, GTK, etc would work? Now GTK is chosen to be the default/official for linux then?

I thought GLFW was the right way as it would work with all of them, even had some modifications that could do YUV420P conversion in shader, I was about to finish it

stuartmorgan-g · 2020-06-18T13:47:51Z

@stuartmorgan I don't get it. Wasn't the goal to be independent from the GUI API? So both Qt, GTK, etc would work? Now GTK is chosen to be the default/official for linux then?

This is covered (as it has always been) in the desktop documentation

I thought GLFW was the right way as it would work with all of them

GLFW didn't work with any of them; neither add-to-app nor PlatformView were viable with GLFW. The documentation has always been very clear that the GLFW embedding was a temporary placeholder that would be replaced.

peerwaya · 2020-06-19T10:00:30Z

@stuartmorgan what's the way forward for bringing texture support to windows?

stuartmorgan-g · 2020-06-20T02:07:21Z

@peerwaya I'm not sure what you're asking, but if you have technical questions about how to implement it the right place would be flutter/flutter#38601, not here.

cloudwebrtc added 10 commits March 29, 2019 00:15

Merge pull request #1 from flutter/master

5302cf8

Merge pull request from flutter/engine

Merge remote-tracking branch 'upstream/master'

2953e13

Merge remote-tracking branch 'upstream/master'

42b7502

Merge remote-tracking branch 'upstream/master'

ee2a336

Merge remote-tracking branch 'upstream/master'

bfd31aa

Add Texture support for GLFW.

08e9d07

Fixed EventChannel for embedder.

0af9192

- Add Success and Error handler for EventSink. - Send Event message or error using MethodCodec.

Fixed compiler error for stub_flutter_api.cc.

bfb06c2

Update texture registrar for glfw.

d6529fb

- Add GLFWPixelBuffer to manage Texture buffer copy. - Remove unnecessary offscreen context in ExternalTextureGL. - Create GL_TEXTURE_2D Texture using the height and width of GLFWPixelBuffer.

clang format.

689f6e6

googlebot added the cla: yes label Jul 13, 2019

stuartmorgan-g reviewed Jul 14, 2019

View reviewed changes

cloudwebrtc added 2 commits July 14, 2019 12:41

Update.

e93c57c

- Remove event_channel.h from this PR - Remove the GLFW prefix for PixelBuffer.

Update.

eaec810

- Remove event_channel.h from this PR - Remove the GLFW prefix for PixelBuffer.

cloudwebrtc mentioned this pull request Jul 16, 2019

Add glad target to glfw/BUILD.gn. flutter/buildroot#275

Closed

cloudwebrtc added 3 commits July 16, 2019 20:34

Add comments and code modify.

63ad181

- Add comments to related classes. - Remove std::shared_ptr<> for PixelBuffer. - Optimize texture rendering code.

Update DEPS

f07279e

Update external_texture_gl.h

d1266a8

cloudwebrtc force-pushed the texture_glfw branch from f724359 to 7796843 Compare July 16, 2019 18:05

Fix compilation errors for linux and clang/gn format.

7391e42

cloudwebrtc force-pushed the texture_glfw branch 2 times, most recently from da077a6 to 8b6234f Compare July 16, 2019 19:02

Fix license check error.

4c5f065

cloudwebrtc force-pushed the texture_glfw branch from 8b6234f to 4c5f065 Compare July 17, 2019 01:14

Fixed the order of glad.h contains a run error.

93fc359

cloudwebrtc added 2 commits November 7, 2019 13:21

update.

eed131a

update unit test.

aba2fa2

stuartmorgan-g suggested changes Dec 8, 2019

View reviewed changes

Fixed typo.

d8efcdb

This comment has been minimized.

Sign in to view

pchampio mentioned this pull request Apr 21, 2020

Plugin for flutter.io/videoPlayer go-flutter-desktop/go-flutter#134

Open

stuartmorgan-g closed this Jun 17, 2020

jnschulze mentioned this pull request Jun 30, 2020

Add windows plugin texture support #19405

Merged

9 tasks

cloudwebrtc deleted the texture_glfw branch July 1, 2020 22:47

cloudwebrtc restored the texture_glfw branch July 1, 2020 22:47

		@@ -0,0 +1,116 @@
		// Copyright 2013 The Flutter Authors. All rights reserved.


		namespace flutter {

		template <typename T>

Texture support for glfw. #9822

Texture support for glfw. #9822

Uh oh!

Conversation

cloudwebrtc commented Jul 13, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Screenshot (Windows 10/ Ubuntu 16.04)

Uh oh!

cloudwebrtc commented Jul 13, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stuartmorgan-g left a comment

Choose a reason for hiding this comment

Uh oh!

stuartmorgan-g Jul 14, 2019

Choose a reason for hiding this comment

Uh oh!

cloudwebrtc Jul 14, 2019

Choose a reason for hiding this comment

Uh oh!

stuartmorgan-g Jul 14, 2019

Choose a reason for hiding this comment

Uh oh!

stuartmorgan-g Jul 14, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

stuartmorgan-g commented Jul 15, 2019

Uh oh!

cloudwebrtc commented Jul 15, 2019

Uh oh!

stuartmorgan-g commented Jul 15, 2019

Uh oh!

cloudwebrtc commented Jul 21, 2019

Uh oh!

lattice0 commented Oct 19, 2019

Uh oh!

stuartmorgan-g commented Oct 19, 2019

Uh oh!

lattice0 commented Oct 20, 2019

Uh oh!

stuartmorgan-g commented Oct 20, 2019

Uh oh!

lattice0 commented Oct 20, 2019

Uh oh!

stuartmorgan-g commented Oct 20, 2019

Uh oh!

cbracken commented Nov 4, 2019

Uh oh!

cloudwebrtc commented Dec 4, 2019

Uh oh!

stuartmorgan-g commented Dec 4, 2019

Uh oh!

rao-subba-venkata commented Dec 8, 2019

Uh oh!

stuartmorgan-g commented Dec 8, 2019

Uh oh!

stuartmorgan-g left a comment

Choose a reason for hiding this comment

Uh oh!

stuartmorgan-g Dec 8, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

This comment has been minimized.

winwisely99 commented Jan 20, 2020

Uh oh!

stuartmorgan-g commented Jan 21, 2020

Uh oh!

andrewmd5 commented May 8, 2020

Uh oh!

peerwaya commented May 28, 2020

Uh oh!

stuartmorgan-g commented Jun 17, 2020

Uh oh!

lattice0 commented Jun 17, 2020

Uh oh!

stuartmorgan-g commented Jun 18, 2020

Uh oh!

cloudwebrtc commented Jul 13, 2019 •

edited

Loading

cloudwebrtc commented Jul 13, 2019 •

edited

Loading