* [PATCH] media: verisilicon: HEVC: Initialize start_bit field
@ 2025-01-20 8:10 Benjamin Gaignard
2025-01-20 10:00 ` Benjamin Gaignard
0 siblings, 1 reply; 6+ messages in thread
From: Benjamin Gaignard @ 2025-01-20 8:10 UTC (permalink / raw)
To: nicolas.dufresne, p.zabel, mchehab
Cc: linux-media, linux-rockchip, linux-kernel, Benjamin Gaignard
Always set start_bit field to 0, if not it could lead to corrupted frames
specially when decoding VP9 bitstreams at the same time since VP9 driver
set it for it own purpose.
Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
---
drivers/media/platform/verisilicon/hantro_g2_hevc_dec.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/media/platform/verisilicon/hantro_g2_hevc_dec.c b/drivers/media/platform/verisilicon/hantro_g2_hevc_dec.c
index 85a44143b378..0e212198dd65 100644
--- a/drivers/media/platform/verisilicon/hantro_g2_hevc_dec.c
+++ b/drivers/media/platform/verisilicon/hantro_g2_hevc_dec.c
@@ -518,6 +518,7 @@ static void set_buffers(struct hantro_ctx *ctx)
hantro_reg_write(vpu, &g2_stream_len, src_len);
hantro_reg_write(vpu, &g2_strm_buffer_len, src_buf_len);
hantro_reg_write(vpu, &g2_strm_start_offset, 0);
+ hantro_reg_write(vpu, &g2_start_bit, 0);
hantro_reg_write(vpu, &g2_write_mvs_e, 1);
hantro_write_addr(vpu, G2_TILE_SIZES_ADDR, ctx->hevc_dec.tile_sizes.dma);
--
2.43.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] media: verisilicon: HEVC: Initialize start_bit field
2025-01-20 8:10 [PATCH] media: verisilicon: HEVC: Initialize start_bit field Benjamin Gaignard
@ 2025-01-20 10:00 ` Benjamin Gaignard
2025-01-20 14:10 ` Nicolas Dufresne
0 siblings, 1 reply; 6+ messages in thread
From: Benjamin Gaignard @ 2025-01-20 10:00 UTC (permalink / raw)
To: nicolas.dufresne, p.zabel, mchehab
Cc: linux-media, linux-rockchip, linux-kernel
Le 20/01/2025 à 09:10, Benjamin Gaignard a écrit :
> Always set start_bit field to 0, if not it could lead to corrupted frames
> specially when decoding VP9 bitstreams at the same time since VP9 driver
> set it for it own purpose.
>
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
I could add this tag:
Fixes: cb5dd5a0fa518 ("media: hantro: Introduce G2/HEVC decoder")
> ---
> drivers/media/platform/verisilicon/hantro_g2_hevc_dec.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/media/platform/verisilicon/hantro_g2_hevc_dec.c b/drivers/media/platform/verisilicon/hantro_g2_hevc_dec.c
> index 85a44143b378..0e212198dd65 100644
> --- a/drivers/media/platform/verisilicon/hantro_g2_hevc_dec.c
> +++ b/drivers/media/platform/verisilicon/hantro_g2_hevc_dec.c
> @@ -518,6 +518,7 @@ static void set_buffers(struct hantro_ctx *ctx)
> hantro_reg_write(vpu, &g2_stream_len, src_len);
> hantro_reg_write(vpu, &g2_strm_buffer_len, src_buf_len);
> hantro_reg_write(vpu, &g2_strm_start_offset, 0);
> + hantro_reg_write(vpu, &g2_start_bit, 0);
> hantro_reg_write(vpu, &g2_write_mvs_e, 1);
>
> hantro_write_addr(vpu, G2_TILE_SIZES_ADDR, ctx->hevc_dec.tile_sizes.dma);
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] media: verisilicon: HEVC: Initialize start_bit field
2025-01-20 10:00 ` Benjamin Gaignard
@ 2025-01-20 14:10 ` Nicolas Dufresne
2025-01-20 14:15 ` Adam Ford
0 siblings, 1 reply; 6+ messages in thread
From: Nicolas Dufresne @ 2025-01-20 14:10 UTC (permalink / raw)
To: Benjamin Gaignard, p.zabel, mchehab
Cc: linux-media, linux-rockchip, linux-kernel
Le lundi 20 janvier 2025 à 11:00 +0100, Benjamin Gaignard a écrit :
> Le 20/01/2025 à 09:10, Benjamin Gaignard a écrit :
> > Always set start_bit field to 0, if not it could lead to corrupted frames
> > specially when decoding VP9 bitstreams at the same time since VP9 driver
> > set it for it own purpose.
its
> >
> > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
>
> I could add this tag:
>
> Fixes: cb5dd5a0fa518 ("media: hantro: Introduce G2/HEVC decoder")
I have tested this on IMX8MQ board using the following GStreamer pipeline.
Before the change, the HEVC window was entirely corrupted. The streams don't
matter as long as they use both HEVC and VP9 codec.
gst-launch-1.0 \
filesrc location=hevc.mp4 ! parsebin ! v4l2slh265dec ! fakevideosink \
filesrc location=vp9.mkv ! parsebin ! v4l2slvp9dec ! fakevideosink
Tested-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
>
> > ---
> > drivers/media/platform/verisilicon/hantro_g2_hevc_dec.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/media/platform/verisilicon/hantro_g2_hevc_dec.c b/drivers/media/platform/verisilicon/hantro_g2_hevc_dec.c
> > index 85a44143b378..0e212198dd65 100644
> > --- a/drivers/media/platform/verisilicon/hantro_g2_hevc_dec.c
> > +++ b/drivers/media/platform/verisilicon/hantro_g2_hevc_dec.c
> > @@ -518,6 +518,7 @@ static void set_buffers(struct hantro_ctx *ctx)
> > hantro_reg_write(vpu, &g2_stream_len, src_len);
> > hantro_reg_write(vpu, &g2_strm_buffer_len, src_buf_len);
> > hantro_reg_write(vpu, &g2_strm_start_offset, 0);
> > + hantro_reg_write(vpu, &g2_start_bit, 0);
> > hantro_reg_write(vpu, &g2_write_mvs_e, 1);
I've also crossed against "decoder_swreg_map_g2.xlsx" documentation, if you are
lucky to have access to that, and within swreg5 there is only g2_tempor_mvp_e
that is also shared, and its already being set in both drivers.
Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> >
> > hantro_write_addr(vpu, G2_TILE_SIZES_ADDR, ctx->hevc_dec.tile_sizes.dma);
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] media: verisilicon: HEVC: Initialize start_bit field
2025-01-20 14:10 ` Nicolas Dufresne
@ 2025-01-20 14:15 ` Adam Ford
2025-01-20 14:19 ` Benjamin Gaignard
2025-01-20 14:43 ` Nicolas Dufresne
0 siblings, 2 replies; 6+ messages in thread
From: Adam Ford @ 2025-01-20 14:15 UTC (permalink / raw)
To: Nicolas Dufresne
Cc: Benjamin Gaignard, p.zabel, mchehab, linux-media, linux-rockchip,
linux-kernel
On Mon, Jan 20, 2025 at 8:10 AM Nicolas Dufresne
<nicolas.dufresne@collabora.com> wrote:
>
> Le lundi 20 janvier 2025 à 11:00 +0100, Benjamin Gaignard a écrit :
> > Le 20/01/2025 à 09:10, Benjamin Gaignard a écrit :
> > > Always set start_bit field to 0, if not it could lead to corrupted frames
> > > specially when decoding VP9 bitstreams at the same time since VP9 driver
> > > set it for it own purpose.
>
> its
>
Does this impact the Fluster score?
> > >
> > > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
> >
> > I could add this tag:
> >
> > Fixes: cb5dd5a0fa518 ("media: hantro: Introduce G2/HEVC decoder")
>
> I have tested this on IMX8MQ board using the following GStreamer pipeline.
> Before the change, the HEVC window was entirely corrupted. The streams don't
> matter as long as they use both HEVC and VP9 codec.
>
> gst-launch-1.0 \
> filesrc location=hevc.mp4 ! parsebin ! v4l2slh265dec ! fakevideosink \
> filesrc location=vp9.mkv ! parsebin ! v4l2slvp9dec ! fakevideosink
>
> Tested-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
>
>
> >
> > > ---
> > > drivers/media/platform/verisilicon/hantro_g2_hevc_dec.c | 1 +
> > > 1 file changed, 1 insertion(+)
> > >
> > > diff --git a/drivers/media/platform/verisilicon/hantro_g2_hevc_dec.c b/drivers/media/platform/verisilicon/hantro_g2_hevc_dec.c
> > > index 85a44143b378..0e212198dd65 100644
> > > --- a/drivers/media/platform/verisilicon/hantro_g2_hevc_dec.c
> > > +++ b/drivers/media/platform/verisilicon/hantro_g2_hevc_dec.c
> > > @@ -518,6 +518,7 @@ static void set_buffers(struct hantro_ctx *ctx)
> > > hantro_reg_write(vpu, &g2_stream_len, src_len);
> > > hantro_reg_write(vpu, &g2_strm_buffer_len, src_buf_len);
> > > hantro_reg_write(vpu, &g2_strm_start_offset, 0);
> > > + hantro_reg_write(vpu, &g2_start_bit, 0);
> > > hantro_reg_write(vpu, &g2_write_mvs_e, 1);
>
> I've also crossed against "decoder_swreg_map_g2.xlsx" documentation, if you are
> lucky to have access to that, and within swreg5 there is only g2_tempor_mvp_e
> that is also shared, and its already being set in both drivers.
>
> Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
>
>
> > >
> > > hantro_write_addr(vpu, G2_TILE_SIZES_ADDR, ctx->hevc_dec.tile_sizes.dma);
> >
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] media: verisilicon: HEVC: Initialize start_bit field
2025-01-20 14:15 ` Adam Ford
@ 2025-01-20 14:19 ` Benjamin Gaignard
2025-01-20 14:43 ` Nicolas Dufresne
1 sibling, 0 replies; 6+ messages in thread
From: Benjamin Gaignard @ 2025-01-20 14:19 UTC (permalink / raw)
To: Adam Ford, Nicolas Dufresne
Cc: p.zabel, mchehab, linux-media, linux-rockchip, linux-kernel
Le 20/01/2025 à 15:15, Adam Ford a écrit :
> On Mon, Jan 20, 2025 at 8:10 AM Nicolas Dufresne
> <nicolas.dufresne@collabora.com> wrote:
>> Le lundi 20 janvier 2025 à 11:00 +0100, Benjamin Gaignard a écrit :
>>> Le 20/01/2025 à 09:10, Benjamin Gaignard a écrit :
>>>> Always set start_bit field to 0, if not it could lead to corrupted frames
>>>> specially when decoding VP9 bitstreams at the same time since VP9 driver
>>>> set it for it own purpose.
>> its
>>
> Does this impact the Fluster score?
No because that happens only when decoding VP9 and HEVC bitstreams at the same time.
>
>>>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
>>> I could add this tag:
>>>
>>> Fixes: cb5dd5a0fa518 ("media: hantro: Introduce G2/HEVC decoder")
>> I have tested this on IMX8MQ board using the following GStreamer pipeline.
>> Before the change, the HEVC window was entirely corrupted. The streams don't
>> matter as long as they use both HEVC and VP9 codec.
>>
>> gst-launch-1.0 \
>> filesrc location=hevc.mp4 ! parsebin ! v4l2slh265dec ! fakevideosink \
>> filesrc location=vp9.mkv ! parsebin ! v4l2slvp9dec ! fakevideosink
>>
>> Tested-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
>>
>>
>>>> ---
>>>> drivers/media/platform/verisilicon/hantro_g2_hevc_dec.c | 1 +
>>>> 1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/drivers/media/platform/verisilicon/hantro_g2_hevc_dec.c b/drivers/media/platform/verisilicon/hantro_g2_hevc_dec.c
>>>> index 85a44143b378..0e212198dd65 100644
>>>> --- a/drivers/media/platform/verisilicon/hantro_g2_hevc_dec.c
>>>> +++ b/drivers/media/platform/verisilicon/hantro_g2_hevc_dec.c
>>>> @@ -518,6 +518,7 @@ static void set_buffers(struct hantro_ctx *ctx)
>>>> hantro_reg_write(vpu, &g2_stream_len, src_len);
>>>> hantro_reg_write(vpu, &g2_strm_buffer_len, src_buf_len);
>>>> hantro_reg_write(vpu, &g2_strm_start_offset, 0);
>>>> + hantro_reg_write(vpu, &g2_start_bit, 0);
>>>> hantro_reg_write(vpu, &g2_write_mvs_e, 1);
>> I've also crossed against "decoder_swreg_map_g2.xlsx" documentation, if you are
>> lucky to have access to that, and within swreg5 there is only g2_tempor_mvp_e
>> that is also shared, and its already being set in both drivers.
>>
>> Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
>>
>>
>>>> hantro_write_addr(vpu, G2_TILE_SIZES_ADDR, ctx->hevc_dec.tile_sizes.dma);
>>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] media: verisilicon: HEVC: Initialize start_bit field
2025-01-20 14:15 ` Adam Ford
2025-01-20 14:19 ` Benjamin Gaignard
@ 2025-01-20 14:43 ` Nicolas Dufresne
1 sibling, 0 replies; 6+ messages in thread
From: Nicolas Dufresne @ 2025-01-20 14:43 UTC (permalink / raw)
To: Adam Ford
Cc: Benjamin Gaignard, p.zabel, mchehab, linux-media, linux-rockchip,
linux-kernel
Le lundi 20 janvier 2025 à 08:15 -0600, Adam Ford a écrit :
> On Mon, Jan 20, 2025 at 8:10 AM Nicolas Dufresne
> <nicolas.dufresne@collabora.com> wrote:
> >
> > Le lundi 20 janvier 2025 à 11:00 +0100, Benjamin Gaignard a écrit :
> > > Le 20/01/2025 à 09:10, Benjamin Gaignard a écrit :
> > > > Always set start_bit field to 0, if not it could lead to corrupted frames
> > > > specially when decoding VP9 bitstreams at the same time since VP9 driver
> > > > set it for it own purpose.
> >
> > its
> >
>
> Does this impact the Fluster score?
Only if you concurrently test VP9 and HEVC at the same time. Fluster does not
parallelized across codecs, could be a nice addition to catch more issues.
Nicolas
>
> > > >
> > > > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
> > >
> > > I could add this tag:
> > >
> > > Fixes: cb5dd5a0fa518 ("media: hantro: Introduce G2/HEVC decoder")
> >
> > I have tested this on IMX8MQ board using the following GStreamer pipeline.
> > Before the change, the HEVC window was entirely corrupted. The streams don't
> > matter as long as they use both HEVC and VP9 codec.
> >
> > gst-launch-1.0 \
> > filesrc location=hevc.mp4 ! parsebin ! v4l2slh265dec ! fakevideosink \
> > filesrc location=vp9.mkv ! parsebin ! v4l2slvp9dec ! fakevideosink
> >
> > Tested-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> >
> >
> > >
> > > > ---
> > > > drivers/media/platform/verisilicon/hantro_g2_hevc_dec.c | 1 +
> > > > 1 file changed, 1 insertion(+)
> > > >
> > > > diff --git a/drivers/media/platform/verisilicon/hantro_g2_hevc_dec.c b/drivers/media/platform/verisilicon/hantro_g2_hevc_dec.c
> > > > index 85a44143b378..0e212198dd65 100644
> > > > --- a/drivers/media/platform/verisilicon/hantro_g2_hevc_dec.c
> > > > +++ b/drivers/media/platform/verisilicon/hantro_g2_hevc_dec.c
> > > > @@ -518,6 +518,7 @@ static void set_buffers(struct hantro_ctx *ctx)
> > > > hantro_reg_write(vpu, &g2_stream_len, src_len);
> > > > hantro_reg_write(vpu, &g2_strm_buffer_len, src_buf_len);
> > > > hantro_reg_write(vpu, &g2_strm_start_offset, 0);
> > > > + hantro_reg_write(vpu, &g2_start_bit, 0);
> > > > hantro_reg_write(vpu, &g2_write_mvs_e, 1);
> >
> > I've also crossed against "decoder_swreg_map_g2.xlsx" documentation, if you are
> > lucky to have access to that, and within swreg5 there is only g2_tempor_mvp_e
> > that is also shared, and its already being set in both drivers.
> >
> > Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> >
> >
> > > >
> > > > hantro_write_addr(vpu, G2_TILE_SIZES_ADDR, ctx->hevc_dec.tile_sizes.dma);
> > >
> >
> >
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-01-20 14:43 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-01-20 8:10 [PATCH] media: verisilicon: HEVC: Initialize start_bit field Benjamin Gaignard
2025-01-20 10:00 ` Benjamin Gaignard
2025-01-20 14:10 ` Nicolas Dufresne
2025-01-20 14:15 ` Adam Ford
2025-01-20 14:19 ` Benjamin Gaignard
2025-01-20 14:43 ` Nicolas Dufresne
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®