[libcamera-devel,v1] pipeline: raspberrypi: Do not mark the Embedded Data stream as external

15264 diff mbox series

Remove the code that marks the Embedded Data stream as external with the Unicam
Image (RAW) stream. This was needed for legacy reasons when matching image and
embedded buffers, but is not needed any more.

Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
---
 src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 9 ---------
 1 file changed, 9 deletions(-)

Hi Naush

Thanks for sending this patch!

On Mon, 10 Jan 2022 at 15:42, Naushir Patuck <naush@raspberrypi.com> wrote:
>
> Remove the code that marks the Embedded Data stream as external with the Unicam
> Image (RAW) stream. This was needed for legacy reasons when matching image and
> embedded buffers, but is not needed any more.
>
> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>

Reviewed-by: David Plowman <david.plowman@raspberrypi.com>
Tested-by: David Plowman <david.plowman@raspberrypi.com>

Thanks!
David

> ---
>  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 9 ---------
>  1 file changed, 9 deletions(-)
>
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index 168bbcef819c..a0cd501c8250 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -896,15 +896,6 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)
>                                         << format.toString();
>                         return ret;
>                 }
> -
> -               /*
> -                * If a RAW/Bayer stream has been requested by the application,
> -                * we must set both Unicam streams as external, even though the
> -                * application may only request RAW frames. This is because we
> -                * match timestamps on both streams to synchronise buffers.
> -                */
> -               if (rawStream)
> -                       data->unicam_[Unicam::Embedded].setExternal(true);
>         }
>
>         /*
> --
> 2.25.1
>

Hi,

Gentle ping for feedback on this change please :)

Thanks,
Naush

On Mon, 10 Jan 2022 at 15:42, Naushir Patuck <naush@raspberrypi.com> wrote:

> Remove the code that marks the Embedded Data stream as external with the
> Unicam
> Image (RAW) stream. This was needed for legacy reasons when matching image
> and
> embedded buffers, but is not needed any more.
>
> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> ---
>  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 9 ---------
>  1 file changed, 9 deletions(-)
>
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index 168bbcef819c..a0cd501c8250 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -896,15 +896,6 @@ int PipelineHandlerRPi::configure(Camera *camera,
> CameraConfiguration *config)
>                                         << format.toString();
>                         return ret;
>                 }
> -
> -               /*
> -                * If a RAW/Bayer stream has been requested by the
> application,
> -                * we must set both Unicam streams as external, even
> though the
> -                * application may only request RAW frames. This is
> because we
> -                * match timestamps on both streams to synchronise buffers.
> -                */
> -               if (rawStream)
> -                       data->unicam_[Unicam::Embedded].setExternal(true);
>         }
>
>         /*
> --
> 2.25.1
>
>

Hi Naush,

Quoting Naushir Patuck (2022-01-10 15:42:26)
> Remove the code that marks the Embedded Data stream as external with the Unicam
> Image (RAW) stream. This was needed for legacy reasons when matching image and
> embedded buffers, but is not needed any more.
> 
> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> ---
>  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 9 ---------
>  1 file changed, 9 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index 168bbcef819c..a0cd501c8250 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -896,15 +896,6 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)
>                                         << format.toString();
>                         return ret;
>                 }
> -
> -               /*
> -                * If a RAW/Bayer stream has been requested by the application,
> -                * we must set both Unicam streams as external, even though the
> -                * application may only request RAW frames. This is because we
> -                * match timestamps on both streams to synchronise buffers.
> -                */
> -               if (rawStream)
> -                       data->unicam_[Unicam::Embedded].setExternal(true);

For such a small patch there's a lot of digging to get my head around
this, but I believe it's fine, and I trust David's testing so...

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

>         }
>  
>         /*
> -- 
> 2.25.1
>