Re: [PATCH v2 0/3] media: vimc: Allow multiple capture devices to use the same sensor

From: Dafna Hirschfeld
Date: Tue Jul 28 2020 - 10:00:53 EST

On 28.07.20 14:07, Dafna Hirschfeld wrote:

On 28.07.20 13:39, Kaaira Gupta wrote:
On Mon, Jul 27, 2020 at 02:54:30PM -0300, Helen Koike wrote:

On 7/27/20 11:31 AM, Kieran Bingham wrote:
Hi all,

+Dafna for the thread discussion, as she's missed from the to/cc list.

On 24/07/2020 13:21, Kaaira Gupta wrote:
On Fri, Jul 24, 2020 at 02:15:21PM +0200, Niklas SÃderlund wrote:

Hi Kaaira,

Thanks for your work.

Thanks for yours :D

On 2020-07-24 17:32:10 +0530, Kaaira Gupta wrote:
This is version 2 of the patch series posted by Niklas for allowing
multiple streams in VIMC.
The original series can be found here:

This series adds support for two (or more) capture devices to be
connected to the same sensors and run simultaneously. Each capture device
can be started and stopped independent of each other.

Patch 1/3 and 2/3 deals with solving the issues that arises once two
capture devices can be part of the same pipeline. While 3/3 allows for
two capture devices to be part of the same pipeline and thus allows for
simultaneously use.

I wonder if these two patches are enough, since each vimc entity also have
a 'process_frame' callback, but only one allocated frame. That means
that the 'process_frame' can be called concurrently by two different streams
on the same frame and cause corruption.

I think we should somehow change the vimc-stream.c code so that we have only
one stream process per pipe. So if one capture is already streaming, then the new
capture that wants to stream uses the same thread so we don't have two threads
both calling 'process_frame'.

The second capture that wants to stream should iterate the topology downwards until
reaching an entity that already belong to the stream path of the other streaming capture
and tell the streamer it wants to read the frames this entity



I'm just curious if you are aware of this series? It would replace the
need for 1/3 and 2/3 of this series right?

v3 of this series replaces the need for 1/3, but not the current version
(ie v4). v4 of patch 2/5 removes the stream_counter that is needed to
keep count of the calls to s_stream. Hence 1/3 becomes relevant again.

So the question really is, how do we best make use of the two current
series, to achieve our goal of supporting multiple streams.

Having not parsed Dafna's series yet, do we need to combine elements of
both ? Or should we work towards starting with this series and get
dafna's patches built on top ?

Or should patch 1/3 and 3/3 of this series be on top of Dafna's v4 ?

(It might be noteworthy to say that Kaaira has reported successful
multiple stream operation from /this/ series and her development branch
on libcamera).

Dafna's patch seems still under discussion, but I don't want to block progress in Vimc either.

So I was wondering if we can move forward with Vimc support for multistreaming,
without considering Dafna's patchset, and we can do the clean up later once we solve that.

What do you think?

I agree with supporting multiple streams with VIMC with this patchset,
and then we can refactor the counters for s_stream in VIMC later (over
this series) if dafna includes them in subsequent version of her patchset.

I also think that adding support in the code will take much longer and should not
stop us from supporting vimc independently.




Changes since v1:
ÂÂÂÂ- All three patches rebased on latest media-tree.
ÂÂÂÂPatch 3:
ÂÂÂÂ- Search for an entity with a non-NULL pipe instead of searching
ÂÂÂÂÂ for sensor. This terminates the search at output itself.

Kaaira Gupta (3):
ÂÂ media: vimc: Add usage count to subdevices
ÂÂ media: vimc: Serialize vimc_streamer_s_stream()
ÂÂ media: vimc: Join pipeline if one already exists

 .../media/test-drivers/vimc/vimc-capture.c | 35 ++++++++++++++++++-
 .../media/test-drivers/vimc/vimc-debayer.c | 8 +++++
 drivers/media/test-drivers/vimc/vimc-scaler.c | 8 +++++
 drivers/media/test-drivers/vimc/vimc-sensor.c | 9 ++++-
 .../media/test-drivers/vimc/vimc-streamer.c | 23 +++++++-----
 5 files changed, 73 insertions(+), 10 deletions(-)


Niklas SÃderlund