All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
To: Chen-Yu Tsai <wenst@chromium.org>,
	Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>,
	Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: linux-media@vger.kernel.org, linux-rockchip@lists.infradead.org,
	linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Subject: Re: [PATCH 0/2] media: rkvdec: Align decoder behavior with Hantro and Cedrus
Date: Fri, 8 Oct 2021 17:01:35 +0200	[thread overview]
Message-ID: <c5393cf3-e2d1-4000-3bd1-00a09bb0ee8f@collabora.com> (raw)
In-Reply-To: <20211008100423.739462-1-wenst@chromium.org>

Hi Chen-Yu Tsai,

W dniu 08.10.2021 o 12:04, Chen-Yu Tsai pisze:
> Hi everyone,
> 
> While working on the rkvdec H.264 decoder for ChromeOS, I noticed some
> behavioral differences compared to Hantro and Cedrus:
> 
> 1. The driver always overrides the sizeimage setting given by userspace
>     for the output format. This results in insufficient buffer space when
>     running the ChromeOS video_decode_accelerator_tests test program,
>     likely due to a small initial resolution followed by dynamic
>     resolution change.
> 
> 2. Doesn't support dynamic resolution change.
> 
> This small series fixes both and aligns the behavior with the other two
> stateless decoder drivers. This was tested on the downstream ChromeOS
> 5.10 kernel with ChromeOS. Also compiled tested on mainline but I don't
> have any other RK3399 devices set up to test video stuff, so testing
> would be very much appreciated.
> 
> Also, I'm not sure if user applications are required to check the value
> of sizeimage upon S_FMT return. If the value is different or too small,
> what can the application do besides fail? AFAICT it can't split the
> data of one frame (or slice) between different buffers.
> 
> Andrzej, I believe the second patch would conflict with your VP9 series.
> 

The conflict is rather trivial to solve. Adopting your version does not
change in any way (neither for better nor for worse) the fluster score
I get for vp9 with rkvdec on a rockpi4 using my vp9 series.

Regards,

Andrzej

> 
> Regards
> ChenYu
> 
> Chen-Yu Tsai (2):
>    media: rkvdec: Do not override sizeimage for output format
>    media: rkvdec: Support dynamic resolution changes
> 
>   drivers/staging/media/rkvdec/rkvdec-h264.c |  5 +--
>   drivers/staging/media/rkvdec/rkvdec.c      | 40 +++++++++++-----------
>   2 files changed, 23 insertions(+), 22 deletions(-)
> 


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

WARNING: multiple messages have this Message-ID (diff)
From: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
To: Chen-Yu Tsai <wenst@chromium.org>,
	Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>,
	Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: linux-media@vger.kernel.org, linux-rockchip@lists.infradead.org,
	linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Subject: Re: [PATCH 0/2] media: rkvdec: Align decoder behavior with Hantro and Cedrus
Date: Fri, 8 Oct 2021 17:01:35 +0200	[thread overview]
Message-ID: <c5393cf3-e2d1-4000-3bd1-00a09bb0ee8f@collabora.com> (raw)
In-Reply-To: <20211008100423.739462-1-wenst@chromium.org>

Hi Chen-Yu Tsai,

W dniu 08.10.2021 o 12:04, Chen-Yu Tsai pisze:
> Hi everyone,
> 
> While working on the rkvdec H.264 decoder for ChromeOS, I noticed some
> behavioral differences compared to Hantro and Cedrus:
> 
> 1. The driver always overrides the sizeimage setting given by userspace
>     for the output format. This results in insufficient buffer space when
>     running the ChromeOS video_decode_accelerator_tests test program,
>     likely due to a small initial resolution followed by dynamic
>     resolution change.
> 
> 2. Doesn't support dynamic resolution change.
> 
> This small series fixes both and aligns the behavior with the other two
> stateless decoder drivers. This was tested on the downstream ChromeOS
> 5.10 kernel with ChromeOS. Also compiled tested on mainline but I don't
> have any other RK3399 devices set up to test video stuff, so testing
> would be very much appreciated.
> 
> Also, I'm not sure if user applications are required to check the value
> of sizeimage upon S_FMT return. If the value is different or too small,
> what can the application do besides fail? AFAICT it can't split the
> data of one frame (or slice) between different buffers.
> 
> Andrzej, I believe the second patch would conflict with your VP9 series.
> 

The conflict is rather trivial to solve. Adopting your version does not
change in any way (neither for better nor for worse) the fluster score
I get for vp9 with rkvdec on a rockpi4 using my vp9 series.

Regards,

Andrzej

> 
> Regards
> ChenYu
> 
> Chen-Yu Tsai (2):
>    media: rkvdec: Do not override sizeimage for output format
>    media: rkvdec: Support dynamic resolution changes
> 
>   drivers/staging/media/rkvdec/rkvdec-h264.c |  5 +--
>   drivers/staging/media/rkvdec/rkvdec.c      | 40 +++++++++++-----------
>   2 files changed, 23 insertions(+), 22 deletions(-)
> 


  parent reply	other threads:[~2021-10-08 15:01 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-08 10:04 [PATCH 0/2] media: rkvdec: Align decoder behavior with Hantro and Cedrus Chen-Yu Tsai
2021-10-08 10:04 ` Chen-Yu Tsai
2021-10-08 10:04 ` [PATCH 1/2] media: rkvdec: Do not override sizeimage for output format Chen-Yu Tsai
2021-10-08 10:04   ` Chen-Yu Tsai
2021-10-08 15:48   ` Nicolas Dufresne
2021-10-08 15:48     ` Nicolas Dufresne
2021-10-13 14:23   ` Ezequiel Garcia
2021-10-13 14:23     ` Ezequiel Garcia
2021-10-08 10:04 ` [PATCH 2/2] media: rkvdec: Support dynamic resolution changes Chen-Yu Tsai
2021-10-08 10:04   ` Chen-Yu Tsai
2021-10-08 15:54   ` Nicolas Dufresne
2021-10-08 15:54     ` Nicolas Dufresne
2021-10-13 14:27   ` Ezequiel Garcia
2021-10-13 14:27     ` Ezequiel Garcia
2021-10-08 15:01 ` Andrzej Pietrasiewicz [this message]
2021-10-08 15:01   ` [PATCH 0/2] media: rkvdec: Align decoder behavior with Hantro and Cedrus Andrzej Pietrasiewicz
2021-10-08 15:42 ` Nicolas Dufresne
2021-10-08 15:42   ` Nicolas Dufresne
2021-10-13  7:05   ` Chen-Yu Tsai
2021-10-13  7:05     ` Chen-Yu Tsai
2021-10-13 13:40     ` Nicolas Dufresne
2021-10-13 13:40       ` Nicolas Dufresne
2021-10-14  7:31       ` Chen-Yu Tsai
2021-10-14  7:31         ` Chen-Yu Tsai
2021-10-14  8:46         ` Ezequiel Garcia
2021-10-14  8:46           ` Ezequiel Garcia
2021-10-14  8:57           ` Chen-Yu Tsai
2021-10-14  8:57             ` Chen-Yu Tsai
2021-10-14  9:21             ` Ezequiel Garcia
2021-10-14  9:21               ` Ezequiel Garcia

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=c5393cf3-e2d1-4000-3bd1-00a09bb0ee8f@collabora.com \
    --to=andrzej.p@collabora.com \
    --cc=ezequiel@vanguardiasur.com.ar \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=linux-staging@lists.linux.dev \
    --cc=mchehab@kernel.org \
    --cc=wenst@chromium.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: link
Be 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.