[libcamera-devel] Pull request from https://github.com/naushir/libcamera.git
mbox series

Message ID CAEmqJPoHqm2W-VY_jbT50waCve-Gk1Qtc6D+0Y9vdbErvxyFSw@mail.gmail.com
State Superseded
Headers show
Series
  • [libcamera-devel] Pull request from https://github.com/naushir/libcamera.git
Related show

Pull-request

https://github.com/naushir/libcamera.git

Message

Naushir Patuck March 7, 2023, 3:58 p.m. UTC
The following changes since commit cde9293cf9899b0fc4ce9cf89dd29f26be77f35c:

  ipa: rkisp1: lsc: Fix integer division error (2023-03-06 18:12:10 +0100)

are available in the Git repository at:

  https://github.com/naushir/libcamera.git

for you to fetch changes up to f194529f9ba35421b66fee5e5bfcb2b8a148bbbf:

  ipa: raspberrypi: imx296: Minor tuning updates (2023-03-07 15:43:08 +0000)

----------------------------------------------------------------
David Plowman (2):
      ipa: raspberrypi: agc: Fix overflow in Y value calculation
      ipa: raspberrypi: imx296: Minor tuning updates

Naushir Patuck (4):
      pipeline: ipa: raspberrypi: Change Unicam timeout handling
      ipa: raspberrypi: Better heuristics for calculating Unicam timeout
      pipeline: raspberrypi: Add a Unicam timeout override config options
      pipeline: raspberrypi: Iterate over all Unicam instances in match()

 include/libcamera/ipa/raspberrypi.mojom              |   2 +-
 src/ipa/raspberrypi/controller/rpi/agc.cpp           |   2 +-
 src/ipa/raspberrypi/data/imx296.json                 | 180
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------------------------
 src/ipa/raspberrypi/data/imx296_mono.json            |  12 ++++++++--
 src/ipa/raspberrypi/raspberrypi.cpp                  |  45
+++++++++++++++++++++++++++++++++++---
 src/libcamera/pipeline/raspberrypi/data/example.yaml |  11 +++++++++-
 src/libcamera/pipeline/raspberrypi/raspberrypi.cpp   | 105
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----------------------------
 7 files changed, 282 insertions(+), 75 deletions(-)

Comments

Laurent Pinchart March 7, 2023, 5:10 p.m. UTC | #1
Hi Naush,

Thanks for the pull request.

In the future, could you try to follow the kernel convention for the
subject line ? It should start with a [GIT PULL] tag, and then have a
short one-line description of the pull request. This one could be, for
instance

[GIT PULL] Raspberry Pi timeout handling improvements & other misc changes

On Tue, Mar 07, 2023 at 03:58:20PM +0000, Naushir Patuck via libcamera-devel wrote:
> The following changes since commit cde9293cf9899b0fc4ce9cf89dd29f26be77f35c:
> 
>   ipa: rkisp1: lsc: Fix integer division error (2023-03-06 18:12:10 +0100)
> 
> are available in the Git repository at:
> 
>   https://github.com/naushir/libcamera.git
> 
> for you to fetch changes up to f194529f9ba35421b66fee5e5bfcb2b8a148bbbf:
> 
>   ipa: raspberrypi: imx296: Minor tuning updates (2023-03-07 15:43:08 +0000)
> 
> ----------------------------------------------------------------
> David Plowman (2):
>       ipa: raspberrypi: agc: Fix overflow in Y value calculation
>       ipa: raspberrypi: imx296: Minor tuning updates
> 
> Naushir Patuck (4):
>       pipeline: ipa: raspberrypi: Change Unicam timeout handling
>       ipa: raspberrypi: Better heuristics for calculating Unicam timeout
>       pipeline: raspberrypi: Add a Unicam timeout override config options
>       pipeline: raspberrypi: Iterate over all Unicam instances in match()
> 
>  include/libcamera/ipa/raspberrypi.mojom              |   2 +-
>  src/ipa/raspberrypi/controller/rpi/agc.cpp           |   2 +-
>  src/ipa/raspberrypi/data/imx296.json                 | 180 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------------------------
>  src/ipa/raspberrypi/data/imx296_mono.json            |  12 ++++++++--
>  src/ipa/raspberrypi/raspberrypi.cpp                  |  45 +++++++++++++++++++++++++++++++++++---
>  src/libcamera/pipeline/raspberrypi/data/example.yaml |  11 +++++++++-
>  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp   | 105 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----------------------------
>  7 files changed, 282 insertions(+), 75 deletions(-)
Kieran Bingham March 7, 2023, 10:24 p.m. UTC | #2
Hi Naush,

Quoting Laurent Pinchart (2023-03-07 17:10:22)
> Hi Naush,
> 
> Thanks for the pull request.
> 
> In the future, could you try to follow the kernel convention for the
> subject line ? It should start with a [GIT PULL] tag, and then have a
> short one-line description of the pull request. This one could be, for
> instance
> 
> [GIT PULL] Raspberry Pi timeout handling improvements & other misc changes

Thank you for trying this as new way to handle your integrations!

Running through the integration scripts on my side failed:

Missing committer Signed-off-by in commit f194529f9ba35421b66fee5e5bfcb2b8a148bbbf
Missing committer Signed-off-by in commit 95e7b5415aadccf1a8d706c3460e114553a49e06
Found 2 errors in refs/heads/master, not pushing

Which was due to our pre-merge validation scripts noting that the
patches you cherry-picked from David were not Signed-off by the
'committer' (now you).

To be able to merge these, I've run git rebase --signoff, which has
recommitted these with my Sign-off. I figured I could have asked you to
resubmit a new pull request with your SoB - but it didn't seem worth the
extra cycle at this time.

With that, they passed my integration tests, and are now merged.

Thankyou!
--
Kieran


> 
> On Tue, Mar 07, 2023 at 03:58:20PM +0000, Naushir Patuck via libcamera-devel wrote:
> > The following changes since commit cde9293cf9899b0fc4ce9cf89dd29f26be77f35c:
> > 
> >   ipa: rkisp1: lsc: Fix integer division error (2023-03-06 18:12:10 +0100)
> > 
> > are available in the Git repository at:
> > 
> >   https://github.com/naushir/libcamera.git
> > 
> > for you to fetch changes up to f194529f9ba35421b66fee5e5bfcb2b8a148bbbf:
> > 
> >   ipa: raspberrypi: imx296: Minor tuning updates (2023-03-07 15:43:08 +0000)
> > 
> > ----------------------------------------------------------------
> > David Plowman (2):
> >       ipa: raspberrypi: agc: Fix overflow in Y value calculation
> >       ipa: raspberrypi: imx296: Minor tuning updates
> > 
> > Naushir Patuck (4):
> >       pipeline: ipa: raspberrypi: Change Unicam timeout handling
> >       ipa: raspberrypi: Better heuristics for calculating Unicam timeout
> >       pipeline: raspberrypi: Add a Unicam timeout override config options
> >       pipeline: raspberrypi: Iterate over all Unicam instances in match()
> > 
> >  include/libcamera/ipa/raspberrypi.mojom              |   2 +-
> >  src/ipa/raspberrypi/controller/rpi/agc.cpp           |   2 +-
> >  src/ipa/raspberrypi/data/imx296.json                 | 180 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------------------------
> >  src/ipa/raspberrypi/data/imx296_mono.json            |  12 ++++++++--
> >  src/ipa/raspberrypi/raspberrypi.cpp                  |  45 +++++++++++++++++++++++++++++++++++---
> >  src/libcamera/pipeline/raspberrypi/data/example.yaml |  11 +++++++++-
> >  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp   | 105 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----------------------------
> >  7 files changed, 282 insertions(+), 75 deletions(-)
> 
> -- 
> Regards,
> 
> Laurent Pinchart
Laurent Pinchart March 8, 2023, 8:24 a.m. UTC | #3
On Tue, Mar 07, 2023 at 10:24:28PM +0000, Kieran Bingham wrote:
> Hi Naush,
> 
> Quoting Laurent Pinchart (2023-03-07 17:10:22)
> > Hi Naush,
> > 
> > Thanks for the pull request.
> > 
> > In the future, could you try to follow the kernel convention for the
> > subject line ? It should start with a [GIT PULL] tag, and then have a
> > short one-line description of the pull request. This one could be, for
> > instance
> > 
> > [GIT PULL] Raspberry Pi timeout handling improvements & other misc changes
> 
> Thank you for trying this as new way to handle your integrations!
> 
> Running through the integration scripts on my side failed:
> 
> Missing committer Signed-off-by in commit f194529f9ba35421b66fee5e5bfcb2b8a148bbbf
> Missing committer Signed-off-by in commit 95e7b5415aadccf1a8d706c3460e114553a49e06
> Found 2 errors in refs/heads/master, not pushing
> 
> Which was due to our pre-merge validation scripts noting that the
> patches you cherry-picked from David were not Signed-off by the
> 'committer' (now you).
> 
> To be able to merge these, I've run git rebase --signoff, which has
> recommitted these with my Sign-off. I figured I could have asked you to
> resubmit a new pull request with your SoB - but it didn't seem worth the
> extra cycle at this time.

Just to make it clear, Naush, could you make sure, in future pull
request, that all commits that list you as the committer have your SoB ?
This includes all commits that you cherry-pick, apply from an mbox file,
or rebase. You can see the committer with `git log --pretty=fuller`.

You can also use the pre-push hook from utils/hooks/ (just copy it to
.git/hooks/) to perform validity checks when pushing a branch. By
default the script only checks branches named `master` or
`integration/*`, but it's easy to modify it to accommodate your
workflow.

> With that, they passed my integration tests, and are now merged.
> 
> > On Tue, Mar 07, 2023 at 03:58:20PM +0000, Naushir Patuck via libcamera-devel wrote:
> > > The following changes since commit cde9293cf9899b0fc4ce9cf89dd29f26be77f35c:
> > > 
> > >   ipa: rkisp1: lsc: Fix integer division error (2023-03-06 18:12:10 +0100)
> > > 
> > > are available in the Git repository at:
> > > 
> > >   https://github.com/naushir/libcamera.git
> > > 
> > > for you to fetch changes up to f194529f9ba35421b66fee5e5bfcb2b8a148bbbf:
> > > 
> > >   ipa: raspberrypi: imx296: Minor tuning updates (2023-03-07 15:43:08 +0000)
> > > 
> > > ----------------------------------------------------------------
> > > David Plowman (2):
> > >       ipa: raspberrypi: agc: Fix overflow in Y value calculation
> > >       ipa: raspberrypi: imx296: Minor tuning updates
> > > 
> > > Naushir Patuck (4):
> > >       pipeline: ipa: raspberrypi: Change Unicam timeout handling
> > >       ipa: raspberrypi: Better heuristics for calculating Unicam timeout
> > >       pipeline: raspberrypi: Add a Unicam timeout override config options
> > >       pipeline: raspberrypi: Iterate over all Unicam instances in match()
> > > 
> > >  include/libcamera/ipa/raspberrypi.mojom              |   2 +-
> > >  src/ipa/raspberrypi/controller/rpi/agc.cpp           |   2 +-
> > >  src/ipa/raspberrypi/data/imx296.json                 | 180 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------------------------
> > >  src/ipa/raspberrypi/data/imx296_mono.json            |  12 ++++++++--
> > >  src/ipa/raspberrypi/raspberrypi.cpp                  |  45 +++++++++++++++++++++++++++++++++++---
> > >  src/libcamera/pipeline/raspberrypi/data/example.yaml |  11 +++++++++-
> > >  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp   | 105 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----------------------------
> > >  7 files changed, 282 insertions(+), 75 deletions(-)
Naushir Patuck March 8, 2023, 8:37 a.m. UTC | #4
Hi all,

On Wed, 8 Mar 2023 at 08:24, Laurent Pinchart <
laurent.pinchart@ideasonboard.com> wrote:

> On Tue, Mar 07, 2023 at 10:24:28PM +0000, Kieran Bingham wrote:
> > Hi Naush,
> >
> > Quoting Laurent Pinchart (2023-03-07 17:10:22)
> > > Hi Naush,
> > >
> > > Thanks for the pull request.
> > >
> > > In the future, could you try to follow the kernel convention for the
> > > subject line ? It should start with a [GIT PULL] tag, and then have a
> > > short one-line description of the pull request. This one could be, for
> > > instance
> > >
> > > [GIT PULL] Raspberry Pi timeout handling improvements & other misc
> changes
> >
> > Thank you for trying this as new way to handle your integrations!
> >
> > Running through the integration scripts on my side failed:
> >
> > Missing committer Signed-off-by in commit
> f194529f9ba35421b66fee5e5bfcb2b8a148bbbf
> > Missing committer Signed-off-by in commit
> 95e7b5415aadccf1a8d706c3460e114553a49e06
> > Found 2 errors in refs/heads/master, not pushing
> >
> > Which was due to our pre-merge validation scripts noting that the
> > patches you cherry-picked from David were not Signed-off by the
> > 'committer' (now you).
> >
> > To be able to merge these, I've run git rebase --signoff, which has
> > recommitted these with my Sign-off. I figured I could have asked you to
> > resubmit a new pull request with your SoB - but it didn't seem worth the
> > extra cycle at this time.
>
> Just to make it clear, Naush, could you make sure, in future pull
> request, that all commits that list you as the committer have your SoB ?
> This includes all commits that you cherry-pick, apply from an mbox file,
> or rebase. You can see the committer with `git log --pretty=fuller`.
>
> You can also use the pre-push hook from utils/hooks/ (just copy it to
> .git/hooks/) to perform validity checks when pushing a branch. By
> default the script only checks branches named `master` or
> `integration/*`, but it's easy to modify it to accommodate your
> workflow.
>

Sure, I'll make sure to do that going forward.  This git request-pull stuff
is
all new to me.


>
> > With that, they passed my integration tests, and are now merged.
>

Excellent, thanks!!

Naush


> >
> > > On Tue, Mar 07, 2023 at 03:58:20PM +0000, Naushir Patuck via
> libcamera-devel wrote:
> > > > The following changes since commit
> cde9293cf9899b0fc4ce9cf89dd29f26be77f35c:
> > > >
> > > >   ipa: rkisp1: lsc: Fix integer division error (2023-03-06 18:12:10
> +0100)
> > > >
> > > > are available in the Git repository at:
> > > >
> > > >   https://github.com/naushir/libcamera.git
> > > >
> > > > for you to fetch changes up to
> f194529f9ba35421b66fee5e5bfcb2b8a148bbbf:
> > > >
> > > >   ipa: raspberrypi: imx296: Minor tuning updates (2023-03-07
> 15:43:08 +0000)
> > > >
> > > > ----------------------------------------------------------------
> > > > David Plowman (2):
> > > >       ipa: raspberrypi: agc: Fix overflow in Y value calculation
> > > >       ipa: raspberrypi: imx296: Minor tuning updates
> > > >
> > > > Naushir Patuck (4):
> > > >       pipeline: ipa: raspberrypi: Change Unicam timeout handling
> > > >       ipa: raspberrypi: Better heuristics for calculating Unicam
> timeout
> > > >       pipeline: raspberrypi: Add a Unicam timeout override config
> options
> > > >       pipeline: raspberrypi: Iterate over all Unicam instances in
> match()
> > > >
> > > >  include/libcamera/ipa/raspberrypi.mojom              |   2 +-
> > > >  src/ipa/raspberrypi/controller/rpi/agc.cpp           |   2 +-
> > > >  src/ipa/raspberrypi/data/imx296.json                 | 180
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------------------------
> > > >  src/ipa/raspberrypi/data/imx296_mono.json            |  12
> ++++++++--
> > > >  src/ipa/raspberrypi/raspberrypi.cpp                  |  45
> +++++++++++++++++++++++++++++++++++---
> > > >  src/libcamera/pipeline/raspberrypi/data/example.yaml |  11
> +++++++++-
> > > >  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp   | 105
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----------------------------
> > > >  7 files changed, 282 insertions(+), 75 deletions(-)
>
> --
> Regards,
>
> Laurent Pinchart
>