Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Fix ImageReader may leak images when onDraw() not called. #23909

Closed
wants to merge 2 commits into from
Closed

Fix ImageReader may leak images when onDraw() not called. #23909

wants to merge 2 commits into from

Conversation

eggfly
Copy link
Member

@eggfly eggfly commented Jan 25, 2021

The onDraw() may not called after invalidate(), this maybe reproduced on some Android devices.
And also ImageReader related logics can be simplified.

image

The imageQueue in old implementation may leaves two or more items after acquireLatestImage() and occure the native level log:
"unable to acquire a buffer item, very likely client tried to acquire more than maximages buffers" in https://android.googlesource.com/platform/frameworks/base/+/master/media/jni/android_media_ImageReader.cpp#517
Because there's no guarantee that onDraw() must be called after invalidate() when device sleeping or on some special devices.

It fixes:
flutter/flutter#63897
flutter/flutter#70290

It fixes:
flutter/flutter#63897
flutter/flutter#70290

The onDraw() may not called after invalidate(), this maybe reproduced on some Android devices.
And also ImageReader related logics can be simplified.
@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@hssdx
Copy link

hssdx commented Jan 25, 2021

牛批,解决了我的难题

@eggfly eggfly changed the title Fix ImageReader may leak images when onDraw() not called Fix ImageReader may leak images when onDraw() not called. Jan 26, 2021
@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@blasten
Copy link

blasten commented Jan 28, 2021

Thanks for the contribution. A few questions:

  1. what devices did you use to test?
  2. this change would need some tests
  3. the code needs to be formatted

@chinmaygarde
Copy link
Member

@eggfly Can you clarify @blasten s queries please so we can make progress on this patch?

@eggfly
Copy link
Member Author

eggfly commented Feb 5, 2021

@chinmaygarde I will soon make a progress.
@blasten I will do the thress things. Thank you for your advises! And also, which code segment lines should be formatted and what's the recommend styles and rules to format the java code of flutter?

@eggfly
Copy link
Member Author

eggfly commented Feb 7, 2021

I made a new pull request here with some new progress on this patch: #24272
So close this old pull request. Thanks @chinmaygarde @blasten ~

@eggfly eggfly closed this Feb 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants