Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1603846 > unrolled thread

Re: [PATCH v5 00/39] i.MX Media Driver

Started byRussell King - ARM Linux <linux@armlinux.org.uk>
First post2017-03-18 20:30 +0100
Last post2017-03-20 14:20 +0100
Articles 20 on this page of 51 — 10 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v5 00/39] i.MX Media Driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-18 20:30 +0100
    Re: [PATCH v5 00/39] i.MX Media Driver Steve Longerbeam <steve_longerbeam@mentor.com> - 2017-03-18 21:00 +0100
      Re: [PATCH v5 00/39] i.MX Media Driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-18 21:50 +0100
        Re: [PATCH v5 00/39] i.MX Media Driver Nicolas Dufresne <nicolas@ndufresne.ca> - 2017-03-19 01:50 +0100
          Re: [PATCH v5 00/39] i.MX Media Driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-19 02:10 +0100
            Re: [PATCH v5 00/39] i.MX Media Driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-19 16:00 +0100
            Re: [PATCH v5 00/39] i.MX Media Driver Nicolas Dufresne <nicolas@ndufresne.ca> - 2017-03-19 16:10 +0100
          Re: [PATCH v5 00/39] i.MX Media Driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-19 11:00 +0100
            Re: [PATCH v5 00/39] i.MX Media Driver Nicolas Dufresne <nicolas@ndufresne.ca> - 2017-03-19 15:50 +0100
        Re: [PATCH v5 00/39] i.MX Media Driver Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com> - 2017-03-19 15:00 +0100
          Re: [PATCH v5 00/39] i.MX Media Driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-19 15:30 +0100
            Re: [PATCH v5 00/39] i.MX Media Driver Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com> - 2017-03-19 16:10 +0100
              Re: [PATCH v5 00/39] i.MX Media Driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-19 16:20 +0100
          Re: [PATCH v5 00/39] i.MX Media Driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-19 15:30 +0100
            Re: [PATCH v5 00/39] i.MX Media Driver Nicolas Dufresne <nicolas@ndufresne.ca> - 2017-03-19 15:50 +0100
      Re: [PATCH v5 00/39] i.MX Media Driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-19 11:50 +0100
        [PATCH 2/4] media: imx: allow bayer pixel formats to be looked up Russell King <rmk+kernel@armlinux.org.uk> - 2017-03-19 11:50 +0100
          Re: [PATCH 2/4] media: imx: allow bayer pixel formats to be looked up Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-19 23:20 +0100
        [PATCH 1/4] media: imx-media-csi: fix v4l2-compliance check Russell King <rmk+kernel@armlinux.org.uk> - 2017-03-19 11:50 +0100
          Re: [PATCH 1/4] media: imx-media-csi: fix v4l2-compliance check Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-19 23:20 +0100
        [PATCH 4/4] media: imx-media-capture: add frame sizes/interval  enumeration Russell King <rmk+kernel@armlinux.org.uk> - 2017-03-19 12:00 +0100
          Re: [PATCH 4/4] media: imx-media-capture: add frame sizes/interval  enumeration Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-19 23:30 +0100
            Re: [PATCH 4/4] media: imx-media-capture: add frame sizes/interval  enumeration Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-19 23:50 +0100
          Re: [PATCH 4/4] media: imx-media-capture: add frame sizes/interval         enumeration Philippe De Muyter <phdm@macq.eu> - 2017-03-20 10:00 +0100
            Re: [PATCH 4/4] media: imx-media-capture: add frame sizes/interval  enumeration Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-20 10:10 +0100
              Re: [PATCH 4/4] media: imx-media-capture: add frame sizes/interval         enumeration Philippe De Muyter <phdm@macq.eu> - 2017-03-20 10:30 +0100
                Re: [PATCH 4/4] media: imx-media-capture: add frame sizes/interval  enumeration Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-20 11:50 +0100
        [PATCH 3/4] media: imx-csi: add frame size/interval enumeration Russell King <rmk+kernel@armlinux.org.uk> - 2017-03-19 12:10 +0100
          Re: [PATCH 3/4] media: imx-csi: add frame size/interval enumeration Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-19 23:30 +0100
          Re: [PATCH 3/4] media: imx-csi: add frame size/interval enumeration Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-22 00:50 +0100
        Re: [PATCH v5 00/39] i.MX Media Driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-19 19:00 +0100
          Re: [PATCH v5 00/39] i.MX Media Driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-19 19:10 +0100
          Re: [PATCH v5 00/39] i.MX Media Driver Hans Verkuil <hverkuil@xs4all.nl> - 2017-03-20 14:10 +0100
            Re: [PATCH v5 00/39] i.MX Media Driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-20 14:40 +0100
              Re: [PATCH v5 00/39] i.MX Media Driver Hans Verkuil <hverkuil@xs4all.nl> - 2017-03-20 15:00 +0100
                Re: [PATCH v5 00/39] i.MX Media Driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-20 15:20 +0100
                  Re: [PATCH v5 00/39] i.MX Media Driver Hans Verkuil <hverkuil@xs4all.nl> - 2017-03-20 17:10 +0100
                    Re: [PATCH v5 00/39] i.MX Media Driver "Niklas Söderlund" <niklas.soderlund@ragnatech.se> - 2017-03-21 11:50 +0100
                      Re: [PATCH v5 00/39] i.MX Media Driver Hans Verkuil <hverkuil@xs4all.nl> - 2017-03-21 12:10 +0100
                        Re: [PATCH v5 00/39] i.MX Media Driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-21 12:50 +0100
                          Re: [PATCH v5 00/39] i.MX Media Driver Nicolas Dufresne <nicolas@ndufresne.ca> - 2017-03-22 19:20 +0100
      Re: [PATCH v5 00/39] i.MX Media Driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-19 13:20 +0100
        Re: [PATCH v5 00/39] i.MX Media Driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-19 20:10 +0100
          Re: [PATCH v5 00/39] i.MX Media Driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-19 21:00 +0100
        Re: [PATCH v5 00/39] i.MX Media Driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-19 21:00 +0100
        Re: [PATCH v5 00/39] i.MX Media Driver Hans Verkuil <hverkuil@xs4all.nl> - 2017-03-20 14:00 +0100
        Re: [PATCH v5 00/39] i.MX Media Driver Philipp Zabel <p.zabel@pengutronix.de> - 2017-03-20 14:30 +0100
          Re: [PATCH v5 00/39] i.MX Media Driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-20 16:50 +0100
            Re: [PATCH v5 00/39] i.MX Media Driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-20 17:40 +0100
            Re: [PATCH v5 00/39] i.MX Media Driver Philipp Zabel <p.zabel@pengutronix.de> - 2017-03-20 17:40 +0100
      Re: [PATCH v5 00/39] i.MX Media Driver Philipp Zabel <p.zabel@pengutronix.de> - 2017-03-20 14:20 +0100

Page 1 of 3  [1] 2 3  Next page →

#1603846 — Re: [PATCH v5 00/39] i.MX Media Driver

FromRussell King - ARM Linux <linux@armlinux.org.uk>
Date2017-03-18 20:30 +0100
SubjectRe: [PATCH v5 00/39] i.MX Media Driver
Message-ID<tmsd4-2zo-19@gated-at.bofh.it>
Hi Steve,

I've just been trying to get gstreamer to capture and h264 encode
video from my camera at various frame rates, and what I've discovered
does not look good.

1) when setting frame rates, media-ctl _always_ calls
   VIDIOC_SUBDEV_S_FRAME_INTERVAL with pad=0.

2) media-ctl never retrieves the frame interval information, so there's
   no way to read it back with standard tools, and no indication that
   this is going on...

3) gstreamer v4l2src is getting upset, because it can't enumerate the
   frame sizes (VIDIOC_ENUM_FRAMESIZES fails), which causes it to
   fallback to using the "tvnorms" to decide about frame rates.  This
   makes it impossible to use frame rates higher than 30000/1001, and
   causes the pipeline validation to fail.

0:00:01.937465845 20954  0x15ffe90 DEBUG                   v4l2 gstv4l2object.c:2474:gst_v4l2_object_probe_caps_for_format:<v4l2src0> Enumerating frame sizes for RGGB
0:00:01.937588518 20954  0x15ffe90 DEBUG                   v4l2 gstv4l2object.c:2601:gst_v4l2_object_probe_caps_for_format:<v4l2src0> Failed to enumerate frame sizes for pixelformat RGGB (Inappropriate ioctl for device)
0:00:01.937879535 20954  0x15ffe90 LOG                     v4l2 gstv4l2object.c:2708:gst_v4l2_object_get_nearest_size:<v4l2src0> getting nearest size to 1x1 with format RGGB
0:00:01.937990874 20954  0x15ffe90 LOG                     v4l2 gstv4l2object.c:2724:gst_v4l2_object_get_nearest_size:<v4l2src0> got nearest size 816x616
0:00:01.938250889 20954  0x15ffe90 ERROR                   v4l2 gstv4l2object.c:1873:gst_v4l2_object_get_interlace_mode: Driver bug detected - check driver with v4l2-compliance from http://git.linuxtv.org/v4l-utils.git
0:00:01.938326893 20954  0x15ffe90 LOG                     v4l2 gstv4l2object.c:2708:gst_v4l2_object_get_nearest_size:<v4l2src0> getting nearest size to 32768x32768 with format RGGB
0:00:01.938431566 20954  0x15ffe90 LOG                     v4l2 gstv4l2object.c:2724:gst_v4l2_object_get_nearest_size:<v4l2src0> got nearest size 816x616
0:00:01.939776641 20954  0x15ffe90 ERROR                   v4l2 gstv4l2object.c:1873:gst_v4l2_object_get_interlace_mode: Driver bug detected - check driver with v4l2-compliance from http://git.linuxtv.org/v4l-utils.git
0:00:01.940110660 20954  0x15ffe90 DEBUG                   v4l2 gstv4l2object.c:1955:gst_v4l2_object_get_colorspace: Unknown enum v4l2_colorspace 0

   This triggers the "/* Since we can't get framerate directly, try to
   use the current norm */" code in v4l2object.c, which causes it to
   select one of the 30000/1001 norms:

0:00:01.955927879 20954  0x15ffe90 INFO                    v4l2 gstv4l2object.c:3811:gst_v4l2_object_get_caps:<v4l2src0> probed caps: video/x-bayer, format=(string)rggb, framerate=(fraction)30000/1001, width=(int)816, height=(int)616, pixel-aspect-ratio=(fraction)1/1; video/x-raw, format=(string)I420, framerate=(fraction)30000/1001, width=(int)816, height=(int)616, interlace-mode=(string)progressive, pixel-aspect-ratio=(fraction)1/1; video/x-raw, format=(string)YV12, framerate=(fraction)30000/1001, width=(int)816, height=(int)616, interlace-mode=(string)progressive, pixel-aspect-ratio=(fraction)1/1; video/x-raw, format=(string)BGR, framerate=(fraction)30000/1001, width=(int)816, height=(int)616, interlace-mode=(string)progressive, pixel-aspect-ratio=(fraction)1/1; video/x-raw, format=(string)RGB, framerate=(fraction)30000/1001, width=(int)816, height=(int)616, interlace-mode=(string)progressive, pixel-aspect-ratio=(fraction)1/1

   despite the media pipeline actually being configured for 60fps.

   Forcing it by adjusting the pipeline only results in gstreamer
   failing, because it believes that v4l2 is unable to operate at
   60fps.

   Also note the complaints from v4l2src about the non-compliance...

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[toc] | [next] | [standalone]

#1603849

FromSteve Longerbeam <steve_longerbeam@mentor.com>
Date2017-03-18 21:00 +0100
Message-ID<tmsG5-2No-11@gated-at.bofh.it>
In reply to#1603846

[Multipart message — attachments visible in raw view] — view raw

On 03/18/2017 12:22 PM, Russell King - ARM Linux wrote:
> Hi Steve,
>
> I've just been trying to get gstreamer to capture and h264 encode
> video from my camera at various frame rates, and what I've discovered
> does not look good.
>
> 1) when setting frame rates, media-ctl _always_ calls
>     VIDIOC_SUBDEV_S_FRAME_INTERVAL with pad=0.
>
> 2) media-ctl never retrieves the frame interval information, so there's
>     no way to read it back with standard tools, and no indication that
>     this is going on...

I think Philipp Zabel submitted a patch which addresses these
in media-ctl. Check with him.

>
> 3) gstreamer v4l2src is getting upset, because it can't enumerate the
>     frame sizes (VIDIOC_ENUM_FRAMESIZES fails),

Right, imx-media-capture.c (the "standard" v4l2 user interface module)
is not implementing VIDIOC_ENUM_FRAMESIZES. It should, but it can only
return the single frame size that the pipeline has configured (the mbus
format of the attached source pad).

>   which causes it to
>     fallback to using the "tvnorms" to decide about frame rates.  This
>     makes it impossible to use frame rates higher than 30000/1001, and
>     causes the pipeline validation to fail.

In v5 I added validation of frame intervals between pads,
but due to negative feedback I've pulled that. So next
version will not attempt to validate frame intervals between
source->sink pads.

Can you share your gstreamer pipeline? For now, until
VIDIOC_ENUM_FRAMESIZES is implemented, try a pipeline that
does not attempt to specify a frame rate. I use the attached
script for testing, which works for me.



>
> 0:00:01.937465845 20954  0x15ffe90 DEBUG                   v4l2 gstv4l2object.c:2474:gst_v4l2_object_probe_caps_for_format:<v4l2src0> Enumerating frame sizes for RGGB
> 0:00:01.937588518 20954  0x15ffe90 DEBUG                   v4l2 gstv4l2object.c:2601:gst_v4l2_object_probe_caps_for_format:<v4l2src0> Failed to enumerate frame sizes for pixelformat RGGB (Inappropriate ioctl for device)
> 0:00:01.937879535 20954  0x15ffe90 LOG                     v4l2 gstv4l2object.c:2708:gst_v4l2_object_get_nearest_size:<v4l2src0> getting nearest size to 1x1 with format RGGB
> 0:00:01.937990874 20954  0x15ffe90 LOG                     v4l2 gstv4l2object.c:2724:gst_v4l2_object_get_nearest_size:<v4l2src0> got nearest size 816x616
> 0:00:01.938250889 20954  0x15ffe90 ERROR                   v4l2 gstv4l2object.c:1873:gst_v4l2_object_get_interlace_mode: Driver bug detected - check driver with v4l2-compliance from http://git.linuxtv.org/v4l-utils.git
> 0:00:01.938326893 20954  0x15ffe90 LOG                     v4l2 gstv4l2object.c:2708:gst_v4l2_object_get_nearest_size:<v4l2src0> getting nearest size to 32768x32768 with format RGGB
> 0:00:01.938431566 20954  0x15ffe90 LOG                     v4l2 gstv4l2object.c:2724:gst_v4l2_object_get_nearest_size:<v4l2src0> got nearest size 816x616
> 0:00:01.939776641 20954  0x15ffe90 ERROR                   v4l2 gstv4l2object.c:1873:gst_v4l2_object_get_interlace_mode: Driver bug detected - check driver with v4l2-compliance from http://git.linuxtv.org/v4l-utils.git
> 0:00:01.940110660 20954  0x15ffe90 DEBUG                   v4l2 gstv4l2object.c:1955:gst_v4l2_object_get_colorspace: Unknown enum v4l2_colorspace 0
>
>     This triggers the "/* Since we can't get framerate directly, try to
>     use the current norm */" code in v4l2object.c, which causes it to
>     select one of the 30000/1001 norms:
>
> 0:00:01.955927879 20954  0x15ffe90 INFO                    v4l2 gstv4l2object.c:3811:gst_v4l2_object_get_caps:<v4l2src0> probed caps: video/x-bayer, format=(string)rggb, framerate=(fraction)30000/1001, width=(int)816, height=(int)616, pixel-aspect-ratio=(fraction)1/1; video/x-raw, format=(string)I420, framerate=(fraction)30000/1001, width=(int)816, height=(int)616, interlace-mode=(string)progressive, pixel-aspect-ratio=(fraction)1/1; video/x-raw, format=(string)YV12, framerate=(fraction)30000/1001, width=(int)816, height=(int)616, interlace-mode=(string)progressive, pixel-aspect-ratio=(fraction)1/1; video/x-raw, format=(string)BGR, framerate=(fraction)30000/1001, width=(int)816, height=(int)616, interlace-mode=(string)progressive, pixel-aspect-ratio=(fraction)1/1; video/x-raw, format=(string)RGB, framerate=(fraction)30000/1001, width=(int)816, height=(int)616, interlace-mode=(string)progressive, pixel-aspect-ratio=(fraction)1/1
>
>     despite the media pipeline actually being configured for 60fps.
>
>     Forcing it by adjusting the pipeline only results in gstreamer
>     failing, because it believes that v4l2 is unable to operate at
>     60fps.
>
>     Also note the complaints from v4l2src about the non-compliance...

Thanks, I've fixed most of v4l2-compliance issues, but this is not
done yet. Is that something you can help with?

Steve

[toc] | [prev] | [next] | [standalone]

#1603858

FromRussell King - ARM Linux <linux@armlinux.org.uk>
Date2017-03-18 21:50 +0100
Message-ID<tmtsu-3nO-7@gated-at.bofh.it>
In reply to#1603849
On Sat, Mar 18, 2017 at 12:58:27PM -0700, Steve Longerbeam wrote:
> Can you share your gstreamer pipeline? For now, until
> VIDIOC_ENUM_FRAMESIZES is implemented, try a pipeline that
> does not attempt to specify a frame rate. I use the attached
> script for testing, which works for me.

It's nothing more than

  gst-launch-1.0 -v v4l2src ! <any needed conversions> ! xvimagesink

in my case, the conversions are bayer2rgbneon.  However, this only shows
you the frame rate negotiated on the pads (which is actually good enough
to show the issue.)

How I stumbled across though this was when I was trying to encode:

 gst-launch-1.0 v4l2src device=/dev/video9 ! bayer2rgbneon ! \
	videoconvert ! x264enc speed-preset=1 ! avimux ! \
	filesink location=test.avi

I noticed that vlc would always say it was playing the resulting AVI
at 30fps.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[toc] | [prev] | [next] | [standalone]

#1603875

FromNicolas Dufresne <nicolas@ndufresne.ca>
Date2017-03-19 01:50 +0100
Message-ID<tmxcJ-64K-3@gated-at.bofh.it>
In reply to#1603858

[Multipart message — attachments visible in raw view] — view raw

Le samedi 18 mars 2017 à 20:43 +0000, Russell King - ARM Linux a
écrit :
> On Sat, Mar 18, 2017 at 12:58:27PM -0700, Steve Longerbeam wrote:
> > Can you share your gstreamer pipeline? For now, until
> > VIDIOC_ENUM_FRAMESIZES is implemented, try a pipeline that
> > does not attempt to specify a frame rate. I use the attached
> > script for testing, which works for me.
> 
> It's nothing more than
> 
>   gst-launch-1.0 -v v4l2src ! <any needed conversions> ! xvimagesink
> 
> in my case, the conversions are bayer2rgbneon.  However, this only
> shows
> you the frame rate negotiated on the pads (which is actually good
> enough
> to show the issue.)
> 
> How I stumbled across though this was when I was trying to encode:
> 
>  gst-launch-1.0 v4l2src device=/dev/video9 ! bayer2rgbneon ! \
>         videoconvert ! x264enc speed-preset=1 ! avimux ! \
>         filesink location=test.avi
> 
> I noticed that vlc would always say it was playing the resulting AVI
> at 30fps.

In practice, I have the impression there is a fair reason why framerate
enumeration isn't implemented (considering there is only 1 valid rate).
Along with the norm fallback, GStreamer could could also consider the
currently set framerate as returned by VIDIOC_G_PARM. At the same time,
implementing that enumeration shall be straightforward, and will make a
large amount of existing userspace work.

regards,
Nicolas

[toc] | [prev] | [next] | [standalone]

#1603876

FromRussell King - ARM Linux <linux@armlinux.org.uk>
Date2017-03-19 02:10 +0100
Message-ID<tmxw5-6sR-1@gated-at.bofh.it>
In reply to#1603875
On Sat, Mar 18, 2017 at 08:41:14PM -0400, Nicolas Dufresne wrote:
> Le samedi 18 mars 2017 à 20:43 +0000, Russell King - ARM Linux a
> écrit :
> > On Sat, Mar 18, 2017 at 12:58:27PM -0700, Steve Longerbeam wrote:
> > > Can you share your gstreamer pipeline? For now, until
> > > VIDIOC_ENUM_FRAMESIZES is implemented, try a pipeline that
> > > does not attempt to specify a frame rate. I use the attached
> > > script for testing, which works for me.
> > 
> > It's nothing more than
> > 
> >   gst-launch-1.0 -v v4l2src ! <any needed conversions> ! xvimagesink
> > 
> > in my case, the conversions are bayer2rgbneon.  However, this only
> > shows
> > you the frame rate negotiated on the pads (which is actually good
> > enough
> > to show the issue.)
> > 
> > How I stumbled across though this was when I was trying to encode:
> > 
> >  gst-launch-1.0 v4l2src device=/dev/video9 ! bayer2rgbneon ! \
> >         videoconvert ! x264enc speed-preset=1 ! avimux ! \
> >         filesink location=test.avi
> > 
> > I noticed that vlc would always say it was playing the resulting AVI
> > at 30fps.
> 
> In practice, I have the impression there is a fair reason why framerate
> enumeration isn't implemented (considering there is only 1 valid rate).

That's actually completely incorrect.

With the capture device interfacing directly with CSI, it's possible
_today_ to select:

* the CSI sink pad's resolution
* the CSI sink pad's resolution with the width and/or height halved
* the CSI sink pad's frame rate
* the CSI sink pad's frame rate divided by the frame drop factor

To put it another way, these are possible:

# v4l2-ctl -d /dev/video10 --list-formats-ext
ioctl: VIDIOC_ENUM_FMT
        Index       : 0
        Type        : Video Capture
        Pixel Format: 'RGGB'
        Name        : 8-bit Bayer RGRG/GBGB
                Size: Discrete 816x616
                        Interval: Discrete 0.040s (25.000 fps)
                        Interval: Discrete 0.048s (20.833 fps)
                        Interval: Discrete 0.050s (20.000 fps)
                        Interval: Discrete 0.053s (18.750 fps)
                        Interval: Discrete 0.060s (16.667 fps)
                        Interval: Discrete 0.067s (15.000 fps)
                        Interval: Discrete 0.080s (12.500 fps)
                        Interval: Discrete 0.100s (10.000 fps)
                        Interval: Discrete 0.120s (8.333 fps)
                        Interval: Discrete 0.160s (6.250 fps)
                        Interval: Discrete 0.200s (5.000 fps)
                        Interval: Discrete 0.240s (4.167 fps)
                Size: Discrete 408x616
<same intervals>
                Size: Discrete 816x308
<same intervals>
                Size: Discrete 408x308
<same intervals>

These don't become possible as a result of implementing the enums,
they're all already requestable through /dev/video10.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[toc] | [prev] | [next] | [standalone]

#1603995

FromRussell King - ARM Linux <linux@armlinux.org.uk>
Date2017-03-19 16:00 +0100
Message-ID<tmKtj-7am-11@gated-at.bofh.it>
In reply to#1603876
On Sun, Mar 19, 2017 at 10:33:25AM -0400, Nicolas Dufresne wrote:
> Le dimanche 19 mars 2017 à 00:54 +0000, Russell King - ARM Linux a
> écrit :
> > > 
> > > In practice, I have the impression there is a fair reason why
> > > framerate
> > > enumeration isn't implemented (considering there is only 1 valid
> > > rate).
> > 
> > That's actually completely incorrect.
> > 
> > With the capture device interfacing directly with CSI, it's possible
> > _today_ to select:
> > 
> > * the CSI sink pad's resolution
> > * the CSI sink pad's resolution with the width and/or height halved
> > * the CSI sink pad's frame rate
> > * the CSI sink pad's frame rate divided by the frame drop factor
> > 
> > To put it another way, these are possible:
> > 
> > # v4l2-ctl -d /dev/video10 --list-formats-ext
> > ioctl: VIDIOC_ENUM_FMT
> >         Index       : 0
> >         Type        : Video Capture
> >         Pixel Format: 'RGGB'
> >         Name        : 8-bit Bayer RGRG/GBGB
> >                 Size: Discrete 816x616
> >                         Interval: Discrete 0.040s (25.000 fps)
> >                         Interval: Discrete 0.048s (20.833 fps)
> >                         Interval: Discrete 0.050s (20.000 fps)
> >                         Interval: Discrete 0.053s (18.750 fps)
> >                         Interval: Discrete 0.060s (16.667 fps)
> >                         Interval: Discrete 0.067s (15.000 fps)
> >                         Interval: Discrete 0.080s (12.500 fps)
> >                         Interval: Discrete 0.100s (10.000 fps)
> >                         Interval: Discrete 0.120s (8.333 fps)
> >                         Interval: Discrete 0.160s (6.250 fps)
> >                         Interval: Discrete 0.200s (5.000 fps)
> >                         Interval: Discrete 0.240s (4.167 fps)
> >                 Size: Discrete 408x616
> > <same intervals>
> >                 Size: Discrete 816x308
> > <same intervals>
> >                 Size: Discrete 408x308
> > <same intervals>
> > 
> > These don't become possible as a result of implementing the enums,
> > they're all already requestable through /dev/video10.
> 
> Ok that wasn't clear. So basically video9 is a front-end to video10,
> and it does not proxy the enumerations.

No.  We've sent .dot graphs which show the structure of the imx capture
driver.

What we have wrt video nodes is (eg):

sensor --->  csi2 ----> mux ---> csi ----+------> csi capture
subdev      subdev    subdev    subdev   |       /dev/video10
                                         |
                                         +---------\
                                         |          \
                                         +--> vdic ---> ic_prpenc ---> ic_prpenc
                                             subdev     subdev         capture

... etc ... for full details, see the .dot diagrams that have been
sent (sorry I can't recall where they are in the threads.)

> I understand this is what you
> are now fixing. And this has to be fixed, because I can image cases
> where the front-end could support only a subset of the sub-dev. So
> having userspace enumerate on another device (and having to find this
> device by walking the tree) is unlikely to work in all scenarios.

The capture blocks (imx-media-capture) all talk to their immediate
upstream subdev and configure its source pad according to the formats,
frame size and frame interval requested by the capture application.
The subdev source pad decides whether the request is valid, and allows
it, modifies it or rejects it as appropriate.

Without working enumeration support, there's no way for an application
to find out what possible settings there are, and, as I've already
explained, the CSI subdev is capable itself of two things:

* Scaling down the image by a factor of two independently in the
  horizontal and vertical directions
* Deterministically dropping frames received from its upstream
  element, thereby reducing the frame rate.

> p.s. This is why caps negotiation is annoyingly complex in GStreamer,
> specially that there is no shortcut, you connect pads, and they figure-
> out what format they will use between each other.

Right, so when you specify video/x-raw,...,framerate=60/1 it introduces
a new element which has one source and sink pad, which only supports
the specification given.  If the neighbour's pad doesn't support it,
gstreamer fails because the caps negotiation fails.

So, if v4l2src believes (via the tvnorms, because it's lacking any
other information) that the capture device can only do 25fps and
30fps, then trying to set 60fps _even if S_PARM may accept it_ will
cause gstreamer to fail - because v4l2src can only advertise that
it supports a source of 25fps and 30fps.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[toc] | [prev] | [next] | [standalone]

#1604000

FromNicolas Dufresne <nicolas@ndufresne.ca>
Date2017-03-19 16:10 +0100
Message-ID<tmKtj-7am-13@gated-at.bofh.it>
In reply to#1603876

[Multipart message — attachments visible in raw view] — view raw

Le dimanche 19 mars 2017 à 00:54 +0000, Russell King - ARM Linux a
écrit :
> > 
> > In practice, I have the impression there is a fair reason why
> > framerate
> > enumeration isn't implemented (considering there is only 1 valid
> > rate).
> 
> That's actually completely incorrect.
> 
> With the capture device interfacing directly with CSI, it's possible
> _today_ to select:
> 
> * the CSI sink pad's resolution
> * the CSI sink pad's resolution with the width and/or height halved
> * the CSI sink pad's frame rate
> * the CSI sink pad's frame rate divided by the frame drop factor
> 
> To put it another way, these are possible:
> 
> # v4l2-ctl -d /dev/video10 --list-formats-ext
> ioctl: VIDIOC_ENUM_FMT
>         Index       : 0
>         Type        : Video Capture
>         Pixel Format: 'RGGB'
>         Name        : 8-bit Bayer RGRG/GBGB
>                 Size: Discrete 816x616
>                         Interval: Discrete 0.040s (25.000 fps)
>                         Interval: Discrete 0.048s (20.833 fps)
>                         Interval: Discrete 0.050s (20.000 fps)
>                         Interval: Discrete 0.053s (18.750 fps)
>                         Interval: Discrete 0.060s (16.667 fps)
>                         Interval: Discrete 0.067s (15.000 fps)
>                         Interval: Discrete 0.080s (12.500 fps)
>                         Interval: Discrete 0.100s (10.000 fps)
>                         Interval: Discrete 0.120s (8.333 fps)
>                         Interval: Discrete 0.160s (6.250 fps)
>                         Interval: Discrete 0.200s (5.000 fps)
>                         Interval: Discrete 0.240s (4.167 fps)
>                 Size: Discrete 408x616
> <same intervals>
>                 Size: Discrete 816x308
> <same intervals>
>                 Size: Discrete 408x308
> <same intervals>
> 
> These don't become possible as a result of implementing the enums,
> they're all already requestable through /dev/video10.

Ok that wasn't clear. So basically video9 is a front-end to video10,
and it does not proxy the enumerations. I understand this is what you
are now fixing. And this has to be fixed, because I can image cases
where the front-end could support only a subset of the sub-dev. So
having userspace enumerate on another device (and having to find this
device by walking the tree) is unlikely to work in all scenarios.

regards,
Nicolas

p.s. This is why caps negotiation is annoyingly complex in GStreamer,
specially that there is no shortcut, you connect pads, and they figure-
out what format they will use between each other.

[toc] | [prev] | [next] | [standalone]

#1603927

FromRussell King - ARM Linux <linux@armlinux.org.uk>
Date2017-03-19 11:00 +0100
Message-ID<tmFN0-3Je-7@gated-at.bofh.it>
In reply to#1603875
On Sat, Mar 18, 2017 at 08:41:14PM -0400, Nicolas Dufresne wrote:
> Along with the norm fallback, GStreamer could could also consider the
> currently set framerate as returned by VIDIOC_G_PARM. At the same time,
> implementing that enumeration shall be straightforward, and will make a
> large amount of existing userspace work.

Since, according to v4l2-compliance, providing the enumeration ioctls
appears to be optional:

1) should v4l2-compliance be checking whether other frame sizes/frame
   intervals are possible, and failing if the enumeration ioctls are
   not supported?

2) would it also make sense to allow gstreamer's v4l2src to try setting
   a these parameters, and only fail if it's unable to set it?  IOW, if
   I use:

gst-launch-1.0 v4l2src device=/dev/video10 ! \
	video/x-bayer,format=RGGB,framerate=20/1 ! ...

   where G_PARM says its currently configured for 25fps, but a S_PARM
   with 20fps would actually succeed.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[toc] | [prev] | [next] | [standalone]

#1603992

FromNicolas Dufresne <nicolas@ndufresne.ca>
Date2017-03-19 15:50 +0100
Message-ID<tmKjE-75i-11@gated-at.bofh.it>
In reply to#1603927

[Multipart message — attachments visible in raw view] — view raw

Le dimanche 19 mars 2017 à 09:55 +0000, Russell King - ARM Linux a
écrit :
> 2) would it also make sense to allow gstreamer's v4l2src to try
> setting
>    a these parameters, and only fail if it's unable to set it?  IOW,
> if
>    I use:
> 
> gst-launch-1.0 v4l2src device=/dev/video10 ! \
>         video/x-bayer,format=RGGB,framerate=20/1 ! ...
> 
>    where G_PARM says its currently configured for 25fps, but a S_PARM
>    with 20fps would actually succeed.

In current design, v4l2src will "probe" all possible formats, cache
this, and use this information for negotiation. So after the caps has
been probed, there will be no TRY_FMT or anything like this happening
until it's too late. You have spotted a bug though, it should be
reading back the parm structure to validate (and probably produce a
not-negotiated error here).

Recently, specially for the IMX work done by Pengutronix, there was
contributions to enhance this probing to support probing capabilities
that are not enumerable (e.g. interlacing, colorimetry) using TRY_FMT.
There is no TRY_PARM in the API to implement similar fallback. Also,
those ended up creating a massive disaster for slow cameras. We now
have UVC cameras that takes 6s or more to start. I have no other choice
but to rewrite that now. We will negotiate the non-enumerable at the
last minute with TRY_FMT (when the subset is at it's smallest). This
will by accident add support for this camera interface, but that wasn't
the goal. It would still fail with application that enumerates the
possible resolutions and framerate and let you select them with a drop-
down (like cheese). In general, I can only conclude that making
everything that matter enumerable is the only working way to go for
generic userspace. 

Nicolas

[toc] | [prev] | [next] | [standalone]

#1603982

FromVladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
Date2017-03-19 15:00 +0100
Message-ID<tmJxg-6on-29@gated-at.bofh.it>
In reply to#1603858
Hi Russell,

On 03/18/2017 10:43 PM, Russell King - ARM Linux wrote:
> On Sat, Mar 18, 2017 at 12:58:27PM -0700, Steve Longerbeam wrote:
>> Can you share your gstreamer pipeline? For now, until
>> VIDIOC_ENUM_FRAMESIZES is implemented, try a pipeline that
>> does not attempt to specify a frame rate. I use the attached
>> script for testing, which works for me.
> 
> It's nothing more than
> 
>   gst-launch-1.0 -v v4l2src ! <any needed conversions> ! xvimagesink
> 
> in my case, the conversions are bayer2rgbneon.  However, this only shows
> you the frame rate negotiated on the pads (which is actually good enough
> to show the issue.)

I'm sorry for potential offtopic, but is bayer2rgbneon element found in
any officially supported by GStreamer plugin? Can it be a point of
failure?

--
With best wishes,
Vladimir

[toc] | [prev] | [next] | [standalone]

#1603987

FromRussell King - ARM Linux <linux@armlinux.org.uk>
Date2017-03-19 15:30 +0100
Message-ID<tmK0h-6VG-1@gated-at.bofh.it>
In reply to#1603982
On Sun, Mar 19, 2017 at 02:21:10PM +0000, Russell King - ARM Linux wrote:
> There's a good reason why I dumped a full debug log using GST_DEBUG=*:9,
> analysed it for the cause of the failure, and tried several different
> pipelines, including the standard bayer2rgb plugin.
> 
> Please don't blame this on random stuff after analysis of the logs _and_
> reading the appropriate plugin code has shown where the problem is.  I
> know gstreamer can be very complex, but it's very possible to analyse
> the cause of problems and pin them down with detailed logs in conjunction
> with the source code.

Oh, and the proof of correct analysis is that fixing the kernel capture
driver to enumerate the frame sizes and intervals fixes the issue, even
with bayer2rgbneon being used.

Therefore, there is _no way_ what so ever that it could be caused by that
plugin.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[toc] | [prev] | [next] | [standalone]

#1603998

FromVladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
Date2017-03-19 16:10 +0100
Message-ID<tmKCZ-7uc-11@gated-at.bofh.it>
In reply to#1603987
On 03/19/2017 04:22 PM, Russell King - ARM Linux wrote:
> On Sun, Mar 19, 2017 at 02:21:10PM +0000, Russell King - ARM Linux wrote:
>> There's a good reason why I dumped a full debug log using GST_DEBUG=*:9,
>> analysed it for the cause of the failure, and tried several different
>> pipelines, including the standard bayer2rgb plugin.
>>
>> Please don't blame this on random stuff after analysis of the logs _and_
>> reading the appropriate plugin code has shown where the problem is.  I
>> know gstreamer can be very complex, but it's very possible to analyse
>> the cause of problems and pin them down with detailed logs in conjunction
>> with the source code.
> 
> Oh, and the proof of correct analysis is that fixing the kernel capture
> driver to enumerate the frame sizes and intervals fixes the issue, even
> with bayer2rgbneon being used.
> 
> Therefore, there is _no way_ what so ever that it could be caused by that
> plugin.
> 

Hey, no blaming of the unknown to me bayer2rgbneon element from my side,
I've just asked an innocent question, thanks for reply. I failed to find
the source code of the plugin, I was interested to compare its performance
and features with mine in-house NEON powered RGGB/BGGR to RGB24 GStreamer
conversion element, which is written years ago. My question was offtopic.

--
With best wishes,
Vladimir

[toc] | [prev] | [next] | [standalone]

#1604001

FromRussell King - ARM Linux <linux@armlinux.org.uk>
Date2017-03-19 16:20 +0100
Message-ID<tmKMF-7B1-13@gated-at.bofh.it>
In reply to#1603998
On Sun, Mar 19, 2017 at 05:00:08PM +0200, Vladimir Zapolskiy wrote:
> On 03/19/2017 04:22 PM, Russell King - ARM Linux wrote:
> > On Sun, Mar 19, 2017 at 02:21:10PM +0000, Russell King - ARM Linux wrote:
> >> There's a good reason why I dumped a full debug log using GST_DEBUG=*:9,
> >> analysed it for the cause of the failure, and tried several different
> >> pipelines, including the standard bayer2rgb plugin.
> >>
> >> Please don't blame this on random stuff after analysis of the logs _and_
> >> reading the appropriate plugin code has shown where the problem is.  I
> >> know gstreamer can be very complex, but it's very possible to analyse
> >> the cause of problems and pin them down with detailed logs in conjunction
> >> with the source code.
> > 
> > Oh, and the proof of correct analysis is that fixing the kernel capture
> > driver to enumerate the frame sizes and intervals fixes the issue, even
> > with bayer2rgbneon being used.
> > 
> > Therefore, there is _no way_ what so ever that it could be caused by that
> > plugin.
> > 
> 
> Hey, no blaming of the unknown to me bayer2rgbneon element from my side,
> I've just asked an innocent question, thanks for reply. I failed to find
> the source code of the plugin, I was interested to compare its performance
> and features with mine in-house NEON powered RGGB/BGGR to RGB24 GStreamer
> conversion element, which is written years ago. My question was offtopic.

If you wanted to know where to get it from, you should've asked that.
You can find all the bits here:

	https://git.phytec.de/

You need bayer2rgb-neon and gst-bayer2rgb-neon, and it requires some
fixes to the configure script and Makefiles get it to build if you
don't have gengenopt available.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[toc] | [prev] | [next] | [standalone]

#1603988

FromRussell King - ARM Linux <linux@armlinux.org.uk>
Date2017-03-19 15:30 +0100
Message-ID<tmK0h-6VG-3@gated-at.bofh.it>
In reply to#1603982
On Sun, Mar 19, 2017 at 03:57:56PM +0200, Vladimir Zapolskiy wrote:
> Hi Russell,
> 
> On 03/18/2017 10:43 PM, Russell King - ARM Linux wrote:
> > On Sat, Mar 18, 2017 at 12:58:27PM -0700, Steve Longerbeam wrote:
> >> Can you share your gstreamer pipeline? For now, until
> >> VIDIOC_ENUM_FRAMESIZES is implemented, try a pipeline that
> >> does not attempt to specify a frame rate. I use the attached
> >> script for testing, which works for me.
> > 
> > It's nothing more than
> > 
> >   gst-launch-1.0 -v v4l2src ! <any needed conversions> ! xvimagesink
> > 
> > in my case, the conversions are bayer2rgbneon.  However, this only shows
> > you the frame rate negotiated on the pads (which is actually good enough
> > to show the issue.)
> 
> I'm sorry for potential offtopic, but is bayer2rgbneon element found in
> any officially supported by GStreamer plugin?

No it isn't.  Google is wonderful, please make use of planetary search
facilities.

> Can it be a point of failure?

There's a good reason why I dumped a full debug log using GST_DEBUG=*:9,
analysed it for the cause of the failure, and tried several different
pipelines, including the standard bayer2rgb plugin.

Please don't blame this on random stuff after analysis of the logs _and_
reading the appropriate plugin code has shown where the problem is.  I
know gstreamer can be very complex, but it's very possible to analyse
the cause of problems and pin them down with detailed logs in conjunction
with the source code.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[toc] | [prev] | [next] | [standalone]

#1603993

FromNicolas Dufresne <nicolas@ndufresne.ca>
Date2017-03-19 15:50 +0100
Message-ID<tmKjE-75i-17@gated-at.bofh.it>
In reply to#1603988

[Multipart message — attachments visible in raw view] — view raw

Le dimanche 19 mars 2017 à 14:21 +0000, Russell King - ARM Linux a
écrit :
> > Can it be a point of failure?
> 
> There's a good reason why I dumped a full debug log using
> GST_DEBUG=*:9,
> analysed it for the cause of the failure, and tried several different
> pipelines, including the standard bayer2rgb plugin.
> 
> Please don't blame this on random stuff after analysis of the logs
> _and_
> reading the appropriate plugin code has shown where the problem is. 
> I
> know gstreamer can be very complex, but it's very possible to analyse
> the cause of problems and pin them down with detailed logs in
> conjunction
> with the source code.

I read your analyses with GStreamer, and it was all correct.

Nicolas

[toc] | [prev] | [next] | [standalone]

#1603936

FromRussell King - ARM Linux <linux@armlinux.org.uk>
Date2017-03-19 11:50 +0100
Message-ID<tmGzo-4lF-13@gated-at.bofh.it>
In reply to#1603849
On Sat, Mar 18, 2017 at 12:58:27PM -0700, Steve Longerbeam wrote:
> Right, imx-media-capture.c (the "standard" v4l2 user interface module)
> is not implementing VIDIOC_ENUM_FRAMESIZES. It should, but it can only
> return the single frame size that the pipeline has configured (the mbus
> format of the attached source pad).

I now have a set of patches that enumerate the frame sizes and intervals
from the source pad of the first subdev (since you're setting the formats
etc there from the capture device, it seems sensible to return what it
can support.)  This means my patch set doesn't add to non-CSI subdevs.

> Can you share your gstreamer pipeline? For now, until
> VIDIOC_ENUM_FRAMESIZES is implemented, try a pipeline that
> does not attempt to specify a frame rate. I use the attached
> script for testing, which works for me.

Note that I'm not specifying a frame rate on gstreamer - I'm setting
the pipeline up for 60fps, but gstreamer in its wisdom is unable to
enumerate the frame sizes, and therefore is unable to enumerate the
frame intervals (frame intervals depend on frame sizes), so it
falls back to the "tvnorms" which are basically 25/1 and 30000/1001.

It sees 60fps via G_PARM, and then decides to set 30000/1001 via S_PARM.
So, we end up with most of the pipeline operating at 60fps, with CSI
doing frame skipping to reduce the frame rate to 30fps.

gstreamer doesn't complain, doesn't issue any warnings, the only way
you can spot this is to enable debugging and look through the copious
debug log, or use -v and check the pad capabilities.

Testing using gstreamer, and only using "does it produce video" is a
good simple test, but it's just that - it's a simple test.  It doesn't
tell you that what you're seeing is what you intended to see (such as
video at the frame rate you expected) without more work.

> Thanks, I've fixed most of v4l2-compliance issues, but this is not
> done yet. Is that something you can help with?

What did you do with:

ioctl(3, VIDIOC_REQBUFS, {count=0, type=0 /* V4L2_BUF_TYPE_??? */, memory=0 /* V4L2_MEMORY_??? */}) = -1 EINVAL (Invalid argument)
                test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
ioctl(3, VIDIOC_EXPBUF, 0xbef405bc)     = -1 EINVAL (Invalid argument)
                fail: v4l2-test-buffers.cpp(571): q.has_expbuf(node)
                test VIDIOC_EXPBUF: FAIL

To me, this looks like a bug in v4l2-compliance (I'm using 1.10.0).
I'm not sure what buffer VIDIOC_EXPBUF is expected to export, since
afaics no buffers have been allocated, so of course it's going to fail.
Either that, or the v4l2 core vb2 code is non-compliant with v4l2's
interface requirements.

In any case, it doesn't look like the buffer management is being
tested at all by v4l2-compliance - we know that gstreamer works, so
buffers _can_ be allocated, and I've also used dmabufs with gstreamer,
so I also know that VIDIOC_EXPBUF works there.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[toc] | [prev] | [next] | [standalone]

#1603938 — [PATCH 2/4] media: imx: allow bayer pixel formats to be looked up

FromRussell King <rmk+kernel@armlinux.org.uk>
Date2017-03-19 11:50 +0100
Subject[PATCH 2/4] media: imx: allow bayer pixel formats to be looked up
Message-ID<tmGzo-4lF-15@gated-at.bofh.it>
In reply to#1603936
Allow imx_media_find_format() to look up bayer formats, which is
required to support frame size and interval enumeration.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/staging/media/imx/imx-media-capture.c | 11 ++++++-----
 drivers/staging/media/imx/imx-media-utils.c   |  6 +++---
 drivers/staging/media/imx/imx-media.h         |  2 +-
 3 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/media/imx/imx-media-capture.c b/drivers/staging/media/imx/imx-media-capture.c
index ee914396080f..cdeb2cd8b1d7 100644
--- a/drivers/staging/media/imx/imx-media-capture.c
+++ b/drivers/staging/media/imx/imx-media-capture.c
@@ -164,10 +164,10 @@ static int capture_try_fmt_vid_cap(struct file *file, void *fh,
 			CS_SEL_YUV : CS_SEL_RGB;
 		fourcc = f->fmt.pix.pixelformat;
 
-		cc = imx_media_find_format(fourcc, cs_sel);
+		cc = imx_media_find_format(fourcc, cs_sel, false);
 		if (!cc) {
 			imx_media_enum_format(&fourcc, 0, cs_sel);
-			cc = imx_media_find_format(fourcc, cs_sel);
+			cc = imx_media_find_format(fourcc, cs_sel, false);
 		}
 	}
 
@@ -193,7 +193,7 @@ static int capture_s_fmt_vid_cap(struct file *file, void *fh,
 
 	priv->vdev.fmt.fmt.pix = f->fmt.pix;
 	priv->vdev.cc = imx_media_find_format(f->fmt.pix.pixelformat,
-					      CS_SEL_ANY);
+					      CS_SEL_ANY, false);
 
 	return 0;
 }
@@ -505,7 +505,8 @@ void imx_media_capture_device_set_format(struct imx_media_video_dev *vdev,
 
 	mutex_lock(&priv->mutex);
 	priv->vdev.fmt.fmt.pix = *pix;
-	priv->vdev.cc = imx_media_find_format(pix->pixelformat, CS_SEL_ANY);
+	priv->vdev.cc = imx_media_find_format(pix->pixelformat, CS_SEL_ANY,
+					      false);
 	mutex_unlock(&priv->mutex);
 }
 EXPORT_SYMBOL_GPL(imx_media_capture_device_set_format);
@@ -614,7 +615,7 @@ int imx_media_capture_device_register(struct imx_media_video_dev *vdev)
 	imx_media_mbus_fmt_to_pix_fmt(&vdev->fmt.fmt.pix,
 				      &fmt_src.format, NULL);
 	vdev->cc = imx_media_find_format(vdev->fmt.fmt.pix.pixelformat,
-					 CS_SEL_ANY);
+					 CS_SEL_ANY, false);
 
 	v4l2_info(sd, "Registered %s as /dev/%s\n", vfd->name,
 		  video_device_node_name(vfd));
diff --git a/drivers/staging/media/imx/imx-media-utils.c b/drivers/staging/media/imx/imx-media-utils.c
index 6eb7e3c5279e..d048e4a080d0 100644
--- a/drivers/staging/media/imx/imx-media-utils.c
+++ b/drivers/staging/media/imx/imx-media-utils.c
@@ -329,9 +329,9 @@ static int enum_format(u32 *fourcc, u32 *code, u32 index,
 }
 
 const struct imx_media_pixfmt *
-imx_media_find_format(u32 fourcc, enum codespace_sel cs_sel)
+imx_media_find_format(u32 fourcc, enum codespace_sel cs_sel, bool allow_bayer)
 {
-	return find_format(fourcc, 0, cs_sel, true, false);
+	return find_format(fourcc, 0, cs_sel, true, allow_bayer);
 }
 EXPORT_SYMBOL_GPL(imx_media_find_format);
 
@@ -524,7 +524,7 @@ int imx_media_ipu_image_to_mbus_fmt(struct v4l2_mbus_framefmt *mbus,
 {
 	const struct imx_media_pixfmt *fmt;
 
-	fmt = imx_media_find_format(image->pix.pixelformat, CS_SEL_ANY);
+	fmt = imx_media_find_format(image->pix.pixelformat, CS_SEL_ANY, false);
 	if (!fmt)
 		return -EINVAL;
 
diff --git a/drivers/staging/media/imx/imx-media.h b/drivers/staging/media/imx/imx-media.h
index 234242271a13..d8c9536bf1f8 100644
--- a/drivers/staging/media/imx/imx-media.h
+++ b/drivers/staging/media/imx/imx-media.h
@@ -178,7 +178,7 @@ enum codespace_sel {
 };
 
 const struct imx_media_pixfmt *
-imx_media_find_format(u32 fourcc, enum codespace_sel cs_sel);
+imx_media_find_format(u32 fourcc, enum codespace_sel cs_sel, bool allow_bayer);
 int imx_media_enum_format(u32 *fourcc, u32 index, enum codespace_sel cs_sel);
 const struct imx_media_pixfmt *
 imx_media_find_mbus_format(u32 code, enum codespace_sel cs_sel,
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1604100 — Re: [PATCH 2/4] media: imx: allow bayer pixel formats to be looked up

FromSteve Longerbeam <slongerbeam@gmail.com>
Date2017-03-19 23:20 +0100
SubjectRe: [PATCH 2/4] media: imx: allow bayer pixel formats to be looked up
Message-ID<tmRl8-3JU-15@gated-at.bofh.it>
In reply to#1603938
This is good too, but if it's all right with you I would prefer to
squash this with the "redo  pixel format enumeration and
negotiation" patch, to keep the patch count down.

Steve


On 03/19/2017 03:48 AM, Russell King wrote:
> Allow imx_media_find_format() to look up bayer formats, which is
> required to support frame size and interval enumeration.
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
>   drivers/staging/media/imx/imx-media-capture.c | 11 ++++++-----
>   drivers/staging/media/imx/imx-media-utils.c   |  6 +++---
>   drivers/staging/media/imx/imx-media.h         |  2 +-
>   3 files changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/staging/media/imx/imx-media-capture.c b/drivers/staging/media/imx/imx-media-capture.c
> index ee914396080f..cdeb2cd8b1d7 100644
> --- a/drivers/staging/media/imx/imx-media-capture.c
> +++ b/drivers/staging/media/imx/imx-media-capture.c
> @@ -164,10 +164,10 @@ static int capture_try_fmt_vid_cap(struct file *file, void *fh,
>   			CS_SEL_YUV : CS_SEL_RGB;
>   		fourcc = f->fmt.pix.pixelformat;
>   
> -		cc = imx_media_find_format(fourcc, cs_sel);
> +		cc = imx_media_find_format(fourcc, cs_sel, false);
>   		if (!cc) {
>   			imx_media_enum_format(&fourcc, 0, cs_sel);
> -			cc = imx_media_find_format(fourcc, cs_sel);
> +			cc = imx_media_find_format(fourcc, cs_sel, false);
>   		}
>   	}
>   
> @@ -193,7 +193,7 @@ static int capture_s_fmt_vid_cap(struct file *file, void *fh,
>   
>   	priv->vdev.fmt.fmt.pix = f->fmt.pix;
>   	priv->vdev.cc = imx_media_find_format(f->fmt.pix.pixelformat,
> -					      CS_SEL_ANY);
> +					      CS_SEL_ANY, false);
>   
>   	return 0;
>   }
> @@ -505,7 +505,8 @@ void imx_media_capture_device_set_format(struct imx_media_video_dev *vdev,
>   
>   	mutex_lock(&priv->mutex);
>   	priv->vdev.fmt.fmt.pix = *pix;
> -	priv->vdev.cc = imx_media_find_format(pix->pixelformat, CS_SEL_ANY);
> +	priv->vdev.cc = imx_media_find_format(pix->pixelformat, CS_SEL_ANY,
> +					      false);
>   	mutex_unlock(&priv->mutex);
>   }
>   EXPORT_SYMBOL_GPL(imx_media_capture_device_set_format);
> @@ -614,7 +615,7 @@ int imx_media_capture_device_register(struct imx_media_video_dev *vdev)
>   	imx_media_mbus_fmt_to_pix_fmt(&vdev->fmt.fmt.pix,
>   				      &fmt_src.format, NULL);
>   	vdev->cc = imx_media_find_format(vdev->fmt.fmt.pix.pixelformat,
> -					 CS_SEL_ANY);
> +					 CS_SEL_ANY, false);
>   
>   	v4l2_info(sd, "Registered %s as /dev/%s\n", vfd->name,
>   		  video_device_node_name(vfd));
> diff --git a/drivers/staging/media/imx/imx-media-utils.c b/drivers/staging/media/imx/imx-media-utils.c
> index 6eb7e3c5279e..d048e4a080d0 100644
> --- a/drivers/staging/media/imx/imx-media-utils.c
> +++ b/drivers/staging/media/imx/imx-media-utils.c
> @@ -329,9 +329,9 @@ static int enum_format(u32 *fourcc, u32 *code, u32 index,
>   }
>   
>   const struct imx_media_pixfmt *
> -imx_media_find_format(u32 fourcc, enum codespace_sel cs_sel)
> +imx_media_find_format(u32 fourcc, enum codespace_sel cs_sel, bool allow_bayer)
>   {
> -	return find_format(fourcc, 0, cs_sel, true, false);
> +	return find_format(fourcc, 0, cs_sel, true, allow_bayer);
>   }
>   EXPORT_SYMBOL_GPL(imx_media_find_format);
>   
> @@ -524,7 +524,7 @@ int imx_media_ipu_image_to_mbus_fmt(struct v4l2_mbus_framefmt *mbus,
>   {
>   	const struct imx_media_pixfmt *fmt;
>   
> -	fmt = imx_media_find_format(image->pix.pixelformat, CS_SEL_ANY);
> +	fmt = imx_media_find_format(image->pix.pixelformat, CS_SEL_ANY, false);
>   	if (!fmt)
>   		return -EINVAL;
>   
> diff --git a/drivers/staging/media/imx/imx-media.h b/drivers/staging/media/imx/imx-media.h
> index 234242271a13..d8c9536bf1f8 100644
> --- a/drivers/staging/media/imx/imx-media.h
> +++ b/drivers/staging/media/imx/imx-media.h
> @@ -178,7 +178,7 @@ enum codespace_sel {
>   };
>   
>   const struct imx_media_pixfmt *
> -imx_media_find_format(u32 fourcc, enum codespace_sel cs_sel);
> +imx_media_find_format(u32 fourcc, enum codespace_sel cs_sel, bool allow_bayer);
>   int imx_media_enum_format(u32 *fourcc, u32 index, enum codespace_sel cs_sel);
>   const struct imx_media_pixfmt *
>   imx_media_find_mbus_format(u32 code, enum codespace_sel cs_sel,

[toc] | [prev] | [next] | [standalone]

#1603940 — [PATCH 1/4] media: imx-media-csi: fix v4l2-compliance check

FromRussell King <rmk+kernel@armlinux.org.uk>
Date2017-03-19 11:50 +0100
Subject[PATCH 1/4] media: imx-media-csi: fix v4l2-compliance check
Message-ID<tmGzo-4lF-19@gated-at.bofh.it>
In reply to#1603936
v4l2-compliance was failing with:

                fail: v4l2-test-formats.cpp(1076): cap->timeperframe.numerator == 0 || cap->timeperframe.denominator == 0
                test VIDIOC_G/S_PARM: FAIL

Fix this.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/staging/media/imx/imx-media-csi.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
index 0336891069dc..65346e789dd6 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -680,8 +680,10 @@ static const struct csi_skip_desc *csi_find_best_skip(struct v4l2_fract *in,
 
 	/* Default to 1:1 ratio */
 	if (out->numerator == 0 || out->denominator == 0 ||
-	    in->numerator == 0 || in->denominator == 0)
+	    in->numerator == 0 || in->denominator == 0) {
+		*out = *in;
 		return best_skip;
+	}
 
 	want_us = div_u64((u64)USEC_PER_SEC * out->numerator, out->denominator);
 
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1604099 — Re: [PATCH 1/4] media: imx-media-csi: fix v4l2-compliance check

FromSteve Longerbeam <slongerbeam@gmail.com>
Date2017-03-19 23:20 +0100
SubjectRe: [PATCH 1/4] media: imx-media-csi: fix v4l2-compliance check
Message-ID<tmRl8-3JU-13@gated-at.bofh.it>
In reply to#1603940
Looks good to me.

Steve


On 03/19/2017 03:48 AM, Russell King wrote:
> v4l2-compliance was failing with:
>
>                  fail: v4l2-test-formats.cpp(1076): cap->timeperframe.numerator == 0 || cap->timeperframe.denominator == 0
>                  test VIDIOC_G/S_PARM: FAIL
>
> Fix this.
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
>   drivers/staging/media/imx/imx-media-csi.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
> index 0336891069dc..65346e789dd6 100644
> --- a/drivers/staging/media/imx/imx-media-csi.c
> +++ b/drivers/staging/media/imx/imx-media-csi.c
> @@ -680,8 +680,10 @@ static const struct csi_skip_desc *csi_find_best_skip(struct v4l2_fract *in,
>   
>   	/* Default to 1:1 ratio */
>   	if (out->numerator == 0 || out->denominator == 0 ||
> -	    in->numerator == 0 || in->denominator == 0)
> +	    in->numerator == 0 || in->denominator == 0) {
> +		*out = *in;
>   		return best_skip;
> +	}
>   
>   	want_us = div_u64((u64)USEC_PER_SEC * out->numerator, out->denominator);
>

[toc] | [prev] | [next] | [standalone]

Page 1 of 3  [1] 2 3  Next page →

Back to top | Article view | linux.kernel

csiph-web