From: Chen-Yu Tsai <wens@csie.org>
To: Tomasz Figa <tfiga@chromium.org>
Cc: Maxime Ripard <maxime.ripard@bootlin.com>,
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>,
Nicolas Dufresne <nicolas.dufresne@collabora.com>,
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: Thu, 4 Apr 2019 22:42:50 +0800 [thread overview]
Message-ID: <CAGb2v642__AF3iHO+d9FKazJ4=uRAQSfxAN_uCUWagH6Jz2twQ@mail.gmail.com> (raw)
In-Reply-To: <CAAFQd5ASMJL2_ovNeFOhCh2JoD86KMtV5XO+7OgM37ckxrxmfA@mail.gmail.com>
On Thu, Apr 4, 2019 at 8:54 PM Tomasz Figa <tfiga@chromium.org> wrote:
>
> Hi,
>
> On Thu, Apr 4, 2019 at 9:26 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> >
> > From: Pawel Osciak <posciak@chromium.org>
> >
> > Stateless video codecs will require both the H264 metadata and slices in
> > order to be able to decode frames.
> >
> > This introduces the definitions for a new pixel format for H264 slices that
> > have been parsed, as well as the structures used to pass the metadata from
> > the userspace to the kernel.
> >
> > Reviewed-by: Tomasz Figa <tfiga@chromium.org>
> > Signed-off-by: Pawel Osciak <posciak@chromium.org>
> > Signed-off-by: Guenter Roeck <groeck@chromium.org>
> > Co-developed-by: Maxime Ripard <maxime.ripard@bootlin.com>
> > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> > ---
> > Documentation/media/uapi/v4l/biblio.rst | 9 +-
> > Documentation/media/uapi/v4l/ext-ctrls-codec.rst | 569 ++++++++++++++-
> > Documentation/media/uapi/v4l/pixfmt-compressed.rst | 19 +-
> > Documentation/media/uapi/v4l/vidioc-queryctrl.rst | 30 +-
> > Documentation/media/videodev2.h.rst.exceptions | 5 +-
> > drivers/media/v4l2-core/v4l2-ctrls.c | 42 +-
> > drivers/media/v4l2-core/v4l2-ioctl.c | 1 +-
> > include/media/h264-ctrls.h | 192 +++++-
> > include/media/v4l2-ctrls.h | 13 +-
> > include/uapi/linux/videodev2.h | 1 +-
> > 10 files changed, 880 insertions(+), 1 deletion(-)
> > create mode 100644 include/media/h264-ctrls.h
> >
> > diff --git a/Documentation/media/uapi/v4l/biblio.rst b/Documentation/media/uapi/v4l/biblio.rst
> > index ec33768c055e..8f4eb8823d82 100644
> > --- a/Documentation/media/uapi/v4l/biblio.rst
> > +++ b/Documentation/media/uapi/v4l/biblio.rst
> > @@ -122,6 +122,15 @@ ITU BT.1119
> >
> > :author: International Telecommunication Union (http://www.itu.ch)
> >
> > +.. _h264:
> > +
> > +ITU-T Rec. H.264 Specification (04/2017 Edition)
> > +================================================
> > +
> > +:title: ITU-T Recommendation H.264 "Advanced Video Coding for Generic Audiovisual Services"
> > +
> > +:author: International Telecommunication Union (http://www.itu.ch)
> > +
> > .. _jfif:
> >
> > JFIF
> > diff --git a/Documentation/media/uapi/v4l/ext-ctrls-codec.rst b/Documentation/media/uapi/v4l/ext-ctrls-codec.rst
> > index 67a122339c0e..1285bfec7d3d 100644
> > --- a/Documentation/media/uapi/v4l/ext-ctrls-codec.rst
> > +++ b/Documentation/media/uapi/v4l/ext-ctrls-codec.rst
> > @@ -1371,6 +1371,575 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type -
> > - Layer number
> >
> >
> > +.. _v4l2-mpeg-h264:
> > +
> > +``V4L2_CID_MPEG_VIDEO_H264_SPS (struct)``
> > + Specifies the sequence parameter set (as extracted from the
> > + bitstream) for the associated H264 slice data. This includes the
> > + necessary parameters for configuring a stateless hardware decoding
> > + pipeline for H264. The bitstream parameters are defined according
> > + to :ref:`h264`, section 7.4.2.1.1 "Sequence Parameter Set Data
> > + Semantics". For further documentation, refer to the above
> > + specification, unless there is an explicit comment stating
> > + otherwise.
> > +
> > + .. note::
> > +
> > + This compound control is not yet part of the public kernel API and
> > + it is expected to change.
> > +
> > +.. c:type:: v4l2_ctrl_h264_sps
> > +
> > +.. cssclass:: longtable
> > +
> > +.. flat-table:: struct v4l2_ctrl_h264_sps
> > + :header-rows: 0
> > + :stub-columns: 0
> > + :widths: 1 1 2
> > +
> > + * - __u8
> > + - ``profile_idc``
> > + -
> > + * - __u8
> > + - ``constraint_set_flags``
> > + - See :ref:`Sequence Parameter Set Constraints Set Flags <h264_sps_constraints_set_flags>`
> > + * - __u8
> > + - ``level_idc``
> > + -
> > + * - __u8
> > + - ``seq_parameter_set_id``
> > + -
> > + * - __u8
> > + - ``chroma_format_idc``
> > + -
> > + * - __u8
> > + - ``bit_depth_luma_minus8``
> > + -
> > + * - __u8
> > + - ``bit_depth_chroma_minus8``
> > + -
> > + * - __u8
> > + - ``log2_max_frame_num_minus4``
> > + -
> > + * - __u8
> > + - ``pic_order_cnt_type``
> > + -
> > + * - __u8
> > + - ``log2_max_pic_order_cnt_lsb_minus4``
> > + -
> > + * - __u8
> > + - ``max_num_ref_frames``
> > + -
> > + * - __u8
> > + - ``num_ref_frames_in_pic_order_cnt_cycle``
> > + -
> > + * - __s32
> > + - ``offset_for_ref_frame[255]``
> > + -
> > + * - __s32
> > + - ``offset_for_non_ref_pic``
> > + -
> > + * - __s32
> > + - ``offset_for_top_to_bottom_field``
> > + -
> > + * - __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?
ChenYu
WARNING: multiple messages have this Message-ID (diff)
From: Chen-Yu Tsai <wens@csie.org>
To: Tomasz Figa <tfiga@chromium.org>
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>,
Maxime Ripard <maxime.ripard@bootlin.com>,
linux-sunxi <linux-sunxi@googlegroups.com>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Jernej Skrabec <jernej.skrabec@gmail.com>,
Paul Kocialkowski <paul.kocialkowski@bootlin.com>,
Jens Kuske <jenskuske@gmail.com>,
Hans Verkuil <hans.verkuil@cisco.com>,
Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
Sakari Ailus <sakari.ailus@linux.intel.com>,
Guenter Roeck <groeck@chromium.org>,
Nicolas Dufresne <nicolas.dufresne@collabora.com>,
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: Thu, 4 Apr 2019 22:42:50 +0800 [thread overview]
Message-ID: <CAGb2v642__AF3iHO+d9FKazJ4=uRAQSfxAN_uCUWagH6Jz2twQ@mail.gmail.com> (raw)
In-Reply-To: <CAAFQd5ASMJL2_ovNeFOhCh2JoD86KMtV5XO+7OgM37ckxrxmfA@mail.gmail.com>
On Thu, Apr 4, 2019 at 8:54 PM Tomasz Figa <tfiga@chromium.org> wrote:
>
> Hi,
>
> On Thu, Apr 4, 2019 at 9:26 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> >
> > From: Pawel Osciak <posciak@chromium.org>
> >
> > Stateless video codecs will require both the H264 metadata and slices in
> > order to be able to decode frames.
> >
> > This introduces the definitions for a new pixel format for H264 slices that
> > have been parsed, as well as the structures used to pass the metadata from
> > the userspace to the kernel.
> >
> > Reviewed-by: Tomasz Figa <tfiga@chromium.org>
> > Signed-off-by: Pawel Osciak <posciak@chromium.org>
> > Signed-off-by: Guenter Roeck <groeck@chromium.org>
> > Co-developed-by: Maxime Ripard <maxime.ripard@bootlin.com>
> > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> > ---
> > Documentation/media/uapi/v4l/biblio.rst | 9 +-
> > Documentation/media/uapi/v4l/ext-ctrls-codec.rst | 569 ++++++++++++++-
> > Documentation/media/uapi/v4l/pixfmt-compressed.rst | 19 +-
> > Documentation/media/uapi/v4l/vidioc-queryctrl.rst | 30 +-
> > Documentation/media/videodev2.h.rst.exceptions | 5 +-
> > drivers/media/v4l2-core/v4l2-ctrls.c | 42 +-
> > drivers/media/v4l2-core/v4l2-ioctl.c | 1 +-
> > include/media/h264-ctrls.h | 192 +++++-
> > include/media/v4l2-ctrls.h | 13 +-
> > include/uapi/linux/videodev2.h | 1 +-
> > 10 files changed, 880 insertions(+), 1 deletion(-)
> > create mode 100644 include/media/h264-ctrls.h
> >
> > diff --git a/Documentation/media/uapi/v4l/biblio.rst b/Documentation/media/uapi/v4l/biblio.rst
> > index ec33768c055e..8f4eb8823d82 100644
> > --- a/Documentation/media/uapi/v4l/biblio.rst
> > +++ b/Documentation/media/uapi/v4l/biblio.rst
> > @@ -122,6 +122,15 @@ ITU BT.1119
> >
> > :author: International Telecommunication Union (http://www.itu.ch)
> >
> > +.. _h264:
> > +
> > +ITU-T Rec. H.264 Specification (04/2017 Edition)
> > +================================================
> > +
> > +:title: ITU-T Recommendation H.264 "Advanced Video Coding for Generic Audiovisual Services"
> > +
> > +:author: International Telecommunication Union (http://www.itu.ch)
> > +
> > .. _jfif:
> >
> > JFIF
> > diff --git a/Documentation/media/uapi/v4l/ext-ctrls-codec.rst b/Documentation/media/uapi/v4l/ext-ctrls-codec.rst
> > index 67a122339c0e..1285bfec7d3d 100644
> > --- a/Documentation/media/uapi/v4l/ext-ctrls-codec.rst
> > +++ b/Documentation/media/uapi/v4l/ext-ctrls-codec.rst
> > @@ -1371,6 +1371,575 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type -
> > - Layer number
> >
> >
> > +.. _v4l2-mpeg-h264:
> > +
> > +``V4L2_CID_MPEG_VIDEO_H264_SPS (struct)``
> > + Specifies the sequence parameter set (as extracted from the
> > + bitstream) for the associated H264 slice data. This includes the
> > + necessary parameters for configuring a stateless hardware decoding
> > + pipeline for H264. The bitstream parameters are defined according
> > + to :ref:`h264`, section 7.4.2.1.1 "Sequence Parameter Set Data
> > + Semantics". For further documentation, refer to the above
> > + specification, unless there is an explicit comment stating
> > + otherwise.
> > +
> > + .. note::
> > +
> > + This compound control is not yet part of the public kernel API and
> > + it is expected to change.
> > +
> > +.. c:type:: v4l2_ctrl_h264_sps
> > +
> > +.. cssclass:: longtable
> > +
> > +.. flat-table:: struct v4l2_ctrl_h264_sps
> > + :header-rows: 0
> > + :stub-columns: 0
> > + :widths: 1 1 2
> > +
> > + * - __u8
> > + - ``profile_idc``
> > + -
> > + * - __u8
> > + - ``constraint_set_flags``
> > + - See :ref:`Sequence Parameter Set Constraints Set Flags <h264_sps_constraints_set_flags>`
> > + * - __u8
> > + - ``level_idc``
> > + -
> > + * - __u8
> > + - ``seq_parameter_set_id``
> > + -
> > + * - __u8
> > + - ``chroma_format_idc``
> > + -
> > + * - __u8
> > + - ``bit_depth_luma_minus8``
> > + -
> > + * - __u8
> > + - ``bit_depth_chroma_minus8``
> > + -
> > + * - __u8
> > + - ``log2_max_frame_num_minus4``
> > + -
> > + * - __u8
> > + - ``pic_order_cnt_type``
> > + -
> > + * - __u8
> > + - ``log2_max_pic_order_cnt_lsb_minus4``
> > + -
> > + * - __u8
> > + - ``max_num_ref_frames``
> > + -
> > + * - __u8
> > + - ``num_ref_frames_in_pic_order_cnt_cycle``
> > + -
> > + * - __s32
> > + - ``offset_for_ref_frame[255]``
> > + -
> > + * - __s32
> > + - ``offset_for_non_ref_pic``
> > + -
> > + * - __s32
> > + - ``offset_for_top_to_bottom_field``
> > + -
> > + * - __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?
ChenYu
_______________________________________________
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-04 14:43 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 [this message]
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
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='CAGb2v642__AF3iHO+d9FKazJ4=uRAQSfxAN_uCUWagH6Jz2twQ@mail.gmail.com' \
--to=wens@csie.org \
--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=nicolas.dufresne@collabora.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 \
/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: link
Be 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.