| Message ID | 20230928053723.20765-1-ming.qian@nxp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: imx-jpeg: notify source chagne event when the first picture parsed | expand |
On 28/09/2023 07:37, Ming Qian wrote: > After gstreamer rework the dynamic resolution change handling, gstreamer > stop doing capture buffer allocation based on guesses and wait for the > source change event when available. It requires driver always notify > source change event in the initialization, even if the size parsed is > equal to the size set on capture queue. otherwise, the pipeline will be > stalled. > > Currently driver may not notify source change event if the parsed format > and size are equal to those previously established, but it may stall the > gstreamer pipeline. > > The link of gstreamer patch is > https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/4437 > > Fixes: b4e1fb8643da ("media: imx-jpeg: Support dynamic resolution change") > Signed-off-by: Ming Qian <ming.qian@nxp.com> > --- > drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c | 7 ++++++- > drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h | 1 + > 2 files changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c > index 3af0af8ac07b..372f3007ff43 100644 > --- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c > +++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c > @@ -1348,7 +1348,8 @@ static bool mxc_jpeg_source_change(struct mxc_jpeg_ctx *ctx, > q_data_cap = mxc_jpeg_get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE); > if (mxc_jpeg_compare_format(q_data_cap->fmt, jpeg_src_buf->fmt)) > jpeg_src_buf->fmt = q_data_cap->fmt; > - if (q_data_cap->fmt != jpeg_src_buf->fmt || > + if (!ctx->source_change_cnt || > + q_data_cap->fmt != jpeg_src_buf->fmt || > q_data_cap->w != jpeg_src_buf->w || > q_data_cap->h != jpeg_src_buf->h) { > dev_dbg(dev, "Detected jpeg res=(%dx%d)->(%dx%d), pixfmt=%c%c%c%c\n", > @@ -1392,6 +1393,7 @@ static bool mxc_jpeg_source_change(struct mxc_jpeg_ctx *ctx, > mxc_jpeg_sizeimage(q_data_cap); > notify_src_chg(ctx); > ctx->source_change = 1; > + ctx->source_change_cnt++; > if (vb2_is_streaming(v4l2_m2m_get_dst_vq(ctx->fh.m2m_ctx))) > mxc_jpeg_set_last_buffer(ctx); > } > @@ -1611,6 +1613,9 @@ static int mxc_jpeg_queue_setup(struct vb2_queue *q, > for (i = 0; i < *nplanes; i++) > sizes[i] = mxc_jpeg_get_plane_size(q_data, i); > > + if (V4L2_TYPE_IS_OUTPUT(q->type)) > + ctx->source_change_cnt = 0; > + > return 0; > } > > diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h > index d80e94cc9d99..b7e94fa50e02 100644 > --- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h > +++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h > @@ -99,6 +99,7 @@ struct mxc_jpeg_ctx { > enum mxc_jpeg_enc_state enc_state; > int slot; > unsigned int source_change; > + unsigned int source_change_cnt; This is a confusing field. It is not a counter at all, it is just a bool to indicate if the initial source change event was raised or not. So something like: bool need_initial_source_change_evt; (feel free to give it a better name!) It is certainly not a counter. Regards, Hans > bool header_parsed; > struct v4l2_ctrl_handler ctrl_handler; > u8 jpeg_quality;
diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c index 3af0af8ac07b..372f3007ff43 100644 --- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c +++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c @@ -1348,7 +1348,8 @@ static bool mxc_jpeg_source_change(struct mxc_jpeg_ctx *ctx, q_data_cap = mxc_jpeg_get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE); if (mxc_jpeg_compare_format(q_data_cap->fmt, jpeg_src_buf->fmt)) jpeg_src_buf->fmt = q_data_cap->fmt; - if (q_data_cap->fmt != jpeg_src_buf->fmt || + if (!ctx->source_change_cnt || + q_data_cap->fmt != jpeg_src_buf->fmt || q_data_cap->w != jpeg_src_buf->w || q_data_cap->h != jpeg_src_buf->h) { dev_dbg(dev, "Detected jpeg res=(%dx%d)->(%dx%d), pixfmt=%c%c%c%c\n", @@ -1392,6 +1393,7 @@ static bool mxc_jpeg_source_change(struct mxc_jpeg_ctx *ctx, mxc_jpeg_sizeimage(q_data_cap); notify_src_chg(ctx); ctx->source_change = 1; + ctx->source_change_cnt++; if (vb2_is_streaming(v4l2_m2m_get_dst_vq(ctx->fh.m2m_ctx))) mxc_jpeg_set_last_buffer(ctx); } @@ -1611,6 +1613,9 @@ static int mxc_jpeg_queue_setup(struct vb2_queue *q, for (i = 0; i < *nplanes; i++) sizes[i] = mxc_jpeg_get_plane_size(q_data, i); + if (V4L2_TYPE_IS_OUTPUT(q->type)) + ctx->source_change_cnt = 0; + return 0; } diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h index d80e94cc9d99..b7e94fa50e02 100644 --- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h +++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h @@ -99,6 +99,7 @@ struct mxc_jpeg_ctx { enum mxc_jpeg_enc_state enc_state; int slot; unsigned int source_change; + unsigned int source_change_cnt; bool header_parsed; struct v4l2_ctrl_handler ctrl_handler; u8 jpeg_quality;
After gstreamer rework the dynamic resolution change handling, gstreamer stop doing capture buffer allocation based on guesses and wait for the source change event when available. It requires driver always notify source change event in the initialization, even if the size parsed is equal to the size set on capture queue. otherwise, the pipeline will be stalled. Currently driver may not notify source change event if the parsed format and size are equal to those previously established, but it may stall the gstreamer pipeline. The link of gstreamer patch is https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/4437 Fixes: b4e1fb8643da ("media: imx-jpeg: Support dynamic resolution change") Signed-off-by: Ming Qian <ming.qian@nxp.com> --- drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c | 7 ++++++- drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h | 1 + 2 files changed, 7 insertions(+), 1 deletion(-)