5

As pointed out in the Android documentation on handling the camera, it is recommended to use a separate thread to open the camera.

Well, I am doing just that but do have some difficulties:

For my camera object I am using a global instance variable. Now, when I start my app, I create a separate thread in onResume() and do all initializations for that camera object in that thread.

Later when I leave the app, I release the camera in onPause(). This works all just fine.

But the problem is: When I did some stress tests and switched really fast between onResume() and onPause() (by hitting the multitask button excessively fast) my app crashed. The reason was that there was a Method called after release().

This makes sense since it is possible that the camera may be released in onPause() but at the same time the thread hasn't finished with its initialization. Thus, the thread tries to make a call on a camera object that was already released.

Now, what can I do to fix this? Maybe not use a global camera object? Or how can I make this thread safe?

user2426316
  • 7,131
  • 20
  • 52
  • 83

2 Answers2

11

You have a couple of options.

Option #1: only touch the Camera from the UI thread.

This is pretty straightforward, since you can manage the camera in onPause() and onResume(). If you want to do work with the Camera object from another thread, you can just use runOnUiThread(). The problem with this approach is that you don't want to do significant amounts of work on the UI thread, so you have to send the preview data elsewhere.

This approach was used in the "Show + capture camera" activity in Grafika. There's a long-winded comment about the thread management here. Note that the camera preview gets handled by the GL rendering thread.

Option #2: only touch the Camera from a dedicated camera thread.

For this to work, you need to send messages to the thread every time you want it to do something with the Camera. This is most easily done with the usual Handler and Looper arrangement. The trick is that, when you send a message, the call returns immediately. If you send a message to tell the thread to shut the camera down from onPause(), the actual camera shutdown may not happen for a while -- and in the mean time, the activity is shutting other things down, and (if events are happening quickly) might be back in onResume() before the shutdown has completed. (I suspect this is what's happening to you.)

For startup and shutdown, you need to wait for completion. You can see an example of this in a different (non-Camera) Grafika activity, where it waits for the render thread to finish initializing before sending it messages. The shutdown is synchronized by join()ing the thread as it stops.

The key thing to remember is that you can only call Camera methods from a single thread -- whatever thread you use to open the camera is the only thread that gets to touch that instance. After that, it's a "simple" matter of concurrency management. (Strictly speaking, the doc says "this class's methods must never be called from multiple threads at once", so you can use multiple threads if you serialize the access carefully. The easiest way to serialize access is to only do the calls from one thread.)

fadden
  • 51,356
  • 5
  • 116
  • 166
  • So I guess Option #2 is the one to go with. So just to make sure I got this right: my camera gets handled in a dedicated thread. If then in my main thread `onPause()` gets called, I then send a signal to the camera thread, wait for it to shutdown the camera and only then let the main thread finish with `onPause()`. – user2426316 Mar 09 '14 at 17:53
  • Yes. That should eliminate two possible races (continuing to use the Camera after it has been closed in a different thread, and attempting to reopen the camera before it has finished closing in a different thread). The easiest way is to have the dedicated thread close the camera and then exit; that way you can just `join()` the thread and know that *everything* the thread needed to clean up is done. – fadden Mar 09 '14 at 18:30
  • I think that `join()` from `onPause()` is not a good idea. The framework expects `onPause()` to do its job and return quickly. `Camera.release()` itself is fast. So I don't see problems with @Mr.Me's solution. – Alex Cohn Mar 10 '14 at 05:14
  • 1
    `join()` won't be slow unless the thread is slow to stop (e.g. closing the camera is slow). If you don't wait for the camera to shut down, you run the risk that the shutdown won't happen before the next time `onResume()` starts, and you'll crash when trying to re-open an open camera (and you'll be opening it on a different thread than the one that's closing it). One alternative is to ensure that the open in `onResume()` happens in the same thread -- you keep the camera worker thread alive across pause/resume -- ensuring serialization, but that requires some complicated thread management. – fadden Mar 10 '14 at 06:19
  • 1
    You don't need complicated thread management, simply use `Executors.newSingleThreadExecutor()`, or better - create and keep [a HandlerThread](http://stackoverflow.com/a/19154438/192373) dedicated to Camera management. – Alex Cohn Jan 11 '15 at 08:58
1

You can use a boolean flag for camera thread to indicate that the camera is no longer necessary (for example in the case of pause).

so your code is going to be something like this:

    private boolean isNeeded;
    private boolean isInitialized;

    @Override
    protected void onResume(){
        super.onResume();
        Thread thread = new Thread(){
            @Override
            public void run(){
                // getCamera();
                // initializeCamera();
                if (isNeeded){
                    inInitialized = true;
                    // do stuff with Camera


                } else {
                    //release camera here
                }
            }

        }
        thread.start();
    }

    protected void onPause(){
        super.onPause();
        isNeeded = false;
        if (isInitialized){
            //release camera here
        }
    }

so your initializing thread checks if the camera isn't needed and release it if so, or initialize the camera successfully and you release the camera from the onPause.

Mr.Me
  • 9,192
  • 5
  • 39
  • 51
  • 1
    `isInitialized` and `isNeeded` must be declared `volatile` or this can fail. (This is essentially the same pattern as "double checked locking" -- google for that, or dive into http://developer.android.com/training/articles/smp.html.) You're also potentially initializing the camera on one thread and releasing it from another thread, which is exactly what you're not supposed to do. – fadden Mar 09 '14 at 17:00
  • @fadden it seems like I need to understand java memory model, so thanks for pointing that out, my question is why does my answer work. I always use non volatile boolean flags and they get updated in any background thread, why is that if volatile is absolute necessity. – Mr.Me Mar 09 '14 at 17:26
  • 1
    It's a race. Most of the time you're going to win, but it's possible that you'll lose and things will behave very strangely. There's enough going on around pause/resume that you'll *probably* never have a problem with general sync issues, but you still have to worry about optimizing compilers... if those boolean flags aren't marked volatile, the compiler is free to assume that only one thread can access them, and might elide loads and stores where you really want them. Not currently a problem with javac + dx + Dalvik, but those are changing. – fadden Mar 09 '14 at 17:40