| Message ID | 20200129033210.278800-24-nicolas@ndufresne.ca |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Nicolas, Thank you for the patch. On Tue, Jan 28, 2020 at 10:32:10PM -0500, Nicolas Dufresne wrote: > From: Nicolas Dufresne <nicolas.dufresne@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@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 ? > + */ > + > #include "gstlibcamerasrc.h" > #include "gstlibcamerapad.h" > #include "gstlibcameraallocator.h"
Le mercredi 12 février 2020 à 02:41 +0200, Laurent Pinchart a écrit : > Hi Nicolas, > > Thank you for the patch. > > On Tue, Jan 28, 2020 at 10:32:10PM -0500, Nicolas Dufresne wrote: > > From: Nicolas Dufresne <nicolas.dufresne@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@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 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. > > > + */ > > + > > #include "gstlibcamerasrc.h" > > #include "gstlibcamerapad.h" > > #include "gstlibcameraallocator.h"
Hi Nicolas, 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@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@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. > 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. > > > + */ > > > + > > > #include "gstlibcamerasrc.h" > > > #include "gstlibcamerapad.h" > > > #include "gstlibcameraallocator.h"
Le mer. 12 févr. 2020 15 h 12, Laurent Pinchart < laurent.pinchart@ideasonboard.com> a écrit : > Hi Nicolas, > > 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@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@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. > > > > + */ > > > > + > > > > #include "gstlibcamerasrc.h" > > > > #include "gstlibcamerapad.h" > > > > #include "gstlibcameraallocator.h" > > -- > Regards, > > 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@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@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"
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 + * - 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 + */ + #include "gstlibcamerasrc.h" #include "gstlibcamerapad.h" #include "gstlibcameraallocator.h"