[libcamera-devel] [PATCH v1 23/23] gst: libcamerasrc: Add a TODO comment

[Laurent Pinchart]

Hi Nicolas,

On Thu, Feb 13, 2020 at 09:26:04AM -0500, Nicolas Dufresne wrote:
> Le mer. 12 févr. 2020 15 h 12, Laurent Pinchart a écrit :
> > On Wed, Feb 12, 2020 at 12:42:02PM -0500, Nicolas Dufresne wrote:
> >> Le mercredi 12 février 2020 à 02:41 +0200, Laurent Pinchart a écrit :
> >>> On Tue, Jan 28, 2020 at 10:32:10PM -0500, Nicolas Dufresne wrote:
> >>>> From: Nicolas Dufresne <nicolas.dufresne at collabora.com>
> >>>>
> >>>> This is to guide upcoming contributors toward what is left to do to get
> >>>> toward a production element.
> >>>>
> >>>> Signed-off-by: Nicolas Dufresne <nicolas.dufresne at collabora.com>
> >>>> ---
> >>>>  src/gstreamer/gstlibcamerasrc.cpp | 20 ++++++++++++++++++++
> >>>>  1 file changed, 20 insertions(+)
> >>>>
> >>>> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp
> >>>> index 947a8bf..878ae2f 100644
> >>>> --- a/src/gstreamer/gstlibcamerasrc.cpp
> >>>> +++ b/src/gstreamer/gstlibcamerasrc.cpp
> >>>> @@ -6,6 +6,26 @@
> >>>>   * gstlibcamerasrc.cpp - GStreamer Capture Element
> >>>>   */
> >>>>
> >>>> +/* TODO
> >>>
> >>> s/TODO/\\todo/
> >>>
> >>>> + *  - Implement GstElement::send_event
> >>>> + *    + Allowing application to send EOS
> >>>> + *    + Allowing application to use FLUSH/FLUSH_STOP
> >>>> + *    + Prevent the main thread from accessing streaming thread
> >>>> + *  - Implement renegotiation (even if slow)
> >>>> + *  - Implement GstElement::request-new-pad (multi stream)
> >>>> + *    + Evaluate if a single streaming thread is fine
> >>>> + *  - Add application driven request (snapshot)
> >>>> + *  - Add framerate control
> >>>> + *
> >>>> + *  Requires new libcamera API:
> >>>> + *  - Add framerate negotiation support
> >>>> + *  - Add colorimetry support
> >>>> + *  - Add timestamp support
> >>>> + *  - Use unique names to select the camera
> >>>> + *  - Add GstVideoMeta support (strides and offsets)
> >>>> + *  - Add buffer importation support
> >>>
> >>> Even if I don't understand most of the items, it's a nice list :-) I'm
> >>> sure we'll get back to you to discuss individual items, but I would like
> >>> to already ask about the last one. What is missing for buffer import ?
> >>
> >> I haven't seen any API that ingest a DMABuf, maybe I missed it. But
> >
> > You can create a FrameBuffer object manually to wrap a set of dmabufs
> > (one per plane) and use it instead of the FrameBuffer instances created
> > by the FrameBufferAllocator. The allocator is a side helper, the main
> > API is designed for the import use case.
> 
> Oh, thanks, I didn't figure out. I'll update the comment to clarify.
> 
> >> it's a bit pointless to implement importation without strides and
> >> offset support as this won't be robust (can produce very bad frames).
> >> So I think it's fine to keep it in that section.
> >
> > Absolutely. We need strides and offsets for sure.
> 
> Just in case someone wants to start on this, be aware that currently v4l2
> does not allow using data_offset for importation and does not offer
> anything else, so if you have a v4l2 driver, we are also limited and must
> do very strong validation, otherwise you get a broken image. I can help on
> that.

That's partly why we have no offset support yet :-( It should be fixed
in V4L2, and Boris had a go at that at the end of last year, but as far
as I know the development has been put on hold. Are you offering help to
resume that work on the kernel side ? ;-)

> >>>> + */
> >>>> +
> >>>>  #include "gstlibcamerasrc.h"
> >>>>  #include "gstlibcamerapad.h"
> >>>>  #include "gstlibcameraallocator.h"

-- 
Regards,

Laurent Pinchart