Message ID | 20250417-b4-rkvdec_h264_high10_and_422_support-v9-3-0e8738ccb46b@collabora.com (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | media: rkvdec: Add H.264 High 10 and 4:2:2 profile support | expand |
Hi Nicolas, On 2025-04-17 23:58, Nicolas Dufresne wrote: > From: Jonas Karlman <jonas@kwiboo.se> > > Add support for a get_image_fmt() ops that returns the required image > format. > > The CAPTURE format is reset when the required image format changes and > the buffer queue is not busy. > > Signed-off-by: Jonas Karlman <jonas@kwiboo.se> > Tested-by: Nicolas Dufresne <nicolas.dufresne@collabora.com> > Co-developed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com> > Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com> > --- > drivers/staging/media/rkvdec/rkvdec.c | 35 +++++++++++++++++++++++++++++++++++ > drivers/staging/media/rkvdec/rkvdec.h | 2 ++ > 2 files changed, 37 insertions(+) > > diff --git a/drivers/staging/media/rkvdec/rkvdec.c b/drivers/staging/media/rkvdec/rkvdec.c > index 7b780392bb6a63cc954655ef940e87146d2b852f..6c6fe411f48772419e1810d869ab40d168848e65 100644 > --- a/drivers/staging/media/rkvdec/rkvdec.c > +++ b/drivers/staging/media/rkvdec/rkvdec.c > @@ -72,6 +72,15 @@ static bool rkvdec_is_valid_fmt(struct rkvdec_ctx *ctx, u32 fourcc, > return false; > } > > +static bool rkvdec_fmt_changed(struct rkvdec_ctx *ctx, > + enum rkvdec_image_fmt image_fmt) Just a small nitpick: Maybe this function should be called rkvdec_image_fmt_changed() and could be moved closer to rkvdec_image_fmt_match() as those two are related to image_fmt and not the pixfmt/fourcc. Regards, Jonas > +{ > + if (image_fmt == RKVDEC_IMG_FMT_ANY) > + return false; > + > + return ctx->image_fmt != image_fmt; > +} > + > static void rkvdec_fill_decoded_pixfmt(struct rkvdec_ctx *ctx, > struct v4l2_pix_format_mplane *pix_mp) > { > @@ -118,8 +127,34 @@ static int rkvdec_try_ctrl(struct v4l2_ctrl *ctrl) > return 0; > } > > +static int rkvdec_s_ctrl(struct v4l2_ctrl *ctrl) > +{ > + struct rkvdec_ctx *ctx = container_of(ctrl->handler, struct rkvdec_ctx, ctrl_hdl); > + const struct rkvdec_coded_fmt_desc *desc = ctx->coded_fmt_desc; > + enum rkvdec_image_fmt image_fmt; > + struct vb2_queue *vq; > + > + /* Check if this change requires a capture format reset */ > + if (!desc->ops->get_image_fmt) > + return 0; > + > + image_fmt = desc->ops->get_image_fmt(ctx, ctrl); > + if (rkvdec_fmt_changed(ctx, image_fmt)) { > + vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, > + V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE); > + if (vb2_is_busy(vq)) > + return -EBUSY; > + > + ctx->image_fmt = image_fmt; > + rkvdec_reset_decoded_fmt(ctx); > + } > + > + return 0; > +} > + > static const struct v4l2_ctrl_ops rkvdec_ctrl_ops = { > .try_ctrl = rkvdec_try_ctrl, > + .s_ctrl = rkvdec_s_ctrl, > }; > > static const struct rkvdec_ctrl_desc rkvdec_h264_ctrl_descs[] = { > diff --git a/drivers/staging/media/rkvdec/rkvdec.h b/drivers/staging/media/rkvdec/rkvdec.h > index 6f8cf50c5d99aad2f52e321f54f3ca17166ddf98..e466a2753ccfc13738e0a672bc578e521af2c3f2 100644 > --- a/drivers/staging/media/rkvdec/rkvdec.h > +++ b/drivers/staging/media/rkvdec/rkvdec.h > @@ -73,6 +73,8 @@ struct rkvdec_coded_fmt_ops { > struct vb2_v4l2_buffer *dst_buf, > enum vb2_buffer_state result); > int (*try_ctrl)(struct rkvdec_ctx *ctx, struct v4l2_ctrl *ctrl); > + enum rkvdec_image_fmt (*get_image_fmt)(struct rkvdec_ctx *ctx, > + struct v4l2_ctrl *ctrl); > }; > > enum rkvdec_image_fmt { >
Le vendredi 18 avril 2025 à 08:22 +0200, Jonas Karlman a écrit : > Hi Nicolas, > > On 2025-04-17 23:58, Nicolas Dufresne wrote: > > From: Jonas Karlman <jonas@kwiboo.se> > > > > Add support for a get_image_fmt() ops that returns the required image > > format. > > > > The CAPTURE format is reset when the required image format changes and > > the buffer queue is not busy. > > > > Signed-off-by: Jonas Karlman <jonas@kwiboo.se> > > Tested-by: Nicolas Dufresne <nicolas.dufresne@collabora.com> > > Co-developed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com> > > Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com> > > --- > > drivers/staging/media/rkvdec/rkvdec.c | 35 +++++++++++++++++++++++++++++++++++ > > drivers/staging/media/rkvdec/rkvdec.h | 2 ++ > > 2 files changed, 37 insertions(+) > > > > diff --git a/drivers/staging/media/rkvdec/rkvdec.c b/drivers/staging/media/rkvdec/rkvdec.c > > index 7b780392bb6a63cc954655ef940e87146d2b852f..6c6fe411f48772419e1810d869ab40d168848e65 100644 > > --- a/drivers/staging/media/rkvdec/rkvdec.c > > +++ b/drivers/staging/media/rkvdec/rkvdec.c > > @@ -72,6 +72,15 @@ static bool rkvdec_is_valid_fmt(struct rkvdec_ctx *ctx, u32 fourcc, > > return false; > > } > > > > +static bool rkvdec_fmt_changed(struct rkvdec_ctx *ctx, > > + enum rkvdec_image_fmt image_fmt) > > Just a small nitpick: > > Maybe this function should be called rkvdec_image_fmt_changed() and > could be moved closer to rkvdec_image_fmt_match() as those two are > related to image_fmt and not the pixfmt/fourcc. Applied locally. With this change, may I have your Rb ? thanks, Nicolas > > Regards, > Jonas > > > +{ > > + if (image_fmt == RKVDEC_IMG_FMT_ANY) > > + return false; > > + > > + return ctx->image_fmt != image_fmt; > > +} > > + > > static void rkvdec_fill_decoded_pixfmt(struct rkvdec_ctx *ctx, > > struct v4l2_pix_format_mplane *pix_mp) > > { > > @@ -118,8 +127,34 @@ static int rkvdec_try_ctrl(struct v4l2_ctrl *ctrl) > > return 0; > > } > > > > +static int rkvdec_s_ctrl(struct v4l2_ctrl *ctrl) > > +{ > > + struct rkvdec_ctx *ctx = container_of(ctrl->handler, struct rkvdec_ctx, ctrl_hdl); > > + const struct rkvdec_coded_fmt_desc *desc = ctx->coded_fmt_desc; > > + enum rkvdec_image_fmt image_fmt; > > + struct vb2_queue *vq; > > + > > + /* Check if this change requires a capture format reset */ > > + if (!desc->ops->get_image_fmt) > > + return 0; > > + > > + image_fmt = desc->ops->get_image_fmt(ctx, ctrl); > > + if (rkvdec_fmt_changed(ctx, image_fmt)) { > > + vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, > > + V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE); > > + if (vb2_is_busy(vq)) > > + return -EBUSY; > > + > > + ctx->image_fmt = image_fmt; > > + rkvdec_reset_decoded_fmt(ctx); > > + } > > + > > + return 0; > > +} > > + > > static const struct v4l2_ctrl_ops rkvdec_ctrl_ops = { > > .try_ctrl = rkvdec_try_ctrl, > > + .s_ctrl = rkvdec_s_ctrl, > > }; > > > > static const struct rkvdec_ctrl_desc rkvdec_h264_ctrl_descs[] = { > > diff --git a/drivers/staging/media/rkvdec/rkvdec.h b/drivers/staging/media/rkvdec/rkvdec.h > > index 6f8cf50c5d99aad2f52e321f54f3ca17166ddf98..e466a2753ccfc13738e0a672bc578e521af2c3f2 100644 > > --- a/drivers/staging/media/rkvdec/rkvdec.h > > +++ b/drivers/staging/media/rkvdec/rkvdec.h > > @@ -73,6 +73,8 @@ struct rkvdec_coded_fmt_ops { > > struct vb2_v4l2_buffer *dst_buf, > > enum vb2_buffer_state result); > > int (*try_ctrl)(struct rkvdec_ctx *ctx, struct v4l2_ctrl *ctrl); > > + enum rkvdec_image_fmt (*get_image_fmt)(struct rkvdec_ctx *ctx, > > + struct v4l2_ctrl *ctrl); > > }; > > > > enum rkvdec_image_fmt { > > >
Hi Nicolas, On 2025-04-18 14:28, Nicolas Dufresne wrote: > Le vendredi 18 avril 2025 à 08:22 +0200, Jonas Karlman a écrit : >> Hi Nicolas, >> >> On 2025-04-17 23:58, Nicolas Dufresne wrote: >>> From: Jonas Karlman <jonas@kwiboo.se> >>> >>> Add support for a get_image_fmt() ops that returns the required image >>> format. >>> >>> The CAPTURE format is reset when the required image format changes and >>> the buffer queue is not busy. >>> >>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se> >>> Tested-by: Nicolas Dufresne <nicolas.dufresne@collabora.com> >>> Co-developed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com> >>> Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com> >>> --- >>> drivers/staging/media/rkvdec/rkvdec.c | 35 +++++++++++++++++++++++++++++++++++ >>> drivers/staging/media/rkvdec/rkvdec.h | 2 ++ >>> 2 files changed, 37 insertions(+) >>> >>> diff --git a/drivers/staging/media/rkvdec/rkvdec.c b/drivers/staging/media/rkvdec/rkvdec.c >>> index 7b780392bb6a63cc954655ef940e87146d2b852f..6c6fe411f48772419e1810d869ab40d168848e65 100644 >>> --- a/drivers/staging/media/rkvdec/rkvdec.c >>> +++ b/drivers/staging/media/rkvdec/rkvdec.c >>> @@ -72,6 +72,15 @@ static bool rkvdec_is_valid_fmt(struct rkvdec_ctx *ctx, u32 fourcc, >>> return false; >>> } >>> >>> +static bool rkvdec_fmt_changed(struct rkvdec_ctx *ctx, >>> + enum rkvdec_image_fmt image_fmt) >> >> Just a small nitpick: >> >> Maybe this function should be called rkvdec_image_fmt_changed() and >> could be moved closer to rkvdec_image_fmt_match() as those two are >> related to image_fmt and not the pixfmt/fourcc. > > Applied locally. With this change, may I have your Rb ? Sure, and thanks for helping getting this old series to land :-) Reviewed-by: Jonas Karlman <jonas@kwiboo.se> Regards, Jonas > > thanks, > Nicolas > >> >> Regards, >> Jonas >> >>> +{ >>> + if (image_fmt == RKVDEC_IMG_FMT_ANY) >>> + return false; >>> + >>> + return ctx->image_fmt != image_fmt; >>> +} >>> + >>> static void rkvdec_fill_decoded_pixfmt(struct rkvdec_ctx *ctx, >>> struct v4l2_pix_format_mplane *pix_mp) >>> { >>> @@ -118,8 +127,34 @@ static int rkvdec_try_ctrl(struct v4l2_ctrl *ctrl) >>> return 0; >>> } >>> >>> +static int rkvdec_s_ctrl(struct v4l2_ctrl *ctrl) >>> +{ >>> + struct rkvdec_ctx *ctx = container_of(ctrl->handler, struct rkvdec_ctx, ctrl_hdl); >>> + const struct rkvdec_coded_fmt_desc *desc = ctx->coded_fmt_desc; >>> + enum rkvdec_image_fmt image_fmt; >>> + struct vb2_queue *vq; >>> + >>> + /* Check if this change requires a capture format reset */ >>> + if (!desc->ops->get_image_fmt) >>> + return 0; >>> + >>> + image_fmt = desc->ops->get_image_fmt(ctx, ctrl); >>> + if (rkvdec_fmt_changed(ctx, image_fmt)) { >>> + vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, >>> + V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE); >>> + if (vb2_is_busy(vq)) >>> + return -EBUSY; >>> + >>> + ctx->image_fmt = image_fmt; >>> + rkvdec_reset_decoded_fmt(ctx); >>> + } >>> + >>> + return 0; >>> +} >>> + >>> static const struct v4l2_ctrl_ops rkvdec_ctrl_ops = { >>> .try_ctrl = rkvdec_try_ctrl, >>> + .s_ctrl = rkvdec_s_ctrl, >>> }; >>> >>> static const struct rkvdec_ctrl_desc rkvdec_h264_ctrl_descs[] = { >>> diff --git a/drivers/staging/media/rkvdec/rkvdec.h b/drivers/staging/media/rkvdec/rkvdec.h >>> index 6f8cf50c5d99aad2f52e321f54f3ca17166ddf98..e466a2753ccfc13738e0a672bc578e521af2c3f2 100644 >>> --- a/drivers/staging/media/rkvdec/rkvdec.h >>> +++ b/drivers/staging/media/rkvdec/rkvdec.h >>> @@ -73,6 +73,8 @@ struct rkvdec_coded_fmt_ops { >>> struct vb2_v4l2_buffer *dst_buf, >>> enum vb2_buffer_state result); >>> int (*try_ctrl)(struct rkvdec_ctx *ctx, struct v4l2_ctrl *ctrl); >>> + enum rkvdec_image_fmt (*get_image_fmt)(struct rkvdec_ctx *ctx, >>> + struct v4l2_ctrl *ctrl); >>> }; >>> >>> enum rkvdec_image_fmt { >>> >>
diff --git a/drivers/staging/media/rkvdec/rkvdec.c b/drivers/staging/media/rkvdec/rkvdec.c index 7b780392bb6a63cc954655ef940e87146d2b852f..6c6fe411f48772419e1810d869ab40d168848e65 100644 --- a/drivers/staging/media/rkvdec/rkvdec.c +++ b/drivers/staging/media/rkvdec/rkvdec.c @@ -72,6 +72,15 @@ static bool rkvdec_is_valid_fmt(struct rkvdec_ctx *ctx, u32 fourcc, return false; } +static bool rkvdec_fmt_changed(struct rkvdec_ctx *ctx, + enum rkvdec_image_fmt image_fmt) +{ + if (image_fmt == RKVDEC_IMG_FMT_ANY) + return false; + + return ctx->image_fmt != image_fmt; +} + static void rkvdec_fill_decoded_pixfmt(struct rkvdec_ctx *ctx, struct v4l2_pix_format_mplane *pix_mp) { @@ -118,8 +127,34 @@ static int rkvdec_try_ctrl(struct v4l2_ctrl *ctrl) return 0; } +static int rkvdec_s_ctrl(struct v4l2_ctrl *ctrl) +{ + struct rkvdec_ctx *ctx = container_of(ctrl->handler, struct rkvdec_ctx, ctrl_hdl); + const struct rkvdec_coded_fmt_desc *desc = ctx->coded_fmt_desc; + enum rkvdec_image_fmt image_fmt; + struct vb2_queue *vq; + + /* Check if this change requires a capture format reset */ + if (!desc->ops->get_image_fmt) + return 0; + + image_fmt = desc->ops->get_image_fmt(ctx, ctrl); + if (rkvdec_fmt_changed(ctx, image_fmt)) { + vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, + V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE); + if (vb2_is_busy(vq)) + return -EBUSY; + + ctx->image_fmt = image_fmt; + rkvdec_reset_decoded_fmt(ctx); + } + + return 0; +} + static const struct v4l2_ctrl_ops rkvdec_ctrl_ops = { .try_ctrl = rkvdec_try_ctrl, + .s_ctrl = rkvdec_s_ctrl, }; static const struct rkvdec_ctrl_desc rkvdec_h264_ctrl_descs[] = { diff --git a/drivers/staging/media/rkvdec/rkvdec.h b/drivers/staging/media/rkvdec/rkvdec.h index 6f8cf50c5d99aad2f52e321f54f3ca17166ddf98..e466a2753ccfc13738e0a672bc578e521af2c3f2 100644 --- a/drivers/staging/media/rkvdec/rkvdec.h +++ b/drivers/staging/media/rkvdec/rkvdec.h @@ -73,6 +73,8 @@ struct rkvdec_coded_fmt_ops { struct vb2_v4l2_buffer *dst_buf, enum vb2_buffer_state result); int (*try_ctrl)(struct rkvdec_ctx *ctx, struct v4l2_ctrl *ctrl); + enum rkvdec_image_fmt (*get_image_fmt)(struct rkvdec_ctx *ctx, + struct v4l2_ctrl *ctrl); }; enum rkvdec_image_fmt {