Hi,
Thanks everybody for comments.
2015-06-22 17:54 GMT+03:00 Kamil Debski <kamil@xxxxxxxxx>:
> Hi,
>
> I am adding Jacek Anaszewski to CC loop. He was working with the
> s5p-jpeg driver some time ago.
> I've spoken with him about questions in this email recently. Jacek,
> thank you for your comments :)
>
> On 18 June 2015 at 20:48, Laurent Pinchart
> <laurent.pinchart@xxxxxxxxxxxxxxxx> wrote:
>> Hi Mikhail,
>>
>> (CC'ing Kamil Debski)
>>
>> On Wednesday 06 May 2015 01:03:10 Mikhail Ulianov wrote:
>>> On Mon, 04 May 2015 02:32:05 +0300 Laurent Pinchart wrote:
>
> [snip]
>
>>> [snip]
>>>
>>> >> +/*
>>> >> + * ====================================================================
>>> >> + * Queue operations
>>> >> + * ====================================================================
>>> >> + */
>>> >> +static int jpu_queue_setup(struct vb2_queue *vq,
>>> >> + const struct v4l2_format *fmt,
>>> >> + unsigned int *nbuffers, unsigned int
>>> >> *nplanes,
>>> >> + unsigned int sizes[], void
>>> >> *alloc_ctxs[])
>>> >> +{
>>> >> + struct jpu_ctx *ctx = vb2_get_drv_priv(vq);
>>> >> + struct jpu_q_data *q_data;
>>> >> + unsigned int count = *nbuffers;
>>> >> + unsigned int i;
>>> >> +
>>> >> + q_data = jpu_get_q_data(ctx, vq->type);
>>> >> +
>>> >> + *nplanes = q_data->format.num_planes;
>>> >> +
>>> >> + /*
>>> >> + * Header is parsed during decoding and parsed information
>>> >> stored
>>> >> + * in the context so we do not allow another buffer to
>>> >> overwrite it.
>>> >> + * For now it works this way, but planned for alternation.
>>> >
>>> > It shouldn't be difficult to create a jpu_buffer structure that
>>> > inherits from vb2_buffer and store the information there instead of
>>> > in the context.
>>>
>>> You are absolutely right. But for this version i want to keep it
>>> simple and also at this moment not everything clear for me with this
>>> format "autodetection" feature we want to have e.g. for decoder if user
>>> requested 2 output buffers and then queue first with some valid JPEG
>>> with format -1-(so we setup queues format here), after that
>>> another one with format -2-... should we discard second one or just
>>> change format of queues? what about same situation if user already
>>> requested capture buffers. I mean relations with buf_prepare and
>>> queue_setup. AFAIU format should remain the same for all requested
>>> buffers. I see only one "solid" solution here - get rid of
>>> "autodetection" feature and rely only on format setted by user, so in
>>> this case we can just discard queued buffers with inappropriate
>>> format(kind of format validation in kernel). This solution will also
>>> work well with NV61, NV21, and semiplanar formats we want to add in next
>>> version. *But* with this solution header parsing must be done twice(in
>>> user and kernel spaces).
>>> I'm a little bit frustrated here :)
>>
>> Yes, it's a bit frustrating indeed. I'm not sure what to advise, I'm not too
>> familiar with the m2m API for JPEG.
>>
>> Kamil, do you have a comment on that ?
>
> I am not sure whether it is good to get rid of header parsing by the
> driver/hardware option. I agree that the buffers should have a
> consistent format and size. Maybe the way to go would be to allow
> header parsing by the hardware, but to stop processing when the format
> has changed? Other solution would be to use the
> V4L2_EVENT_SOURCE_CHANGE event to inform user space about the change.
> User space then could check whether the buffers are sufficient or
> reallocate them. Similar to what happens in MFC when format changes.
>
> For me implementing resolution change in JPEG seems like an overkill,
> but maybe you have a use case that would benefit from this. Initially
> the JPEG decoder was designed and written as a one shot device. Could
> you give an example of such use case?
>
> The possible use case I can imagine is having an M-JPEG stream where
> all JPEGs have the same dimensions and format. There I can see some
> benefits from having more than one buffer on the queues. Then there
> would be no change in the buffer parameters, so this should work.
That's correct. It's exactly our use case, but we have few cameras. So
we serialize buffers in user space and sometimes one(or more) cameras
have different configuration. I think i should go with stop processing
option.
>>> [snip]
>
> [snip]
>
> Best wishes,
> Kamil Debski
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
