From: "Jernej Škrabec" <jernej.skrabec@siol.net> To: Maxime Ripard <maxime.ripard@bootlin.com> Cc: paul.kocialkowski@bootlin.com, wens@csie.org, mchehab@kernel.org, gregkh@linuxfoundation.org, linux-media@vger.kernel.org, devel@driverdev.osuosl.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, nicolas@ndufresne.ca, boris.brezillon@collabora.com, jonas@kwiboo.se Subject: Re: [PATCH 2/7] media: cedrus: Fix H264 default reference index count Date: Mon, 03 Jun 2019 17:34:03 +0200 [thread overview] Message-ID: <8861995.Bqp6Xp4pol@jernej-laptop> (raw) In-Reply-To: <20190603114620.vsvbxz6gyhg3tn7b@flea> Dne ponedeljek, 03. junij 2019 ob 13:46:20 CEST je Maxime Ripard napisal(a): > On Thu, May 30, 2019 at 11:15:11PM +0200, Jernej Skrabec wrote: > > Reference index count in VE_H264_PPS reg should come from PPS control. > > However, this is not really important because reference index count is > > in our case always overridden by that from slice header. > > > > Cc: nicolas@ndufresne.ca > > Cc: boris.brezillon@collabora.com > > Cc: jonas@kwiboo.se > > > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net> > > Acked-by: Maxime Ripard <maxime.ripard@bootlin.com> > > > --- > > We have to decide if we drop pps->num_ref_idx_l0_default_active_minus1 > > and pps->num_ref_idx_l1_default_active_minus1 fields or add > > num_ref_idx_l0_active_override_flag and > > num_ref_idx_l0_active_override_flag > > to slice control. Actually only one flag is in bitstream valid for both l0 and l1 ref list. > > > > Current control doesn't have those two flags, so in Cedrus override flag > > is > > always set and we rely on userspace to set > > slice->num_ref_idx_l0_active_minus1 and > > slice->num_ref_idx_l1_active_minus1 to correct values. This means that > > values stored in PPS are not needed and always ignored by VPU. > > > > If I understand correctly, algorithm is very simple: > > > > ref_count = PPS->ref_count > > if (override_flag) > > > > ref_count = slice->ref_count > > > > It seems that VAAPI provides only final value. In my opinion we should do > > the same - get rid of PPS default ref index count fields. > > The rationale was to be as conservative as possible and just expose > everything that is in the bitstream in those controls to accomodate > for as many weird hardware as possible. Ok, so then we should add that override flag, which would align with h264 specs and you can still do same trick in VAAPI library which it's currently used in Cedrus driver - always set override flag and fill out only slice reflist count. At the end it shouldn't matter for proper decoding in any driver. Best regards, Jernej
WARNING: multiple messages have this Message-ID (diff)
From: "Jernej Škrabec" <jernej.skrabec@siol.net> To: Maxime Ripard <maxime.ripard@bootlin.com> Cc: devel@driverdev.osuosl.org, jonas@kwiboo.se, gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org, nicolas@ndufresne.ca, paul.kocialkowski@bootlin.com, wens@csie.org, boris.brezillon@collabora.com, mchehab@kernel.org, linux-arm-kernel@lists.infradead.org, linux-media@vger.kernel.org Subject: Re: [PATCH 2/7] media: cedrus: Fix H264 default reference index count Date: Mon, 03 Jun 2019 17:34:03 +0200 [thread overview] Message-ID: <8861995.Bqp6Xp4pol@jernej-laptop> (raw) In-Reply-To: <20190603114620.vsvbxz6gyhg3tn7b@flea> Dne ponedeljek, 03. junij 2019 ob 13:46:20 CEST je Maxime Ripard napisal(a): > On Thu, May 30, 2019 at 11:15:11PM +0200, Jernej Skrabec wrote: > > Reference index count in VE_H264_PPS reg should come from PPS control. > > However, this is not really important because reference index count is > > in our case always overridden by that from slice header. > > > > Cc: nicolas@ndufresne.ca > > Cc: boris.brezillon@collabora.com > > Cc: jonas@kwiboo.se > > > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net> > > Acked-by: Maxime Ripard <maxime.ripard@bootlin.com> > > > --- > > We have to decide if we drop pps->num_ref_idx_l0_default_active_minus1 > > and pps->num_ref_idx_l1_default_active_minus1 fields or add > > num_ref_idx_l0_active_override_flag and > > num_ref_idx_l0_active_override_flag > > to slice control. Actually only one flag is in bitstream valid for both l0 and l1 ref list. > > > > Current control doesn't have those two flags, so in Cedrus override flag > > is > > always set and we rely on userspace to set > > slice->num_ref_idx_l0_active_minus1 and > > slice->num_ref_idx_l1_active_minus1 to correct values. This means that > > values stored in PPS are not needed and always ignored by VPU. > > > > If I understand correctly, algorithm is very simple: > > > > ref_count = PPS->ref_count > > if (override_flag) > > > > ref_count = slice->ref_count > > > > It seems that VAAPI provides only final value. In my opinion we should do > > the same - get rid of PPS default ref index count fields. > > The rationale was to be as conservative as possible and just expose > everything that is in the bitstream in those controls to accomodate > for as many weird hardware as possible. Ok, so then we should add that override flag, which would align with h264 specs and you can still do same trick in VAAPI library which it's currently used in Cedrus driver - always set override flag and fill out only slice reflist count. At the end it shouldn't matter for proper decoding in any driver. Best regards, Jernej _______________________________________________ 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-06-03 15:34 UTC|newest] Thread overview: 65+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-05-30 21:15 [PATCH 0/7] media: cedrus: Improvements/cleanup Jernej Skrabec 2019-05-30 21:15 ` Jernej Skrabec 2019-05-30 21:15 ` [PATCH 1/7] media: cedrus: Disable engine after each slice decoding Jernej Skrabec 2019-05-30 21:15 ` Jernej Skrabec 2019-06-03 11:38 ` Maxime Ripard 2019-06-03 11:38 ` Maxime Ripard 2019-08-12 13:28 ` Ezequiel Garcia 2019-08-12 13:28 ` Ezequiel Garcia 2019-08-12 13:28 ` Ezequiel Garcia 2019-05-30 21:15 ` [PATCH 2/7] media: cedrus: Fix H264 default reference index count Jernej Skrabec 2019-05-30 21:15 ` Jernej Skrabec 2019-06-03 11:46 ` Maxime Ripard 2019-06-03 11:46 ` Maxime Ripard 2019-06-03 15:34 ` Jernej Škrabec [this message] 2019-06-03 15:34 ` Jernej Škrabec 2019-05-30 21:15 ` [PATCH 3/7] media: cedrus: Fix decoding for some H264 videos Jernej Skrabec 2019-05-30 21:15 ` Jernej Skrabec 2019-06-03 11:55 ` Maxime Ripard 2019-06-03 11:55 ` Maxime Ripard 2019-06-03 15:37 ` Jernej Škrabec 2019-06-03 15:37 ` Jernej Škrabec 2019-06-06 12:45 ` Dan Carpenter 2019-06-06 12:45 ` Dan Carpenter 2019-05-30 21:15 ` [PATCH 4/7] media: cedrus: Remove dst_bufs from context Jernej Skrabec 2019-05-30 21:15 ` Jernej Skrabec 2019-06-03 12:13 ` Maxime Ripard 2019-06-03 12:13 ` Maxime Ripard 2019-06-05 21:07 ` Paul Kocialkowski 2019-06-05 21:07 ` Paul Kocialkowski 2019-08-12 13:42 ` Ezequiel Garcia 2019-08-12 13:42 ` Ezequiel Garcia 2019-08-12 13:42 ` Ezequiel Garcia 2019-05-30 21:15 ` [PATCH 5/7] media: cedrus: Don't set chroma size for scale & rotation Jernej Skrabec 2019-05-30 21:15 ` Jernej Skrabec 2019-06-05 21:08 ` Paul Kocialkowski 2019-06-05 21:08 ` Paul Kocialkowski 2019-05-30 21:15 ` [PATCH 6/7] media: cedrus: Add infra for extra buffers connected to capture buffers Jernej Skrabec 2019-05-30 21:15 ` Jernej Skrabec 2019-06-03 12:18 ` Maxime Ripard 2019-06-03 12:18 ` Maxime Ripard 2019-06-03 15:48 ` Jernej Škrabec 2019-06-03 15:48 ` Jernej Škrabec 2019-06-05 21:10 ` Paul Kocialkowski 2019-06-05 21:10 ` Paul Kocialkowski 2019-06-05 21:52 ` Jernej Škrabec 2019-06-05 21:52 ` Jernej Škrabec 2019-06-06 8:33 ` Maxime Ripard 2019-06-06 8:33 ` Maxime Ripard 2019-05-30 21:15 ` [PATCH 7/7] media: cedrus: Improve H264 memory efficiency Jernej Skrabec 2019-05-30 21:15 ` Jernej Skrabec 2019-06-03 12:23 ` Maxime Ripard 2019-06-03 12:23 ` Maxime Ripard 2019-06-03 16:37 ` Jernej Škrabec 2019-06-03 16:37 ` Jernej Škrabec 2019-06-05 21:12 ` Paul Kocialkowski 2019-06-05 21:12 ` Paul Kocialkowski 2019-08-12 12:12 ` [PATCH 0/7] media: cedrus: Improvements/cleanup Hans Verkuil 2019-08-12 12:12 ` Hans Verkuil 2019-08-12 12:12 ` Hans Verkuil 2019-08-12 12:21 ` Jernej Škrabec 2019-08-12 12:21 ` Jernej Škrabec 2019-08-12 12:21 ` Jernej Škrabec 2019-08-12 13:55 ` Maxime Ripard 2019-08-12 13:55 ` Maxime Ripard 2019-08-12 13:55 ` 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=8861995.Bqp6Xp4pol@jernej-laptop \ --to=jernej.skrabec@siol.net \ --cc=boris.brezillon@collabora.com \ --cc=devel@driverdev.osuosl.org \ --cc=gregkh@linuxfoundation.org \ --cc=jonas@kwiboo.se \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-media@vger.kernel.org \ --cc=maxime.ripard@bootlin.com \ --cc=mchehab@kernel.org \ --cc=nicolas@ndufresne.ca \ --cc=paul.kocialkowski@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.