RE: [PATCH v2 1/2] v4l: Add resolution change event.

[Kamil Debski <k.debski@samsung.com>]

Hi Pawel,

> From: Pawel Osciak [mailto:posciak@chromium.org]
> Sent: Tuesday, April 22, 2014 9:46 AM
> To: Kamil Debski
> Cc: Arun Kumar K; linux-media@vger.kernel.org; linux-samsung-soc;
> Sylwester Nawrocki; Hans Verkuil; Laurent Pinchart
> Subject: Re: [PATCH v2 1/2] v4l: Add resolution change event.
> 
> Hi Kamil,
> 
> On Tue, Apr 22, 2014 at 4:34 PM, Kamil Debski <k.debski@samsung.com>
> wrote:
> 
> 
> 	Hi Pawel,
> 
> 	> From: Pawel Osciak [mailto:posciak@chromium.org]
> 	> Sent: Monday, April 21, 2014 12:27 PM
> 	> To: Arun Kumar K
> 	> Cc: linux-media@vger.kernel.org; linux-samsung-soc; Kamil
> Debski;
> 	> Sylwester Nawrocki; Hans Verkuil; Laurent Pinchart
> 	> Subject: Re: [PATCH v2 1/2] v4l: Add resolution change event.
> 
> 	>
> 	> As a side note, because this is not really codified in the API,
> I would
> 	> like this event to indicate not only resolution change mid-
> stream, but
> 	> also detection of initial resolution, which should be a subset
> of
> 	> resolution change. I think this would make sense for the codec
> 	> interface:
> 	>
> 	> Video decode:
> 	> 1. S_FMT to given codec on OUTPUT queue.
> 	> 2. REQBUFS(n) and STREAMON on OUTPUT queue.
> 	> 3. Keep QBUFing until we get an resolution change event on the
> CAPTURE
> 	> queue; until then, the driver/codec HW will operate on the
> OUTPUT queue
> 	> only and try to detect relevant headers in the OUTPUT buffers,
> and will
> 	> send resolution change event once it finds resolution, profile,
> etc.
> 	> info). DQEVENT.
> 	> 4. G_FMT on CAPTURE to get the discovered output format
> (resolution),
> 	> REQBUFS and STREAMON on the CAPTURE queue.
> 	> 5. Normal mem-to-mem decoding.
> 	> 6. If a resolution change event arrives on CAPTURE queue,
> DQEVENT,
> 	> STREAMOFF, REQBUFS(0) only on CAPTURE queue, and goto 4. OUTPUT
> queue
> 	> operates completely independently of this.
> 	>
> 	> Also, this event should invariably indicate all of the below:
> 	> - all output buffers from before resolution change are already
> ready on
> 	> the CAPTURE queue to DQBUF (so it's ready to REQBUFS(0) after
> DQBUFs),
> 	> and
> 	> - there will be no more new ready buffers on the CAPTURE queue
> until
> 	> the streamoff-reqbufs(0)-g_fmt-reqbufs()-streamon is performed,
> and
> 	> - OUTPUT queue is completely independent of all of the above
> and can be
> 	> still used as normal, i.e. stream buffers can still keep being
> queued
> 	> at any stage of the resolution change and they will be decoded
> after
> 	> resolution change sequence is finished;
> 	>
> 	> If we all agree to the above, I will prepare a subsequent patch
> for the
> 	> documentation to include the above.
> 
> 
> 	If I understand correctly this will keep the old application
> working.
> 	By this I mean application that do not use events and rely on the
> current
> 	mechanism to detect initial header parsing and resolution change.
> 
> 	If backward compatibility is kept I am all for the changes
> proposed by you.
> 
> 
> 
> MFC codec depends on the userspace to parse the stream and pass the
> stream header with resolution info before calling G_FMT. So if it
> ignores the events but keeps doing this, things should keep working I
> think, as the G_FMT should still work as before.
> 
> This event will help userspace that doesn't want to bother itself with
> parsing the stream to know if the right header was queued and rely on
> events to know when to call G_FMT instead.
> 
> What do you think? Am I missing something?

Do you think this will work with all MFC versions? I guess that your focus
is on the newer MFC version such as v6 or even v7. I had worked mostly with
the v5 version and I am not sure how well it will handle the situation when
it does not receive the header as the first frame. Do you have a target with
v5 of MFC at hand?

I really like your idea. If you haven't got the possibility to check how it
works on v5 then I suggest you go ahead with patches and later I will do some
testing on MFC v5.

Best wishes,
-- 
Kamil Debski
Samsung R&D Institute Poland