From: Philipp Zabel <p.zabel@pengutronix.de>
To: Jonas Karlman <jonas@kwiboo.se>,
Ezequiel Garcia <ezequiel@collabora.com>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>,
Hans Verkuil <hverkuil@xs4all.nl>,
Boris Brezillon <boris.brezillon@collabora.com>,
Paul Kocialkowski <paul.kocialkowski@bootlin.com>,
"linux-media@vger.kernel.org" <linux-media@vger.kernel.org>,
"linux-rockchip@lists.infradead.org"
<linux-rockchip@lists.infradead.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 02/12] media: hantro: Do not reorder H264 scaling list
Date: Tue, 03 Sep 2019 11:56:39 +0200 [thread overview]
Message-ID: <1567504599.5229.1.camel@pengutronix.de> (raw)
In-Reply-To: <HE1PR06MB4011A8F99D58E5ACFAE3CECAACBE0@HE1PR06MB4011.eurprd06.prod.outlook.com>
On Mon, 2019-09-02 at 16:18 +0000, Jonas Karlman wrote:
> On 2019-09-02 16:00, Philipp Zabel wrote:
> > Hi Jonas,
> >
> > On Sun, 2019-09-01 at 12:45 +0000, Jonas Karlman wrote:
> > > Scaling list supplied from userspace using ffmpeg and libva-v4l2-request
> > > is already in matrix order and can be used without applying the inverse
> > > scanning process.
> >
> > "in matrix order" is equivalent to "in raster scan order"?
>
> The values supplied by ffmpeg and libva-v4l2-request is in the order after the
> inverse scanning process has been applied (scaling list has been transformed
> into a scaling matrix). Not sure what this is called, "matrix order" seemed
> close enough.
Ok, after reading chapters
8.5.6 Inverse scanning process for 4x4 transform coefficients and scaling lists
8.5.7 Inverse scanning process for 8x8 transform coefficients and scaling lists
of ITU-T Rec. H.264, this seems clear enough. I just asked to make sure,
because libva documentation uses the term "raster scan" [1].
[1] http://intel.github.io/libva/structVAIQMatrixBufferH264.html
> Since there is two scan orders, zig-zag and field, and cedrus already expecting
> the values in "matrix" order, it seems more logical to let userspace handle the
> inverse scanning process.
I agree.
[...]
> > This changes the size of struct hantro_h264_dec_priv_tbl. Did this
> > describe the auxiliary buffer format incorrectly before?
>
> Based on RKMPP and Hantro SDK the HW expects the 8x8 inter/intra list for
> Y-component to be located at indices 0 and 1, lists for Cr/Cb is only used for
> 4:4:4 and HW only supports 4:0:0/4:2:0 if I am not mistaken. So the unused
> extra 4 lists at the end of the auxiliary buffer seemed like a waste,
> also RKMPP and Hantro SDK only seemed to allocate space for 2 lists.
Ok.
> > After this change, the second 8x8 scaling list has moved to a different
> > offset. Is this where the hardware has always been looking for it, or is
> > there a change missing in another place?
>
> As mentioned above HW only looks at indices 0 and 1, and ffmpeg will store the
> inter/intra Y list at indices 0 and 3 as seen at [1], in similar way cedrus only
> use indices 0 and 3 at [2].
> FFmpeg memcpy entire scaling_matrix8 to scaling_list_8x8 for v4l2-request-api
> and memcpy scaling_matrix8[0] and scaling_matrix8[3] for vaapi.
>
> You can see the effect of this patch using the h264_tivo_sample.ts sample from
> cover letter, patch 3-8 must be applied. With this patch applied the green
> football field will stay green, without the patch the field will shift in colors.
>
> [1] https://github.com/FFmpeg/FFmpeg/blob/master/libavcodec/h264_ps.c#L299-L308
> [2] https://git.linuxtv.org/media_tree.git/tree/drivers/staging/media/sunxi/cedrus/cedrus_h264.c#n231
Thank you, I'll try this.
regards
Philipp
next prev parent reply other threads:[~2019-09-03 9:56 UTC|newest]
Thread overview: 45+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-01 12:42 [PATCH RFC 00/12] media: hantro: H264 fixes and improvements Jonas Karlman
2019-09-01 12:45 ` [PATCH 01/12] media: hantro: Fix H264 max frmsize supported on RK3288 Jonas Karlman
2019-09-04 13:07 ` Ezequiel Garcia
2019-09-09 19:25 ` Jonas Karlman
[not found] ` <20190901124531.23645-1-jonas@kwiboo.se>
2019-09-01 12:45 ` [PATCH 02/12] media: hantro: Do not reorder H264 scaling list Jonas Karlman
2019-09-02 14:00 ` Philipp Zabel
2019-09-02 16:18 ` Jonas Karlman
2019-09-03 7:54 ` Jonas Karlman
2019-09-03 12:53 ` Philipp Zabel
2019-09-03 9:56 ` Philipp Zabel [this message]
2019-09-10 10:14 ` Ezequiel Garcia
2019-09-01 12:45 ` [PATCH 03/12] media: hantro: Fix H264 motion vector buffer offset Jonas Karlman
2019-09-03 10:58 ` Philipp Zabel
2019-09-03 20:13 ` Jonas Karlman
2019-09-10 10:18 ` Ezequiel Garcia
2019-09-10 11:34 ` Ezequiel Garcia
2019-09-01 12:45 ` [PATCH 05/12] media: hantro: Remove now unused H264 pic_size Jonas Karlman
2019-09-01 12:45 ` [PATCH 04/12] media: hantro: Reduce H264 extra space for motion vectors Jonas Karlman
2019-09-01 12:45 ` [PATCH 06/12] media: hantro: Set H264 FIELDPIC_FLAG_E flag correctly Jonas Karlman
2019-09-01 12:45 ` [RFC 08/12] media: hantro: Fix H264 decoding of field encoded content Jonas Karlman
2019-09-03 13:21 ` Philipp Zabel
2019-09-03 14:02 ` Jonas Karlman
2019-09-03 15:01 ` Philipp Zabel
2019-09-03 19:47 ` Jonas Karlman
2019-09-01 12:45 ` [RFC 07/12] media: uapi: h264: Add DPB entry field reference flags Jonas Karlman
2020-07-10 4:21 ` Ezequiel Garcia
2020-07-10 8:13 ` Boris Brezillon
2020-07-10 8:48 ` Jonas Karlman
2020-07-10 12:18 ` Ezequiel Garcia
2020-07-10 11:50 ` Ezequiel Garcia
2020-07-10 12:05 ` Boris Brezillon
2020-07-10 12:25 ` Ezequiel Garcia
2020-07-10 21:49 ` Nicolas Dufresne
2020-07-11 10:21 ` Jonas Karlman
2020-07-11 18:36 ` Nicolas Dufresne
2020-07-12 22:59 ` Ezequiel Garcia
2020-07-14 16:04 ` Nicolas Dufresne
2019-09-01 12:45 ` [RFC 09/12] media: hantro: Refactor G1 H264 code Jonas Karlman
2019-09-01 12:45 ` [RFC 10/12] media: hantro: Add support for H264 decoding on RK3399 Jonas Karlman
2019-09-02 11:46 ` Hans Verkuil
2019-09-02 15:25 ` Jonas Karlman
2019-09-01 12:45 ` [RFC 11/12] media: hantro: Enable " Jonas Karlman
2019-09-01 12:45 ` [RFC 12/12] media: hantro: Enable H264 decoding on RK3328 Jonas Karlman
2019-09-02 13:02 ` [PATCH RFC 00/12] media: hantro: H264 fixes and improvements Ezequiel Garcia
2019-09-02 16:28 ` Jonas Karlman
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=1567504599.5229.1.camel@pengutronix.de \
--to=p.zabel@pengutronix.de \
--cc=boris.brezillon@collabora.com \
--cc=ezequiel@collabora.com \
--cc=hverkuil@xs4all.nl \
--cc=jonas@kwiboo.se \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=mchehab@kernel.org \
--cc=paul.kocialkowski@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 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).