Camera API: Excessive GC caused by preview callbacks

[Tom Gibara]

[This is a repost from the now closed android-framework  - I'm reposting now because the issue is collecting some useful comments that probably don't belong there]

I want to start a discussion on how the Camera API could be substantially improved regarding the performance of preview callbacks. As far as I am able to tell (by browsing the git repository) no coding has been done to address it.

The core of the problem is that the glue between the native Camera code and the Java framework (in the form of android_hardware_Camera.cpp)  allocates a new Java array on each preview frame delivered to the PreviewCallback supplied by the application. In short, this generates crazy amounts of garbage and kills performance. A defect has been reported at: http://code.google.com/p/android/issues/detail?id=2794

Having read some of the source code I remain ignorant as to why the API operates in the way it does, and have assumed that the API is strongly moulded by hardware/driver constraints. So I've tried to remain within conservative (but guessed) boundaries for how the API can be adjusted.

My suggestion (which I acknowledge up-front might be unworkable since I don't understand the constraints) is to expose a ByteBuffer via the Camera class, together with a read lock, something like:

  public ByteBuffer getPreviewData();

  public  Lock getPreviewLock();

The ByteBuffer would be backed directly by system memory that stores the preview. Java applications wanting to use this data would have to:

  * always obtain the lock before reading data from the buffer

  * always explicitly release the lock after acquiring it

  * accommodate whatever data encoding is used within the buffer

The application developer would essentially have two (and a half) choices:

  1a) On acquiring the lock, copy the ByteBuffer into a Java array, release the lock, process the preview. In this instance we are better off because the developer is in control and knows how/when to reuse the Java array. Excessive garbage collection can be avoided.

  1b) As above except releasing the lock after processing has finished. This would prevent the framework from supplying frame previews that the application isn't ready to accept (see below).

  2) Or, after acquiring the lock within the Java code, the ByteBuffer can be passed to native code for processing, after which the Java application would release the lock (though lock control could be performed by the native code). This avoids mem-copying to the greatest extent possible and should provide good performance.

The framework would attempt to take the lock before writing preview data to the ByteBuffer and would release it immediately thereafter. If the lock were already held (ie. by the application) then the preview frame would simply be discarded. Discarding data in this way prevents applications from blocking the system thread that is posting the preview frame. It also allows them to signal back to the framework that they are not ready for more data, avoiding unnecessary writes to the buffer.

I realize that this approach will probably force the framework to maintain an extra preview buffer (since I assume that the camera driver writes directly and unimpeded to the currently used buffer). For this reason, the ByteBuffer would not be used by the framework, or made available to an application, until a callback had been registered to indicate the application's readiness to receive preview data in this way, something like:

public interface PreviewDataCallback {

  void onPreviewFrameData(Camera camera);

}

with onPreviewFrameData() called after each write to the ByteBuffer.

In the event that an implementation of this (or something like it) can be realized without an extra buffer, the calls to PreviewCallback.onPreviewFrame() could then be driven from the ByteBuffer. In either case, I can't see any reason why this improved API couldn't coexist with the current implementation. But comments on its viability are most welcome - I don't know enough to say if this suggestion is workable and I'd love to see improvements in this area.

Tom.

[Sergey Povzner]

I'm a little surprised this issue didn't get more attention in this
group. I saw a number of threads about the need to convert from YUV to
RGB format, but before you even to get to the point of image
conversion, you have a serious drop in the framerate due to GC calls.

Once the application subscribes to receive Preview Callbacks, you get
continuous GC calls even if your handler is empty and is not doing
anything. The main advice on writing real-time apps is to avoid GC by
all possible means, yet with the current implementation of the Preview
Callback it seems impossible to avoid GC. Any ideas on possible
workarounds are appreciated.

[Tom Gibara]

I was surprised too. The issue is starting to gather comments, but I think they're best captured here.

I actually find the repeated GCs more visually disturbing when the application's intrinsic framerate is higher - it causes bursts of smooth motion to be punctuated by long pauses. I can work around other limitations in the camera API, but there's no way I can hide this one from the users. For this reason I think it's the most serious camera issue.

Tom.

2009/10/2 Sergey Povzner <ser...@gmail.com>

--
Tom Gibara
email: m...@tomgibara.com
web: http://www.tomgibara.com
blog: http://blog.tomgibara.com
twitter: tomgibara

[Martin Storsjö]

On Fri, 2 Oct 2009, Tom Gibara wrote:

> I want to start a discussion on how the Camera API could be substantially
> improved regarding the performance of preview callbacks. As far as I am able
> to tell (by browsing the git repository) no coding has been done to address
> it.
> The core of the problem is that the glue between the native Camera code and
> the Java framework (in the form of android_hardware_Camera.cpp)  allocates a
> new Java array on each preview frame delivered to the PreviewCallback
> supplied by the application. In short, this generates crazy amounts of
> garbage and kills performance. A defect has been reported
> at: http://code.google.com/p/android/issues/detail?id=2794

Yes, I'd like to chime in on this - I've encountered the same issue, and
it's absolutely killing the performance.

> Having read some of the source code I remain ignorant as to why the API
> operates in the way it does, and have assumed that the API is strongly
> moulded by hardware/driver constraints. So I've tried to remain within
> conservative (but guessed) boundaries for how the API can be adjusted.

I've got another suggestion, that ought to fit in as a backwards
compatible enhancement that doesn't break existing applications using the
current API.

In addition to the current PreviewCallback interface, another interface
GetBufferCallback would be added. This interface has one method, byte[]
getPreviewBuffer(int size), that is called for each frame that would be
returned by onPreviewFrame.

If no GetBufferCallback is set, the API would work just as it does now,
allocating a new byte[] for each frame, otherwise the buffer returned by
the callback is used for returning the next frame in onPreviewFrame.
If the getPreviewBuffer method returns null, it signals that the
application doesn't want any frame at the moment (e.g. still busy
processing the last one).

Then all buffer management is up to the application, which can choose to
reuse one single buffer for all processing, or use a pool of buffers.

Does this sound sensible? I could try hacking this together as a proper
patch if you want to try my idea out - the modifications it requires
really are quite minimal.

// Martin

[Gergely Kis]

Hello,

I think it would be a good idea to collect the use cases of the API
that we want to create.

For the Camera.PreviewCallback I can see 1 main use case: get access
to the raw frames without compression for further processing. (Simply
displaying the frames is not an issue, since there is an extra API for
that.)
Now there are basically 2 variants that I can see:
1/a: Process the frames in java code
1/b: Process the frames in native code

I think that in such resource intensive algorithms, like image
processing, the usage of native code will increase, so the API should
try to be efficient for such use cases.

For handling the actual callback I would like to make the following proposal:

PreviewCallback interface:
onPreviewCallback(CameraBuffer buffer)

CameraBuffer interface:
void release()
int size()
byte[] toArray(byte[] buffer)
int handle()

This interface implements the Dispose design pattern, which can be
probably seen as a corner case of the reference counted object design
pattern.

A Camera implementation would allocate a set of objects that implement
the CameraBuffer interface. These objects are filled with the image
data and passed to the callback. When the application is done with the
processing it calls the release() method on the buffer, which in turn
tells the Camera implementation, that the buffer can be reused for the
next frame.

The frame data can be accessed by using the toArray() method. It
follows the pattern of the Java Runtime: If the parameter is not null,
and big enough to hold the image data, it is used, otherwise a new
array is allocated.

Why not simply have a byte[] getBuffer() method in the interface? Here
comes the 1/b use case.

If this object is passed to native code, an appropriate NDK API may be
used to get direct access to the memory buffer allocated by the camera
implementation, potentially without copying it.

For example:
const char *CameraBuffer_Get_Pointer(int handle) // value of the
handle() method

This solution has the following advantages in my opinion:
- It makes the handling of callbacks more efficient, no GC hell
- It keeps the buffer allocation handling in the camera
implementation. This way, depending on the camera hardware, the
implementation can perform additional memory optimizations.
- The interface makes sure that the image data is only copied to a
java array, when it is needed. If it will be processed by an algorithm
that is implemented in native code, it can be passed through without
unnecessary copying.
- The application may still apply its own additional buffer management
algorithms.

What do you think?

There are of course many details to be worked out, e.g. whether
putting a releaseBuffer(CameraBuffer) method into the Camera class
makes more sense than using release() on CameraBuffer. I am not sure
whether the handle() method is necessary or not.

Also, instead of changing PreviewCallback, probably a new name will
have to be introduced to maintain backwards compatibility.

Best Regards,
Gergely

[Tom Gibara]

On the surface this does seem like a fairly unintrusive way of ameliorating the API. It definitely has the strong benefit of simplicity. I have a two reservations though:

1) It doesn't assist native code that needs access to the byte array. Accessing large Java byte arrays from C is expensive (at least one, if not two copies) whereas byte buffers are very cheap. I think most code which needs to execute quickly over the preview data well end up being native code.

2) Passing control back to the application on the thread that drives the camera preview probably invites disaster. What happens if the getPreviewBuffer() implementation blocks (say on a synchronized block?). I suspect the application would crash - just as it does now if the onPreviewFrame() method takes "too long". Repeating an existing weakness of the current API is disappointing.

Tom.

2009/10/3 Martin Storsjö <mar...@martin.st>

[Fred Grott(Android Expert)]

I agree with your cocnerns..

To be blunt I am being kind of thrown as several upcoming things I am
doing will be in this area..

I now its kind of hard before the next code drop for 1.6r2 but I would
like if we could somehow basically come up with summary of the state
of the proposal so that everyone can see
where every thing stands and where we intend to focus on..

Tom, btw Ilked your grab camera source demo..very nice..


one advantage of it being in Java fro now is that we could use before
the APi gets approved..the downside is how do we take care of the
performance issues..

[Tom Gibara]

I think it's unlikely that an additional API would be exposed in the NDK for something that could be handled conveniently by exposing a ByteBuffer (which could then be passed to a native method) to the application.

Maintaining a recyclable pool of buffers available to the camera implementation does prevent the application from blocking the camera thread and would reduce garbage collection. But it introduces other considerations - how large should the pool be? What happens when the pool is exhausted?

Tom.

2009/10/4 Gergely Kis <gerge...@gmail.com>

[Tom Gibara]

From my perspective, a general concern is: has anything already been done in this area that we aren't aware of?

I don't think I've seen any posts by a core platform developer about the Camera API for a very long time. And the API has seen very little (visible) improvement since its debut. So there's a wide spectrum of possibilities ranging from "nothing" to "rewrite".

Since I'm not in a position to contribute an implementation myself at the moment, I don't think I have the right to make any demands for information. But it would be nice to hear something about what's happening with the camera API, no matter how vague.

Starting this thread was just a way of trying gain ground before coding.

Tom.

2009/10/12 Fred Grott(Android Expert) <fred....@gmail.com>

[Gergely Kis]

On Tue, Oct 13, 2009 at 12:45 AM, Tom Gibara <m...@tomgibara.com> wrote:
> I think it's unlikely that an additional API would be exposed in the NDK for
> something that could be handled conveniently by exposing a ByteBuffer (which
> could then be passed to a native method) to the application.

Using a direct ByteBuffer is probably a good idea. Would it make sense
in your opinion to pass additional information together with the frame
data? E.g. frame timing information might be useful.

> Maintaining a recyclable pool of buffers available to the camera
> implementation does prevent the application from blocking the camera thread
> and would reduce garbage collection. But it introduces other considerations
> - how large should the pool be? What happens when the pool is exhausted?

The pool size could be determined by the implementation, based on the
size of available memory in the device. It could even change the pool
size when conditions change (e.g. if the user starts another
application).
It would be probably a good idea to add an API which can set the
minimum size of the pool.

If the pool is exhausted, then the Camera would simply drop the frame.
Maybe it would make sense to change the callback interface so the
Camera can report the dropped frames, so the application can detect
this situation and act on it (increase the pool size if possible)

Best Regards,
Gergely

> 2009/10/4 Gergely Kis <gerge...@gmail.com>
[...]

[blindfold]

> 1/a: Process the frames in java code
> 1/b: Process the frames in native code

I've already moved the bulk of my live image processing to native code
processing, so any Java-level stubs and preview callbacks in between
for arraycopy'ing to native code only mean unwanted overhead.
Preferably I'd want access to the camera preview buffer directly from
native code (with some locking mechanism to make sure preview data is
always valid when available). Until now I have been unable to reduce
image processing latency to acceptable levels (for my use case) that I
had in Java ME on Nokia phones, and I'm not even sure what is causing
the large content latency in fetching previews on ADP1. Whatever I did
so far, preview content seemed to lag by about two preview frames. It
is my main performance bottleneck now since native code processing is
fast enough. Has anyone else noticed unexpectedly large latencies in
preview *content* as captured through the preview callback?

[Fred Grott(Android Expert)]

I am somewhat in the same boat as you Tom..

Need the functionality but may not be in the position to add code at
the moment because of time constraints..

I will see if I can ask in a different way that may get a Google
response.. I seem to be good at that skill lately..:)

On Oct 12, 5:59 pm, Tom Gibara <m...@tomgibara.com> wrote:
> From my perspective, a general concern is: has anything already been done in
> this area that we aren't aware of?
> I don't think I've seen any posts by a core platform developer about the
> Camera API for a very long time. And the API has seen very little (visible)
> improvement since its debut. So there's a wide spectrum of possibilities
> ranging from "nothing" to "rewrite".
>
> Since I'm not in a position to contribute an implementation myself at the
> moment, I don't think I have the right to make any demands for information.
> But it would be nice to hear something about what's happening with the
> camera API, no matter how vague.
>
> Starting this thread was just a way of trying gain ground before coding.
>
> Tom.
>

> 2009/10/12 Fred Grott(Android Expert) <fred.gr...@gmail.com>

[Anders Johansson]

Hi Tom,

As yet another professional suffering from this problem, I feel
obligated to second your motion in this thread.

As per our earlier discussion in
http://groups.google.com/group/android-developers/browse_thread/thread/a9a8714a97f0911c/5010e9df460d77eb#
I agree fully with your proposal, and would also like to underline the
need to have the preview data delivered in a direct ByteBuffer,
thereby enabling native app code to access it without copying.

If changes are made to the Camera APIs, I would also hope to see a
move away from the current requirement of having a PreviewDisplay
surface set in order to start the camera, as this also has quite
severe effects on performance in an application where the screen
rendering is done differently.

best regards
Anders

p.s. I also hope santa makes it possible to access SkBitmaps from the
NDK, but I digress... d.s.

On Oct 13, 12:59 am, Tom Gibara <m...@tomgibara.com> wrote:
> From my perspective, a general concern is: has anything already been done in
> this area that we aren't aware of?
> I don't think I've seen any posts by a core platform developer about the
> Camera API for a very long time. And the API has seen very little (visible)
> improvement since its debut. So there's a wide spectrum of possibilities
> ranging from "nothing" to "rewrite".
>
> Since I'm not in a position to contribute an implementation myself at the
> moment, I don't think I have the right to make any demands for information.
> But it would be nice to hear something about what's happening with the
> camera API, no matter how vague.
>
> Starting this thread was just a way of trying gain ground before coding.
>
> Tom.
>

> 2009/10/12 Fred Grott(Android Expert) <fred.gr...@gmail.com>

[Anders Johansson]

Well, curiosity got the better of me so I decided to make some changes
to the platform and see what would happen...

What I did was to expose a new API in android.hardware.Camera,
allowing the app to set a direct ByteBuffer as the preview buffer.
I then hooked that into JNICameraContext (android_hardware_camera.cpp)
and changed the copyAndPost function with an if statement allowing
circumvention of the allocation of the infamous byte[] that causes all
the GC, instead doing a memcpy to the bytebuffer if it exists.

All in all, this now causes two memcpy's per frame (one in my code and
one in CameraService::Client::copyFrameAndPostCopiedFrame), but it did
yield a massive performance boost as theorized by Tom and others.

I'm now going to look into a mutex/lock scenario and try to cut down
on the memcpy'ing, to see if further improvements can be made.

Please note that what I've done is a hack, and is in no way or shape
ready to be submitted for review.

However, if you're like me and need to demo something on a single
device, drop me a line and I'll happily provide more details.

best regards
Anders

On Oct 13, 3:30 pm, Anders Johansson <svi...@gmail.com> wrote:
> Hi Tom,
>
> As yet another professional suffering from this problem, I feel
> obligated to second your motion in this thread.
>

> As per our earlier discussion inhttp://groups.google.com/group/android-developers/browse_thread/threa...

[Will 'Varfar']

I'm also suffering from this issue; so will the Camera API ever get
the much-needed overhaul?

* Fast access to the raw frames (could be exposed in the 'preview'
callback)
* support for multiple cameras
* and YUV_??_SP support in libjpeg
* and so on?

I'm curious, who actually can submit stuff? If a 3rd party with a
stake in it goes and makes an API without the current limitations,
will it ever find its way into the platform?

Yours hopefully,

Will