2022-12-23 18:41:28

by Nicolas Dufresne

Subject: [PATCH v2] hantro: Fix JPEG encoder ENUM_FRMSIZE on RK3399

Since 79c987de8b354, enumerating framesize on format set with "MODE_NONE"
(any raw formats) is reporting an invalid frmsize.

Size: Stepwise 0x0 - 0x0 with step 0/0

Before this change, the driver would return EINVAL, which is also invalid but
worked in GStreamer. The original intent was not to implement it, hence the
-ENOTTY return in this change. While drivers should implement ENUM_FRMSIZE for
all formats and queues, this change is limited in scope to fix the regression.

This fixes taking picture in Gnome Cheese software, or any software using
GSteamer to encode JPEG with hardware acceleration.

Fixes: 79c987de8b354 ("media: hantro: Use post processor scaling capacities")
Reported-by: Robert Mader <[email protected]>
Signed-off-by: Nicolas Dufresne <[email protected]>
---
drivers/media/platform/verisilicon/hantro_v4l2.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/verisilicon/hantro_v4l2.c b/drivers/media/platform/verisilicon/hantro_v4l2.c
index 2c7a805289e7b..30e650edaea8a 100644
--- a/drivers/media/platform/verisilicon/hantro_v4l2.c
+++ b/drivers/media/platform/verisilicon/hantro_v4l2.c
@@ -161,8 +161,11 @@ static int vidioc_enum_framesizes(struct file *file, void *priv,
}

/* For non-coded formats check if postprocessing scaling is possible */
- if (fmt->codec_mode == HANTRO_MODE_NONE && hantro_needs_postproc(ctx, fmt)) {
- return hanto_postproc_enum_framesizes(ctx, fsize);
+ if (fmt->codec_mode == HANTRO_MODE_NONE) {
+ if (hantro_needs_postproc(ctx, fmt))
+ return hanto_postproc_enum_framesizes(ctx, fsize);
+ else
+ return -ENOTTY;
} else if (fsize->index != 0) {
vpu_debug(0, "invalid frame size index (expected 0, got %d)\n",
fsize->index);
--
2.38.1

2022-12-24 08:50:31

by Benjamin Gaignard

Subject: Re: [PATCH v2] hantro: Fix JPEG encoder ENUM_FRMSIZE on RK3399

Le 23/12/2022 à 19:16, Nicolas Dufresne a écrit :
> Since 79c987de8b354, enumerating framesize on format set with "MODE_NONE"
> (any raw formats) is reporting an invalid frmsize.
>
> Size: Stepwise 0x0 - 0x0 with step 0/0
>
> Before this change, the driver would return EINVAL, which is also invalid but
> worked in GStreamer. The original intent was not to implement it, hence the
> -ENOTTY return in this change. While drivers should implement ENUM_FRMSIZE for
> all formats and queues, this change is limited in scope to fix the regression.
>
> This fixes taking picture in Gnome Cheese software, or any software using
> GSteamer to encode JPEG with hardware acceleration.
>
> Fixes: 79c987de8b354 ("media: hantro: Use post processor scaling capacities")
> Reported-by: Robert Mader <[email protected]>
> Signed-off-by: Nicolas Dufresne <[email protected]>

Reviewed-by: Benjamin Gaignard <[email protected]>

> ---
> drivers/media/platform/verisilicon/hantro_v4l2.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/platform/verisilicon/hantro_v4l2.c b/drivers/media/platform/verisilicon/hantro_v4l2.c
> index 2c7a805289e7b..30e650edaea8a 100644
> --- a/drivers/media/platform/verisilicon/hantro_v4l2.c
> +++ b/drivers/media/platform/verisilicon/hantro_v4l2.c
> @@ -161,8 +161,11 @@ static int vidioc_enum_framesizes(struct file *file, void *priv,
> }
>
> /* For non-coded formats check if postprocessing scaling is possible */
> - if (fmt->codec_mode == HANTRO_MODE_NONE && hantro_needs_postproc(ctx, fmt)) {
> - return hanto_postproc_enum_framesizes(ctx, fsize);
> + if (fmt->codec_mode == HANTRO_MODE_NONE) {
> + if (hantro_needs_postproc(ctx, fmt))
> + return hanto_postproc_enum_framesizes(ctx, fsize);
> + else
> + return -ENOTTY;
> } else if (fsize->index != 0) {
> vpu_debug(0, "invalid frame size index (expected 0, got %d)\n",
> fsize->index);

2022-12-24 18:40:54

by Ezequiel Garcia

Subject: Re: [PATCH v2] hantro: Fix JPEG encoder ENUM_FRMSIZE on RK3399

On Sat, Dec 24, 2022 at 09:03:59AM +0100, Benjamin Gaignard wrote:
>
> Le 23/12/2022 ? 19:16, Nicolas Dufresne a ?crit?:
> > Since 79c987de8b354, enumerating framesize on format set with "MODE_NONE"
> > (any raw formats) is reporting an invalid frmsize.
> >
> > Size: Stepwise 0x0 - 0x0 with step 0/0
> >
> > Before this change, the driver would return EINVAL, which is also invalid but
> > worked in GStreamer. The original intent was not to implement it, hence the
> > -ENOTTY return in this change. While drivers should implement ENUM_FRMSIZE for
> > all formats and queues, this change is limited in scope to fix the regression.
> >
> > This fixes taking picture in Gnome Cheese software, or any software using
> > GSteamer to encode JPEG with hardware acceleration.
> >
> > Fixes: 79c987de8b354 ("media: hantro: Use post processor scaling capacities")
> > Reported-by: Robert Mader <[email protected]>
> > Signed-off-by: Nicolas Dufresne <[email protected]>
>
> Reviewed-by: Benjamin Gaignard <[email protected]>
>

Reviewed-by: Ezequiel Garcia <[email protected]>

Thanks,
Ezequiel

> > ---
> > drivers/media/platform/verisilicon/hantro_v4l2.c | 7 +++++--
> > 1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/media/platform/verisilicon/hantro_v4l2.c b/drivers/media/platform/verisilicon/hantro_v4l2.c
> > index 2c7a805289e7b..30e650edaea8a 100644
> > --- a/drivers/media/platform/verisilicon/hantro_v4l2.c
> > +++ b/drivers/media/platform/verisilicon/hantro_v4l2.c
> > @@ -161,8 +161,11 @@ static int vidioc_enum_framesizes(struct file *file, void *priv,
> > }
> > /* For non-coded formats check if postprocessing scaling is possible */
> > - if (fmt->codec_mode == HANTRO_MODE_NONE && hantro_needs_postproc(ctx, fmt)) {
> > - return hanto_postproc_enum_framesizes(ctx, fsize);
> > + if (fmt->codec_mode == HANTRO_MODE_NONE) {
> > + if (hantro_needs_postproc(ctx, fmt))
> > + return hanto_postproc_enum_framesizes(ctx, fsize);
> > + else
> > + return -ENOTTY;
> > } else if (fsize->index != 0) {
> > vpu_debug(0, "invalid frame size index (expected 0, got %d)\n",
> > fsize->index);

2022-12-25 00:53:49

by Robert Mader

Subject: Re: [PATCH v2] hantro: Fix JPEG encoder ENUM_FRMSIZE on RK3399

On 24.12.22 18:03, Ezequiel Garcia wrote:
> On Sat, Dec 24, 2022 at 09:03:59AM +0100, Benjamin Gaignard wrote:
>> Le 23/12/2022 à 19:16, Nicolas Dufresne a écrit :
>>> Since 79c987de8b354, enumerating framesize on format set with "MODE_NONE"
>>> (any raw formats) is reporting an invalid frmsize.
>>>
>>> Size: Stepwise 0x0 - 0x0 with step 0/0
>>>
>>> Before this change, the driver would return EINVAL, which is also invalid but
>>> worked in GStreamer. The original intent was not to implement it, hence the
>>> -ENOTTY return in this change. While drivers should implement ENUM_FRMSIZE for
>>> all formats and queues, this change is limited in scope to fix the regression.
>>>
>>> This fixes taking picture in Gnome Cheese software, or any software using
>>> GSteamer to encode JPEG with hardware acceleration.
>>>
>>> Fixes: 79c987de8b354 ("media: hantro: Use post processor scaling capacities")
>>> Reported-by: Robert Mader <[email protected]>
>>> Signed-off-by: Nicolas Dufresne <[email protected]>
>> Reviewed-by: Benjamin Gaignard <[email protected]>
>>
> Reviewed-by: Ezequiel Garcia <[email protected]>
>
> Thanks,
> Ezequiel

Tested-by: Robert Mader <[email protected]>

Thanks a lot for this Christmas present, confirmed that it fixes the
affected apps / use-cases for me :)

>
>>> ---
>>> drivers/media/platform/verisilicon/hantro_v4l2.c | 7 +++++--
>>> 1 file changed, 5 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/media/platform/verisilicon/hantro_v4l2.c b/drivers/media/platform/verisilicon/hantro_v4l2.c
>>> index 2c7a805289e7b..30e650edaea8a 100644
>>> --- a/drivers/media/platform/verisilicon/hantro_v4l2.c
>>> +++ b/drivers/media/platform/verisilicon/hantro_v4l2.c
>>> @@ -161,8 +161,11 @@ static int vidioc_enum_framesizes(struct file *file, void *priv,
>>> }
>>> /* For non-coded formats check if postprocessing scaling is possible */
>>> - if (fmt->codec_mode == HANTRO_MODE_NONE && hantro_needs_postproc(ctx, fmt)) {
>>> - return hanto_postproc_enum_framesizes(ctx, fsize);
>>> + if (fmt->codec_mode == HANTRO_MODE_NONE) {
>>> + if (hantro_needs_postproc(ctx, fmt))
>>> + return hanto_postproc_enum_framesizes(ctx, fsize);
>>> + else
>>> + return -ENOTTY;
>>> } else if (fsize->index != 0) {
>>> vpu_debug(0, "invalid frame size index (expected 0, got %d)\n",
>>> fsize->index);

--
Robert Mader
Consultant Software Developer

Collabora Ltd.
Platinum Building, St John's Innovation Park, Cambridge CB4 0DS, UK
Registered in England & Wales, no. 5513718