| Message ID | 20190201123239.7d4eacfb@silica.lan (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Gstreamer and vim2m with bayer capture formats | expand |
Le vendredi 01 février 2019 à 12:32 -0200, Mauro Carvalho Chehab a écrit : > Hi Nicolas, > > I just added a patch for the vim2m Kernel driver to also support bayer formats, > but only in capture mode: > > https://git.linuxtv.org/mchehab/experimental.git/commit/?h=vim2m&id=7fd6ccf110b7c167a2304ffd482e6c04252c4909 > > The goal here is to be able to use vim2m to simulate a webcam. Creating > a bayer output is trivial, but converting from bayer to RGB is a way more > complex, and probably not too useful for this Kernel testing driver. So, > I opted to not implement bayer conversion only for the V4L2 output mode > (with is actually the m2m input). > > After such patch and the ones that I merged yesterday at the linux-media > development git tree, what we have is: > > $ v4l2-ctl --list-formats --list-formats-out > ioctl: VIDIOC_ENUM_FMT > Type: Video Capture > > [0]: 'RGBP' (16-bit RGB 5-6-5) > [1]: 'RGBR' (16-bit RGB 5-6-5 BE) > [2]: 'RGB3' (24-bit RGB 8-8-8) > [3]: 'BGR3' (24-bit BGR 8-8-8) > [4]: 'YUYV' (YUYV 4:2:2) > [5]: 'BA81' (8-bit Bayer BGBG/GRGR) > [6]: 'GBRG' (8-bit Bayer GBGB/RGRG) > [7]: 'GRBG' (8-bit Bayer GRGR/BGBG) > [8]: 'RGGB' (8-bit Bayer RGRG/GBGB) > > ioctl: VIDIOC_ENUM_FMT > Type: Video Output > > [0]: 'RGBP' (16-bit RGB 5-6-5) > [1]: 'RGBR' (16-bit RGB 5-6-5 BE) > [2]: 'RGB3' (24-bit RGB 8-8-8) > [3]: 'BGR3' (24-bit BGR 8-8-8) > [4]: 'YUYV' (YUYV 4:2:2) > > With that, I got two problems with gstreamer V4L2 plugin: > > 1) Right now, it only creates a v4l2video?convert source if a M2M device > is symmetric, e. g. exactly the same set of formats should be supported by > both capture and output types. That was never a V4L2 API requirement. Not exactly a question of symmetry. The v4l2transform element is means for RAW video processors, so the classification heuristic is that all input and output formats must be RAW video. What I believe the bug might be is that the bayer formats may not have been classified as RAW video (they are not considered RAW in GStreamer at the moment). > > Actually, vim2m capture driver had always asymmetric formats since when m2m > support was introduced. It only became symmetric for a short period of > time (this week) when I added more formats to it. Yet, after my new patch > (planned to be merged next week), it will be asymmetric again. Again, it has nothing to do with symmetry. > > So, the enclosed patch is required (or something similar) in order to fix > gstreamer with regards to V4L2 M2M API support. > > With such patch applied, and building it with --enable-v4l2-plugin, the > gst V4L2 plugin works with a pipeline like: > > $ gst-launch-1.0 videotestsrc ! video/x-raw,format=BGR ! v4l2video0convert disable-passthrough=1 extra-controls="s,horizontal_flip=1,vertical_flip=1" ! video/x-raw,format=BGR ! videoconvert ! fpsdisplaysink > > 2) I'm not sure how to use gstreamer to use a bayer format for capture type. > > I tried this: > > $ gst-launch-1.0 videotestsrc ! video/x-raw,format=BGR ! v4l2video0convert disable-passthrough=1 extra-controls="s,horizontal_flip=1,vertical_flip=1" ! video/x-bayer,format=bggr ! bayer2rgb ! videoconvert ! fpsdisplaysink > > But it failed: > Setting pipeline to PAUSED ... > failed to open /usr/lib64/dri/hybrid_drv_video.so > Not using hybrid_drv_video.so > failed to open /usr/lib64/dri/hybrid_drv_video.so > Not using hybrid_drv_video.so > Pipeline is PREROLLING ... > Got context from element 'fps-display-video_sink-actual-sink-vaapi': gst.vaapi.Display=context, gst.vaapi.Display=(GstVaapiDisplay)"\(GstVaapiDisplayGLX\)\ vaapidisplayglx1"; > ERROR: from element /GstPipeline:pipeline0/GstVideoTestSrc:videotestsrc0: Internal data stream error. > Additional debug info: > gstbasesrc.c(3055): gst_base_src_loop (): /GstPipeline:pipeline0/GstVideoTestSrc:videotestsrc0: > streaming stopped, reason not-negotiated (-4) > ERROR: pipeline doesn't want to preroll. > Setting pipeline to NULL ... > Freeing pipeline ... > > PS.: I'm sure that the vim2m is working fine with bayer, as I tested it with: > > $ qvidcap -p & > $ for i in BA81 GBRRG GRBG RGGB; do v4l2-ctl --stream-mmap --stream-out-mmap --stream-to-host localhost --stream-lossless --stream-out-hor-speed 1 -v pixelformat=$i --stream-count 40; done > > Cheers, > Mauro > > diff --git a/sys/v4l2/gstv4l2.c b/sys/v4l2/gstv4l2.c > index 2674d9cd3449..9031de657d71 100644 > --- a/sys/v4l2/gstv4l2.c > +++ b/sys/v4l2/gstv4l2.c > @@ -204,7 +204,7 @@ gst_v4l2_probe_and_register (GstPlugin * plugin) > if (gst_v4l2_is_vp9_enc (sink_caps, src_caps)) > gst_v4l2_vp9_enc_register (plugin, basename, it->device_path, > sink_caps, src_caps); > - } else if (gst_v4l2_is_transform (sink_caps, src_caps)) { > + } else { > gst_v4l2_transform_register (plugin, basename, it->device_path, > sink_caps, src_caps); Of course this by-pass the classification. The intention is not that v4l2convert becomes a fallthrough, the reason is that the caps negotiation has semantic related to the expected functions the m2m driver will do (right now we support csc and scaling). So the intention is that GstV4l2Transform becomes an abstract class, and GstV4l2Convert would subclass it to implement csc and scaling. And then other specialized subclass can be made to support other type of hardware. But before this can happen, proper API for HW function classification will be needed. Right now, this is all base on guessing. For the vim2m case, we should simply properly classify bayer format as RAW video, and that should solve your issue. If my change you have resources or time to work on a proper patch, be aware that patch submissions works through gitlab.freedesktop.org Merge Request (basically pushing a branch on a fork there and doing couple of webui clicks). > } > diff --git a/sys/v4l2/gstv4l2transform.c b/sys/v4l2/gstv4l2transform.c > index e92f984ff40a..487b7750d17b 100644 > --- a/sys/v4l2/gstv4l2transform.c > +++ b/sys/v4l2/gstv4l2transform.c > @@ -1171,17 +1171,6 @@ gst_v4l2_transform_subclass_init (gpointer g_class, gpointer data) > } > > /* Probing functions */ > -gboolean > -gst_v4l2_is_transform (GstCaps * sink_caps, GstCaps * src_caps) > -{ > - gboolean ret = FALSE; > - > - if (gst_caps_is_subset (sink_caps, gst_v4l2_object_get_raw_caps ()) > - && gst_caps_is_subset (src_caps, gst_v4l2_object_get_raw_caps ())) > - ret = TRUE; > - > - return ret; > -} > > void > gst_v4l2_transform_register (GstPlugin * plugin, const gchar * basename, > diff --git a/sys/v4l2/gstv4l2transform.h b/sys/v4l2/gstv4l2transform.h > index 29f3f3c655b7..afdc289db545 100644 > --- a/sys/v4l2/gstv4l2transform.h > +++ b/sys/v4l2/gstv4l2transform.h > @@ -73,7 +73,6 @@ struct _GstV4l2TransformClass > > GType gst_v4l2_transform_get_type (void); > > -gboolean gst_v4l2_is_transform (GstCaps * sink_caps, GstCaps * src_caps); > void gst_v4l2_transform_register (GstPlugin * plugin, > const gchar *basename, > const gchar *device_path, >
Em Fri, 01 Feb 2019 12:03:49 -0500 Nicolas Dufresne <nicolas@ndufresne.ca> escreveu: > Le vendredi 01 février 2019 à 12:32 -0200, Mauro Carvalho Chehab a > écrit : > > Hi Nicolas, > > > > I just added a patch for the vim2m Kernel driver to also support bayer formats, > > but only in capture mode: > > > > https://git.linuxtv.org/mchehab/experimental.git/commit/?h=vim2m&id=7fd6ccf110b7c167a2304ffd482e6c04252c4909 > > > > The goal here is to be able to use vim2m to simulate a webcam. Creating > > a bayer output is trivial, but converting from bayer to RGB is a way more > > complex, and probably not too useful for this Kernel testing driver. So, > > I opted to not implement bayer conversion only for the V4L2 output mode > > (with is actually the m2m input). > > > > After such patch and the ones that I merged yesterday at the linux-media > > development git tree, what we have is: > > > > $ v4l2-ctl --list-formats --list-formats-out > > ioctl: VIDIOC_ENUM_FMT > > Type: Video Capture > > > > [0]: 'RGBP' (16-bit RGB 5-6-5) > > [1]: 'RGBR' (16-bit RGB 5-6-5 BE) > > [2]: 'RGB3' (24-bit RGB 8-8-8) > > [3]: 'BGR3' (24-bit BGR 8-8-8) > > [4]: 'YUYV' (YUYV 4:2:2) > > [5]: 'BA81' (8-bit Bayer BGBG/GRGR) > > [6]: 'GBRG' (8-bit Bayer GBGB/RGRG) > > [7]: 'GRBG' (8-bit Bayer GRGR/BGBG) > > [8]: 'RGGB' (8-bit Bayer RGRG/GBGB) > > > > ioctl: VIDIOC_ENUM_FMT > > Type: Video Output > > > > [0]: 'RGBP' (16-bit RGB 5-6-5) > > [1]: 'RGBR' (16-bit RGB 5-6-5 BE) > > [2]: 'RGB3' (24-bit RGB 8-8-8) > > [3]: 'BGR3' (24-bit BGR 8-8-8) > > [4]: 'YUYV' (YUYV 4:2:2) > > > > With that, I got two problems with gstreamer V4L2 plugin: > > > > 1) Right now, it only creates a v4l2video?convert source if a M2M device > > is symmetric, e. g. exactly the same set of formats should be supported by > > both capture and output types. That was never a V4L2 API requirement. > > Not exactly a question of symmetry. The v4l2transform element is means > for RAW video processors, so the classification heuristic is that all > input and output formats must be RAW video. > > What I believe the bug might be is that the bayer formats may not have > been classified as RAW video (they are not considered RAW in GStreamer > at the moment). Ah, I see. What happens if a single m2m device could convert from non-raw and raw formats? As a large amount of m2m devices actually map into some ISP, with is fimware-based, nothing really prevents someone to implement a m2m device that would accept a wide range of formats as input and output (both raw and non-raw). So, it would be possible to have one of such devices doing weird things like supporting, as input, RGB, YUV, Bayer, MJPEG, and H.264 and a similar set (probably not identical) as output. So, one could set it, for example, to convert from MJPEG to H.264. With the current model, V4L2 plugin won't be able to map it (as it won't match the current concept of encoder, decoder or transform). IMO, it would make sense to keep encoders and decoders as is, to be used only when it is clearly a simple format encoding or decoding, using v4l2transform for anything else. > > > > > Actually, vim2m capture driver had always asymmetric formats since when m2m > > support was introduced. It only became symmetric for a short period of > > time (this week) when I added more formats to it. Yet, after my new patch > > (planned to be merged next week), it will be asymmetric again. > > Again, it has nothing to do with symmetry. > > > > > So, the enclosed patch is required (or something similar) in order to fix > > gstreamer with regards to V4L2 M2M API support. > > > > With such patch applied, and building it with --enable-v4l2-plugin, the > > gst V4L2 plugin works with a pipeline like: > > > > $ gst-launch-1.0 videotestsrc ! video/x-raw,format=BGR ! v4l2video0convert disable-passthrough=1 extra-controls="s,horizontal_flip=1,vertical_flip=1" ! video/x-raw,format=BGR ! videoconvert ! fpsdisplaysink > > > > 2) I'm not sure how to use gstreamer to use a bayer format for capture type. > > > > I tried this: > > > > $ gst-launch-1.0 videotestsrc ! video/x-raw,format=BGR ! v4l2video0convert disable-passthrough=1 extra-controls="s,horizontal_flip=1,vertical_flip=1" ! video/x-bayer,format=bggr ! bayer2rgb ! videoconvert ! fpsdisplaysink > > > > But it failed: > > Setting pipeline to PAUSED ... > > failed to open /usr/lib64/dri/hybrid_drv_video.so > > Not using hybrid_drv_video.so > > failed to open /usr/lib64/dri/hybrid_drv_video.so > > Not using hybrid_drv_video.so > > Pipeline is PREROLLING ... > > Got context from element 'fps-display-video_sink-actual-sink-vaapi': gst.vaapi.Display=context, gst.vaapi.Display=(GstVaapiDisplay)"\(GstVaapiDisplayGLX\)\ vaapidisplayglx1"; > > ERROR: from element /GstPipeline:pipeline0/GstVideoTestSrc:videotestsrc0: Internal data stream error. > > Additional debug info: > > gstbasesrc.c(3055): gst_base_src_loop (): /GstPipeline:pipeline0/GstVideoTestSrc:videotestsrc0: > > streaming stopped, reason not-negotiated (-4) > > ERROR: pipeline doesn't want to preroll. > > Setting pipeline to NULL ... > > Freeing pipeline ... > > > > PS.: I'm sure that the vim2m is working fine with bayer, as I tested it with: > > > > $ qvidcap -p & > > $ for i in BA81 GBRRG GRBG RGGB; do v4l2-ctl --stream-mmap --stream-out-mmap --stream-to-host localhost --stream-lossless --stream-out-hor-speed 1 -v pixelformat=$i --stream-count 40; done > > > > Cheers, > > Mauro > > > > diff --git a/sys/v4l2/gstv4l2.c b/sys/v4l2/gstv4l2.c > > index 2674d9cd3449..9031de657d71 100644 > > --- a/sys/v4l2/gstv4l2.c > > +++ b/sys/v4l2/gstv4l2.c > > @@ -204,7 +204,7 @@ gst_v4l2_probe_and_register (GstPlugin * plugin) > > if (gst_v4l2_is_vp9_enc (sink_caps, src_caps)) > > gst_v4l2_vp9_enc_register (plugin, basename, it->device_path, > > sink_caps, src_caps); > > - } else if (gst_v4l2_is_transform (sink_caps, src_caps)) { > > + } else { > > gst_v4l2_transform_register (plugin, basename, it->device_path, > > sink_caps, src_caps); > > Of course this by-pass the classification. The intention is not that > v4l2convert becomes a fallthrough, the reason is that the caps > negotiation has semantic related to the expected functions the m2m > driver will do (right now we support csc and scaling). So the intention > is that GstV4l2Transform becomes an abstract class, and GstV4l2Convert > would subclass it to implement csc and scaling. And then other > specialized subclass can be made to support other type of hardware. The problem I see is that the sky is the limit. In practice, a m2m device maps into an entry point to an Image Signal Processor firmware, sending them a video stream and receiving another video stream. It can do multiple things at one. I mean, a single m2m device could potentially do all sorts of transformation at the same time: format changes, encoding, decoding, colorspace conversion, scaling, cropping, frame rate interpolation/decimation, image enhancements, 3A, etc. What a m2m device does basically depends on the imagination of the hardware vendor, and the V4L2 API limits. Also, what it will actually do depends on the firmware it is currently using[1]. [1] There are some already existing m2m drivers that accept different firmwares, and depending on its version (or at the hardware release), it exposes a different set of controls and formats. So, I would try to stick with "just" encoding, decoding, having a generic "transform" for all the rest. > But before this can happen, proper API for HW function classification > will be needed. Right now, this is all base on guessing. For the vim2m > case, we should simply properly classify bayer format as RAW video, and > that should solve your issue. For the vim2m case, this would work. Yet, as vim2m was the first m2m driver merged, and the gstreamer's issue with it is still present at gst 1.14.4 [2], I suspect that there could be other cases where the v4l2 plugin could not be exposing a m2m device, because it won't fit at the simple csc/scaling use case. [2] granted, this is not a real hardware-based m2m device, so nobody should use vim2m in production, as the Kernel is not the right place for conversions - also, the conversion logic there was broken. That may explain why nobody fixed gstream support for it so far. > If my change you have resources or time to work on a proper patch, be > aware that patch submissions works through gitlab.freedesktop.org Merge > Request (basically pushing a branch on a fork there and doing couple of > webui clicks). Ah, ok. Well, the intention here was just to do a RFC and check with you about the proper solution. While I do have a limited amount of time, due to my kernel duties, I could try to write a different patch for it once I understand better what should be done. While that doesn't happen, IMHO, the best is to send RFC patches via e-mail, as it allows c/c the discussions to the linux-media ML. > > > } > > diff --git a/sys/v4l2/gstv4l2transform.c b/sys/v4l2/gstv4l2transform.c > > index e92f984ff40a..487b7750d17b 100644 > > --- a/sys/v4l2/gstv4l2transform.c > > +++ b/sys/v4l2/gstv4l2transform.c > > @@ -1171,17 +1171,6 @@ gst_v4l2_transform_subclass_init (gpointer g_class, gpointer data) > > } > > > > /* Probing functions */ > > -gboolean > > -gst_v4l2_is_transform (GstCaps * sink_caps, GstCaps * src_caps) > > -{ > > - gboolean ret = FALSE; > > - > > - if (gst_caps_is_subset (sink_caps, gst_v4l2_object_get_raw_caps ()) > > - && gst_caps_is_subset (src_caps, gst_v4l2_object_get_raw_caps ())) > > - ret = TRUE; > > - > > - return ret; > > -} > > > > void > > gst_v4l2_transform_register (GstPlugin * plugin, const gchar * basename, > > diff --git a/sys/v4l2/gstv4l2transform.h b/sys/v4l2/gstv4l2transform.h > > index 29f3f3c655b7..afdc289db545 100644 > > --- a/sys/v4l2/gstv4l2transform.h > > +++ b/sys/v4l2/gstv4l2transform.h > > @@ -73,7 +73,6 @@ struct _GstV4l2TransformClass > > > > GType gst_v4l2_transform_get_type (void); > > > > -gboolean gst_v4l2_is_transform (GstCaps * sink_caps, GstCaps * src_caps); > > void gst_v4l2_transform_register (GstPlugin * plugin, > > const gchar *basename, > > const gchar *device_path, > > Cheers, Mauro
Em Fri, 1 Feb 2019 15:55:06 -0200 Mauro Carvalho Chehab <mchehab+samsung@kernel.org> escreveu: > Em Fri, 01 Feb 2019 12:03:49 -0500 > Nicolas Dufresne <nicolas@ndufresne.ca> escreveu: > > > If my change you have resources or time to work on a proper patch, be > > aware that patch submissions works through gitlab.freedesktop.org Merge > > Request (basically pushing a branch on a fork there and doing couple of > > webui clicks). > > Ah, ok. Well, the intention here was just to do a RFC and check with > you about the proper solution. > > While I do have a limited amount of time, due to my kernel duties, > I could try to write a different patch for it once I understand > better what should be done. > > While that doesn't happen, IMHO, the best is to send RFC patches via > e-mail, as it allows c/c the discussions to the linux-media ML. Cheers, Mauro Ah, I guess I see the issue... gst_v4l2_object_get_raw_caps() doesn't associate bayer formats as a format that a v4l2transform would accept. So, a change like the one below would make it to recognize the 4 bayer formats that was added to vim2m as belonging to a v4l2convert type, right? Indeed with this patch it is now recognizing the formats and a v4l2video0convert block was recognized. Unfortunately, this is not working yet: $ gst-launch-1.0 videotestsrc ! video/x-raw,format=BGR ! v4l2video0convert disable-passthrough=1 extra-controls="s,horizontal_flip=1,vertical_flip=1" ! video/x-bayer,format=bggr ! bayer2rgb ! videoconvert ! ximagesink Maybe I need to change something else too for it to negotiate between a v4l2 capture sink from videoconvert and bayer2rgb source. - diff --git a/sys/v4l2/gstv4l2object.c b/sys/v4l2/gstv4l2object.c index 124c778c626d..5a1e52989903 100644 --- a/sys/v4l2/gstv4l2object.c +++ b/sys/v4l2/gstv4l2object.c @@ -157,10 +157,10 @@ static const GstV4L2FormatDesc gst_v4l2_formats[] = { {V4L2_PIX_FMT_NV42, TRUE, GST_V4L2_RAW}, /* Bayer formats - see http://www.siliconimaging.com/RGB%20Bayer.htm */ - {V4L2_PIX_FMT_SBGGR8, TRUE, GST_V4L2_CODEC}, - {V4L2_PIX_FMT_SGBRG8, TRUE, GST_V4L2_CODEC}, - {V4L2_PIX_FMT_SGRBG8, TRUE, GST_V4L2_CODEC}, - {V4L2_PIX_FMT_SRGGB8, TRUE, GST_V4L2_CODEC}, + {V4L2_PIX_FMT_SBGGR8, TRUE, GST_V4L2_RAW | GST_V4L2_CODEC}, + {V4L2_PIX_FMT_SGBRG8, TRUE, GST_V4L2_RAW | GST_V4L2_CODEC}, + {V4L2_PIX_FMT_SGRBG8, TRUE, GST_V4L2_RAW | GST_V4L2_CODEC}, + {V4L2_PIX_FMT_SRGGB8, TRUE, GST_V4L2_RAW | GST_V4L2_CODEC}, /* compressed formats */ {V4L2_PIX_FMT_MJPEG, FALSE, GST_V4L2_CODEC},
diff --git a/sys/v4l2/gstv4l2.c b/sys/v4l2/gstv4l2.c index 2674d9cd3449..9031de657d71 100644 --- a/sys/v4l2/gstv4l2.c +++ b/sys/v4l2/gstv4l2.c @@ -204,7 +204,7 @@ gst_v4l2_probe_and_register (GstPlugin * plugin) if (gst_v4l2_is_vp9_enc (sink_caps, src_caps)) gst_v4l2_vp9_enc_register (plugin, basename, it->device_path, sink_caps, src_caps); - } else if (gst_v4l2_is_transform (sink_caps, src_caps)) { + } else { gst_v4l2_transform_register (plugin, basename, it->device_path, sink_caps, src_caps); } diff --git a/sys/v4l2/gstv4l2transform.c b/sys/v4l2/gstv4l2transform.c index e92f984ff40a..487b7750d17b 100644 --- a/sys/v4l2/gstv4l2transform.c +++ b/sys/v4l2/gstv4l2transform.c @@ -1171,17 +1171,6 @@ gst_v4l2_transform_subclass_init (gpointer g_class, gpointer data) } /* Probing functions */ -gboolean -gst_v4l2_is_transform (GstCaps * sink_caps, GstCaps * src_caps) -{ - gboolean ret = FALSE; - - if (gst_caps_is_subset (sink_caps, gst_v4l2_object_get_raw_caps ()) - && gst_caps_is_subset (src_caps, gst_v4l2_object_get_raw_caps ())) - ret = TRUE; - - return ret; -} void gst_v4l2_transform_register (GstPlugin * plugin, const gchar * basename, diff --git a/sys/v4l2/gstv4l2transform.h b/sys/v4l2/gstv4l2transform.h index 29f3f3c655b7..afdc289db545 100644 --- a/sys/v4l2/gstv4l2transform.h +++ b/sys/v4l2/gstv4l2transform.h @@ -73,7 +73,6 @@ struct _GstV4l2TransformClass GType gst_v4l2_transform_get_type (void); -gboolean gst_v4l2_is_transform (GstCaps * sink_caps, GstCaps * src_caps); void gst_v4l2_transform_register (GstPlugin * plugin, const gchar *basename, const gchar *device_path,