Re: [PATCH 10/12] [media] coda: fail to start streaming if userspace set invalid formats
Hi Kamil, Am Mittwoch, den 18.03.2015, 11:44 +0100 schrieb Kamil Debski: > Hi Philipp, > > > From: Jean-Michel Hautbois [mailto:jhautb...@gmail.com] > > Sent: Friday, February 27, 2015 10:00 AM > > To: Philipp Zabel > > Cc: Kamil Debski; Peter Seiderer; Linux Media Mailing List; Sascha > > Hauer > > Subject: Re: [PATCH 10/12] [media] coda: fail to start streaming if > > userspace set invalid formats > > > > Hi Philipp, > > > > 2015-02-23 16:20 GMT+01:00 Philipp Zabel : > > Could you add a description to this patch? I think I'll have to solve this differently. > Also, please remember about this patch > https://patchwork.linuxtv.org/patch/28016. > It also needs a description. Thank you for the reminder. regards Philipp -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 10/12] [media] coda: fail to start streaming if userspace set invalid formats
Am Freitag, den 27.02.2015, 09:59 +0100 schrieb Jean-Michel Hautbois:
> Hi Philipp,
>
> 2015-02-23 16:20 GMT+01:00 Philipp Zabel :
> > Signed-off-by: Philipp Zabel
> > ---
> > drivers/media/platform/coda/coda-common.c | 13 -
> > 1 file changed, 12 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/platform/coda/coda-common.c
> > b/drivers/media/platform/coda/coda-common.c
> > index b42ccfc..4441179 100644
> > --- a/drivers/media/platform/coda/coda-common.c
> > +++ b/drivers/media/platform/coda/coda-common.c
> > @@ -1282,12 +1282,23 @@ static int coda_start_streaming(struct vb2_queue
> > *q, unsigned int count)
> > if (!(ctx->streamon_out & ctx->streamon_cap))
> > return 0;
> >
> > + q_data_dst = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE);
> > + if ((q_data_src->width != q_data_dst->width &&
> > +round_up(q_data_src->width, 16) != q_data_dst->width) ||
> > + (q_data_src->height != q_data_dst->height &&
> > +round_up(q_data_src->height, 16) != q_data_dst->height)) {
> > + v4l2_err(v4l2_dev, "can't convert %dx%d to %dx%d\n",
> > +q_data_src->width, q_data_src->height,
> > +q_data_dst->width, q_data_dst->height);
> > + ret = -EINVAL;
> > + goto err;
> > + }
> > +
>
> Shouldn't the driver check on queues related to encoding or decoding only ?
> We don't need to set correct width/height from userspace if we are
> encoding, or it should be done by s_fmt itself.
Good point, the notes from the V4L2 codec API session during ELCE2014
say:
"the coded format is always the master; changing format on it
changes the set of supported formats on the other queue;"
Since an unsupported format shouldn't be selected, this implies that
S_FMT on an encoder capture queue should possibly change the format on
the output queue at the same time.
If I enforce compatible output and capture formats during S_FMT,
this check is indeed not needed.
regards
Philipp
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 10/12] [media] coda: fail to start streaming if userspace set invalid formats
Hi Philipp,
> From: Jean-Michel Hautbois [mailto:jhautb...@gmail.com]
> Sent: Friday, February 27, 2015 10:00 AM
> To: Philipp Zabel
> Cc: Kamil Debski; Peter Seiderer; Linux Media Mailing List; Sascha
> Hauer
> Subject: Re: [PATCH 10/12] [media] coda: fail to start streaming if
> userspace set invalid formats
>
> Hi Philipp,
>
> 2015-02-23 16:20 GMT+01:00 Philipp Zabel :
Could you add a description to this patch?
Also, please remember about this patch
https://patchwork.linuxtv.org/patch/28016.
It also needs a description.
Best wishes,
--
Kamil Debski
Samsung R&D Institute Poland
> > Signed-off-by: Philipp Zabel
> > ---
> > drivers/media/platform/coda/coda-common.c | 13 -
> > 1 file changed, 12 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/platform/coda/coda-common.c
> > b/drivers/media/platform/coda/coda-common.c
> > index b42ccfc..4441179 100644
> > --- a/drivers/media/platform/coda/coda-common.c
> > +++ b/drivers/media/platform/coda/coda-common.c
> > @@ -1282,12 +1282,23 @@ static int coda_start_streaming(struct
> vb2_queue *q, unsigned int count)
> > if (!(ctx->streamon_out & ctx->streamon_cap))
> > return 0;
> >
> > + q_data_dst = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE);
> > + if ((q_data_src->width != q_data_dst->width &&
> > +round_up(q_data_src->width, 16) != q_data_dst->width) ||
> > + (q_data_src->height != q_data_dst->height &&
> > +round_up(q_data_src->height, 16) != q_data_dst->height))
> {
> > + v4l2_err(v4l2_dev, "can't convert %dx%d to %dx%d\n",
> > +q_data_src->width, q_data_src->height,
> > +q_data_dst->width, q_data_dst->height);
> > + ret = -EINVAL;
> > + goto err;
> > + }
> > +
>
> Shouldn't the driver check on queues related to encoding or decoding
> only ?
> We don't need to set correct width/height from userspace if we are
> encoding, or it should be done by s_fmt itself.
>
> Thanks,
> JM
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 10/12] [media] coda: fail to start streaming if userspace set invalid formats
Hi Philipp,
2015-02-23 16:20 GMT+01:00 Philipp Zabel :
> Signed-off-by: Philipp Zabel
> ---
> drivers/media/platform/coda/coda-common.c | 13 -
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/coda/coda-common.c
> b/drivers/media/platform/coda/coda-common.c
> index b42ccfc..4441179 100644
> --- a/drivers/media/platform/coda/coda-common.c
> +++ b/drivers/media/platform/coda/coda-common.c
> @@ -1282,12 +1282,23 @@ static int coda_start_streaming(struct vb2_queue *q,
> unsigned int count)
> if (!(ctx->streamon_out & ctx->streamon_cap))
> return 0;
>
> + q_data_dst = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE);
> + if ((q_data_src->width != q_data_dst->width &&
> +round_up(q_data_src->width, 16) != q_data_dst->width) ||
> + (q_data_src->height != q_data_dst->height &&
> +round_up(q_data_src->height, 16) != q_data_dst->height)) {
> + v4l2_err(v4l2_dev, "can't convert %dx%d to %dx%d\n",
> +q_data_src->width, q_data_src->height,
> +q_data_dst->width, q_data_dst->height);
> + ret = -EINVAL;
> + goto err;
> + }
> +
Shouldn't the driver check on queues related to encoding or decoding only ?
We don't need to set correct width/height from userspace if we are
encoding, or it should be done by s_fmt itself.
Thanks,
JM
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
