From: Nicolas Dufresne <nicolas.dufresne@collabora.com> To: Maxime Ripard <maxime.ripard@bootlin.com> Cc: Chen-Yu Tsai <wens@csie.org>, Tomasz Figa <tfiga@chromium.org>, Hans Verkuil <hans.verkuil@cisco.com>, Alexandre Courbot <acourbot@chromium.org>, Sakari Ailus <sakari.ailus@linux.intel.com>, Laurent Pinchart <laurent.pinchart@ideasonboard.com>, Pawel Osciak <posciak@chromium.org>, Paul Kocialkowski <paul.kocialkowski@bootlin.com>, Linux Kernel Mailing List <linux-kernel@vger.kernel.org>, "list@263.net:IOMMU DRIVERS <iommu@lists.linux-foundation.org>, Joerg " "Roedel <joro@8bytes.org>," <linux-arm-kernel@lists.infradead.org>, Linux Media Mailing List <linux-media@vger.kernel.org>, Jens Kuske <jenskuske@gmail.com>, Jernej Skrabec <jernej.skrabec@gmail.com>, Jonas Karlman <jonas@kwiboo.se>, Ezequiel Garcia <ezequiel@collabora.com>, linux-sunxi <linux-sunxi@googlegroups.com>, Thomas Petazzoni <thomas.petazzoni@bootlin.com>, Guenter Roeck <groeck@chromium.org> Subject: Re: [PATCH RESEND v7 1/2] media: uapi: Add H264 low-level decoder API compound controls. Date: Fri, 05 Apr 2019 12:27:48 -0400 [thread overview] Message-ID: <05078223a4098c96f6260f77e9ef2a01b966daa6.camel@collabora.com> (raw) In-Reply-To: <20190405151552.biesirbs35uivk7d@flea> [-- Attachment #1: Type: text/plain, Size: 2806 bytes --] Le vendredi 05 avril 2019 à 17:15 +0200, Maxime Ripard a écrit : > Hi Nicolas, > > On Thu, Apr 04, 2019 at 11:41:13AM -0400, Nicolas Dufresne wrote: > > > > > + * - __u16 > > > > > + - ``pic_width_in_mbs_minus1`` > > > > > + - > > > > > + * - __u16 > > > > > + - ``pic_height_in_map_units_minus1`` > > > > > + - > > > > > > > > We recently had some reflection with Alex that this is redundant with > > > > the width and height in the OUTPUT format. It may also apply to some > > > > other fields in these structs. I feel like they should be removed and > > > > passed via corresponding generic V4L2 properties - format, selection, > > > > etc. > > > > > > > > The same problem is also present in the MPEG2 controls. In fact, there > > > > was a patch already which used some fields from the controls to > > > > calculate the destination buffer strides, rather than bytesperline in > > > > the format. > > > > > > > > Since we're in staging, it could be done with a follow-up patch, though. > > > > > > Just my two cents. I played with some codecs a while back. IIRC some > > > specify a "codec" size in addition to the actual picture size, like > > > when the encoder does padding to fit the requirements of the codec > > > (spec). Is this needed anywhere? > > > > With state-less encoders, the headers, which contains the crop > > information is created by userspace and for state less decoder, the > > headers that contains this information is parsed by userspace. So I > > believe that in theory, the accelerator does not strictly need to be > > aware of the cropped dimensions. > > > > Another thing, is that there is not guarantied matches between e.g. > > depth of the chrome/luma and the final image buffers. Some hardware may > > have bandwidth limitation or internal converter and could possibly > > decode 10bit data into 8bit buffers. > > > > A third reason why I would not try and encode this header information > > is that there can be multiple PPS/SPS at the same time, and I think > > it's confusing if the relevant information to differentiate them is > > removed. > > Sorry if that sounds a bit dumb, but it's not really clear to me if > you're arguing for the removal of the data as Tomasz suggests, or if > you want to keep them. > > The first paragrah seems to advocate for the former, but the two > others for the latter. I think the data should stay. As I said, there can be multiple SPS/PPS, while there is only one format. That being said, how does SPS/PPS activation works ? How do you tell the driver about all the SPS/PPS and which one is being activated ? > > Thanks! > Maxime > > -- > Maxime Ripard, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 195 bytes --]
WARNING: multiple messages have this Message-ID (diff)
From: Nicolas Dufresne <nicolas.dufresne@collabora.com> To: Maxime Ripard <maxime.ripard@bootlin.com> Cc: "list@263.net:IOMMU DRIVERS <iommu@lists.linux-foundation.org>, Joerg Roedel <joro@8bytes.org>, " <linux-arm-kernel@lists.infradead.org>, Thomas Petazzoni <thomas.petazzoni@bootlin.com>, Alexandre Courbot <acourbot@chromium.org>, Jonas Karlman <jonas@kwiboo.se>, Jens Kuske <jenskuske@gmail.com>, linux-sunxi <linux-sunxi@googlegroups.com>, Linux Kernel Mailing List <linux-kernel@vger.kernel.org>, Jernej Skrabec <jernej.skrabec@gmail.com>, Tomasz Figa <tfiga@chromium.org>, Paul Kocialkowski <paul.kocialkowski@bootlin.com>, Chen-Yu Tsai <wens@csie.org>, Hans Verkuil <hans.verkuil@cisco.com>, Laurent Pinchart <laurent.pinchart@ideasonboard.com>, Sakari Ailus <sakari.ailus@linux.intel.com>, Guenter Roeck <groeck@chromium.org>, Ezequiel Garcia <ezequiel@collabora.com>, Pawel Osciak <posciak@chromium.org>, Linux Media Mailing List <linux-media@vger.kernel.org> Subject: Re: [PATCH RESEND v7 1/2] media: uapi: Add H264 low-level decoder API compound controls. Date: Fri, 05 Apr 2019 12:27:48 -0400 [thread overview] Message-ID: <05078223a4098c96f6260f77e9ef2a01b966daa6.camel@collabora.com> (raw) In-Reply-To: <20190405151552.biesirbs35uivk7d@flea> [-- Attachment #1.1: Type: text/plain, Size: 2806 bytes --] Le vendredi 05 avril 2019 à 17:15 +0200, Maxime Ripard a écrit : > Hi Nicolas, > > On Thu, Apr 04, 2019 at 11:41:13AM -0400, Nicolas Dufresne wrote: > > > > > + * - __u16 > > > > > + - ``pic_width_in_mbs_minus1`` > > > > > + - > > > > > + * - __u16 > > > > > + - ``pic_height_in_map_units_minus1`` > > > > > + - > > > > > > > > We recently had some reflection with Alex that this is redundant with > > > > the width and height in the OUTPUT format. It may also apply to some > > > > other fields in these structs. I feel like they should be removed and > > > > passed via corresponding generic V4L2 properties - format, selection, > > > > etc. > > > > > > > > The same problem is also present in the MPEG2 controls. In fact, there > > > > was a patch already which used some fields from the controls to > > > > calculate the destination buffer strides, rather than bytesperline in > > > > the format. > > > > > > > > Since we're in staging, it could be done with a follow-up patch, though. > > > > > > Just my two cents. I played with some codecs a while back. IIRC some > > > specify a "codec" size in addition to the actual picture size, like > > > when the encoder does padding to fit the requirements of the codec > > > (spec). Is this needed anywhere? > > > > With state-less encoders, the headers, which contains the crop > > information is created by userspace and for state less decoder, the > > headers that contains this information is parsed by userspace. So I > > believe that in theory, the accelerator does not strictly need to be > > aware of the cropped dimensions. > > > > Another thing, is that there is not guarantied matches between e.g. > > depth of the chrome/luma and the final image buffers. Some hardware may > > have bandwidth limitation or internal converter and could possibly > > decode 10bit data into 8bit buffers. > > > > A third reason why I would not try and encode this header information > > is that there can be multiple PPS/SPS at the same time, and I think > > it's confusing if the relevant information to differentiate them is > > removed. > > Sorry if that sounds a bit dumb, but it's not really clear to me if > you're arguing for the removal of the data as Tomasz suggests, or if > you want to keep them. > > The first paragrah seems to advocate for the former, but the two > others for the latter. I think the data should stay. As I said, there can be multiple SPS/PPS, while there is only one format. That being said, how does SPS/PPS activation works ? How do you tell the driver about all the SPS/PPS and which one is being activated ? > > Thanks! > Maxime > > -- > Maxime Ripard, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com [-- Attachment #1.2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 195 bytes --] [-- Attachment #2: Type: text/plain, Size: 176 bytes --] _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2019-04-05 16:27 UTC|newest] Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-04-04 12:26 [PATCH RESEND v7 0/2] media: cedrus: Add H264 decoding support Maxime Ripard 2019-04-04 12:26 ` Maxime Ripard 2019-04-04 12:26 ` [PATCH RESEND v7 1/2] media: uapi: Add H264 low-level decoder API compound controls Maxime Ripard 2019-04-04 12:26 ` Maxime Ripard 2019-04-04 12:31 ` Hans Verkuil (hansverk) 2019-04-04 12:31 ` Hans Verkuil (hansverk) 2019-04-04 12:36 ` Hans Verkuil 2019-04-04 12:36 ` Hans Verkuil 2019-04-04 12:54 ` Tomasz Figa 2019-04-04 12:54 ` Tomasz Figa 2019-04-04 14:42 ` Chen-Yu Tsai 2019-04-04 14:42 ` Chen-Yu Tsai 2019-04-04 15:41 ` Nicolas Dufresne 2019-04-04 15:41 ` Nicolas Dufresne 2019-04-05 15:15 ` Maxime Ripard 2019-04-05 15:15 ` Maxime Ripard 2019-04-05 16:27 ` Nicolas Dufresne [this message] 2019-04-05 16:27 ` Nicolas Dufresne 2019-04-11 15:57 ` Maxime Ripard 2019-04-11 15:57 ` Maxime Ripard 2019-04-16 7:16 ` Tomasz Figa 2019-04-16 7:16 ` Tomasz Figa 2019-04-16 11:54 ` Nicolas Dufresne 2019-04-16 11:54 ` Nicolas Dufresne 2019-04-04 12:26 ` [PATCH RESEND v7 2/2] media: cedrus: Add H264 decoding support Maxime Ripard 2019-04-04 12:26 ` Maxime Ripard
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=05078223a4098c96f6260f77e9ef2a01b966daa6.camel@collabora.com \ --to=nicolas.dufresne@collabora.com \ --cc=acourbot@chromium.org \ --cc=ezequiel@collabora.com \ --cc=groeck@chromium.org \ --cc=hans.verkuil@cisco.com \ --cc=jenskuske@gmail.com \ --cc=jernej.skrabec@gmail.com \ --cc=jonas@kwiboo.se \ --cc=laurent.pinchart@ideasonboard.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-media@vger.kernel.org \ --cc=linux-sunxi@googlegroups.com \ --cc=maxime.ripard@bootlin.com \ --cc=paul.kocialkowski@bootlin.com \ --cc=posciak@chromium.org \ --cc=sakari.ailus@linux.intel.com \ --cc=tfiga@chromium.org \ --cc=thomas.petazzoni@bootlin.com \ --cc=wens@csie.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.