Fixing The Android Camera API

Bozhidar BozhanovApril 2nd, 2014Last Updated: April 1st, 2014

0 65 4 minutes read

The other day I participated in a company hackathon and I decided to make use of the Android camera. I’ve always said that the Android APIs are very bad (to put it mildly), but I’ve never actually tried to explicitly say what is wrong and how it could be better. Until now.

So, the Camera API is crappy. If you haven’t seen it before, take a look for a minute. You can use it in a lot of wrong ways, you can forget many important things, you don’t easily find out what the problem is, even with stackoverflow, and it just doesn’t feel good. To put it differently – there is something wrong with an API that requires you to read a list of 10 steps, some of which are highlighted as “Important”. How would you write that API? Let’s improve it.

And so I did – my EasyCamera wrapper is on GitHub. Below are the changes that I made and the reason behind the change:

setPreviewDisplay(..) is required before startPreview(), and it in turn is required before taking pictures. Why not enforce that? We could simply throw an exception “Preview display not set”, but that’s not good enough. Let’s get rid of the method that sets the preview display and then make startPreview(..) take the surface as parameter. Overload it to be able to take a SurfaceTexture as well.
We’ve enforced the preview setting, so now let’s enforce starting the preview. We can again throw a “Preview not started” exception from takePicture(..), but that happens at runtime. Let’s instead get rid of the takePicture(..) method out of the Camera class and put it in a CameraActions class. Together with other methods that are only valid after preview is started (I don’t know which ones exactly they are – it is not clear from the current API). How do we obtain the CameraActions instance? It is returned by startPreview(..)
So far we’ve made the main use-case straightforward and less error-prone by enforcing the right set of steps. But the takePicture(..) method still feels odd. You can supply nulls to all parameters, but somehow there are two overloaded methods. Arguably, passing null is fine, but there are other options. One is to introduce a PictureCallback interface that has all the four methods that can be invoked, and provide a blank implementation of all of them in a BasePictureCallback class. That, however, might not be applicable in this case, because it makes a difference if you pass null vs callback that does nothing (at least on my phone, if I pass a shutter callback, the shutter sound is played, and if I pass null, it is not). So, let’s introduce the Callbacks which is a builder-like class to contain all callbacks that you like.
So far so good, but you need to restart preview after a picture is taken. And restarting it automatically may not be a good default. But in the situation we are in, you only have CameraActions and calling startPreview(..) now requires a surface to be passed. Should we introduce a restartPreview() method? No. We can make our interface methods return boolean and if the developer wants to restart preview, they should just return true. That would be fine, if there weren’t 4 different callbacks, and calculating the outcome based on all 4 is tricky. That’s why a sensible option is to add a restartPreview property to the Callbacks class, and restart only after the last callback is invoked and only if the property is set to true.
The main process is improved now, but there are other things to improve. For example, there’s an asymmetry between the methods for opening and closing the camera. “open” and “release”. It should be either “open” and “close” or “acquire” and “release”. I prefer to have a .close(), and then (if you can use Java 7), make use of the AutoClosable interface, and therefore the try-with-resource construct.
When you call getParameters() you can a copy of the parameters. That’s ok, but then you should set them back, and that’s counter-intuitive at first. Providing a simple camera.setParameter(..) method would be easier to work with, but the Parameters class has a lot of methods, so it’s not an option to bring them to the Camera class. We can make parameters mutable? (That isn’t implemented yet)
One of the reasons the API is so cumbersome to use is the error reporting. You almost exclusively get “Came.takePicture failed”, regardless of the reason. With the above steps we eliminated the need of exceptions in some cases, but it would still be good to get better reports on the exact reason.
We need to make the Camera mock-friendly (currently it isn’t). So EasyCamera is an interface, which you can easily mock. CameraActions is a simple mockable class as well.

My EasyCamera project is only an initial version now and hasn’t been used in production code, but I’d like to get feedback on whether it’s better than the original and how to get it improved. At some point it can wrap other cumbersome Camera functionality, like getting front and back camera, etc.

Broken APIs are the reason for hundreds of wasted hours, money and neurons. That’s why, when you design an API, you need some very specific skills and need to ask yourself many questions. Josh Bloch’s talk on API design is very relevant and I recommend it. I actually violated one of his advice – “if in doubt, leave it out” – you have access to the whole Camera in your CameraActions, and that might allow you to do things that don’t work. However, I am not fully aware of all features of the Camera, that’s why I didn’t want to limit users and make them invent clumsy workarounds.

When I started this post, I didn’t plan to actually write EasyCamera. But it turned out to take 2 hours, so I did it. And I would suggest that developers, whenever confronted with a bad API, do the above exercise – think of how they would’ve written it. Then it may turn out to be a lot less effort to actually fix it or wrap it, than continue using it as it is.

Reference:

Fixing The Android Camera API from our JCG partner Bozhidar Bozhanov at the Bozho’s tech blog blog.