| Message ID | 20200819180442.11630-10-kgupta@es.iitr.ac.in (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: vimc: Multiple stream support in vimc | expand |
Hi Kaaira, On 19/08/2020 19:04, Kaaira Gupta wrote: > If multiple captures try to enable stream, start their stream but do not > initialise the pipeline again. Also, don't start the thread separately. > Starting their streams will update the use count and their frames would > be processed by the already running thread. > > Signed-off-by: Kaaira Gupta <kgupta@es.iitr.ac.in> > --- > drivers/media/test-drivers/vimc/vimc-streamer.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/test-drivers/vimc/vimc-streamer.c b/drivers/media/test-drivers/vimc/vimc-streamer.c > index fade37bee26d..880c31759cc0 100644 > --- a/drivers/media/test-drivers/vimc/vimc-streamer.c > +++ b/drivers/media/test-drivers/vimc/vimc-streamer.c > @@ -275,13 +275,14 @@ int vimc_streamer_s_stream(struct vimc_stream *stream, > return ret; > > if (enable) { > - if (stream->kthread) > - return 0; > > ret = vimc_streamer_stream_start(ved); > if (ret) > goto out; > > + if (stream->kthread) > + goto out; > + This goto out makes it look like it's an error path. So that probably warrants a comment along the lines of 'don't reinitialise the pipeline if it has already started'. ? I wonder if it's better to handle the pipeline_init during _stream_start 'only' in the code path where it's the first stream ? Then similarly, the pipeline_terminate would happen on stream_stop 'only' when it's the last stream. Or I guess that is handled by the refcount ... Hrm it would be nice to be able to make/keep it symmetrical somehow. > ret = vimc_streamer_pipeline_init(stream, ved); > if (ret) > goto out; >
On Wed, Sep 02, 2020 at 11:46:50AM +0100, Kieran Bingham wrote: > Hi Kaaira, > > On 19/08/2020 19:04, Kaaira Gupta wrote: > > If multiple captures try to enable stream, start their stream but do not > > initialise the pipeline again. Also, don't start the thread separately. > > Starting their streams will update the use count and their frames would > > be processed by the already running thread. > > > > Signed-off-by: Kaaira Gupta <kgupta@es.iitr.ac.in> > > --- > > drivers/media/test-drivers/vimc/vimc-streamer.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/media/test-drivers/vimc/vimc-streamer.c b/drivers/media/test-drivers/vimc/vimc-streamer.c > > index fade37bee26d..880c31759cc0 100644 > > --- a/drivers/media/test-drivers/vimc/vimc-streamer.c > > +++ b/drivers/media/test-drivers/vimc/vimc-streamer.c > > @@ -275,13 +275,14 @@ int vimc_streamer_s_stream(struct vimc_stream *stream, > > return ret; > > > > if (enable) { > > - if (stream->kthread) > > - return 0; > > > > ret = vimc_streamer_stream_start(ved); > > if (ret) > > goto out; > > > > + if (stream->kthread) > > + goto out; > > + > > This goto out makes it look like it's an error path. So that probably > warrants a comment along the lines of 'don't reinitialise the pipeline > if it has already started'. ? > > I wonder if it's better to handle the pipeline_init during _stream_start > 'only' in the code path where it's the first stream ? stream_start needs to be called for both (or all) the captures, while init must be called just once...so I think keeping it here inside s_stream and calling it just once will be okay too as that wouldn't need refcounts..but yes I think I should keep them symmetrical for better readability of code. What do you think is a better method? > > Then similarly, the pipeline_terminate would happen on stream_stop > 'only' when it's the last stream. > > Or I guess that is handled by the refcount ... Hrm it would be nice to > be able to make/keep it symmetrical somehow. > > > > ret = vimc_streamer_pipeline_init(stream, ved); > > if (ret) > > goto out; > > > > -- > Regards > -- > Kieran
diff --git a/drivers/media/test-drivers/vimc/vimc-streamer.c b/drivers/media/test-drivers/vimc/vimc-streamer.c index fade37bee26d..880c31759cc0 100644 --- a/drivers/media/test-drivers/vimc/vimc-streamer.c +++ b/drivers/media/test-drivers/vimc/vimc-streamer.c @@ -275,13 +275,14 @@ int vimc_streamer_s_stream(struct vimc_stream *stream, return ret; if (enable) { - if (stream->kthread) - return 0; ret = vimc_streamer_stream_start(ved); if (ret) goto out; + if (stream->kthread) + goto out; + ret = vimc_streamer_pipeline_init(stream, ved); if (ret) goto out;
If multiple captures try to enable stream, start their stream but do not initialise the pipeline again. Also, don't start the thread separately. Starting their streams will update the use count and their frames would be processed by the already running thread. Signed-off-by: Kaaira Gupta <kgupta@es.iitr.ac.in> --- drivers/media/test-drivers/vimc/vimc-streamer.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)