| Message ID | 20200803161816.107113-11-kieran.bingham@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Kieran On Mon, Aug 03, 2020 at 05:18:14PM +0100, Kieran Bingham wrote: > Generate the ResultMetadata before performing JPEG compression so that > JPEG specific metadata can be added to the metadata when it has been > processed. > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > src/android/camera_device.cpp | 25 ++++++++++++++----------- > 1 file changed, 14 insertions(+), 11 deletions(-) > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp > index ae52a5ca8b86..e23ab055d012 100644 > --- a/src/android/camera_device.cpp > +++ b/src/android/camera_device.cpp > @@ -1138,6 +1138,8 @@ void CameraDevice::requestComplete(Request *request) > const std::map<Stream *, FrameBuffer *> &buffers = request->buffers(); > camera3_buffer_status status = CAMERA3_BUFFER_STATUS_OK; > std::unique_ptr<CameraMetadata> resultMetadata; > + Camera3RequestDescriptor *descriptor = > + reinterpret_cast<Camera3RequestDescriptor *>(request->cookie()); > > if (request->status() != Request::RequestComplete) { > LOG(HAL, Error) << "Request not successfully completed: " > @@ -1145,9 +1147,18 @@ void CameraDevice::requestComplete(Request *request) > status = CAMERA3_BUFFER_STATUS_ERROR; > } > > + /* > + * \todo The timestamp used for the metadata is currently always taken > + * from the first buffer (which may be the first stream) in the Request. > + * It might be appropriate to return a 'correct' (as determined by > + * pipeline handlers) timestamp in the Request itself. > + */ > + FrameBuffer *buffer = buffers.begin()->second; > + resultMetadata = getResultMetadata(descriptor->frameNumber, > + buffer->metadata().timestamp); > + > /* Prepare to call back the Android camera stack. */ > - Camera3RequestDescriptor *descriptor = > - reinterpret_cast<Camera3RequestDescriptor *>(request->cookie()); > + One empty line is enough > > camera3_capture_result_t captureResult = {}; > captureResult.frame_number = descriptor->frameNumber; > @@ -1160,21 +1171,13 @@ void CameraDevice::requestComplete(Request *request) > captureResult.output_buffers = > const_cast<const camera3_stream_buffer_t *>(descriptor->buffers); > > - /* > - * \todo The timestamp used for the metadata is currently always taken > - * from the first buffer (which may be the first stream) in the Request. > - * It might be appropriate to return a 'correct' (as determined by > - * pipeline handlers) timestamp in the Request itself. > - */ > - FrameBuffer *buffer = buffers.begin()->second; > > if (status == CAMERA3_BUFFER_STATUS_OK) { > notifyShutter(descriptor->frameNumber, > buffer->metadata().timestamp); > > captureResult.partial_result = 1; > - resultMetadata = getResultMetadata(descriptor->frameNumber, > - buffer->metadata().timestamp); > + I would drop this one too Overall I think this is fine so far, as we generate metadata statically, but going forward if we have to access the Controls returned by a completed Request, we shall make sure it completed successfully. Can we record that with a \todo entry ? That apart Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> Thanks j > captureResult.result = resultMetadata->get(); > } > > -- > 2.25.1 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Jacopo, On 04/08/2020 12:08, Jacopo Mondi wrote: > Hi Kieran > > On Mon, Aug 03, 2020 at 05:18:14PM +0100, Kieran Bingham wrote: >> Generate the ResultMetadata before performing JPEG compression so that >> JPEG specific metadata can be added to the metadata when it has been >> processed. >> >> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> >> --- >> src/android/camera_device.cpp | 25 ++++++++++++++----------- >> 1 file changed, 14 insertions(+), 11 deletions(-) >> >> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp >> index ae52a5ca8b86..e23ab055d012 100644 >> --- a/src/android/camera_device.cpp >> +++ b/src/android/camera_device.cpp >> @@ -1138,6 +1138,8 @@ void CameraDevice::requestComplete(Request *request) >> const std::map<Stream *, FrameBuffer *> &buffers = request->buffers(); >> camera3_buffer_status status = CAMERA3_BUFFER_STATUS_OK; >> std::unique_ptr<CameraMetadata> resultMetadata; >> + Camera3RequestDescriptor *descriptor = >> + reinterpret_cast<Camera3RequestDescriptor *>(request->cookie()); >> >> if (request->status() != Request::RequestComplete) { >> LOG(HAL, Error) << "Request not successfully completed: " >> @@ -1145,9 +1147,18 @@ void CameraDevice::requestComplete(Request *request) >> status = CAMERA3_BUFFER_STATUS_ERROR; >> } >> >> + /* >> + * \todo The timestamp used for the metadata is currently always taken >> + * from the first buffer (which may be the first stream) in the Request. >> + * It might be appropriate to return a 'correct' (as determined by >> + * pipeline handlers) timestamp in the Request itself. >> + */ >> + FrameBuffer *buffer = buffers.begin()->second; >> + resultMetadata = getResultMetadata(descriptor->frameNumber, >> + buffer->metadata().timestamp); >> + >> /* Prepare to call back the Android camera stack. */ >> - Camera3RequestDescriptor *descriptor = >> - reinterpret_cast<Camera3RequestDescriptor *>(request->cookie()); >> + > > One empty line is enough > >> >> camera3_capture_result_t captureResult = {}; >> captureResult.frame_number = descriptor->frameNumber; >> @@ -1160,21 +1171,13 @@ void CameraDevice::requestComplete(Request *request) >> captureResult.output_buffers = >> const_cast<const camera3_stream_buffer_t *>(descriptor->buffers); >> >> - /* >> - * \todo The timestamp used for the metadata is currently always taken >> - * from the first buffer (which may be the first stream) in the Request. >> - * It might be appropriate to return a 'correct' (as determined by >> - * pipeline handlers) timestamp in the Request itself. >> - */ >> - FrameBuffer *buffer = buffers.begin()->second; >> >> if (status == CAMERA3_BUFFER_STATUS_OK) { >> notifyShutter(descriptor->frameNumber, >> buffer->metadata().timestamp); >> >> captureResult.partial_result = 1; >> - resultMetadata = getResultMetadata(descriptor->frameNumber, >> - buffer->metadata().timestamp); >> + > > I would drop this one too > > Overall I think this is fine so far, as we generate metadata > statically, but going forward if we have to access the Controls > returned by a completed Request, we shall make sure it completed > successfully. Can we record that with a \todo entry ? > I'm not sure I fully understand your comment here, do you mean the libcamera request should be completed successfully before updating the resultMetaData? It will still only be assigned to the captureResult.result if the status == CAMERA3_BUFFER_STATUS_OK check passes still... You'll see how this gets used in 12/12 of course. > That apart > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> Thanks, > > Thanks > j > >> captureResult.result = resultMetadata->get(); >> } >> >> -- >> 2.25.1 >> >> _______________________________________________ >> libcamera-devel mailing list >> libcamera-devel@lists.libcamera.org >> https://lists.libcamera.org/listinfo/libcamera-devel
Hi Jacopo, On 04/08/2020 12:55, Jacopo Mondi wrote: > Hi Kieran, > > On Tue, Aug 04, 2020 at 12:10:45PM +0100, Kieran Bingham wrote: >> Hi Jacopo, >> >> On 04/08/2020 12:08, Jacopo Mondi wrote: >>> Hi Kieran >>> >>> On Mon, Aug 03, 2020 at 05:18:14PM +0100, Kieran Bingham wrote: >>>> Generate the ResultMetadata before performing JPEG compression so that >>>> JPEG specific metadata can be added to the metadata when it has been >>>> processed. >>>> >>>> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> >>>> --- >>>> src/android/camera_device.cpp | 25 ++++++++++++++----------- >>>> 1 file changed, 14 insertions(+), 11 deletions(-) >>>> >>>> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp >>>> index ae52a5ca8b86..e23ab055d012 100644 >>>> --- a/src/android/camera_device.cpp >>>> +++ b/src/android/camera_device.cpp >>>> @@ -1138,6 +1138,8 @@ void CameraDevice::requestComplete(Request *request) >>>> const std::map<Stream *, FrameBuffer *> &buffers = request->buffers(); >>>> camera3_buffer_status status = CAMERA3_BUFFER_STATUS_OK; >>>> std::unique_ptr<CameraMetadata> resultMetadata; >>>> + Camera3RequestDescriptor *descriptor = >>>> + reinterpret_cast<Camera3RequestDescriptor *>(request->cookie()); >>>> >>>> if (request->status() != Request::RequestComplete) { >>>> LOG(HAL, Error) << "Request not successfully completed: " >>>> @@ -1145,9 +1147,18 @@ void CameraDevice::requestComplete(Request *request) >>>> status = CAMERA3_BUFFER_STATUS_ERROR; >>>> } >>>> >>>> + /* >>>> + * \todo The timestamp used for the metadata is currently always taken >>>> + * from the first buffer (which may be the first stream) in the Request. >>>> + * It might be appropriate to return a 'correct' (as determined by >>>> + * pipeline handlers) timestamp in the Request itself. >>>> + */ >>>> + FrameBuffer *buffer = buffers.begin()->second; >>>> + resultMetadata = getResultMetadata(descriptor->frameNumber, >>>> + buffer->metadata().timestamp); >>>> + >>>> /* Prepare to call back the Android camera stack. */ >>>> - Camera3RequestDescriptor *descriptor = >>>> - reinterpret_cast<Camera3RequestDescriptor *>(request->cookie()); >>>> + >>> >>> One empty line is enough >>> >>>> >>>> camera3_capture_result_t captureResult = {}; >>>> captureResult.frame_number = descriptor->frameNumber; >>>> @@ -1160,21 +1171,13 @@ void CameraDevice::requestComplete(Request *request) >>>> captureResult.output_buffers = >>>> const_cast<const camera3_stream_buffer_t *>(descriptor->buffers); >>>> >>>> - /* >>>> - * \todo The timestamp used for the metadata is currently always taken >>>> - * from the first buffer (which may be the first stream) in the Request. >>>> - * It might be appropriate to return a 'correct' (as determined by >>>> - * pipeline handlers) timestamp in the Request itself. >>>> - */ >>>> - FrameBuffer *buffer = buffers.begin()->second; >>>> >>>> if (status == CAMERA3_BUFFER_STATUS_OK) { >>>> notifyShutter(descriptor->frameNumber, >>>> buffer->metadata().timestamp); >>>> >>>> captureResult.partial_result = 1; >>>> - resultMetadata = getResultMetadata(descriptor->frameNumber, >>>> - buffer->metadata().timestamp); >>>> + >>> >>> I would drop this one too >>> >>> Overall I think this is fine so far, as we generate metadata >>> statically, but going forward if we have to access the Controls >>> returned by a completed Request, we shall make sure it completed >>> successfully. Can we record that with a \todo entry ? >>> >> >> I'm not sure I fully understand your comment here, do you mean the >> libcamera request should be completed successfully before updating the >> resultMetaData? > > What I meant to say is that before this change the metadata buffer was > generated in the > if (status == CAMERA3_BUFFER_STATUS_OK) > case, while now, if I got this right, it's generated unconditionally. > When we'll assemble it inspecting the ControlList associated with the > just completed Request, if it has not completed succesfully, we might > try to access an invalid control list or a control list with unset > values. If that's correct, I think it has to be recorded. No, I don't touch any control list. The added information comes only from the generated JPEG content, which is local to the android layer, not the underlying libcamera stream. -- K > Thanks > j > >> >> It will still only be assigned to the captureResult.result if the >> status == CAMERA3_BUFFER_STATUS_OK >> >> check passes still... >> >> You'll see how this gets used in 12/12 of course. >> >> >>> That apart >>> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> >> >> Thanks, >> >> >>> >>> Thanks >>> j >>> >>>> captureResult.result = resultMetadata->get(); >>>> } >>>> >>>> -- >>>> 2.25.1 >>>> >>>> _______________________________________________ >>>> libcamera-devel mailing list >>>> libcamera-devel@lists.libcamera.org >>>> https://lists.libcamera.org/listinfo/libcamera-devel >> >> -- >> Regards >> -- >> Kieran
Hi Kieran, On Tue, Aug 04, 2020 at 12:10:45PM +0100, Kieran Bingham wrote: > Hi Jacopo, > > On 04/08/2020 12:08, Jacopo Mondi wrote: > > Hi Kieran > > > > On Mon, Aug 03, 2020 at 05:18:14PM +0100, Kieran Bingham wrote: > >> Generate the ResultMetadata before performing JPEG compression so that > >> JPEG specific metadata can be added to the metadata when it has been > >> processed. > >> > >> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > >> --- > >> src/android/camera_device.cpp | 25 ++++++++++++++----------- > >> 1 file changed, 14 insertions(+), 11 deletions(-) > >> > >> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp > >> index ae52a5ca8b86..e23ab055d012 100644 > >> --- a/src/android/camera_device.cpp > >> +++ b/src/android/camera_device.cpp > >> @@ -1138,6 +1138,8 @@ void CameraDevice::requestComplete(Request *request) > >> const std::map<Stream *, FrameBuffer *> &buffers = request->buffers(); > >> camera3_buffer_status status = CAMERA3_BUFFER_STATUS_OK; > >> std::unique_ptr<CameraMetadata> resultMetadata; > >> + Camera3RequestDescriptor *descriptor = > >> + reinterpret_cast<Camera3RequestDescriptor *>(request->cookie()); > >> > >> if (request->status() != Request::RequestComplete) { > >> LOG(HAL, Error) << "Request not successfully completed: " > >> @@ -1145,9 +1147,18 @@ void CameraDevice::requestComplete(Request *request) > >> status = CAMERA3_BUFFER_STATUS_ERROR; > >> } > >> > >> + /* > >> + * \todo The timestamp used for the metadata is currently always taken > >> + * from the first buffer (which may be the first stream) in the Request. > >> + * It might be appropriate to return a 'correct' (as determined by > >> + * pipeline handlers) timestamp in the Request itself. > >> + */ > >> + FrameBuffer *buffer = buffers.begin()->second; > >> + resultMetadata = getResultMetadata(descriptor->frameNumber, > >> + buffer->metadata().timestamp); > >> + > >> /* Prepare to call back the Android camera stack. */ > >> - Camera3RequestDescriptor *descriptor = > >> - reinterpret_cast<Camera3RequestDescriptor *>(request->cookie()); > >> + > > > > One empty line is enough > > > >> > >> camera3_capture_result_t captureResult = {}; > >> captureResult.frame_number = descriptor->frameNumber; > >> @@ -1160,21 +1171,13 @@ void CameraDevice::requestComplete(Request *request) > >> captureResult.output_buffers = > >> const_cast<const camera3_stream_buffer_t *>(descriptor->buffers); > >> > >> - /* > >> - * \todo The timestamp used for the metadata is currently always taken > >> - * from the first buffer (which may be the first stream) in the Request. > >> - * It might be appropriate to return a 'correct' (as determined by > >> - * pipeline handlers) timestamp in the Request itself. > >> - */ > >> - FrameBuffer *buffer = buffers.begin()->second; > >> > >> if (status == CAMERA3_BUFFER_STATUS_OK) { > >> notifyShutter(descriptor->frameNumber, > >> buffer->metadata().timestamp); > >> > >> captureResult.partial_result = 1; > >> - resultMetadata = getResultMetadata(descriptor->frameNumber, > >> - buffer->metadata().timestamp); > >> + > > > > I would drop this one too > > > > Overall I think this is fine so far, as we generate metadata > > statically, but going forward if we have to access the Controls > > returned by a completed Request, we shall make sure it completed > > successfully. Can we record that with a \todo entry ? > > > > I'm not sure I fully understand your comment here, do you mean the > libcamera request should be completed successfully before updating the > resultMetaData? What I meant to say is that before this change the metadata buffer was generated in the if (status == CAMERA3_BUFFER_STATUS_OK) case, while now, if I got this right, it's generated unconditionally. When we'll assemble it inspecting the ControlList associated with the just completed Request, if it has not completed succesfully, we might try to access an invalid control list or a control list with unset values. If that's correct, I think it has to be recorded. Thanks j > > It will still only be assigned to the captureResult.result if the > status == CAMERA3_BUFFER_STATUS_OK > > check passes still... > > You'll see how this gets used in 12/12 of course. > > > > That apart > > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> > > Thanks, > > > > > > Thanks > > j > > > >> captureResult.result = resultMetadata->get(); > >> } > >> > >> -- > >> 2.25.1 > >> > >> _______________________________________________ > >> libcamera-devel mailing list > >> libcamera-devel@lists.libcamera.org > >> https://lists.libcamera.org/listinfo/libcamera-devel > > -- > Regards > -- > Kieran
Hi Kieran, I think we're not getting each other here :) On Tue, Aug 04, 2020 at 12:54:04PM +0100, Kieran Bingham wrote: > Hi Jacopo, > [snip] > >>>> - /* > >>>> - * \todo The timestamp used for the metadata is currently always taken > >>>> - * from the first buffer (which may be the first stream) in the Request. > >>>> - * It might be appropriate to return a 'correct' (as determined by > >>>> - * pipeline handlers) timestamp in the Request itself. > >>>> - */ > >>>> - FrameBuffer *buffer = buffers.begin()->second; > >>>> > >>>> if (status == CAMERA3_BUFFER_STATUS_OK) { > >>>> notifyShutter(descriptor->frameNumber, > >>>> buffer->metadata().timestamp); > >>>> > >>>> captureResult.partial_result = 1; > >>>> - resultMetadata = getResultMetadata(descriptor->frameNumber, > >>>> - buffer->metadata().timestamp); > >>>> + > >>> > >>> I would drop this one too > >>> > >>> Overall I think this is fine so far, as we generate metadata > >>> statically, but going forward if we have to access the Controls > >>> returned by a completed Request, we shall make sure it completed > >>> successfully. Can we record that with a \todo entry ? > >>> > >> > >> I'm not sure I fully understand your comment here, do you mean the > >> libcamera request should be completed successfully before updating the > >> resultMetaData? > > > > What I meant to say is that before this change the metadata buffer was > > generated in the > > if (status == CAMERA3_BUFFER_STATUS_OK) > > case, while now, if I got this right, it's generated unconditionally. > > When we'll assemble it inspecting the ControlList associated with the > > just completed Request, if it has not completed succesfully, we might > > try to access an invalid control list or a control list with unset > > values. If that's correct, I think it has to be recorded. > > No, I don't touch any control list. The added information comes only > from the generated JPEG content, which is local to the android layer, > not the underlying libcamera stream. You don't need to for JPEG, but the metadata we -currently- return to the framework is assembled with fixed values. We'll need to construct it inspecting the Request ControlList, and if the Request has not succeeded, it might contain invalid values. > > -- > K > > > Thanks > > j > > > >> > >> It will still only be assigned to the captureResult.result if the > >> status == CAMERA3_BUFFER_STATUS_OK > >> > >> check passes still... > >> > >> You'll see how this gets used in 12/12 of course. > >> > >> > >>> That apart > >>> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> > >> > >> Thanks, > >> > >> > >>> > >>> Thanks > >>> j > >>> > >>>> captureResult.result = resultMetadata->get(); > >>>> } > >>>> > >>>> -- > >>>> 2.25.1 > >>>> > >>>> _______________________________________________ > >>>> libcamera-devel mailing list > >>>> libcamera-devel@lists.libcamera.org > >>>> https://lists.libcamera.org/listinfo/libcamera-devel > >> > >> -- > >> Regards > >> -- > >> Kieran > > -- > Regards > -- > Kieran
Hi Jacopo, On 04/08/2020 13:37, Jacopo Mondi wrote: > Hi Kieran, > I think we're not getting each other here :) Maybe ;-) > On Tue, Aug 04, 2020 at 12:54:04PM +0100, Kieran Bingham wrote: >> Hi Jacopo, >> > > [snip] > >>>>>> - /* >>>>>> - * \todo The timestamp used for the metadata is currently always taken >>>>>> - * from the first buffer (which may be the first stream) in the Request. >>>>>> - * It might be appropriate to return a 'correct' (as determined by >>>>>> - * pipeline handlers) timestamp in the Request itself. >>>>>> - */ >>>>>> - FrameBuffer *buffer = buffers.begin()->second; >>>>>> >>>>>> if (status == CAMERA3_BUFFER_STATUS_OK) { >>>>>> notifyShutter(descriptor->frameNumber, >>>>>> buffer->metadata().timestamp); >>>>>> >>>>>> captureResult.partial_result = 1; >>>>>> - resultMetadata = getResultMetadata(descriptor->frameNumber, >>>>>> - buffer->metadata().timestamp); >>>>>> + >>>>> >>>>> I would drop this one too >>>>> >>>>> Overall I think this is fine so far, as we generate metadata >>>>> statically, but going forward if we have to access the Controls >>>>> returned by a completed Request, we shall make sure it completed >>>>> successfully. Can we record that with a \todo entry ? >>>>> >>>> >>>> I'm not sure I fully understand your comment here, do you mean the >>>> libcamera request should be completed successfully before updating the >>>> resultMetaData? >>> >>> What I meant to say is that before this change the metadata buffer was >>> generated in the >>> if (status == CAMERA3_BUFFER_STATUS_OK) >>> case, while now, if I got this right, it's generated unconditionally. >>> When we'll assemble it inspecting the ControlList associated with the >>> just completed Request, if it has not completed succesfully, we might >>> try to access an invalid control list or a control list with unset >>> values. If that's correct, I think it has to be recorded. >> >> No, I don't touch any control list. The added information comes only >> from the generated JPEG content, which is local to the android layer, >> not the underlying libcamera stream. > > You don't need to for JPEG, but the metadata we -currently- return to > the framework is assembled with fixed values. We'll need to construct > it inspecting the Request ControlList, and if the Request has not > succeeded, it might contain invalid values. But if the request fails, then in CameraDevice::requestComplete(), the request status is checked: if (request->status() != Request::RequestComplete) { LOG(HAL, Error) << "Request not successfully completed: " << request->status(); status = CAMERA3_BUFFER_STATUS_ERROR; } So status is set to error: Which means, the metadata will never be assigned anyway... if (status == CAMERA3_BUFFER_STATUS_OK) { notifyShutter(descriptor->frameNumber, buffer->metadata().timestamp); captureResult.partial_result = 1; captureResult.result = resultMetadata->get(); } And thus notifyError would get called: if (status == CAMERA3_BUFFER_STATUS_ERROR || !captureResult.result) { notifyError(descriptor->frameNumber, descriptor->buffers[0].stream); } So I'm not sure what todo to add, as we already ensure that the metadata isn't added to the captureResult if there is an error in either case, and if we parse controls which depend on the request being successfully completed ... well then it's up to that code to decide to only parse the request if it was successful. If you'd still like a specific todo adding - let me know the text and where, and I'll paste it in. -- Kieran > >> >> -- >> K >> >>> Thanks >>> j >>> >>>> >>>> It will still only be assigned to the captureResult.result if the >>>> status == CAMERA3_BUFFER_STATUS_OK >>>> >>>> check passes still... >>>> >>>> You'll see how this gets used in 12/12 of course. >>>> >>>> >>>>> That apart >>>>> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> >>>> >>>> Thanks, >>>> >>>> >>>>> >>>>> Thanks >>>>> j >>>>> >>>>>> captureResult.result = resultMetadata->get(); >>>>>> } >>>>>> >>>>>> -- >>>>>> 2.25.1 >>>>>> >>>>>> _______________________________________________ >>>>>> libcamera-devel mailing list >>>>>> libcamera-devel@lists.libcamera.org >>>>>> https://lists.libcamera.org/listinfo/libcamera-devel >>>> >>>> -- >>>> Regards >>>> -- >>>> Kieran >> >> -- >> Regards >> -- >> Kieran
diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index ae52a5ca8b86..e23ab055d012 100644 --- a/src/android/camera_device.cpp +++ b/src/android/camera_device.cpp @@ -1138,6 +1138,8 @@ void CameraDevice::requestComplete(Request *request) const std::map<Stream *, FrameBuffer *> &buffers = request->buffers(); camera3_buffer_status status = CAMERA3_BUFFER_STATUS_OK; std::unique_ptr<CameraMetadata> resultMetadata; + Camera3RequestDescriptor *descriptor = + reinterpret_cast<Camera3RequestDescriptor *>(request->cookie()); if (request->status() != Request::RequestComplete) { LOG(HAL, Error) << "Request not successfully completed: " @@ -1145,9 +1147,18 @@ void CameraDevice::requestComplete(Request *request) status = CAMERA3_BUFFER_STATUS_ERROR; } + /* + * \todo The timestamp used for the metadata is currently always taken + * from the first buffer (which may be the first stream) in the Request. + * It might be appropriate to return a 'correct' (as determined by + * pipeline handlers) timestamp in the Request itself. + */ + FrameBuffer *buffer = buffers.begin()->second; + resultMetadata = getResultMetadata(descriptor->frameNumber, + buffer->metadata().timestamp); + /* Prepare to call back the Android camera stack. */ - Camera3RequestDescriptor *descriptor = - reinterpret_cast<Camera3RequestDescriptor *>(request->cookie()); + camera3_capture_result_t captureResult = {}; captureResult.frame_number = descriptor->frameNumber; @@ -1160,21 +1171,13 @@ void CameraDevice::requestComplete(Request *request) captureResult.output_buffers = const_cast<const camera3_stream_buffer_t *>(descriptor->buffers); - /* - * \todo The timestamp used for the metadata is currently always taken - * from the first buffer (which may be the first stream) in the Request. - * It might be appropriate to return a 'correct' (as determined by - * pipeline handlers) timestamp in the Request itself. - */ - FrameBuffer *buffer = buffers.begin()->second; if (status == CAMERA3_BUFFER_STATUS_OK) { notifyShutter(descriptor->frameNumber, buffer->metadata().timestamp); captureResult.partial_result = 1; - resultMetadata = getResultMetadata(descriptor->frameNumber, - buffer->metadata().timestamp); + captureResult.result = resultMetadata->get(); }
Generate the ResultMetadata before performing JPEG compression so that JPEG specific metadata can be added to the metadata when it has been processed. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> --- src/android/camera_device.cpp | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-)