Hi Steve,

I think your first approach is fine, i.e. having the camera return the
stride through a read-only key/value pair.

I'd like to understand more about dynamic changes in the preview heap.
Is it the case that the preview heap size may change while preview is
active? Also, is the preview heap chaning in response to a request by
the application, or is it something that can happen at any time due to
some internal state in the camera driver changing?

We are making a lot of changes in the area around the camera and video
sinks right now to support video overlay, which is leading to better
hardware abstraction. One area that we still don't have a good
solution for is surfacing device specific capabilities, particularly
where they cross driver boundaries. A good example of this is for
video recording, where the camera sensor has certain capabilities and
the video encoder has certain capabilities, and the actual device
capabilities are an intersection of the two.

On Jan 26, 7:10 am, steve2641 <steve2...@gmail.com> wrote:
> Hello,
>
> Within the startPreview() function of Camera Services there are
> several hardcoded values that need to be addressed in order to
> properly support different preview formats, buffer strides, and
> potentially dynamic preview heap re-allocations.  This last point
> would be needed in cases such as image stabilization where the preview
> heap may need to be re-sized on the fly.
>
> I'm looking for feedback on a couple of possible approaches for this
> support.
>
> The first approach would be to simply extend what currently exists, as
> in:
>
> =================================
> (CameraService.cpp, changes starting ~line 272)
>     // XXX: This needs to be improved. remove all hardcoded stuff
>
>     int w, h, ws, hs, frmt;
>     CameraParameters params(mHardware->getParameters());
>     params.getPreviewSize(&w, &h);
>     // New "preview-stride" = "<wstride>x<hstride>" parameter (new
> params function)
>     // Would have to be set only by HAL, the app should not set it
>     params.getPreviewStride(&ws, &hs);
>     // Retrieve the preview format and convert to a surface format
> (new function)
>     frmt = convertToSurfaceFormat( params.getPreviewFormat() );
>
>     mSurface->unregisterBuffers();
>
> #if DEBUG_DUMP_PREVIEW_FRAME_TO_FILE
>     debug_frame_cnt = 0;
> #endif
>
>     status_t ret = mHardware->startPreview(previewCallback,
>                                            mCameraService.get());
>     if (ret == NO_ERROR) {
>         mSurface->registerBuffers(w,h,ws,hs,frmt,
>                                    mHardware->getPreviewHeap());
>     }
>     else LOGE("mHardware->startPreview() failed with status %d\n",
>               ret);
> =================================
>
> This approach would add an odd camera parameter, in that only the HAL
> should set it, but this would prevent any HAL API changes.  Another
> option would be to add a "mHardware->getPreviewStride()" function to
> get the needed stride data, but this obviously would require a HAL API
> change.  Also, this approch does not address the possibility of the
> preview heap being re-allocated, so it's only a partial solution.
>
> If the HAL API is going to be modified, I'd propose a more complete
> solution which adds a new HAL -> Camera Services callback to denote
> when the preview heap has changed, as in:
>
> =================================
> (CameraHardwareInterface.h, add ~line 28)
>
> /** Callback when the preview heap is updated */
> typedef void (*pvheap_callback)(sp<IMemoryHeap> heap, int w, int h,
> int wStride, int hStride, int format, void* user);
>
> (CameraHardwareInterface.h, change ~line 91)
> virtual status_t startPreview(preview_callback frameCb,
> pvheap_callback heapCb, void* user) = 0;
>
> (CameraService.cpp, changes starting ~line 272)
> #if DEBUG_DUMP_PREVIEW_FRAME_TO_FILE
>     debug_frame_cnt = 0;
> #endif
>
>     // The Preview Heap Callback will update the frame heap
>
>     status_t ret = mHardware->startPreview(previewCallback,
>                                            pvHeapCallback,
>                                            mCameraService.get());
>     if (ret != NO_ERROR)
>         LOGE("mHardware->startPreview() failed with status %d\n",
> ret);
>
> (CameraService.cpp, add)
> // preview heap callback - called when an aspect of the preview heap
> changes
> void CameraService::Client::pvHeapCallback
> (sp<IMemoryHeap> heap, int w, int h, int wStride, int hStride, int
> format, void *user)
> {
>     LOGV("pvChangeCallback");
>     sp<Client> client = getClientFromCookie(user);
>     if (client == 0) {
>         return;
>     }
>
>     Mutex::Autolock lock(client->mLock);
>
>     client->mSurface->unregisterBuffers();
>     client->mSurface->registerBuffers(w, h, wStride, hStride, format,
> heap);}
>
> =================================
>
> With this approach HAL would be required to call the pvheap_callback
> function whenever the preview heap is allocated or re-allocated,
> including the initial allocation.  The pvHeap_callback would have to
> be called prior to a frame using the new heap is provided via the
> preview_callback function.  I'm not sure if there are any impacts to
> Surface Flinger here, I hope not.  Also, as a cleanup the HAL
> getPreviewHeap() function could potentially be removed.
>
> Obviously I prefer the latter approach, since it solves all of the
> issues I was attempting to cover, but the cost is a HAL API change.
>
> Any comment or other views?
>
> Steve.
--~--~---------~--~----~------------~-------~--~----~
unsubscribe: android-porting+unsubscr...@googlegroups.com
website: http://groups.google.com/group/android-porting
-~----------~----~----~----~------~----~------~--~---

Reply via email to