| Message ID | 20211008100423.739462-2-wenst@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: rkvdec: Align decoder behavior with Hantro and Cedrus | expand |
Le vendredi 08 octobre 2021 à 18:04 +0800, Chen-Yu Tsai a écrit : > The rkvdec H.264 decoder currently overrides sizeimage for the output > format. This causes issues when userspace requires and requests a larger > buffer, but ends up with one of insufficient size. > > Instead, only provide a default size if none was requested. This fixes > the video_decode_accelerator_tests from Chromium failing on the first > frame due to insufficient buffer space. It also aligns the behavior > of the rkvdec driver with the Hantro and Cedrus drivers. > > Fixes: cd33c830448b ("media: rkvdec: Add the rkvdec driver") > Cc: <stable@vger.kernel.org> > Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> > --- > drivers/staging/media/rkvdec/rkvdec-h264.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/staging/media/rkvdec/rkvdec-h264.c b/drivers/staging/media/rkvdec/rkvdec-h264.c > index 76e97cbe2512..951e19231da2 100644 > --- a/drivers/staging/media/rkvdec/rkvdec-h264.c > +++ b/drivers/staging/media/rkvdec/rkvdec-h264.c > @@ -1015,8 +1015,9 @@ static int rkvdec_h264_adjust_fmt(struct rkvdec_ctx *ctx, > struct v4l2_pix_format_mplane *fmt = &f->fmt.pix_mp; > > fmt->num_planes = 1; > - fmt->plane_fmt[0].sizeimage = fmt->width * fmt->height * > - RKVDEC_H264_MAX_DEPTH_IN_BYTES; > + if (!fmt->plane_fmt[0].sizeimage) > + fmt->plane_fmt[0].sizeimage = fmt->width * fmt->height * > + RKVDEC_H264_MAX_DEPTH_IN_BYTES; Note that the test is more strict then the spec, since this behaviour is within spec. But in general, the application may have more information about the incoming stream, the maximum encoded frame size would even be encoded in the container (which is parsed in userspace). So I agree it will be more flexible to accept userspace desired size. If that size is too small, userspace will fail at filling it in the first place, and decoding won't be possible, that's all. Perhaps we could make a recommendation in that sense in the spec ? Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com> > return 0; > } >
Hi Chen-Yu, On Fri, Oct 08, 2021 at 06:04:22PM +0800, Chen-Yu Tsai wrote: > The rkvdec H.264 decoder currently overrides sizeimage for the output > format. This causes issues when userspace requires and requests a larger > buffer, but ends up with one of insufficient size. > > Instead, only provide a default size if none was requested. This fixes > the video_decode_accelerator_tests from Chromium failing on the first > frame due to insufficient buffer space. It also aligns the behavior > of the rkvdec driver with the Hantro and Cedrus drivers. > > Fixes: cd33c830448b ("media: rkvdec: Add the rkvdec driver") > Cc: <stable@vger.kernel.org> > Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> Reviewed-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> Thanks for taking care of this! Ezequiel > --- > drivers/staging/media/rkvdec/rkvdec-h264.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/staging/media/rkvdec/rkvdec-h264.c b/drivers/staging/media/rkvdec/rkvdec-h264.c > index 76e97cbe2512..951e19231da2 100644 > --- a/drivers/staging/media/rkvdec/rkvdec-h264.c > +++ b/drivers/staging/media/rkvdec/rkvdec-h264.c > @@ -1015,8 +1015,9 @@ static int rkvdec_h264_adjust_fmt(struct rkvdec_ctx *ctx, > struct v4l2_pix_format_mplane *fmt = &f->fmt.pix_mp; > > fmt->num_planes = 1; > - fmt->plane_fmt[0].sizeimage = fmt->width * fmt->height * > - RKVDEC_H264_MAX_DEPTH_IN_BYTES; > + if (!fmt->plane_fmt[0].sizeimage) > + fmt->plane_fmt[0].sizeimage = fmt->width * fmt->height * > + RKVDEC_H264_MAX_DEPTH_IN_BYTES; > return 0; > } > > -- > 2.33.0.882.g93a45727a2-goog >
diff --git a/drivers/staging/media/rkvdec/rkvdec-h264.c b/drivers/staging/media/rkvdec/rkvdec-h264.c index 76e97cbe2512..951e19231da2 100644 --- a/drivers/staging/media/rkvdec/rkvdec-h264.c +++ b/drivers/staging/media/rkvdec/rkvdec-h264.c @@ -1015,8 +1015,9 @@ static int rkvdec_h264_adjust_fmt(struct rkvdec_ctx *ctx, struct v4l2_pix_format_mplane *fmt = &f->fmt.pix_mp; fmt->num_planes = 1; - fmt->plane_fmt[0].sizeimage = fmt->width * fmt->height * - RKVDEC_H264_MAX_DEPTH_IN_BYTES; + if (!fmt->plane_fmt[0].sizeimage) + fmt->plane_fmt[0].sizeimage = fmt->width * fmt->height * + RKVDEC_H264_MAX_DEPTH_IN_BYTES; return 0; }
The rkvdec H.264 decoder currently overrides sizeimage for the output format. This causes issues when userspace requires and requests a larger buffer, but ends up with one of insufficient size. Instead, only provide a default size if none was requested. This fixes the video_decode_accelerator_tests from Chromium failing on the first frame due to insufficient buffer space. It also aligns the behavior of the rkvdec driver with the Hantro and Cedrus drivers. Fixes: cd33c830448b ("media: rkvdec: Add the rkvdec driver") Cc: <stable@vger.kernel.org> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> --- drivers/staging/media/rkvdec/rkvdec-h264.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)