[camera]fix a sample buffer memory leak on pause resume recording #5927
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jmagman
reviewed
Jan 18, 2024
Member
There was a problem hiding this comment.
Is CF_RETURNS_RETAINED needed with the copy rename? I thought that was only needed if it was a copy but the name didn't include it.
Contributor
Author
There was a problem hiding this comment.
you are right. let me remove it. just wanted to put copy in the name so +1 is clear from caller site
Member
There was a problem hiding this comment.
Is the bug that this is retained but not released before the if(_audioIsDisconnected) early returns?
Member
There was a problem hiding this comment.
(I think you're right it doesn't need to be retained at all since sampleBuffer isn't used outside the scope of this method)
Clients that need to reference the CMSampleBuffer object outside of the scope of this method must CFRetain it and then CFRelease it when they are finished with it.
hellohuanlin
force-pushed
the
camera_sample_buffer_leak_on_pause_resume
branch
from
0a0ac12 to
d0bbf91
Compare
remove annotation run format
hellohuanlin
force-pushed
the
camera_sample_buffer_leak_on_pause_resume
branch
from
ad79bbc to
d08c2d4
Compare
jmagman
approved these changes
Jan 19, 2024
hellohuanlin
added
the
autosubmit
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Jan 22, 2024
auto-submit bot
pushed a commit
to flutter/flutter
that referenced
this pull request
Jan 22, 2024
flutter/packages@129e08c...e4cbf23 2024-01-21 [email protected] Roll Flutter from ddf60fb to 5dea6b9 (5 revisions) (flutter/packages#5951) 2024-01-21 [email protected] Update platform label rules for shared iOS/macOS (flutter/packages#5801) 2024-01-20 [email protected] [pigeon] Support other hosts in generated file CI checks (flutter/packages#5944) 2024-01-20 [email protected] [pigeon] Improve style of generated Swift code (flutter/packages#5938) 2024-01-20 [email protected] Roll Flutter from 684247a to ddf60fb (12 revisions) (flutter/packages#5949) 2024-01-20 [email protected] [camera]fix a sample buffer memory leak on pause resume recording (flutter/packages#5927) 2024-01-19 [email protected] [ci] Run Swift formatter and linter during CI formatting (flutter/packages#5928) 2024-01-19 [email protected] Manual roll Flutter from f77f824 to 684247a (39 revisions) (flutter/packages#5948) 2024-01-19 [email protected] Expose registered widget libraries and local widget library widgets. (flutter/packages#5936) 2024-01-19 49699333+dependabot[bot]@users.noreply.github.com Bump actions/upload-artifact from 4.1.0 to 4.2.0 (flutter/packages#5937) 2024-01-19 [email protected] Manual roll Flutter (stable) from ef1af02 to 67457e6 (1 revision) (flutter/packages#5932) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
16 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This memory leak happens when pause/resume the recording. Depending on camera resolution, about 1-2MB leaks for every resume, so it can add up pretty fast if we frequently pause/resume.
We only reference this sample buffer inside the scope of this method (e.g. directly piping into file IO), so no need to retain it again, as it's already retained and will be released afterwards by apple. (We did keep around the pixel buffer, but we already manually retain and release that)
Bug was introduced in flutter/plugins#1370.
List which issues are fixed by this PR. You must list at least one issue.
flutter/flutter#132013
If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.
Pre-launch Checklist
dart format.)[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style.///).If you need help, consider asking for advice on the #hackers-new channel on Discord.