Message ID | 20210910070638.467294-5-umang.jain@ideasonboard.com |
---|---|
State | Superseded |
Delegated to: | Umang Jain |
Headers | show |
Series |
|
Related | show |
Hi Umang, Thank you for the patch. On Fri, Sep 10, 2021 at 12:36:33PM +0530, Umang Jain wrote: > We should be able to know if post-processing has been completed > successfully or encountered some errors. This commit introduces a > Signal<> which when connected, will notify that the post-processing The signal notifies completion regardless of whether it's connected or not, the same way that a tree falling in an empty forest still makes sound :-) > has been completed. The status of PostProcessor::process() will be > passed as a PostProcessor::Status argument. The signal will be required > when the post-processor is meant to run asynchronously (in subsequent > commits). > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> > --- > src/android/jpeg/post_processor_jpeg.cpp | 7 ++++++- > src/android/post_processor.h | 8 ++++++++ > src/android/yuv/post_processor_yuv.cpp | 4 ++++ > 3 files changed, 18 insertions(+), 1 deletion(-) > > diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp > index 741eeb21..1df3dfb1 100644 > --- a/src/android/jpeg/post_processor_jpeg.cpp > +++ b/src/android/jpeg/post_processor_jpeg.cpp > @@ -103,8 +103,10 @@ int PostProcessorJpeg::process(const FrameBuffer *source, > CameraMetadata *resultMetadata, > const Camera3RequestDescriptor *context) > { > - if (!encoder_) > + if (!encoder_) { > + processComplete.emit(PostProcessor::Status::Failed, context); As PostProcessorJpeg inherits from PostProcessor, I think you can write processComplete.emit(Status::Failed, context); here. Same below. > return 0; > + } > > ASSERT(destination->numPlanes() == 1); > > @@ -197,6 +199,7 @@ int PostProcessorJpeg::process(const FrameBuffer *source, > exif.data(), quality); > if (jpeg_size < 0) { > LOG(JPEG, Error) << "Failed to encode stream image"; > + processComplete.emit(PostProcessor::Status::Failed, context); > return jpeg_size; > } > > @@ -211,5 +214,7 @@ int PostProcessorJpeg::process(const FrameBuffer *source, > /* Update the JPEG result Metadata. */ > resultMetadata->addEntry(ANDROID_JPEG_SIZE, jpeg_size); > > + processComplete.emit(PostProcessor::Status::Succeeded, context); > + > return 0; > } > diff --git a/src/android/post_processor.h b/src/android/post_processor.h > index 1213e7e6..25c9411f 100644 > --- a/src/android/post_processor.h > +++ b/src/android/post_processor.h > @@ -7,6 +7,8 @@ > #ifndef __ANDROID_POST_PROCESSOR_H__ > #define __ANDROID_POST_PROCESSOR_H__ > > +#include <libcamera/base/signal.h> > + > #include <libcamera/framebuffer.h> > #include <libcamera/stream.h> > > @@ -28,6 +30,12 @@ public: > const CameraMetadata &requestMetadata, > CameraMetadata *resultMetadata, > const Camera3RequestDescriptor *context) = 0; > + > + enum Status { > + Succeeded, > + Failed, > + }; This should move right after "public", we define types before fucntions. We have enum Status { FrameSuccess, FrameError, FrameCancelled, }; in FrameMetadata, I'd thus name the enumerators Success and Error. I'd also make this an enum class to make usage of the Status:: prefix mandatory, given that you already use it everywhere. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + libcamera::Signal<Status, const Camera3RequestDescriptor *> processComplete; > }; > > #endif /* __ANDROID_POST_PROCESSOR_H__ */ > diff --git a/src/android/yuv/post_processor_yuv.cpp b/src/android/yuv/post_processor_yuv.cpp > index 13a78abe..d2df110a 100644 > --- a/src/android/yuv/post_processor_yuv.cpp > +++ b/src/android/yuv/post_processor_yuv.cpp > @@ -61,6 +61,7 @@ int PostProcessorYuv::process(const FrameBuffer *source, > const MappedFrameBuffer sourceMapped(source, MappedFrameBuffer::MapFlag::Read); > if (!sourceMapped.isValid()) { > LOG(YUV, Error) << "Failed to mmap camera frame buffer"; > + processComplete.emit(PostProcessor::Status::Failed, context); > return -EINVAL; > } > > @@ -78,9 +79,12 @@ int PostProcessorYuv::process(const FrameBuffer *source, > libyuv::FilterMode::kFilterBilinear); > if (ret) { > LOG(YUV, Error) << "Failed NV12 scaling: " << ret; > + processComplete.emit(PostProcessor::Status::Failed, context); > return -EINVAL; > } > > + processComplete.emit(PostProcessor::Status::Succeeded, context); > + > return 0; > } >
diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp index 741eeb21..1df3dfb1 100644 --- a/src/android/jpeg/post_processor_jpeg.cpp +++ b/src/android/jpeg/post_processor_jpeg.cpp @@ -103,8 +103,10 @@ int PostProcessorJpeg::process(const FrameBuffer *source, CameraMetadata *resultMetadata, const Camera3RequestDescriptor *context) { - if (!encoder_) + if (!encoder_) { + processComplete.emit(PostProcessor::Status::Failed, context); return 0; + } ASSERT(destination->numPlanes() == 1); @@ -197,6 +199,7 @@ int PostProcessorJpeg::process(const FrameBuffer *source, exif.data(), quality); if (jpeg_size < 0) { LOG(JPEG, Error) << "Failed to encode stream image"; + processComplete.emit(PostProcessor::Status::Failed, context); return jpeg_size; } @@ -211,5 +214,7 @@ int PostProcessorJpeg::process(const FrameBuffer *source, /* Update the JPEG result Metadata. */ resultMetadata->addEntry(ANDROID_JPEG_SIZE, jpeg_size); + processComplete.emit(PostProcessor::Status::Succeeded, context); + return 0; } diff --git a/src/android/post_processor.h b/src/android/post_processor.h index 1213e7e6..25c9411f 100644 --- a/src/android/post_processor.h +++ b/src/android/post_processor.h @@ -7,6 +7,8 @@ #ifndef __ANDROID_POST_PROCESSOR_H__ #define __ANDROID_POST_PROCESSOR_H__ +#include <libcamera/base/signal.h> + #include <libcamera/framebuffer.h> #include <libcamera/stream.h> @@ -28,6 +30,12 @@ public: const CameraMetadata &requestMetadata, CameraMetadata *resultMetadata, const Camera3RequestDescriptor *context) = 0; + + enum Status { + Succeeded, + Failed, + }; + libcamera::Signal<Status, const Camera3RequestDescriptor *> processComplete; }; #endif /* __ANDROID_POST_PROCESSOR_H__ */ diff --git a/src/android/yuv/post_processor_yuv.cpp b/src/android/yuv/post_processor_yuv.cpp index 13a78abe..d2df110a 100644 --- a/src/android/yuv/post_processor_yuv.cpp +++ b/src/android/yuv/post_processor_yuv.cpp @@ -61,6 +61,7 @@ int PostProcessorYuv::process(const FrameBuffer *source, const MappedFrameBuffer sourceMapped(source, MappedFrameBuffer::MapFlag::Read); if (!sourceMapped.isValid()) { LOG(YUV, Error) << "Failed to mmap camera frame buffer"; + processComplete.emit(PostProcessor::Status::Failed, context); return -EINVAL; } @@ -78,9 +79,12 @@ int PostProcessorYuv::process(const FrameBuffer *source, libyuv::FilterMode::kFilterBilinear); if (ret) { LOG(YUV, Error) << "Failed NV12 scaling: " << ret; + processComplete.emit(PostProcessor::Status::Failed, context); return -EINVAL; } + processComplete.emit(PostProcessor::Status::Succeeded, context); + return 0; }
We should be able to know if post-processing has been completed successfully or encountered some errors. This commit introduces a Signal<> which when connected, will notify that the post-processing has been completed. The status of PostProcessor::process() will be passed as a PostProcessor::Status argument. The signal will be required when the post-processor is meant to run asynchronously (in subsequent commits). Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> --- src/android/jpeg/post_processor_jpeg.cpp | 7 ++++++- src/android/post_processor.h | 8 ++++++++ src/android/yuv/post_processor_yuv.cpp | 4 ++++ 3 files changed, 18 insertions(+), 1 deletion(-)