From patchwork Fri Mar 10 20:42:28 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mauro Carvalho Chehab X-Patchwork-Id: 9617953 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 7FF7F604D9 for ; Fri, 10 Mar 2017 20:43:11 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 7DB032879C for ; Fri, 10 Mar 2017 20:43:11 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 723832879E; Fri, 10 Mar 2017 20:43:11 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.9 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID autolearn=unavailable version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [65.50.211.133]) (using TLSv1.2 with cipher AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id EA3872879C for ; Fri, 10 Mar 2017 20:43:10 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Message-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=oXm5llHUP6cp8za+FVnoRoD86xjji8sVBiMT/fnAs6U=; b=CZZ9BEo1xoeEyR aiK+0T0ofU4Vk+KOcpjuSYrFv4//KoDPaJCpwoPg9i4qagtgnv8xMdUY7sCOrFvmOY11hw3e0B5Bp 3/qcSb4ggUagjxJu504ncBhdsChb6lS06VCMw1n8R4leqYI4s2BhOPQNTjspXG7M6Jga+0VKwt6OF HsIVLkOQzbyTnqf2e8X7U5JOVfCQkl58k8ROxy0i1Tle8WUv9uzGqyMur5VXNPWYDd4S2Oo1ehpie G7X0ggR0wiFIYx6Ld4a7paAk80ORUJwZxbti6fihpJJ6sO4Fy7Tsc34RTwX7YABI3FvOCWWxEk1pT y93h882l3NGWL8Cj6quA==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux)) id 1cmRNW-0000cM-Ke; Fri, 10 Mar 2017 20:43:10 +0000 Received: from ec2-52-27-115-49.us-west-2.compute.amazonaws.com ([52.27.115.49] helo=osg.samsung.com) by bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux)) id 1cmRNP-0000Zb-Sd for linux-arm-kernel@lists.infradead.org; Fri, 10 Mar 2017 20:43:08 +0000 Received: from localhost (localhost [127.0.0.1]) by osg.samsung.com (Postfix) with ESMTP id 1D48EA0DE2; Fri, 10 Mar 2017 20:43:04 +0000 (UTC) X-Virus-Scanned: amavisd-new at osg.samsung.com Received: from osg.samsung.com ([127.0.0.1]) by localhost (s-opensource.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id GaOgPCf8O0uz; Fri, 10 Mar 2017 20:43:03 +0000 (UTC) Received: from vento.lan (unknown [191.33.137.53]) by osg.samsung.com (Postfix) with ESMTPSA id BA0DEA0C73; Fri, 10 Mar 2017 20:42:54 +0000 (UTC) Date: Fri, 10 Mar 2017 17:42:28 -0300 From: Mauro Carvalho Chehab To: Russell King - ARM Linux Subject: Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit controls from a pipeline Message-ID: <20170310174228.3fe39100@vento.lan> In-Reply-To: <20170310155708.GX21222@n2100.armlinux.org.uk> References: <1487211578-11360-1-git-send-email-steve_longerbeam@mentor.com> <1487211578-11360-15-git-send-email-steve_longerbeam@mentor.com> <20170302160257.GK3220@valkosipuli.retiisi.org.uk> <20170303230645.GR21222@n2100.armlinux.org.uk> <20170304131329.GV3220@valkosipuli.retiisi.org.uk> <20170310130733.GU21222@n2100.armlinux.org.uk> <20170310122634.0ffda7c6@vento.lan> <20170310155708.GX21222@n2100.armlinux.org.uk> Organization: Samsung X-Mailer: Claws Mail 3.14.1 (GTK+ 2.24.31; x86_64-redhat-linux-gnu) MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20170310_124303_979958_B047E86D X-CRM114-Status: GOOD ( 30.19 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: mark.rutland@arm.com, andrew-ct.chen@mediatek.com, minghsiu.tsai@mediatek.com, sakari.ailus@linux.intel.com, nick@shmanahar.org, songjun.wu@microchip.com, Hans Verkuil , Steve Longerbeam , pavel@ucw.cz, robert.jarzmik@free.fr, devel@driverdev.osuosl.org, markus.heiser@darmarIT.de, laurent.pinchart+renesas@ideasonboard.com, shuah@kernel.org, geert@linux-m68k.org, Steve Longerbeam , linux-media@vger.kernel.org, devicetree@vger.kernel.org, kernel@pengutronix.de, arnd@arndb.de, mchehab@kernel.org, bparrot@ti.com, robh+dt@kernel.org, horms+renesas@verge.net.au, tiffany.lin@mediatek.com, linux-arm-kernel@lists.infradead.org, niklas.soderlund+renesas@ragnatech.se, gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org, Sakari Ailus , jean-christophe.trotin@st.com, p.zabel@pengutronix.de, fabio.estevam@nxp.com, shawnguo@kernel.org, sudipm.mukherjee@gmail.com Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP Em Fri, 10 Mar 2017 15:57:09 +0000 Russell King - ARM Linux escreveu: > On Fri, Mar 10, 2017 at 12:26:34PM -0300, Mauro Carvalho Chehab wrote: > > Hi Russell, > > > > Em Fri, 10 Mar 2017 13:07:33 +0000 > > Russell King - ARM Linux escreveu: > > > > > The idea that the v4l libraries should intercept the format negotiation > > > between the application and kernel is a particularly painful one - the > > > default gstreamer build detects the v4l libraries, and links against it. > > > That much is fine. > > > > > > However, the problem comes when you're trying to use bayer formats. The > > > v4l libraries "helpfully" (or rather unhelpfully) intercept the format > > > negotiation, and decide that they'll invoke v4lconvert to convert the > > > bayer to RGB for you, whether you want them to do that or not. > > > > > > v4lconvert may not be the most efficient way to convert, or even what > > > is desired (eg, you may want to receive the raw bayer image.) However, > > > since the v4l libraries/v4lconvert gives you no option but to have its > > > conversion forced into the pipeline, other options (such as using the > > > gstreamer neon accelerated de-bayer plugin) isn't an option > > > > That's not true. There is an special flag, used only by libv4l2 > > emulated formats, that indicates when a video format is handled > > via v4lconvert: > > I'm afraid that my statement comes from trying to use gstreamer with > libv4l2 and _not_ being able to use the 8-bit bayer formats there at > all - they are simply not passed across to the application through > libv4l2/v4lconvert. > > Instead, the formats that are passed across are the emulated formats. > As I said above, that forces applications to use only the v4lconvert > formats, the raw formats are not available. > > So, the presence or absence of the V4L2_FMT_FLAG_EMULATED is quite > meaningless if you can't even enumerate the non-converted formats. > > The problem comes from the "always needs conversion" stuff in > v4lconvert coupled with the way this subdev stuff works - since it > requires manual configuration of all the pads within the kernel > media pipeline, the kernel ends up only advertising _one_ format > to userspace - in my case, that's RGGB8. > > When v4lconvert_create_with_dev_ops() enumerates the formats from > the kernel, it gets only RGGB8. That causes always_needs_conversion > in there to remain true, so the special v4l control which enables/ > disables conversion gets created with a default value of "true". > The RGGB8 bit is also set in data->supported_src_formats. > > This causes v4lconvert_supported_dst_fmt_only() to return true. > > What this all means is that v4lconvert_enum_fmt() will _not_ return > any of the kernel formats, only the faked formats. > > Ergo, the RGGB8 format from the kernel is completely hidden from the > application, and only the emulated format is made available. As I > said above, this forces v4lconvert's debayering on the application, > whether you want it or not. > > In the gstreamer case, it knows nothing about this special control, > which means that trying to use this gstreamer pipeline: > > $ gst-launch-1.0 v4l2src device=/dev/video6 ! bayer2rgbneon ! xvimagesink > > is completely impossible without first rebuilding gstreamer _without_ > libv4l support. Build gstreamer without libv4l support, and the above > works. > > Enabling debug output in gstreamer's v4l2src plugin confirms that > the kernel's bayer format are totally hidden from gstreamer when > linked with libv4l2, but are present when it isn't linked with > libv4l2. Argh! that is indeed a bug at libv4l (and maybe at gstreamer). I guess that the always_needs_conversion logic was meant to be used to really odd proprietary formats, e. g: /* Vendor-specific formats */ #define V4L2_PIX_FMT_CPIA1 v4l2_fourcc('C', 'P', 'I', 'A') /* cpia1 YUV */ #define V4L2_PIX_FMT_WNVA v4l2_fourcc('W', 'N', 'V', 'A') /* Winnov hw compress */ #define V4L2_PIX_FMT_SN9C10X v4l2_fourcc('S', '9', '1', '0') /* SN9C10x compression */ ... I suspect that nobody uses libv4l2 with MC-based V4L2 devices. That's likely why nobody reported this bug before (that I know of). In any case, for non-proprietary formats, the default should be to always offer both the emulated format and the original one. I suspect that the enclosed patch should fix the issue with bayer formats. > Thanks, Mauro [PATCH RFC] libv4lconvert: by default, offer the original format to the client Applications should have the right to decide between using a libv4lconvert emulated format or to implement the decoding themselves, as this may have significative performance impact. So, change the default to always show both formats. Change also the default for Bayer encoded formats, as userspace likely will want to handle it directly. Signed-off-by: Mauro Carvalho Chehab diff --git a/lib/libv4lconvert/libv4lconvert.c b/lib/libv4lconvert/libv4lconvert.c index da718918b030..2e5458fa420d 100644 --- a/lib/libv4lconvert/libv4lconvert.c +++ b/lib/libv4lconvert/libv4lconvert.c @@ -118,10 +118,10 @@ static const struct v4lconvert_pixfmt supported_src_pixfmts[] = { { V4L2_PIX_FMT_OV511, 0, 7, 7, 1 }, { V4L2_PIX_FMT_OV518, 0, 7, 7, 1 }, /* uncompressed bayer */ - { V4L2_PIX_FMT_SBGGR8, 8, 8, 8, 1 }, - { V4L2_PIX_FMT_SGBRG8, 8, 8, 8, 1 }, - { V4L2_PIX_FMT_SGRBG8, 8, 8, 8, 1 }, - { V4L2_PIX_FMT_SRGGB8, 8, 8, 8, 1 }, + { V4L2_PIX_FMT_SBGGR8, 8, 8, 8, 0 }, + { V4L2_PIX_FMT_SGBRG8, 8, 8, 8, 0 }, + { V4L2_PIX_FMT_SGRBG8, 8, 8, 8, 0 }, + { V4L2_PIX_FMT_SRGGB8, 8, 8, 8, 0 }, { V4L2_PIX_FMT_STV0680, 8, 8, 8, 1 }, /* compressed bayer */ { V4L2_PIX_FMT_SPCA561, 0, 9, 9, 1 }, @@ -178,7 +178,7 @@ struct v4lconvert_data *v4lconvert_create_with_dev_ops(int fd, void *dev_ops_pri /* This keeps tracks of devices which have only formats for which apps most likely will need conversion and we can thus safely add software processing controls without a performance impact. */ - int always_needs_conversion = 1; + int always_needs_conversion = 0; if (!data) { fprintf(stderr, "libv4lconvert: error: out of memory!\n"); @@ -208,8 +208,8 @@ struct v4lconvert_data *v4lconvert_create_with_dev_ops(int fd, void *dev_ops_pri if (j < ARRAY_SIZE(supported_src_pixfmts)) { data->supported_src_formats |= 1ULL << j; v4lconvert_get_framesizes(data, fmt.pixelformat, j); - if (!supported_src_pixfmts[j].needs_conversion) - always_needs_conversion = 0; + if (supported_src_pixfmts[j].needs_conversion) + always_needs_conversion = 1; } else always_needs_conversion = 0; }