[libcamera-devel,5/5] Documentation: guides: pipeline-handler: Fix capture flag usage
diff mbox series

Message ID 20210317192831.359014-6-nfraprado@collabora.com
State Accepted
Headers show
Series
  • Minor improvements to the documentation
Related show

Commit Message

Nícolas F. R. A. Prado March 17, 2021, 7:28 p.m. UTC
The number of frames passed to the -C flag of cam should come right
after it, without a space, otherwise the value is discarded.
The log below already showed the correct usage, but the command in the
code-block was wrong, so fix it.

Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
---
 Documentation/guides/pipeline-handler.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Sebastian Fricke March 18, 2021, 5:21 a.m. UTC | #1
Hey Nícolas,

Thank you for the patch.

Reviewed-by: Sebastian Fricke <sebastian.fricke@posteo.net>

On 17.03.2021 16:28, Nícolas F. R. A. Prado wrote:
>The number of frames passed to the -C flag of cam should come right
>after it, without a space, otherwise the value is discarded.
>The log below already showed the correct usage, but the command in the
>code-block was wrong, so fix it.
>
>Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
>---
> Documentation/guides/pipeline-handler.rst | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst
>index bcce86793ccd..8ef83fea9837 100644
>--- a/Documentation/guides/pipeline-handler.rst
>+++ b/Documentation/guides/pipeline-handler.rst
>@@ -1433,7 +1433,7 @@ capture through the pipeline through both of the cam and qcam utilities.
> .. code-block:: shell
>
>    ninja -C build
>-   ./build/src/cam/cam -c vivid -C 5
>+   ./build/src/cam/cam -c vivid -C5

I think this could be even more descriptive by writing:
```
./build/src/cam/cam --camera=vivid --capture=5
```
But that is probably just my own preference.

>
> To test that the pipeline handler can detect a device, and capture input.
>
>-- 
>2.30.2
>
>_______________________________________________
>libcamera-devel mailing list
>libcamera-devel@lists.libcamera.org
>https://lists.libcamera.org/listinfo/libcamera-devel
Laurent Pinchart March 19, 2021, 12:02 a.m. UTC | #2
Hi Nicolas,

Thank you for the patch.

On Thu, Mar 18, 2021 at 06:21:20AM +0100, Sebastian Fricke wrote:
> Hey Nícolas,
> 
> Thank you for the patch.
> 
> Reviewed-by: Sebastian Fricke <sebastian.fricke@posteo.net>
> 
> On 17.03.2021 16:28, Nícolas F. R. A. Prado wrote:
> > The number of frames passed to the -C flag of cam should come right
> > after it, without a space, otherwise the value is discarded.
> > The log below already showed the correct usage, but the command in the
> > code-block was wrong, so fix it.
> >
> > Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> > ---
> >  Documentation/guides/pipeline-handler.rst | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst
> > index bcce86793ccd..8ef83fea9837 100644
> > --- a/Documentation/guides/pipeline-handler.rst
> > +++ b/Documentation/guides/pipeline-handler.rst
> > @@ -1433,7 +1433,7 @@ capture through the pipeline through both of the cam and qcam utilities.
> >  .. code-block:: shell
> > 
> >     ninja -C build
> > -   ./build/src/cam/cam -c vivid -C 5
> > +   ./build/src/cam/cam -c vivid -C5
> 
> I think this could be even more descriptive by writing:
> ```
> ./build/src/cam/cam --camera=vivid --capture=5
> ```
> But that is probably just my own preference.

That's nice too. I don't have a strong preference. I'll push patch 1/5
to 4/5 already, and let you (Nicolas) decide what option you prefer for
this.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> >
> >  To test that the pipeline handler can detect a device, and capture input.
> >
Nícolas F. R. A. Prado March 19, 2021, 12:19 p.m. UTC | #3
Em 2021-03-18 21:02, Laurent Pinchart escreveu:

> Hi Nicolas,
>
> Thank you for the patch.
>
> On Thu, Mar 18, 2021 at 06:21:20AM +0100, Sebastian Fricke wrote:
> > Hey Nícolas,
> > 
> > Thank you for the patch.
> > 
> > Reviewed-by: Sebastian Fricke <sebastian.fricke@posteo.net>
> > 
> > On 17.03.2021 16:28, Nícolas F. R. A. Prado wrote:
> > > The number of frames passed to the -C flag of cam should come right
> > > after it, without a space, otherwise the value is discarded.
> > > The log below already showed the correct usage, but the command in the
> > > code-block was wrong, so fix it.
> > >
> > > Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> > > ---
> > >  Documentation/guides/pipeline-handler.rst | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst
> > > index bcce86793ccd..8ef83fea9837 100644
> > > --- a/Documentation/guides/pipeline-handler.rst
> > > +++ b/Documentation/guides/pipeline-handler.rst
> > > @@ -1433,7 +1433,7 @@ capture through the pipeline through both of the cam and qcam utilities.
> > >  .. code-block:: shell
> > > 
> > >     ninja -C build
> > > -   ./build/src/cam/cam -c vivid -C 5
> > > +   ./build/src/cam/cam -c vivid -C5
> > 
> > I think this could be even more descriptive by writing:
> > ```
> > ./build/src/cam/cam --camera=vivid --capture=5
> > ```
> > But that is probably just my own preference.
>
> That's nice too. I don't have a strong preference. I'll push patch 1/5
> to 4/5 already, and let you (Nicolas) decide what option you prefer for
> this.

I agree with Sebastian that using `--capture=` is more descriptive, but that
usage is obvious from the `--help`, while the lack of space after `-C` is not.
So I'd keep the `-C` usage (moreover, people are lazy and will use the shorter
forms, so it should be clear how to do so :)).

Thanks,
Nícolas

>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> > >
> > >  To test that the pipeline handler can detect a device, and capture input.
> > >
>
> --
> Regards,
>
> Laurent Pinchart

Patch
diff mbox series

diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst
index bcce86793ccd..8ef83fea9837 100644
--- a/Documentation/guides/pipeline-handler.rst
+++ b/Documentation/guides/pipeline-handler.rst
@@ -1433,7 +1433,7 @@  capture through the pipeline through both of the cam and qcam utilities.
 .. code-block:: shell
 
    ninja -C build
-   ./build/src/cam/cam -c vivid -C 5
+   ./build/src/cam/cam -c vivid -C5
 
 To test that the pipeline handler can detect a device, and capture input.