* [PATCH for 5.4] media: hantro: Fix s_fmt for dynamic resolution changes
@ 2019-09-03 17:12 Ezequiel Garcia
2019-09-04 10:13 ` Philipp Zabel
0 siblings, 1 reply; 6+ messages in thread
From: Ezequiel Garcia @ 2019-09-03 17:12 UTC (permalink / raw)
To: linux-media
Cc: kernel, Nicolas Dufresne, Tomasz Figa, linux-rockchip,
Heiko Stuebner, Jonas Karlman, Philipp Zabel, Boris Brezillon,
Alexandre Courbot, fbuergisser, linux-kernel, Ezequiel Garcia
Commit 953aaa1492c53 ("media: rockchip/vpu: Prepare things to support decoders")
changed the conditions under S_FMT was allowed for OUTPUT
CAPTURE buffers.
However, and according to the mem-to-mem stateless decoder specification,
in order to support dynamic resolution changes, S_FMT should be allowed
even if OUTPUT buffers have been allocated.
Relax decoder S_FMT restrictions on OUTPUT buffers, allowing a resolution
modification, provided the pixel format stays the same.
Tested on RK3288 platforms using ChromiumOS Video Decode/Encode Accelerator Unittests.
Fixes: 953aaa1492c53 ("media: rockchip/vpu: Prepare things to support decoders")
Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
drivers/staging/media/hantro/hantro_v4l2.c | 22 ++++++++++++++++------
1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/media/hantro/hantro_v4l2.c b/drivers/staging/media/hantro/hantro_v4l2.c
index 3dae52abb96c..d48b548842cf 100644
--- a/drivers/staging/media/hantro/hantro_v4l2.c
+++ b/drivers/staging/media/hantro/hantro_v4l2.c
@@ -367,19 +367,22 @@ vidioc_s_fmt_out_mplane(struct file *file, void *priv, struct v4l2_format *f)
{
struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp;
struct hantro_ctx *ctx = fh_to_ctx(priv);
+ struct vb2_queue *vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, f->type);
const struct hantro_fmt *formats;
unsigned int num_fmts;
- struct vb2_queue *vq;
int ret;
- /* Change not allowed if queue is busy. */
- vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, f->type);
- if (vb2_is_busy(vq))
- return -EBUSY;
-
if (!hantro_is_encoder_ctx(ctx)) {
struct vb2_queue *peer_vq;
+ /*
+ * In other to support dynamic resolution change,
+ * the decoder admits a resolution change, as long
+ * as the pixelformat remains. Can't be done if streaming.
+ */
+ if (vb2_is_streaming(vq) || (vb2_is_busy(vq) &&
+ pix_mp->pixelformat != ctx->src_fmt.pixelformat))
+ return -EBUSY;
/*
* Since format change on the OUTPUT queue will reset
* the CAPTURE queue, we can't allow doing so
@@ -389,6 +392,13 @@ vidioc_s_fmt_out_mplane(struct file *file, void *priv, struct v4l2_format *f)
V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
if (vb2_is_busy(peer_vq))
return -EBUSY;
+ } else {
+ /*
+ * The encoder doesn't admit a format change if
+ * there are OUTPUT buffers allocated.
+ */
+ if (vb2_is_busy(vq))
+ return -EBUSY;
}
ret = vidioc_try_fmt_out_mplane(file, priv, f);
--
2.22.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH for 5.4] media: hantro: Fix s_fmt for dynamic resolution changes
2019-09-03 17:12 [PATCH for 5.4] media: hantro: Fix s_fmt for dynamic resolution changes Ezequiel Garcia
@ 2019-09-04 10:13 ` Philipp Zabel
2019-09-04 13:01 ` Ezequiel Garcia
0 siblings, 1 reply; 6+ messages in thread
From: Philipp Zabel @ 2019-09-04 10:13 UTC (permalink / raw)
To: Ezequiel Garcia, linux-media
Cc: kernel, Nicolas Dufresne, Tomasz Figa, linux-rockchip,
Heiko Stuebner, Jonas Karlman, Boris Brezillon,
Alexandre Courbot, fbuergisser, linux-kernel
Hi Ezequiel,
On Tue, 2019-09-03 at 14:12 -0300, Ezequiel Garcia wrote:
> Commit 953aaa1492c53 ("media: rockchip/vpu: Prepare things to support decoders")
> changed the conditions under S_FMT was allowed for OUTPUT
> CAPTURE buffers.
>
> However, and according to the mem-to-mem stateless decoder specification,
> in order to support dynamic resolution changes, S_FMT should be allowed
> even if OUTPUT buffers have been allocated.
>
> Relax decoder S_FMT restrictions on OUTPUT buffers, allowing a resolution
> modification, provided the pixel format stays the same.
>
> Tested on RK3288 platforms using ChromiumOS Video Decode/Encode Accelerator Unittests.
>
> Fixes: 953aaa1492c53 ("media: rockchip/vpu: Prepare things to support decoders")
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> ---
> drivers/staging/media/hantro/hantro_v4l2.c | 22 ++++++++++++++++------
> 1 file changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/staging/media/hantro/hantro_v4l2.c b/drivers/staging/media/hantro/hantro_v4l2.c
> index 3dae52abb96c..d48b548842cf 100644
> --- a/drivers/staging/media/hantro/hantro_v4l2.c
> +++ b/drivers/staging/media/hantro/hantro_v4l2.c
> @@ -367,19 +367,22 @@ vidioc_s_fmt_out_mplane(struct file *file, void *priv, struct v4l2_format *f)
> {
> struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp;
> struct hantro_ctx *ctx = fh_to_ctx(priv);
> + struct vb2_queue *vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, f->type);
> const struct hantro_fmt *formats;
> unsigned int num_fmts;
> - struct vb2_queue *vq;
> int ret;
>
> - /* Change not allowed if queue is busy. */
> - vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, f->type);
> - if (vb2_is_busy(vq))
> - return -EBUSY;
> -
> if (!hantro_is_encoder_ctx(ctx)) {
> struct vb2_queue *peer_vq;
>
> + /*
> + * In other to support dynamic resolution change,
> + * the decoder admits a resolution change, as long
> + * as the pixelformat remains. Can't be done if streaming.
> + */
> + if (vb2_is_streaming(vq) || (vb2_is_busy(vq) &&
> + pix_mp->pixelformat != ctx->src_fmt.pixelformat))
Before using contents of the v4l2_format f for comparison, we should run
vidioc_try_fmt_out_mplane over it. Also, besides pixelformat, sizeimage
shouldn't change either, at least if this is a VB2_MMAP queue.
> + return -EBUSY;
> /*
> * Since format change on the OUTPUT queue will reset
> * the CAPTURE queue, we can't allow doing so
> @@ -389,6 +392,13 @@ vidioc_s_fmt_out_mplane(struct file *file, void *priv, struct v4l2_format *f)
> V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
> if (vb2_is_busy(peer_vq))
> return -EBUSY;
> + } else {
> + /*
> + * The encoder doesn't admit a format change if
> + * there are OUTPUT buffers allocated.
> + */
> + if (vb2_is_busy(vq))
> + return -EBUSY;
> }
>
> ret = vidioc_try_fmt_out_mplane(file, priv, f);
I think this needs to be moved up.
regards
Philipp
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH for 5.4] media: hantro: Fix s_fmt for dynamic resolution changes
2019-09-04 10:13 ` Philipp Zabel
@ 2019-09-04 13:01 ` Ezequiel Garcia
2019-09-04 13:28 ` Philipp Zabel
0 siblings, 1 reply; 6+ messages in thread
From: Ezequiel Garcia @ 2019-09-04 13:01 UTC (permalink / raw)
To: Philipp Zabel, linux-media
Cc: kernel, Nicolas Dufresne, Tomasz Figa, linux-rockchip,
Heiko Stuebner, Jonas Karlman, Boris Brezillon,
Alexandre Courbot, fbuergisser, linux-kernel
On Wed, 2019-09-04 at 12:13 +0200, Philipp Zabel wrote:
> Hi Ezequiel,
>
> On Tue, 2019-09-03 at 14:12 -0300, Ezequiel Garcia wrote:
> > Commit 953aaa1492c53 ("media: rockchip/vpu: Prepare things to support decoders")
> > changed the conditions under S_FMT was allowed for OUTPUT
> > CAPTURE buffers.
> >
> > However, and according to the mem-to-mem stateless decoder specification,
> > in order to support dynamic resolution changes, S_FMT should be allowed
> > even if OUTPUT buffers have been allocated.
> >
> > Relax decoder S_FMT restrictions on OUTPUT buffers, allowing a resolution
> > modification, provided the pixel format stays the same.
> >
> > Tested on RK3288 platforms using ChromiumOS Video Decode/Encode Accelerator Unittests.
> >
> > Fixes: 953aaa1492c53 ("media: rockchip/vpu: Prepare things to support decoders")
> > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> > ---
> > drivers/staging/media/hantro/hantro_v4l2.c | 22 ++++++++++++++++------
> > 1 file changed, 16 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/staging/media/hantro/hantro_v4l2.c b/drivers/staging/media/hantro/hantro_v4l2.c
> > index 3dae52abb96c..d48b548842cf 100644
> > --- a/drivers/staging/media/hantro/hantro_v4l2.c
> > +++ b/drivers/staging/media/hantro/hantro_v4l2.c
> > @@ -367,19 +367,22 @@ vidioc_s_fmt_out_mplane(struct file *file, void *priv, struct v4l2_format *f)
> > {
> > struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp;
> > struct hantro_ctx *ctx = fh_to_ctx(priv);
> > + struct vb2_queue *vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, f->type);
> > const struct hantro_fmt *formats;
> > unsigned int num_fmts;
> > - struct vb2_queue *vq;
> > int ret;
> >
> > - /* Change not allowed if queue is busy. */
> > - vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, f->type);
> > - if (vb2_is_busy(vq))
> > - return -EBUSY;
> > -
> > if (!hantro_is_encoder_ctx(ctx)) {
> > struct vb2_queue *peer_vq;
> >
> > + /*
> > + * In other to support dynamic resolution change,
> > + * the decoder admits a resolution change, as long
> > + * as the pixelformat remains. Can't be done if streaming.
> > + */
> > + if (vb2_is_streaming(vq) || (vb2_is_busy(vq) &&
> > + pix_mp->pixelformat != ctx->src_fmt.pixelformat))
>
> Before using contents of the v4l2_format f for comparison, we should run
> vidioc_try_fmt_out_mplane over it.
Right, good catch.
> Also, besides pixelformat, sizeimage
> shouldn't change either, at least if this is a VB2_MMAP queue.
>
This is the OUTPUT queue, so I don't see why the sizeimage
of the coded buffers should stay the same. Maybe I'm missing
something?
> > + return -EBUSY;
> > /*
> > * Since format change on the OUTPUT queue will reset
> > * the CAPTURE queue, we can't allow doing so
> > @@ -389,6 +392,13 @@ vidioc_s_fmt_out_mplane(struct file *file, void *priv, struct v4l2_format *f)
> > V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
> > if (vb2_is_busy(peer_vq))
> > return -EBUSY;
> > + } else {
> > + /*
> > + * The encoder doesn't admit a format change if
> > + * there are OUTPUT buffers allocated.
> > + */
> > + if (vb2_is_busy(vq))
> > + return -EBUSY;
> > }
> >
> > ret = vidioc_try_fmt_out_mplane(file, priv, f);
>
> I think this needs to be moved up.
>
Right.
Thanks,
Ezequiel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH for 5.4] media: hantro: Fix s_fmt for dynamic resolution changes
2019-09-04 13:01 ` Ezequiel Garcia
@ 2019-09-04 13:28 ` Philipp Zabel
2019-09-05 18:00 ` Ezequiel Garcia
0 siblings, 1 reply; 6+ messages in thread
From: Philipp Zabel @ 2019-09-04 13:28 UTC (permalink / raw)
To: Ezequiel Garcia, linux-media
Cc: kernel, Nicolas Dufresne, Tomasz Figa, linux-rockchip,
Heiko Stuebner, Jonas Karlman, Boris Brezillon,
Alexandre Courbot, fbuergisser, linux-kernel
On Wed, 2019-09-04 at 10:01 -0300, Ezequiel Garcia wrote:
> On Wed, 2019-09-04 at 12:13 +0200, Philipp Zabel wrote:
> > Hi Ezequiel,
> >
> > On Tue, 2019-09-03 at 14:12 -0300, Ezequiel Garcia wrote:
> > > Commit 953aaa1492c53 ("media: rockchip/vpu: Prepare things to support decoders")
> > > changed the conditions under S_FMT was allowed for OUTPUT
> > > CAPTURE buffers.
> > >
> > > However, and according to the mem-to-mem stateless decoder specification,
> > > in order to support dynamic resolution changes, S_FMT should be allowed
> > > even if OUTPUT buffers have been allocated.
> > >
> > > Relax decoder S_FMT restrictions on OUTPUT buffers, allowing a resolution
> > > modification, provided the pixel format stays the same.
> > >
> > > Tested on RK3288 platforms using ChromiumOS Video Decode/Encode Accelerator Unittests.
> > >
> > > Fixes: 953aaa1492c53 ("media: rockchip/vpu: Prepare things to support decoders")
> > > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> > > ---
> > > drivers/staging/media/hantro/hantro_v4l2.c | 22 ++++++++++++++++------
> > > 1 file changed, 16 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/drivers/staging/media/hantro/hantro_v4l2.c b/drivers/staging/media/hantro/hantro_v4l2.c
> > > index 3dae52abb96c..d48b548842cf 100644
> > > --- a/drivers/staging/media/hantro/hantro_v4l2.c
> > > +++ b/drivers/staging/media/hantro/hantro_v4l2.c
> > > @@ -367,19 +367,22 @@ vidioc_s_fmt_out_mplane(struct file *file, void *priv, struct v4l2_format *f)
> > > {
> > > struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp;
> > > struct hantro_ctx *ctx = fh_to_ctx(priv);
> > > + struct vb2_queue *vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, f->type);
> > > const struct hantro_fmt *formats;
> > > unsigned int num_fmts;
> > > - struct vb2_queue *vq;
> > > int ret;
> > >
> > > - /* Change not allowed if queue is busy. */
> > > - vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, f->type);
> > > - if (vb2_is_busy(vq))
> > > - return -EBUSY;
> > > -
> > > if (!hantro_is_encoder_ctx(ctx)) {
> > > struct vb2_queue *peer_vq;
> > >
> > > + /*
> > > + * In other to support dynamic resolution change,
> > > + * the decoder admits a resolution change, as long
> > > + * as the pixelformat remains. Can't be done if streaming.
> > > + */
> > > + if (vb2_is_streaming(vq) || (vb2_is_busy(vq) &&
> > > + pix_mp->pixelformat != ctx->src_fmt.pixelformat))
> >
> > Before using contents of the v4l2_format f for comparison, we should run
> > vidioc_try_fmt_out_mplane over it.
>
> Right, good catch.
>
> > Also, besides pixelformat, sizeimage
> > shouldn't change either, at least if this is a VB2_MMAP queue.
> >
>
> This is the OUTPUT queue, so I don't see why the sizeimage
> of the coded buffers should stay the same. Maybe I'm missing
> something?
If the OUTPUT vb2_queue is busy, we already have some buffers of the old
size allocated. We can't change their size dynamically with just
VIDIOC_S_FMT.
Maybe this should correct sizeimage to the old size instead of returning
-EBUSY? Either way, if the old buffer size is too small to reasonably
decode the new resolution, the OUTPUT buffers have to be reallocated.
regards
Philipp
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH for 5.4] media: hantro: Fix s_fmt for dynamic resolution changes
2019-09-04 13:28 ` Philipp Zabel
@ 2019-09-05 18:00 ` Ezequiel Garcia
2019-09-06 9:26 ` Philipp Zabel
0 siblings, 1 reply; 6+ messages in thread
From: Ezequiel Garcia @ 2019-09-05 18:00 UTC (permalink / raw)
To: Philipp Zabel, linux-media
Cc: kernel, Nicolas Dufresne, Tomasz Figa, linux-rockchip,
Heiko Stuebner, Jonas Karlman, Boris Brezillon,
Alexandre Courbot, fbuergisser, linux-kernel
On Wed, 2019-09-04 at 15:28 +0200, Philipp Zabel wrote:
> On Wed, 2019-09-04 at 10:01 -0300, Ezequiel Garcia wrote:
> > On Wed, 2019-09-04 at 12:13 +0200, Philipp Zabel wrote:
> > > Hi Ezequiel,
> > >
> > > On Tue, 2019-09-03 at 14:12 -0300, Ezequiel Garcia wrote:
> > > > Commit 953aaa1492c53 ("media: rockchip/vpu: Prepare things to support decoders")
> > > > changed the conditions under S_FMT was allowed for OUTPUT
> > > > CAPTURE buffers.
> > > >
> > > > However, and according to the mem-to-mem stateless decoder specification,
> > > > in order to support dynamic resolution changes, S_FMT should be allowed
> > > > even if OUTPUT buffers have been allocated.
> > > >
> > > > Relax decoder S_FMT restrictions on OUTPUT buffers, allowing a resolution
> > > > modification, provided the pixel format stays the same.
> > > >
> > > > Tested on RK3288 platforms using ChromiumOS Video Decode/Encode Accelerator Unittests.
> > > >
> > > > Fixes: 953aaa1492c53 ("media: rockchip/vpu: Prepare things to support decoders")
> > > > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> > > > ---
> > > > drivers/staging/media/hantro/hantro_v4l2.c | 22 ++++++++++++++++------
> > > > 1 file changed, 16 insertions(+), 6 deletions(-)
> > > >
> > > > diff --git a/drivers/staging/media/hantro/hantro_v4l2.c b/drivers/staging/media/hantro/hantro_v4l2.c
> > > > index 3dae52abb96c..d48b548842cf 100644
> > > > --- a/drivers/staging/media/hantro/hantro_v4l2.c
> > > > +++ b/drivers/staging/media/hantro/hantro_v4l2.c
> > > > @@ -367,19 +367,22 @@ vidioc_s_fmt_out_mplane(struct file *file, void *priv, struct v4l2_format *f)
> > > > {
> > > > struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp;
> > > > struct hantro_ctx *ctx = fh_to_ctx(priv);
> > > > + struct vb2_queue *vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, f->type);
> > > > const struct hantro_fmt *formats;
> > > > unsigned int num_fmts;
> > > > - struct vb2_queue *vq;
> > > > int ret;
> > > >
> > > > - /* Change not allowed if queue is busy. */
> > > > - vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, f->type);
> > > > - if (vb2_is_busy(vq))
> > > > - return -EBUSY;
> > > > -
> > > > if (!hantro_is_encoder_ctx(ctx)) {
> > > > struct vb2_queue *peer_vq;
> > > >
> > > > + /*
> > > > + * In other to support dynamic resolution change,
> > > > + * the decoder admits a resolution change, as long
> > > > + * as the pixelformat remains. Can't be done if streaming.
> > > > + */
> > > > + if (vb2_is_streaming(vq) || (vb2_is_busy(vq) &&
> > > > + pix_mp->pixelformat != ctx->src_fmt.pixelformat))
> > >
> > > Before using contents of the v4l2_format f for comparison, we should run
> > > vidioc_try_fmt_out_mplane over it.
> >
> > Right, good catch.
> >
> > > Also, besides pixelformat, sizeimage
> > > shouldn't change either, at least if this is a VB2_MMAP queue.
> > >
> >
> > This is the OUTPUT queue, so I don't see why the sizeimage
> > of the coded buffers should stay the same. Maybe I'm missing
> > something?
>
> If the OUTPUT vb2_queue is busy, we already have some buffers of the old
> size allocated. We can't change their size dynamically with just
> VIDIOC_S_FMT.
>
> Maybe this should correct sizeimage to the old size instead of returning
> -EBUSY? Either way, if the old buffer size is too small to reasonably
> decode the new resolution, the OUTPUT buffers have to be reallocated.
>
Note that for a decoder, the OUTPUT side buffers are coded. Is there any
straightforward correlation between the buffer size and the resolution?
In other words, how does the driver figure out if the size is
resonably large?
Regards,
Ezequiel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH for 5.4] media: hantro: Fix s_fmt for dynamic resolution changes
2019-09-05 18:00 ` Ezequiel Garcia
@ 2019-09-06 9:26 ` Philipp Zabel
0 siblings, 0 replies; 6+ messages in thread
From: Philipp Zabel @ 2019-09-06 9:26 UTC (permalink / raw)
To: Ezequiel Garcia, linux-media
Cc: kernel, Nicolas Dufresne, Tomasz Figa, linux-rockchip,
Heiko Stuebner, Jonas Karlman, Boris Brezillon,
Alexandre Courbot, fbuergisser, linux-kernel
On Thu, 2019-09-05 at 15:00 -0300, Ezequiel Garcia wrote:
> On Wed, 2019-09-04 at 15:28 +0200, Philipp Zabel wrote:
> > On Wed, 2019-09-04 at 10:01 -0300, Ezequiel Garcia wrote:
> > > On Wed, 2019-09-04 at 12:13 +0200, Philipp Zabel wrote:
> > > > Hi Ezequiel,
> > > >
> > > > On Tue, 2019-09-03 at 14:12 -0300, Ezequiel Garcia wrote:
> > > > > Commit 953aaa1492c53 ("media: rockchip/vpu: Prepare things to support decoders")
> > > > > changed the conditions under S_FMT was allowed for OUTPUT
> > > > > CAPTURE buffers.
> > > > >
> > > > > However, and according to the mem-to-mem stateless decoder specification,
> > > > > in order to support dynamic resolution changes, S_FMT should be allowed
> > > > > even if OUTPUT buffers have been allocated.
> > > > >
> > > > > Relax decoder S_FMT restrictions on OUTPUT buffers, allowing a resolution
> > > > > modification, provided the pixel format stays the same.
> > > > >
> > > > > Tested on RK3288 platforms using ChromiumOS Video Decode/Encode Accelerator Unittests.
> > > > >
> > > > > Fixes: 953aaa1492c53 ("media: rockchip/vpu: Prepare things to support decoders")
> > > > > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> > > > > ---
> > > > > drivers/staging/media/hantro/hantro_v4l2.c | 22 ++++++++++++++++------
> > > > > 1 file changed, 16 insertions(+), 6 deletions(-)
> > > > >
> > > > > diff --git a/drivers/staging/media/hantro/hantro_v4l2.c b/drivers/staging/media/hantro/hantro_v4l2.c
> > > > > index 3dae52abb96c..d48b548842cf 100644
> > > > > --- a/drivers/staging/media/hantro/hantro_v4l2.c
> > > > > +++ b/drivers/staging/media/hantro/hantro_v4l2.c
> > > > > @@ -367,19 +367,22 @@ vidioc_s_fmt_out_mplane(struct file *file, void *priv, struct v4l2_format *f)
> > > > > {
> > > > > struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp;
> > > > > struct hantro_ctx *ctx = fh_to_ctx(priv);
> > > > > + struct vb2_queue *vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, f->type);
> > > > > const struct hantro_fmt *formats;
> > > > > unsigned int num_fmts;
> > > > > - struct vb2_queue *vq;
> > > > > int ret;
> > > > >
> > > > > - /* Change not allowed if queue is busy. */
> > > > > - vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, f->type);
> > > > > - if (vb2_is_busy(vq))
> > > > > - return -EBUSY;
> > > > > -
> > > > > if (!hantro_is_encoder_ctx(ctx)) {
> > > > > struct vb2_queue *peer_vq;
> > > > >
> > > > > + /*
> > > > > + * In other to support dynamic resolution change,
> > > > > + * the decoder admits a resolution change, as long
> > > > > + * as the pixelformat remains. Can't be done if streaming.
> > > > > + */
> > > > > + if (vb2_is_streaming(vq) || (vb2_is_busy(vq) &&
> > > > > + pix_mp->pixelformat != ctx->src_fmt.pixelformat))
> > > >
> > > > Before using contents of the v4l2_format f for comparison, we should run
> > > > vidioc_try_fmt_out_mplane over it.
> > >
> > > Right, good catch.
> > >
> > > > Also, besides pixelformat, sizeimage
> > > > shouldn't change either, at least if this is a VB2_MMAP queue.
> > > >
> > >
> > > This is the OUTPUT queue, so I don't see why the sizeimage
> > > of the coded buffers should stay the same. Maybe I'm missing
> > > something?
> >
> > If the OUTPUT vb2_queue is busy, we already have some buffers of the old
> > size allocated. We can't change their size dynamically with just
> > VIDIOC_S_FMT.
> >
> > Maybe this should correct sizeimage to the old size instead of returning
> > -EBUSY? Either way, if the old buffer size is too small to reasonably
> > decode the new resolution, the OUTPUT buffers have to be reallocated.
> >
>
> Note that for a decoder, the OUTPUT side buffers are coded. Is there any
> straightforward correlation between the buffer size and the resolution?
>
> In other words, how does the driver figure out if the size is
> resonably large?
The decoded frame size seems like a reasonable upper bound at first, but
especially for formats like H.264 it is trivial to construct slices that
are larger than that with I_PCM macroblocks.
I think for H.264 some limit depending on profile and level can be
constructed from spec chapter A.3. "Levels", by looking at the rules for
the sum of NumBytesInNALunit, though not quite straightforward.
regards
Philipp
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-09-06 9:26 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-03 17:12 [PATCH for 5.4] media: hantro: Fix s_fmt for dynamic resolution changes Ezequiel Garcia
2019-09-04 10:13 ` Philipp Zabel
2019-09-04 13:01 ` Ezequiel Garcia
2019-09-04 13:28 ` Philipp Zabel
2019-09-05 18:00 ` Ezequiel Garcia
2019-09-06 9:26 ` Philipp Zabel
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.