[Android] Add a way to request new Surfaces from SurfaceProducer and avoid SurfaceProducer returning invalid Surface

[camsim99]

Note

For anyone reviewing this PR, see flutter/packages#9360 for how I'd use the getSurface(boolean forceNewSurface) method.

Note

For anyone coming across this PR post-landing, in the code review process, I renamed getSurface(boolean forceNewSurface) to getForcedNewSurface().

What this does

(1) Adds method getSurface(boolean forceNewSurface) to SurfaceProducer that will force the creation of a new Surface when SurfaceProducer.getSurface() is called.
(2) Fixes SurfaceProducer to avoid returning invalid Surfaces when SurfaceProducer.getSurface()/getSurface(boolean forceNewSurface) is called.

Why we should...

(1) Add getSurface(boolean forceNewSurface)

My motivation for adding this is directly tied to #155294. The camera_android_camerax plugin supports a camera preview use case that requires providing a Surface to in order to render the preview. It does so via SurfaceProducer; we provide a Surface retrieved from SurfaceProducer.getSurface() to the CameraX library to render a camera preview, and when the camera preview is done, a callback that we provide is called, reporting that Surface as used and that it now should be released/invalidated (see SurfaceRequest, SurfaceRequest.Result). However, the CameraX library makes no guarantees about when requests for new Surfaces are made, so the following race condition can happen:

1. CameraX requests a Surface
2. We provide Surface A from SurfaceProducer.getSurface()
3. The camera preview was paused; CameraX requests a new Surface
4. The camera preview is resumed
5. We provide Surface A from SurfaceProducer.getSurface() because it is still technically valid
6. CameraX calls our callback and now Surface A is released/invalid

I believe SurfaceProducer currently has no way to handle the level of complexity of Surface handling required for use cases like the camera_android_camerax camera preview, where a Surface may be invalidated between calls to SurfaceProducer.getSurface(). So, getSurface(boolean forceNewSurface) can support these use cases.*

getSurface(boolean forceNewSurface) simply will force SurfaceProducer to return a new Surface when SurfaceProducer.getSurface is called versus potentially returning the same Surface as the previous invocation.

For camera_android_camerax, see flutter/packages#9360 for an example of how it would be used.

*I'd like to note that I also played around with creating new SurfaceProducers to solve the camera problem, which was successful and probably could be generally for that use case, but I think it's cleaner to support this functionality from within the same SurfaceProducer since we can 🤷‍♀️ It also helps avoid the user from needing to create/manage multiple Flutter Textures, from my understanding.

(2) Fix SurfaceProducer.getSurface()/getSurface(boolean forceNewSurface) to never return an invalid Surfaces

I noticed that SurfaceProducer.getSurface()/getSurface(boolean forceNewSurface) does not check for Surface validity before returning them while I was working on (1). Honestly, this just feels right? We should ensure that a Surface is valid and can be used before returning it and potentially confusing the user.

Pre-launch Checklist

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• I read the Tree Hygiene wiki page, which explains my responsibilities.
• I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
• I signed the CLA.
• I listed at least one issue that this PR fixes in the description above.
• I updated/added relevant documentation (doc comments with ///).
• I added new tests to check the change I am making, or this PR is test-exempt.
• I followed the breaking change policy and added Data Driven Fixes where supported.
• All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

[jonahwilliams]

LGTM

[matanlurey]

What do you think of this API, versus possible alternatives?

• void invalidateSurface() (surface = null), which is called before getSurface to force a new surface.
• Surface getForceNewSurface(), identical this one, but without using a boolean parameter.

[camsim99]

To clarify, you suggesting we provide two different ways to force getting a new surface, i.e. a user could call invalidateSurface then getSurface or just call getForceNewSurface?

void invalidateSurface() (surface = null), which is called before getSurface to force a new surface.

Also, as an aside, I think we should avoid the invalidateSurface name because we aren't actually invalidating the Surface. Maybe requestNewSurface or something?

[matanlurey]

Yeah I just know if getSurface(true) really ... screams anything, like I wouldn't notice it in a code review.

[camsim99]

Hahaha no that's such a good point. I'm thinking we do Surface getForceNewSurface() then so it's clearer.

Edit: Chose getForcedNewSurface so "forced new" is kinda like a descriptor for the surface.

[camsim99]

Gonna go ahead and land this! I'll monitor the dashboard too in case I need to revert in case folks aren't around.