Re: [PATCH v7] media: imx: add mem2mem device

[Tim Harvey <tharvey@gateworks.com>]

On Fri, Feb 15, 2019 at 3:10 AM Philipp Zabel <p.zabel@pengutronix.de> wrote:
>
> Hi Tim,
>
> On Tue, 2019-02-12 at 11:01 -0800, Tim Harvey wrote:
> > On Thu, Jan 17, 2019 at 7:50 AM Philipp Zabel <p.zabel@pengutronix.de> wrote:
> > >
> > > Add a single imx-media mem2mem video device that uses the IPU IC PP
> > > (image converter post processing) task for scaling and colorspace
> > > conversion.
> > > On i.MX6Q/DL SoCs with two IPUs currently only the first IPU is used.
> > >
> > > The hardware only supports writing to destination buffers up to
> > > 1024x1024 pixels in a single pass, arbitrary sizes can be achieved
> > > by rendering multiple tiles per frame.
> > >
> > > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> > > [slongerbeam@gmail.com: use ipu_image_convert_adjust(), fix
> > >  device_run() error handling, add missing media-device header,
> > >  unregister and remove the mem2mem device in error paths in
> > >  imx_media_probe_complete() and in imx_media_remove()]
> > > Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
> > > ---
> > > Changes since v6 [1]:
> > >  - Change driver name in querycap to imx-media-csc-scaler
> > >  - Drop V4L2_SEL_TGT_COMPOSE_PADDED from vidioc_g_selection
> > >  - Simplify error handling in ipu_csc_scaler_init_controls
> > >
> > > [1] https://patchwork.linuxtv.org/patch/53757/
> > > ---
> >
> > Hi Philipp,
> >
> > Thanks for this driver - this is providing support that I need to
> > overcome direct CSI->IC limitations.
> >
> > Can you give me some examples on how your using this? I'm testing this
> > on top of linux-media and trying the following gstreamer pipelines
> > (gstreamer recent master) and running into trouble but it could very
> > likely be me doing something wrong in my pipelines:
> >
> > # upscale
> > gst-launch-1.0 videotestsrc ! video/x-raw,width=320,height=240 !
> > v4l2convert output-io-mode=dmabuf-import !
> > video/x-raw,width=640,height=480 ! kmssink
>
> You can't have v4l2convert import dmabufs because videotestsrc doesn't
> produce any. I used:
>
> gst-launch-1.0 videotestsrc ! video/x-raw,width=320,height=240 ! v4l2video10convert ! video/x-raw,width=640,height=480 ! kmssink
>
> That should work, passing dmabufs between v4l2 and kms automatically.
>
> I usually use kmssink but waylandsink, glimagesink, or v4l2h264enc for
> testing though.
>

Philipp,

That makes sense and jives with what Nicolas was saying about alignment.

I'm currently testing linux-media/master with your v7 mem2mem patch
and 'gst-launch-1.0 videotestsrc ! video/x-raw,width=320,height=240 !
v4l2video10convert ! video/x-raw,width=640,height=480 ! kmssink' hangs
the system. Do you have some other patches queued up?

> > # downscale
> > gst-launch-1.0 videotestsrc ! video/x-raw,width=640,height=480 !
> > v4l2convert output-io-mode=dmabuf-import !
> > video/x-raw,width=320,height=280 ! kmssink
>
> Drop output-io-mode unless your source is capable of producing dmabufs.
> I think kmssink is trying to scale in this case, which imx-drm doesn't
> support. You may have to either keep aspect ratio, or give kmssink the
> can-scale=false parameter.
>
> > # downscale using videotstsrc defaults
> > gst-launch-1.0 videotestsrc ! v4l2convert output-io-mode=dmabuf-import
> > ! video/x-raw,width=100,height=200 ! kmssink
> > ^^^ works
>
> That will probably just negotiate 100x200 on the input side and bypass
> conversion altogether.
>
> > # rotation
> > gst-launch-1.0 videotestsrc ! v4l2convert output-io-mode=dmabuf-import
> > extra-controls=cid,rotate=90 ! kmssink
>
> This will likely negotiate the same format and dimensions on both sides
> and bypass conversion as well. GStreamer has no concept of the rotation
> as of yet. Try:
>
> gst-launch-1.0 videotestsrc ! video/x-raw,width=320,height=240 ! v4l2video10convert extra-controls=cid,rotate=90 ! video/x-raw,width=240,height=320 ! kmssink can-scale=false
>
> > I'm also not sure how to specify hflip/vflip... I don't think
> > extra-controls parses 'hflip', 'vflip' as ipu_csc_scaler_s_ctrl gets
> > called with V4L2_CID_HFLIP/V4L2_CID_VFLIP but ctrl->val is always 0.
>
> You can use v4l2-ctl -L to list the CID names, they are horizontal_flip
> and vertical_flip, respectively. Again, the input and output formats
> must be different because GStreamer doesn't know about the flipping yet:
>
> gst-launch-1.0 videotestsrc ! video/x-raw,width=320,height=240 ! v4l2video10convert extra-controls=cid,horizontal_flip=1 ! video/x-raw,width=640,height=480 ! kmssink can-scale=false
>
> We'd have to add actual properties for rotate/flip to v4l2convert,
> instead of using theextra-controls workaround, probable something
> similar to the video-direction property of the software videoflip
> element.
>

Philipp,

Removing the dmabuf options makes sense along with what Nicolas was
saying about the dma buffer alignment.

The fact that Gstreamer doesn't understand flip/rotate also makes
complete sense - it bypasses the conversion completely unless you
change the format.

All of these work perfectly:
# upscale
gst-launch-1.0 videotestsrc ! video/x-raw,width=320,height=240 !
v4l2convert ! video/x-raw,width=640,height=480 ! kmssink
can-scale=false
# downscale
gst-launch-1.0 videotestsrc ! video/x-raw,width=640,height=480 !
v4l2convert ! video/x-raw,width=320,height=240 ! kmssink
can-scale=false
# rotate
gst-launch-1.0 videotestsrc ! video/x-raw,width=320,height=240 !
v4l2convert extra-controls=cid,rotate=90 !
video/x-raw,width=240,height=320 ! kmssink can-scale=false
# flip
gst-launch-1.0 videotestsrc ! video/x-raw,width=320,height=240 !
v4l2convert extra-controls=cid,horizontal_flip=1 !
video/x-raw,width=640,height=480 ! kmssink can-scale=false
gst-launch-1.0 videotestsrc ! video/x-raw,width=320,height=240 !
v4l2convert extra-controls=cid,vertical_flip=1 !
video/x-raw,width=640,height=480 ! kmssink can-scale=false

As well as the following using an imx-media capture pipeline (480p
source on the sensor) where we can now use aligned dmabuf's:
# encode
gst-launch-1.0 v4l2src device=/dev/video4 ! v4l2convert
output-io-mode=dmabuf-import ! v4l2h264enc
output-io-mode=dmabuf-import ! rtph264pay ! udpsink host=172.24.20.19
port=5001
# scale/encode
gst-launch-1.0 v4l2src device=/dev/video4 ! v4l2convert
output-io-mode=dmabuf-import ! video/x-raw,width=1440,height=960 !
v4l2h264enc output-io-mode=dmabuf-import ! rtph264pay ! udpsink
host=172.24.20.19 port=5001
# scale/flip/encode
gst-launch-1.0 v4l2src device=/dev/video4 ! v4l2convert
output-io-mode=dmabuf-import extra-controls=cid,horizontal_flip=1 !
video/x-raw,width=1440,height=960 ! v4l2h264enc
output-io-mode=dmabuf-import ! rtph264pay ! udpsink host=172.24.20.19
port=5001
# scale/rotate/encode
gst-launch-1.0 v4l2src device=/dev/video4 ! v4l2convert
output-io-mode=dmabuf-import extra-controls=cid,rotate=90 !
video/x-raw,width=1440,height=960 ! v4l2h264enc
output-io-mode=dmabuf-import ! rtph264pay ! udpsink host=172.24.20.19
port=5001

Many thanks - hoping to see this merged soon!

Tim