From: Ezequiel Garcia <ezequiel@collabora.com>
To: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
Cc: linux-media@vger.kernel.org, kernel@collabora.com,
Nicolas Dufresne <nicolas.dufresne@collabora.com>,
Tomasz Figa <tfiga@chromium.org>,
linux-rockchip@lists.infradead.org,
Heiko Stuebner <heiko@sntech.de>, Jonas Karlman <jonas@kwiboo.se>,
Philipp Zabel <p.zabel@pengutronix.de>,
Boris Brezillon <boris.brezillon@collabora.com>,
Alexandre Courbot <acourbot@chromium.org>,
fbuergisser@chromium.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v7 02/11] media: uapi: h264: Rename pixel format
Date: Mon, 19 Aug 2019 11:38:51 -0300 [thread overview]
Message-ID: <a0b1f5806a0a5e5a58f1702485e36ec669c32f81.camel@collabora.com> (raw)
In-Reply-To: <20190819124110.GB32182@aptenodytes>
On Mon, 2019-08-19 at 14:41 +0200, Paul Kocialkowski wrote:
> Hi,
>
> On Fri 16 Aug 19, 13:01, Ezequiel Garcia wrote:
> > The V4L2_PIX_FMT_H264_SLICE_RAW name was originally suggested
> > because the pixel format would represent H264 slices without any
> > start code.
> >
> > However, as we will now introduce a start code menu control,
> > give the pixel format a more meaningful name, while it's
> > still early enough to do so.
>
> I definitely agree that SLICE_RAW is not the suffix we are looking for, but I'm
> not sure that _SLICE is self-describing given that we can operate either
> per-frame or per-slice, and _SLICE sort of implies the latter.
This is not entirely so, per-frame or per-slice mode refer to the granularity
of the stateless API, the pixel format is still composed of H264 NALU slices.
As per the documentation, the decode_mode and the start_code are modifiers
of the pixel format. As long as this is properly speced, the
V4L2_PIX_FMT_H264_SLICE looks pretty OK to me.
> Also, VP8 uses
> _FRAME to clearly indicate that it operates per-frame.
>
This is because VP8 doesn't have any concept of slices, the encoded unit
is a video frame.
> In addition, the _SLICE suffix is used by MPEG-2 in the stable API. Since we
> certainly want MPEG-2 to allow per-slice and per-frame decoding as well as
> H.264 and that the _SLICE format is specified to be the broken "concatenated
> slices" that cedrus expects, we probably want to use another suffix. This way,
> we could deprecated MPEG2_SLICE and introduce a new format for MPEG-2 that would
> have consistent naming with the other mpeg formats.
>
> One suggestion I had was to call it H264_PARSED (and apply this to MPEG-2 and
> HEVC when similar controls to H.264 are set in place for them). I think Hans had
> another suggestion for the name but I don't recall what it was at this point.
>
> Either way, if this has to be some debate, we could perhaps take it off your
> series and stay with SLICE_RAW for now, as long as we do rename it before making
> the API stable.
>
> What do you think?
>
With the new pixel format modifiers (decode_mode and start_code), the _RAW suffix
now has no meaning. Now, we could name it _PARSED or _SLICE. As long as this is
properly documented (as it is now), that'd be fine.
Now, to be completely honest, this discussion sounds like
bikeshedding to me.
Thanks,
Eze
next prev parent reply other threads:[~2019-08-19 14:39 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-08-16 16:01 [PATCH v7 00/11] media: hantro: Add support for H264 decoding Ezequiel Garcia
2019-08-16 16:01 ` [PATCH v7 01/11] lib/sort.c: implement sort() variant taking context argument Ezequiel Garcia
2019-08-16 16:01 ` [PATCH v7 02/11] media: uapi: h264: Rename pixel format Ezequiel Garcia
2019-08-19 12:41 ` Paul Kocialkowski
2019-08-19 14:38 ` Ezequiel Garcia [this message]
2019-08-19 15:53 ` Hans Verkuil
2019-08-22 11:54 ` Paul Kocialkowski
2019-08-22 13:47 ` Hans Verkuil
2019-08-22 14:38 ` Ezequiel Garcia
2019-08-22 14:42 ` Paul Kocialkowski
2019-08-16 16:01 ` [PATCH v7 03/11] media: uapi: h264: Add the concept of decoding mode Ezequiel Garcia
2019-08-16 16:01 ` [PATCH v7 04/11] media: uapi: h264: Add the concept of start code Ezequiel Garcia
2019-08-19 12:14 ` Paul Kocialkowski
2020-02-09 20:08 ` Nicolas Dufresne
2019-08-16 16:01 ` [PATCH v7 05/11] media: uapi: h264: Get rid of the p0/b0/b1 ref-lists Ezequiel Garcia
2019-08-16 16:01 ` [PATCH v7 06/11] media: cedrus: Cleanup control initialization Ezequiel Garcia
2019-08-19 12:31 ` Paul Kocialkowski
2019-08-16 16:01 ` [PATCH v7 07/11] media: cedrus: Specify H264 startcode and decoding mode Ezequiel Garcia
2019-08-22 12:31 ` Paul Kocialkowski
2019-08-16 16:01 ` [PATCH v7 08/11] media: hantro: Move copy_metadata() before doing a decode operation Ezequiel Garcia
2019-08-16 16:01 ` [PATCH v7 09/11] media: hantro: Add core bits to support H264 decoding Ezequiel Garcia
2019-08-16 16:01 ` [PATCH v7 10/11] media: hantro: Add support for H264 decoding on G1 Ezequiel Garcia
2019-08-16 16:01 ` [PATCH v7 11/11] media: hantro: Enable H264 decoding on rk3288 Ezequiel Garcia
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=a0b1f5806a0a5e5a58f1702485e36ec669c32f81.camel@collabora.com \
--to=ezequiel@collabora.com \
--cc=acourbot@chromium.org \
--cc=boris.brezillon@collabora.com \
--cc=fbuergisser@chromium.org \
--cc=heiko@sntech.de \
--cc=jonas@kwiboo.se \
--cc=kernel@collabora.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=nicolas.dufresne@collabora.com \
--cc=p.zabel@pengutronix.de \
--cc=paul.kocialkowski@bootlin.com \
--cc=tfiga@chromium.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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).