This series contains fixes and improvements for the hantro H264 decoder.
Patch 1-3 is updated patches from the "media: hantro: Collected fixes for v5.4"
series.
Patch 4-8 fixes issues and limitations observed when preparing support
for field encoded content.
Patch 9 introduce new DPB entry flags that is used to signal how a reference
frame is referenced. This information is needed to correctly build a
reference list for field encoded content.
Patch 10 adds bits to handle field encoded content, this is a rough patch
and should be reworked with proper code style and formatting.
Please get back with feedback on how to improve this.
The following samples from [1] are now playable with this series
- H264_1080i-25-interlace_Kaesescheibchen.mkv
- H264_10_1080i_50_AC3-Astra19.2_ProSieben_HD.ts
- big_buck_bunny_1080p_H264_AAC_25fps_7200K.mp4
- h264_tivo_sample.ts
This series has been tested using ffmpeg v4l2 request hwaccel at [2]
[1] http://kwiboo.libreelec.tv/test/samples/
[2] https://github.com/Kwiboo/FFmpeg/compare/4.0.4-Leia-18.4...v4l2-request-hwaccel-4.0.4-hantro
Changes in v2:
- scaling list changes split to its own series
- address feedback from Philipp and Ezequiel
Regards,
Jonas
Francois Buergisser (2):
media: hantro: Fix motion vectors usage condition
media: hantro: Fix picture order count table enable
Jonas Karlman (8):
media: hantro: Fix H264 max frmsize supported on RK3288
media: hantro: Fix H264 motion vector buffer offset
media: hantro: Reduce H264 extra space for motion vectors
media: hantro: Use capture buffer width and height for H264 decoding
media: hantro: Remove now unused H264 pic_size
media: hantro: Set H264 FIELDPIC_FLAG_E flag correctly
media: uapi: h264: Add DPB entry field reference flags
media: hantro: Fix H264 decoding of field encoded content
.../media/uapi/v4l/ext-ctrls-codec.rst | 12 ++
.../staging/media/hantro/hantro_g1_h264_dec.c | 62 ++++-----
drivers/staging/media/hantro/hantro_h264.c | 126 +++++++++++-------
drivers/staging/media/hantro/hantro_hw.h | 5 +-
drivers/staging/media/hantro/hantro_v4l2.c | 6 +-
drivers/staging/media/hantro/rk3288_vpu_hw.c | 4 +-
include/media/h264-ctrls.h | 4 +
7 files changed, 135 insertions(+), 84 deletions(-)
--
2.17.1
The FIELDPIC_FLAG_E bit should be set when field_pic_flag exists in stream,
it is currently set based on field_pic_flag of current frame.
The PIC_FIELDMODE_E bit is correctly set based on the field_pic_flag.
Fix this by setting the FIELDPIC_FLAG_E bit when frame_mbs_only is not set.
Fixes: dea0a82f3d22 ("media: hantro: Add support for H264 decoding on G1")
Signed-off-by: Jonas Karlman <[email protected]>
---
drivers/staging/media/hantro/hantro_g1_h264_dec.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/media/hantro/hantro_g1_h264_dec.c b/drivers/staging/media/hantro/hantro_g1_h264_dec.c
index eeed11366135..c07da0ee4973 100644
--- a/drivers/staging/media/hantro/hantro_g1_h264_dec.c
+++ b/drivers/staging/media/hantro/hantro_g1_h264_dec.c
@@ -63,7 +63,7 @@ static void set_params(struct hantro_ctx *ctx)
/* always use the matrix sent from userspace */
reg |= G1_REG_DEC_CTRL2_TYPE1_QUANT_E;
- if (slices[0].flags & V4L2_H264_SLICE_FLAG_FIELD_PIC)
+ if (!(sps->flags & V4L2_H264_SPS_FLAG_FRAME_MBS_ONLY))
reg |= G1_REG_DEC_CTRL2_FIELDPIC_FLAG_E;
vdpu_write_relaxed(vpu, reg, G1_REG_DEC_CTRL2);
--
2.17.1
TRM specify supported image size 48x48 to 4096x2304 at step size 16 pixels,
change frmsize max_width/max_height to match TRM at [1].
This patch makes it possible to decode the 4096x2304 sample at [2].
[1] http://www.t-firefly.com/download/firefly-rk3288/docs/TRM/rk3288-chapter-25-video-encoder-decoder-unit-(vcodec).pdf
[2] https://4ksamples.com/puppies-bath-in-4k/
Fixes: 760327930e10 ("media: hantro: Enable H264 decoding on rk3288")
Signed-off-by: Jonas Karlman <[email protected]>
---
Changes in v2:
- updated commit message with reference to TRM and sample video
---
drivers/staging/media/hantro/rk3288_vpu_hw.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/media/hantro/rk3288_vpu_hw.c b/drivers/staging/media/hantro/rk3288_vpu_hw.c
index c0bdd6c02520..f8db6fcaad73 100644
--- a/drivers/staging/media/hantro/rk3288_vpu_hw.c
+++ b/drivers/staging/media/hantro/rk3288_vpu_hw.c
@@ -67,10 +67,10 @@ static const struct hantro_fmt rk3288_vpu_dec_fmts[] = {
.max_depth = 2,
.frmsize = {
.min_width = 48,
- .max_width = 3840,
+ .max_width = 4096,
.step_width = MB_DIM,
.min_height = 48,
- .max_height = 2160,
+ .max_height = 2304,
.step_height = MB_DIM,
},
},
--
2.17.1
On Tue, 29 Oct 2019 01:24:47 +0000
Jonas Karlman <[email protected]> wrote:
> TRM specify supported image size 48x48 to 4096x2304 at step size 16 pixels,
> change frmsize max_width/max_height to match TRM at [1].
>
> This patch makes it possible to decode the 4096x2304 sample at [2].
>
> [1] http://www.t-firefly.com/download/firefly-rk3288/docs/TRM/rk3288-chapter-25-video-encoder-decoder-unit-(vcodec).pdf
> [2] https://4ksamples.com/puppies-bath-in-4k/
>
> Fixes: 760327930e10 ("media: hantro: Enable H264 decoding on rk3288")
> Signed-off-by: Jonas Karlman <[email protected]>
Reviewed-by: Boris Brezillon <[email protected]>
Tested-by: Boris Brezillon <[email protected]>
Let's also add
Cc: <[email protected]>
just in case this patch doesn't make it to 5.4.
> ---
> Changes in v2:
> - updated commit message with reference to TRM and sample video
> ---
> drivers/staging/media/hantro/rk3288_vpu_hw.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/media/hantro/rk3288_vpu_hw.c b/drivers/staging/media/hantro/rk3288_vpu_hw.c
> index c0bdd6c02520..f8db6fcaad73 100644
> --- a/drivers/staging/media/hantro/rk3288_vpu_hw.c
> +++ b/drivers/staging/media/hantro/rk3288_vpu_hw.c
> @@ -67,10 +67,10 @@ static const struct hantro_fmt rk3288_vpu_dec_fmts[] = {
> .max_depth = 2,
> .frmsize = {
> .min_width = 48,
> - .max_width = 3840,
> + .max_width = 4096,
> .step_width = MB_DIM,
> .min_height = 48,
> - .max_height = 2160,
> + .max_height = 2304,
> .step_height = MB_DIM,
Hans, Mauro, we were intending to have this fix merged in 5.4 or at
the very least be backported to the 5.4 stable branch at some point,
the problem is, this patch is based on media/master which contains the
s/MB_DIM_H264/MB_DIM/ change. I can send a new version based on
media/fixes, but that means Linus will have a conflict when merging the
media 5.5 PR in his tree. Are you fine dealing with this conflict
(letting Linus know about the expected resolution or backmerging the -rc
containing the fix in media/master so that he doesn't even have to deal
with it), or should we just let this patch go in media/master and
backport it later?
> },
> },
On Tue, 29 Oct 2019 01:26:00 +0000
Jonas Karlman <[email protected]> wrote:
> The FIELDPIC_FLAG_E bit should be set when field_pic_flag exists in stream,
> it is currently set based on field_pic_flag of current frame.
> The PIC_FIELDMODE_E bit is correctly set based on the field_pic_flag.
>
> Fix this by setting the FIELDPIC_FLAG_E bit when frame_mbs_only is not set.
>
> Fixes: dea0a82f3d22 ("media: hantro: Add support for H264 decoding on G1")
> Signed-off-by: Jonas Karlman <[email protected]>
Reviewed-by: Boris Brezillon <[email protected]>
> ---
> drivers/staging/media/hantro/hantro_g1_h264_dec.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/media/hantro/hantro_g1_h264_dec.c b/drivers/staging/media/hantro/hantro_g1_h264_dec.c
> index eeed11366135..c07da0ee4973 100644
> --- a/drivers/staging/media/hantro/hantro_g1_h264_dec.c
> +++ b/drivers/staging/media/hantro/hantro_g1_h264_dec.c
> @@ -63,7 +63,7 @@ static void set_params(struct hantro_ctx *ctx)
> /* always use the matrix sent from userspace */
> reg |= G1_REG_DEC_CTRL2_TYPE1_QUANT_E;
>
> - if (slices[0].flags & V4L2_H264_SLICE_FLAG_FIELD_PIC)
> + if (!(sps->flags & V4L2_H264_SPS_FLAG_FRAME_MBS_ONLY))
> reg |= G1_REG_DEC_CTRL2_FIELDPIC_FLAG_E;
> vdpu_write_relaxed(vpu, reg, G1_REG_DEC_CTRL2);
>
On 10/31/19 9:52 AM, Boris Brezillon wrote:
> On Tue, 29 Oct 2019 01:24:47 +0000
> Jonas Karlman <[email protected]> wrote:
>
>> TRM specify supported image size 48x48 to 4096x2304 at step size 16 pixels,
>> change frmsize max_width/max_height to match TRM at [1].
>>
>> This patch makes it possible to decode the 4096x2304 sample at [2].
>>
>> [1] http://www.t-firefly.com/download/firefly-rk3288/docs/TRM/rk3288-chapter-25-video-encoder-decoder-unit-(vcodec).pdf
>> [2] https://4ksamples.com/puppies-bath-in-4k/
>>
>> Fixes: 760327930e10 ("media: hantro: Enable H264 decoding on rk3288")
>> Signed-off-by: Jonas Karlman <[email protected]>
>
> Reviewed-by: Boris Brezillon <[email protected]>
> Tested-by: Boris Brezillon <[email protected]>
>
> Let's also add
>
> Cc: <[email protected]>
>
> just in case this patch doesn't make it to 5.4.
>
>
>> ---
>> Changes in v2:
>> - updated commit message with reference to TRM and sample video
>> ---
>> drivers/staging/media/hantro/rk3288_vpu_hw.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/staging/media/hantro/rk3288_vpu_hw.c b/drivers/staging/media/hantro/rk3288_vpu_hw.c
>> index c0bdd6c02520..f8db6fcaad73 100644
>> --- a/drivers/staging/media/hantro/rk3288_vpu_hw.c
>> +++ b/drivers/staging/media/hantro/rk3288_vpu_hw.c
>> @@ -67,10 +67,10 @@ static const struct hantro_fmt rk3288_vpu_dec_fmts[] = {
>> .max_depth = 2,
>> .frmsize = {
>> .min_width = 48,
>> - .max_width = 3840,
>> + .max_width = 4096,
>> .step_width = MB_DIM,
>> .min_height = 48,
>> - .max_height = 2160,
>> + .max_height = 2304,
>> .step_height = MB_DIM,
>
> Hans, Mauro, we were intending to have this fix merged in 5.4 or at
> the very least be backported to the 5.4 stable branch at some point,
> the problem is, this patch is based on media/master which contains the
> s/MB_DIM_H264/MB_DIM/ change. I can send a new version based on
> media/fixes, but that means Linus will have a conflict when merging the
> media 5.5 PR in his tree. Are you fine dealing with this conflict
> (letting Linus know about the expected resolution or backmerging the -rc
> containing the fix in media/master so that he doesn't even have to deal
> with it), or should we just let this patch go in media/master and
> backport it later?
Backport it later once it is merged in mainline.
This patch doesn't fix a bug, it is really an enhancement, so I think this
can safely be delayed.
Regards,
Hans
>
>> },
>> },
>
On Fri, 1 Nov 2019 09:36:55 +0100
Hans Verkuil <[email protected]> wrote:
> On 10/31/19 9:52 AM, Boris Brezillon wrote:
> > On Tue, 29 Oct 2019 01:24:47 +0000
> > Jonas Karlman <[email protected]> wrote:
> >
> >> TRM specify supported image size 48x48 to 4096x2304 at step size 16 pixels,
> >> change frmsize max_width/max_height to match TRM at [1].
> >>
> >> This patch makes it possible to decode the 4096x2304 sample at [2].
> >>
> >> [1] http://www.t-firefly.com/download/firefly-rk3288/docs/TRM/rk3288-chapter-25-video-encoder-decoder-unit-(vcodec).pdf
> >> [2] https://4ksamples.com/puppies-bath-in-4k/
> >>
> >> Fixes: 760327930e10 ("media: hantro: Enable H264 decoding on rk3288")
> >> Signed-off-by: Jonas Karlman <[email protected]>
> >
> > Reviewed-by: Boris Brezillon <[email protected]>
> > Tested-by: Boris Brezillon <[email protected]>
> >
> > Let's also add
> >
> > Cc: <[email protected]>
> >
> > just in case this patch doesn't make it to 5.4.
> >
> >
> >> ---
> >> Changes in v2:
> >> - updated commit message with reference to TRM and sample video
> >> ---
> >> drivers/staging/media/hantro/rk3288_vpu_hw.c | 4 ++--
> >> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/staging/media/hantro/rk3288_vpu_hw.c b/drivers/staging/media/hantro/rk3288_vpu_hw.c
> >> index c0bdd6c02520..f8db6fcaad73 100644
> >> --- a/drivers/staging/media/hantro/rk3288_vpu_hw.c
> >> +++ b/drivers/staging/media/hantro/rk3288_vpu_hw.c
> >> @@ -67,10 +67,10 @@ static const struct hantro_fmt rk3288_vpu_dec_fmts[] = {
> >> .max_depth = 2,
> >> .frmsize = {
> >> .min_width = 48,
> >> - .max_width = 3840,
> >> + .max_width = 4096,
> >> .step_width = MB_DIM,
> >> .min_height = 48,
> >> - .max_height = 2160,
> >> + .max_height = 2304,
> >> .step_height = MB_DIM,
> >
> > Hans, Mauro, we were intending to have this fix merged in 5.4 or at
> > the very least be backported to the 5.4 stable branch at some point,
> > the problem is, this patch is based on media/master which contains the
> > s/MB_DIM_H264/MB_DIM/ change. I can send a new version based on
> > media/fixes, but that means Linus will have a conflict when merging the
> > media 5.5 PR in his tree. Are you fine dealing with this conflict
> > (letting Linus know about the expected resolution or backmerging the -rc
> > containing the fix in media/master so that he doesn't even have to deal
> > with it), or should we just let this patch go in media/master and
> > backport it later?
>
> Backport it later once it is merged in mainline.
>
> This patch doesn't fix a bug, it is really an enhancement, so I think this
> can safely be delayed.
Okay, let's make things simple. Can we have patches 1 to 3 applied to
media/master? I'll take care of backporting those patches to 5.4 once
5.5-rc1 is out.
Thanks,
Boris