Skip to content
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

Texture api change #86

Conversation

xiaowei-guan
Copy link

1.Merge flutter_tizen_texture_registrar.h into common interface

we add GpuBufferTexture in Texture_registrar.h for tbm surface.

2.Delete ref/unref tbm_surface code.

we add destruction_callback in GpuBufferTexture, notify app to destory tbm surface.

3.Support pixel buffer texture.

we can implement pixel buffer texture.

This code only completes the basic architecture,we don't implement the detail,
We need to discuss the feasibility of this code first, and need more suggestions.

@xiaowei-guan xiaowei-guan requested a review from a team May 11, 2021 08:04
@bwikbs
Copy link
Member

bwikbs commented May 11, 2021

FIrst of all, Thanks !!! I love this patch! 👍 ❤️

we add GpuBufferTexture in Texture_registrar.h for tbm surface.

But I don't know whether we can handle these difference...
I hope to hear enough opinions from others as we proceed this refactoring.

@bbrto21
Copy link

bbrto21 commented May 11, 2021

@xiaowei-guan
Could you resolve conflicting files? and you can convert this to draft not only title

@xiaowei-guan
Copy link
Author

don't know whether we can handle these difference...
About add GpuBufferTexture,

  1. we can't affect the function of pixel buffer texture, so we just need add code.
  2. we need make it common, not just for tbm surface.

@xiaowei-guan
Copy link
Author

@xiaowei-guan
Could you resolve conflicting files? and you can convert this to draft not only title

ok.

@xuelian-bai
Copy link

xuelian-bai commented May 11, 2021

we add GpuBufferTexture in Texture_registrar.h for tbm surface.

But I don't know whether we can handle these difference...

For this, you may see comments inside Texture_registrar.h,

// The available texture variants.
// Only PixelBufferTexture is currently implemented.
// Other variants are expected to be added in the future.
typedef std::variant<PixelBufferTexture> TextureVariant;

If we make it common, maybe we can contribute it to community

@xiaowei-guan xiaowei-guan marked this pull request as draft May 11, 2021 10:37
@xiaowei-guan xiaowei-guan changed the title Texture api change(Draft) Texture api change May 11, 2021
@bwikbs bwikbs mentioned this pull request May 11, 2021
@bwikbs
Copy link
Member

bwikbs commented May 11, 2021

If we make it common, maybe we can contribute it to community

If this can be the starting point for contributing to flutter, I'll be absolutely happy. 😄
If that happens, our useless efforts will be reduced a lot... but honestly, I'm not sure what will happen next stage..
So, for now, it is necessary to be careful for painless rebasing.

1.Delete tizen texture api, use common api, add GpuBufferTexture for gpu
buffer.

2.Delete ref/unref tbm_surface code in engine, add destructioncall for
delete tbm_surface.

3.Support pixel buffer texture.
@bwikbs
Copy link
Member

bwikbs commented May 13, 2021

@xiaowei-guan
I know this patch is still in preparation, but...
Could you show me the code for calling `TextureRegistrarImpl::RegisterTexture(TextureVariant* texture)`` as soon as you make it?
Because I tried to do related work, but when you're ready, I'm going to do something else.

@xiaowei-guan
Copy link
Author

xiaowei-guan commented May 13, 2021

@xiaowei-guan
I know this patch is still in preparation, but...
Could you show me the code for calling `TextureRegistrarImpl::RegisterTexture(TextureVariant* texture)`` as soon as you make it?
Because I tried to do related work, but when you're ready, I'm going to do something else.

1.Get texture register

flutter::PluginRegistrarManager::GetInstance()
          ->GetRegistrar<flutter::PluginRegistrar>(registrar)->texture_registrar()

2.RegisterTexture

 textureVariant_ =
      std::make_unique<flutter::TextureVariant>(flutter::GpuBufferTexture(
          [this](size_t width,
                 size_t height) -> const FlutterDesktopGpuBuffer * {
            this->CopyGpuBuffer(width, height);
          },
          [this]() -> void { this->Destruction(); }));

  LOG_INFO("[VideoPlayer] register texture");
  textureId_ = textureRegistrar->RegisterTexture(textureVariant_.get());

3.MarkTextureFrameAvailable

  textureRegistrar_->MarkTextureFrameAvailable(player->textureId_);

4.UnregisterTexture

textureRegistrar_->UnregisterTexture(textureId_);

@bwikbs
Copy link
Member

bwikbs commented May 13, 2021

Thanks for quick response:exclamation:

Well, please see if I understand it or not:

  1. When we get some packet from the decoder, we call GpuBufferTexture 's some api for storing its surface data.
  2. Call MarkTextureFrameAvailable
  3. FlutterDesktopGpuBufferTextureCallback is called, we make a texture using GL_TEXTURE_EXTERNAL_OES to fill up FlutterOpenGLTexture

Is it correct what I understand?

@xiaowei-guan
Copy link
Author

xiaowei-guan commented May 13, 2021

  1. packet from the decoder, we call GpuBufferTexture 's some api for storing its surface data.

Thanks for quick response❗

Well, please see if I understand it or not:

  1. When we get some packet from the decoder, we call GpuBufferTexture 's some api for storing its surface data.
  2. Call MarkTextureFrameAvailable
  3. FlutterDesktopGpuBufferTextureCallback is called, we make a texture using GL_TEXTURE_EXTERNAL_OES to fill up FlutterOpenGLTexture

Is it correct what I understand?

1.when we get packet from decoder,store the packet, and call MarkTextureFrameAvailable
player->mediaPacket_ = packet;
player->textureRegistrar_->MarkTextureFrameAvailable(player->textureId_);

  1. when we receive copy gpu buffer callback.
    FlutterDesktopGpuBuffer *VideoPlayer::CopyGpuBuffer(size_t width,
    size_t height) {
    std::lock_guardstd::mutex lock(mutex_);
    if (mediaPacket_ == nullptr) {
    LOG_ERROR("[VideoPlayer.CopyGpuBuffer] mediaPacket_ is null");
    return nullptr;
    }
    tbm_surface_h surface;
    int ret = media_packet_get_tbm_surface(mediaPacket_, &surface);
    if (ret != MEDIA_PACKET_ERROR_NONE) {
    LOG_ERROR(
    "[VideoPlayer.onVideoFrameDecoded] media_packet_get_tbm_surface "
    "failed, error: %d",
    ret);
    media_packet_destroy(mediaPacket_);
    mediaPacket_ = nullptr;
    return nullptr;
    }
    FlutterDesktopGpuBuffer gpuBuffer;
    gpuBuffer.buffer = surface;
    gpuBuffer.width = width;
    gpuBuffer.height = height;
    return &gpuBuffer;
    }

3.when Flutter GL render finish , will trigger destruction_gpu_buffer_callback_
void VideoPlayer::Destruction() {
std::lock_guardstd::mutex lock(mutex_);
if (mediaPacket_) {
media_packet_destroy(mediaPacket_);
mediaPacket_ = nullptr;
}
}

@xiaowei-guan
Copy link
Author

  1. packet from the decoder, we call GpuBufferTexture 's some api for storing its surface data.

Thanks for quick response❗
Well, please see if I understand it or not:

  1. When we get some packet from the decoder, we call GpuBufferTexture 's some api for storing its surface data.
  2. Call MarkTextureFrameAvailable
  3. FlutterDesktopGpuBufferTextureCallback is called, we make a texture using GL_TEXTURE_EXTERNAL_OES to fill up FlutterOpenGLTexture

Is it correct what I understand?

1.when we get packet from decoder,store the packet, and call MarkTextureFrameAvailable
player->mediaPacket_ = packet;
player->textureRegistrar_->MarkTextureFrameAvailable(player->textureId_);

  1. when we receive copy gpu buffer callback.
    FlutterDesktopGpuBuffer *VideoPlayer::CopyGpuBuffer(size_t width,
    size_t height) {
    std::lock_guardstd::mutex lock(mutex_);
    if (mediaPacket_ == nullptr) {
    LOG_ERROR("[VideoPlayer.CopyGpuBuffer] mediaPacket_ is null");
    return nullptr;
    }
    tbm_surface_h surface;
    int ret = media_packet_get_tbm_surface(mediaPacket_, &surface);
    if (ret != MEDIA_PACKET_ERROR_NONE) {
    LOG_ERROR(
    "[VideoPlayer.onVideoFrameDecoded] media_packet_get_tbm_surface "
    "failed, error: %d",
    ret);
    media_packet_destroy(mediaPacket_);
    mediaPacket_ = nullptr;
    return nullptr;
    }
    FlutterDesktopGpuBuffer gpuBuffer;
    gpuBuffer.buffer = surface;
    gpuBuffer.width = width;
    gpuBuffer.height = height;
    return &gpuBuffer;
    }

3.when Flutter GL render finish , will trigger destruction_gpu_buffer_callback_
void VideoPlayer::Destruction() {
std::lock_guardstd::mutex lock(mutex_);
if (mediaPacket_) {
media_packet_destroy(mediaPacket_);
mediaPacket_ = nullptr;
}
}

I have upload some code, it can work.

@bwikbs
Copy link
Member

bwikbs commented May 13, 2021

I have upload some code, it can work.

Thanks! I'll check that code first!

@bwikbs
Copy link
Member

bwikbs commented May 13, 2021

Thank you.
Now, I understand what you're trying to do at this work.

I'll ask you one more thing

player->mediaPacket_ = packet;

  1. At this time, if there is an existing packet, how about replacing it with a new packet? I've been wanting to suggest it before.

  2. ExternalTexturePixelGL will replace ExternalTextureGL later? or Is there any special plan for ExternalTexturePixelGL?
    I look forward to complete this patch 😄

1.Implement GPU buffer texture.
2.Remove ref/unref tbm surface code.
@xiaowei-guan
Copy link
Author

xiaowei-guan commented May 14, 2021

2. ExternalTexturePixelGL will replace ExternalTextureGL later? or Is there any special plan for ExternalTexturePixelGL?
I look forward to complete this patch 😄

I view the Engine code, I find we can call MarkTextureFrameAvailable multiple, It can be successful only once, during the rendering process of one frame . so :
1.we can store the media_packet in a stack, and call MarkTextureFrameAvailable when receive onVideoFrameDecoded.
2.when receive copy buffer callback, we only need return the media_packet which on the top and clear the stack.
3.handle destruction callback.
In this way, we can reduce the lost frames at plugin side.

1.Use C++ style casts
2.Rename variable and function, make the code more friendly.
3.Add copyright info.
1.Rename static veriable(nextTextureId -> kNextTextureId)
2.Modify comment, make it more clear.
3.Rename function name(FlutterDesktopDestructionCallback)
Copy link
Member

@bwikbs bwikbs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can see the end of the long journey 👏
Overall, the texture-related code seems to be neat, so I love it.
Thanks for your hard working!

@xiaowei-guan
Copy link
Author

he texture-related code seems to be neat, so I love it.

I think we can see the end of the long journey 👏
Overall, the texture-related code seems to be neat, so I love it.
Thanks for your hard working!

To complete this task, you gave a lot of useful suggestions, thank you very much~

Copy link
Member

@swift-kim swift-kim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we going to submit the change related to the public API and client_wrapper to the upstream?

@xiaowei-guan
Copy link
Author

Are we going to submit the change related to the public API and client_wrapper to the upstream?

Yes, we want to try submit this change to upstream.
before submit, we still have some work to do:
1.we need implement unit test for gpu buffer texture.
2.we want to implement sample code with gpu buffer texure on windows, to verify our interface.

Copy link

@bbrto21 bbrto21 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@bbrto21 bbrto21 merged commit 9f08d18 into flutter-tizen:flutter-2.0.1-tizen May 27, 2021
This warning log shouldn't be displayed when launching a headless
app.
@xiaowei-guan
Copy link
Author

swift-kim pushed a commit that referenced this pull request Jun 7, 2021
* Add platfrom surface buffer structor

* Texture api change(draft)

1.Delete tizen texture api, use common api, add GpuBufferTexture for gpu
buffer.

2.Delete ref/unref tbm_surface code in engine, add destructioncall for
delete tbm_surface.

3.Support pixel buffer texture.

* Revert code to original

* Support GPU buffer texture

1.Implement GPU buffer texture.
2.Remove ref/unref tbm surface code.

* Change file permission

* Add FlutterDesktopGpuBufferTextureConfig in FlutterDesktopTextureInfo.

* Add buffer parameter at Destruction callback

* Change class name ExternalTextureGL to ExternalTextureSurfaceGL

Class ExternamTextureSurfaceGL handle tbm surface texture render.

* Remove not used code

* Fix wild pointer issue

If plugin call UnregisterTexture, then will remove external texture,
But at this time, gpu render is not finish. when gpu render finished,
tiggle destruction callback, need check whether external texture pointer
is wild pointer

* Convert unique_ptr to shared_ptr when create external texture

* Code format

* Fix arm64 build error

* Remove not used file

* Remove unnecessary headers

* Refactor based on comments

1.Remove not used file.
2.Rename define of head file.
2.Rename of variable.

* Refactor based on comments

1.Return nullptr for default case when create external texture.
2.Rename methode name from texture_registrar to GetTextureRegistrar.
3.Change OnCollectTextur to static function.

* Fix code review issue

1.Use C++ style casts
2.Rename variable and function, make the code more friendly.
3.Add copyright info.

* Fix code review issue

1.Rename static veriable(nextTextureId -> kNextTextureId)
2.Modify comment, make it more clear.
3.Rename function name(FlutterDesktopDestructionCallback)

* Fix code review issue

* Remove warning log

This warning log shouldn't be displayed when launching a headless
app.
swift-kim pushed a commit that referenced this pull request Sep 27, 2021
* Add platfrom surface buffer structor

* Texture api change(draft)

1.Delete tizen texture api, use common api, add GpuBufferTexture for gpu
buffer.

2.Delete ref/unref tbm_surface code in engine, add destructioncall for
delete tbm_surface.

3.Support pixel buffer texture.

* Revert code to original

* Support GPU buffer texture

1.Implement GPU buffer texture.
2.Remove ref/unref tbm surface code.

* Change file permission

* Add FlutterDesktopGpuBufferTextureConfig in FlutterDesktopTextureInfo.

* Add buffer parameter at Destruction callback

* Change class name ExternalTextureGL to ExternalTextureSurfaceGL

Class ExternamTextureSurfaceGL handle tbm surface texture render.

* Remove not used code

* Fix wild pointer issue

If plugin call UnregisterTexture, then will remove external texture,
But at this time, gpu render is not finish. when gpu render finished,
tiggle destruction callback, need check whether external texture pointer
is wild pointer

* Convert unique_ptr to shared_ptr when create external texture

* Code format

* Fix arm64 build error

* Remove not used file

* Remove unnecessary headers

* Refactor based on comments

1.Remove not used file.
2.Rename define of head file.
2.Rename of variable.

* Refactor based on comments

1.Return nullptr for default case when create external texture.
2.Rename methode name from texture_registrar to GetTextureRegistrar.
3.Change OnCollectTextur to static function.

* Fix code review issue

1.Use C++ style casts
2.Rename variable and function, make the code more friendly.
3.Add copyright info.

* Fix code review issue

1.Rename static veriable(nextTextureId -> kNextTextureId)
2.Modify comment, make it more clear.
3.Rename function name(FlutterDesktopDestructionCallback)

* Fix code review issue

* Remove warning log

This warning log shouldn't be displayed when launching a headless
app.
swift-kim pushed a commit that referenced this pull request Nov 14, 2021
* Add platfrom surface buffer structor

* Texture api change(draft)

1.Delete tizen texture api, use common api, add GpuBufferTexture for gpu
buffer.

2.Delete ref/unref tbm_surface code in engine, add destructioncall for
delete tbm_surface.

3.Support pixel buffer texture.

* Revert code to original

* Support GPU buffer texture

1.Implement GPU buffer texture.
2.Remove ref/unref tbm surface code.

* Change file permission

* Add FlutterDesktopGpuBufferTextureConfig in FlutterDesktopTextureInfo.

* Add buffer parameter at Destruction callback

* Change class name ExternalTextureGL to ExternalTextureSurfaceGL

Class ExternamTextureSurfaceGL handle tbm surface texture render.

* Remove not used code

* Fix wild pointer issue

If plugin call UnregisterTexture, then will remove external texture,
But at this time, gpu render is not finish. when gpu render finished,
tiggle destruction callback, need check whether external texture pointer
is wild pointer

* Convert unique_ptr to shared_ptr when create external texture

* Code format

* Fix arm64 build error

* Remove not used file

* Remove unnecessary headers

* Refactor based on comments

1.Remove not used file.
2.Rename define of head file.
2.Rename of variable.

* Refactor based on comments

1.Return nullptr for default case when create external texture.
2.Rename methode name from texture_registrar to GetTextureRegistrar.
3.Change OnCollectTextur to static function.

* Fix code review issue

1.Use C++ style casts
2.Rename variable and function, make the code more friendly.
3.Add copyright info.

* Fix code review issue

1.Rename static veriable(nextTextureId -> kNextTextureId)
2.Modify comment, make it more clear.
3.Rename function name(FlutterDesktopDestructionCallback)

* Fix code review issue

* Remove warning log

This warning log shouldn't be displayed when launching a headless
app.
swift-kim pushed a commit that referenced this pull request Dec 9, 2021
* Add platfrom surface buffer structor

* Texture api change(draft)

1.Delete tizen texture api, use common api, add GpuBufferTexture for gpu
buffer.

2.Delete ref/unref tbm_surface code in engine, add destructioncall for
delete tbm_surface.

3.Support pixel buffer texture.

* Revert code to original

* Support GPU buffer texture

1.Implement GPU buffer texture.
2.Remove ref/unref tbm surface code.

* Change file permission

* Add FlutterDesktopGpuBufferTextureConfig in FlutterDesktopTextureInfo.

* Add buffer parameter at Destruction callback

* Change class name ExternalTextureGL to ExternalTextureSurfaceGL

Class ExternamTextureSurfaceGL handle tbm surface texture render.

* Remove not used code

* Fix wild pointer issue

If plugin call UnregisterTexture, then will remove external texture,
But at this time, gpu render is not finish. when gpu render finished,
tiggle destruction callback, need check whether external texture pointer
is wild pointer

* Convert unique_ptr to shared_ptr when create external texture

* Code format

* Fix arm64 build error

* Remove not used file

* Remove unnecessary headers

* Refactor based on comments

1.Remove not used file.
2.Rename define of head file.
2.Rename of variable.

* Refactor based on comments

1.Return nullptr for default case when create external texture.
2.Rename methode name from texture_registrar to GetTextureRegistrar.
3.Change OnCollectTextur to static function.

* Fix code review issue

1.Use C++ style casts
2.Rename variable and function, make the code more friendly.
3.Add copyright info.

* Fix code review issue

1.Rename static veriable(nextTextureId -> kNextTextureId)
2.Modify comment, make it more clear.
3.Rename function name(FlutterDesktopDestructionCallback)

* Fix code review issue

* Remove warning log

This warning log shouldn't be displayed when launching a headless
app.
@xiaowei-guan xiaowei-guan deleted the flutter-2.0.1-tizen-texture-2 branch December 15, 2021 03:31
swift-kim pushed a commit that referenced this pull request Dec 17, 2021
* Add platfrom surface buffer structure

* Support GPU buffer texture

* Change file permission

* Add FlutterDesktopGpuBufferTextureConfig in FlutterDesktopTextureInfo.

* Add buffer parameter at Destruction callback

* Change class name ExternalTextureGL to ExternalTextureSurfaceGL

* Fix wild pointer issue

* Convert unique_ptr to shared_ptr when create external texture

* Fix arm64 build error
swift-kim pushed a commit that referenced this pull request Feb 7, 2022
* Add platfrom surface buffer structure

* Support GPU buffer texture

* Change file permission

* Add FlutterDesktopGpuBufferTextureConfig in FlutterDesktopTextureInfo.

* Add buffer parameter at Destruction callback

* Change class name ExternalTextureGL to ExternalTextureSurfaceGL

* Fix wild pointer issue

* Convert unique_ptr to shared_ptr when create external texture

* Fix arm64 build error
swift-kim pushed a commit that referenced this pull request Feb 11, 2022
* Add platfrom surface buffer structure

* Support GPU buffer texture

* Change file permission

* Add FlutterDesktopGpuBufferTextureConfig in FlutterDesktopTextureInfo.

* Add buffer parameter at Destruction callback

* Change class name ExternalTextureGL to ExternalTextureSurfaceGL

* Fix wild pointer issue

* Convert unique_ptr to shared_ptr when create external texture

* Fix arm64 build error
swift-kim pushed a commit that referenced this pull request May 12, 2022
* Add platfrom surface buffer structure

* Support GPU buffer texture

* Change file permission

* Add FlutterDesktopGpuBufferTextureConfig in FlutterDesktopTextureInfo.

* Add buffer parameter at Destruction callback

* Change class name ExternalTextureGL to ExternalTextureSurfaceGL

* Fix wild pointer issue

* Convert unique_ptr to shared_ptr when create external texture

* Fix arm64 build error
swift-kim pushed a commit that referenced this pull request Aug 5, 2022
* Add platfrom surface buffer structure

* Support GPU buffer texture

* Change file permission

* Add FlutterDesktopGpuBufferTextureConfig in FlutterDesktopTextureInfo.

* Add buffer parameter at Destruction callback

* Change class name ExternalTextureGL to ExternalTextureSurfaceGL

* Fix wild pointer issue

* Convert unique_ptr to shared_ptr when create external texture

* Fix arm64 build error
swift-kim pushed a commit that referenced this pull request Sep 1, 2022
* Add platfrom surface buffer structure

* Support GPU buffer texture

* Change file permission

* Add FlutterDesktopGpuBufferTextureConfig in FlutterDesktopTextureInfo.

* Add buffer parameter at Destruction callback

* Change class name ExternalTextureGL to ExternalTextureSurfaceGL

* Fix wild pointer issue

* Convert unique_ptr to shared_ptr when create external texture

* Fix arm64 build error
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants