| Message ID | 20201126145005.8838-1-david.plowman@raspberrypi.com |
|---|---|
| Headers | show |
| Series |
|
| Related | show |
Hi everyone I was wondering if I could give this one its customary Monday morning nudge? I notice that there was still some question last week over whether metadata (that's not allowed as a control) should be able to signal its maximum/minimum values. It doesn't really feel like an issue to me - I just report what the pipeline did, I'm not really seeing a need to report what the pipeline *might* have done! Other than that, it seems we were largely agreed...? Thanks again! David On Thu, 26 Nov 2020 at 14:50, David Plowman <david.plowman@raspberrypi.com> wrote: > > Hi everyone > > Here (after much talking ourselves in circles!) is a second version of > the digital gain control. I've taken Jacopo's text for the control > description, and added a second patch that makes use of the control in > the Raspberry Pi IPAs to add the digital gain to the metadata. > > This second patch doesn't really tell you very much. In our libcamera > version of raspistill (still image capture app) there will be a line > like this: > > iso *= metadata.get<float>(controls::DigitalGain); > > and that's about it! > > Best regards > David > > David Plowman (2): > libcamera: controls: Add DigitalGain control > src: ipa: raspberrypi: Add digital gain to libcamera metadata > > src/ipa/raspberrypi/raspberrypi.cpp | 4 +++- > src/libcamera/control_ids.yaml | 17 +++++++++++++++++ > 2 files changed, 20 insertions(+), 1 deletion(-) > > -- > 2.20.1 >
Hi David, On Mon, Nov 30, 2020 at 09:59:21AM +0000, David Plowman wrote: > Hi everyone > > I was wondering if I could give this one its customary Monday morning nudge? > > I notice that there was still some question last week over whether > metadata (that's not allowed as a control) should be able to signal > its maximum/minimum values. It doesn't really feel like an issue to me > - I just report what the pipeline did, I'm not really seeing a need to > report what the pipeline *might* have done! Other than that, it seems > we were largely agreed...? My question on the metadata min-max was mostly to plan ahead if we ever need to expose the metadata limits from the Camera API, something we currently don't have. Not something to block this patch on. I see the patch itself has my tags only. I would wait for an explicit ack from Laurent then merge it. Thanks j > > Thanks again! > David > > On Thu, 26 Nov 2020 at 14:50, David Plowman > <david.plowman@raspberrypi.com> wrote: > > > > Hi everyone > > > > Here (after much talking ourselves in circles!) is a second version of > > the digital gain control. I've taken Jacopo's text for the control > > description, and added a second patch that makes use of the control in > > the Raspberry Pi IPAs to add the digital gain to the metadata. > > > > This second patch doesn't really tell you very much. In our libcamera > > version of raspistill (still image capture app) there will be a line > > like this: > > > > iso *= metadata.get<float>(controls::DigitalGain); > > > > and that's about it! > > > > Best regards > > David > > > > David Plowman (2): > > libcamera: controls: Add DigitalGain control > > src: ipa: raspberrypi: Add digital gain to libcamera metadata > > > > src/ipa/raspberrypi/raspberrypi.cpp | 4 +++- > > src/libcamera/control_ids.yaml | 17 +++++++++++++++++ > > 2 files changed, 20 insertions(+), 1 deletion(-) > > > > -- > > 2.20.1 > > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi everyone I think you can guess what I might be about to say.... :) So this is just my little weekly nudge to see where we are with this. We were having some discussions about the pipeline model as a whole, as I recall, though I don't think we were wanting to block these patches. But please let me know what you think! Thanks and sorry for being a nuisance... David On Mon, 30 Nov 2020 at 10:30, Jacopo Mondi <jacopo@jmondi.org> wrote: > > Hi David, > > On Mon, Nov 30, 2020 at 09:59:21AM +0000, David Plowman wrote: > > Hi everyone > > > > I was wondering if I could give this one its customary Monday morning nudge? > > > > I notice that there was still some question last week over whether > > metadata (that's not allowed as a control) should be able to signal > > its maximum/minimum values. It doesn't really feel like an issue to me > > - I just report what the pipeline did, I'm not really seeing a need to > > report what the pipeline *might* have done! Other than that, it seems > > we were largely agreed...? > > My question on the metadata min-max was mostly to plan ahead if we > ever need to expose the metadata limits from the Camera API, something > we currently don't have. > > Not something to block this patch on. > > I see the patch itself has my tags only. I would wait for an explicit > ack from Laurent then merge it. > > Thanks > j > > > > > Thanks again! > > David > > > > On Thu, 26 Nov 2020 at 14:50, David Plowman > > <david.plowman@raspberrypi.com> wrote: > > > > > > Hi everyone > > > > > > Here (after much talking ourselves in circles!) is a second version of > > > the digital gain control. I've taken Jacopo's text for the control > > > description, and added a second patch that makes use of the control in > > > the Raspberry Pi IPAs to add the digital gain to the metadata. > > > > > > This second patch doesn't really tell you very much. In our libcamera > > > version of raspistill (still image capture app) there will be a line > > > like this: > > > > > > iso *= metadata.get<float>(controls::DigitalGain); > > > > > > and that's about it! > > > > > > Best regards > > > David > > > > > > David Plowman (2): > > > libcamera: controls: Add DigitalGain control > > > src: ipa: raspberrypi: Add digital gain to libcamera metadata > > > > > > src/ipa/raspberrypi/raspberrypi.cpp | 4 +++- > > > src/libcamera/control_ids.yaml | 17 +++++++++++++++++ > > > 2 files changed, 20 insertions(+), 1 deletion(-) > > > > > > -- > > > 2.20.1 > > > > > _______________________________________________ > > libcamera-devel mailing list > > libcamera-devel@lists.libcamera.org > > https://lists.libcamera.org/listinfo/libcamera-devel
Hi everyone Thanks for all the reviews and merging that happened towards the end of last week. I think we're getting really close now! So I wondered if I might be able to give this little patch a final nudge over the line...? Thanks and best regards David On Mon, 7 Dec 2020 at 08:58, David Plowman <david.plowman@raspberrypi.com> wrote: > > Hi everyone > > I think you can guess what I might be about to say.... :) > > So this is just my little weekly nudge to see where we are with this. > We were having some discussions about the pipeline model as a whole, > as I recall, though I don't think we were wanting to block these > patches. But please let me know what you think! > > Thanks and sorry for being a nuisance... > David > > On Mon, 30 Nov 2020 at 10:30, Jacopo Mondi <jacopo@jmondi.org> wrote: > > > > Hi David, > > > > On Mon, Nov 30, 2020 at 09:59:21AM +0000, David Plowman wrote: > > > Hi everyone > > > > > > I was wondering if I could give this one its customary Monday morning nudge? > > > > > > I notice that there was still some question last week over whether > > > metadata (that's not allowed as a control) should be able to signal > > > its maximum/minimum values. It doesn't really feel like an issue to me > > > - I just report what the pipeline did, I'm not really seeing a need to > > > report what the pipeline *might* have done! Other than that, it seems > > > we were largely agreed...? > > > > My question on the metadata min-max was mostly to plan ahead if we > > ever need to expose the metadata limits from the Camera API, something > > we currently don't have. > > > > Not something to block this patch on. > > > > I see the patch itself has my tags only. I would wait for an explicit > > ack from Laurent then merge it. > > > > Thanks > > j > > > > > > > > Thanks again! > > > David > > > > > > On Thu, 26 Nov 2020 at 14:50, David Plowman > > > <david.plowman@raspberrypi.com> wrote: > > > > > > > > Hi everyone > > > > > > > > Here (after much talking ourselves in circles!) is a second version of > > > > the digital gain control. I've taken Jacopo's text for the control > > > > description, and added a second patch that makes use of the control in > > > > the Raspberry Pi IPAs to add the digital gain to the metadata. > > > > > > > > This second patch doesn't really tell you very much. In our libcamera > > > > version of raspistill (still image capture app) there will be a line > > > > like this: > > > > > > > > iso *= metadata.get<float>(controls::DigitalGain); > > > > > > > > and that's about it! > > > > > > > > Best regards > > > > David > > > > > > > > David Plowman (2): > > > > libcamera: controls: Add DigitalGain control > > > > src: ipa: raspberrypi: Add digital gain to libcamera metadata > > > > > > > > src/ipa/raspberrypi/raspberrypi.cpp | 4 +++- > > > > src/libcamera/control_ids.yaml | 17 +++++++++++++++++ > > > > 2 files changed, 20 insertions(+), 1 deletion(-) > > > > > > > > -- > > > > 2.20.1 > > > > > > > _______________________________________________ > > > libcamera-devel mailing list > > > libcamera-devel@lists.libcamera.org > > > https://lists.libcamera.org/listinfo/libcamera-devel
Hi David, On Mon, Dec 14, 2020 at 10:34:48AM +0000, David Plowman wrote: > Hi everyone > > Thanks for all the reviews and merging that happened towards the end > of last week. I think we're getting really close now! So I wondered if > I might be able to give this little patch a final nudge over the > line...? Absolutely. Now that the other patches have gone in, it's on top of my review todo list, with Naush's frame rate series. Feel free to let me know which one I should handle first, otherwise I'll start with this one. > On Mon, 7 Dec 2020 at 08:58, David Plowman wrote: > > > > Hi everyone > > > > I think you can guess what I might be about to say.... :) > > > > So this is just my little weekly nudge to see where we are with this. > > We were having some discussions about the pipeline model as a whole, > > as I recall, though I don't think we were wanting to block these > > patches. But please let me know what you think! > > > > Thanks and sorry for being a nuisance... > > David > > > > On Mon, 30 Nov 2020 at 10:30, Jacopo Mondi wrote: > > > On Mon, Nov 30, 2020 at 09:59:21AM +0000, David Plowman wrote: > > > > Hi everyone > > > > > > > > I was wondering if I could give this one its customary Monday morning nudge? > > > > > > > > I notice that there was still some question last week over whether > > > > metadata (that's not allowed as a control) should be able to signal > > > > its maximum/minimum values. It doesn't really feel like an issue to me > > > > - I just report what the pipeline did, I'm not really seeing a need to > > > > report what the pipeline *might* have done! Other than that, it seems > > > > we were largely agreed...? > > > > > > My question on the metadata min-max was mostly to plan ahead if we > > > ever need to expose the metadata limits from the Camera API, something > > > we currently don't have. > > > > > > Not something to block this patch on. > > > > > > I see the patch itself has my tags only. I would wait for an explicit > > > ack from Laurent then merge it. > > > > > > > On Thu, 26 Nov 2020 at 14:50, David Plowman wrote: > > > > > > > > > > Hi everyone > > > > > > > > > > Here (after much talking ourselves in circles!) is a second version of > > > > > the digital gain control. I've taken Jacopo's text for the control > > > > > description, and added a second patch that makes use of the control in > > > > > the Raspberry Pi IPAs to add the digital gain to the metadata. > > > > > > > > > > This second patch doesn't really tell you very much. In our libcamera > > > > > version of raspistill (still image capture app) there will be a line > > > > > like this: > > > > > > > > > > iso *= metadata.get<float>(controls::DigitalGain); > > > > > > > > > > and that's about it! > > > > > > > > > > Best regards > > > > > David > > > > > > > > > > David Plowman (2): > > > > > libcamera: controls: Add DigitalGain control > > > > > src: ipa: raspberrypi: Add digital gain to libcamera metadata > > > > > > > > > > src/ipa/raspberrypi/raspberrypi.cpp | 4 +++- > > > > > src/libcamera/control_ids.yaml | 17 +++++++++++++++++ > > > > > 2 files changed, 20 insertions(+), 1 deletion(-)
Hi Laurent On Tue, 15 Dec 2020 at 04:58, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > Hi David, > > On Mon, Dec 14, 2020 at 10:34:48AM +0000, David Plowman wrote: > > Hi everyone > > > > Thanks for all the reviews and merging that happened towards the end > > of last week. I think we're getting really close now! So I wondered if > > I might be able to give this little patch a final nudge over the > > line...? > > Absolutely. Now that the other patches have gone in, it's on top of my > review todo list, with Naush's frame rate series. Feel free to let me > know which one I should handle first, otherwise I'll start with this > one. Great, thank you very much. These are the final two that I think we feel we need before letting users try our applications. I don't think I have any preference on the order, the digital gain one first is fine! Thanks and best regards David > > > On Mon, 7 Dec 2020 at 08:58, David Plowman wrote: > > > > > > Hi everyone > > > > > > I think you can guess what I might be about to say.... :) > > > > > > So this is just my little weekly nudge to see where we are with this. > > > We were having some discussions about the pipeline model as a whole, > > > as I recall, though I don't think we were wanting to block these > > > patches. But please let me know what you think! > > > > > > Thanks and sorry for being a nuisance... > > > David > > > > > > On Mon, 30 Nov 2020 at 10:30, Jacopo Mondi wrote: > > > > On Mon, Nov 30, 2020 at 09:59:21AM +0000, David Plowman wrote: > > > > > Hi everyone > > > > > > > > > > I was wondering if I could give this one its customary Monday morning nudge? > > > > > > > > > > I notice that there was still some question last week over whether > > > > > metadata (that's not allowed as a control) should be able to signal > > > > > its maximum/minimum values. It doesn't really feel like an issue to me > > > > > - I just report what the pipeline did, I'm not really seeing a need to > > > > > report what the pipeline *might* have done! Other than that, it seems > > > > > we were largely agreed...? > > > > > > > > My question on the metadata min-max was mostly to plan ahead if we > > > > ever need to expose the metadata limits from the Camera API, something > > > > we currently don't have. > > > > > > > > Not something to block this patch on. > > > > > > > > I see the patch itself has my tags only. I would wait for an explicit > > > > ack from Laurent then merge it. > > > > > > > > > On Thu, 26 Nov 2020 at 14:50, David Plowman wrote: > > > > > > > > > > > > Hi everyone > > > > > > > > > > > > Here (after much talking ourselves in circles!) is a second version of > > > > > > the digital gain control. I've taken Jacopo's text for the control > > > > > > description, and added a second patch that makes use of the control in > > > > > > the Raspberry Pi IPAs to add the digital gain to the metadata. > > > > > > > > > > > > This second patch doesn't really tell you very much. In our libcamera > > > > > > version of raspistill (still image capture app) there will be a line > > > > > > like this: > > > > > > > > > > > > iso *= metadata.get<float>(controls::DigitalGain); > > > > > > > > > > > > and that's about it! > > > > > > > > > > > > Best regards > > > > > > David > > > > > > > > > > > > David Plowman (2): > > > > > > libcamera: controls: Add DigitalGain control > > > > > > src: ipa: raspberrypi: Add digital gain to libcamera metadata > > > > > > > > > > > > src/ipa/raspberrypi/raspberrypi.cpp | 4 +++- > > > > > > src/libcamera/control_ids.yaml | 17 +++++++++++++++++ > > > > > > 2 files changed, 20 insertions(+), 1 deletion(-) > > -- > Regards, > > Laurent Pinchart
Hi everyone Here (after much talking ourselves in circles!) is a second version of the digital gain control. I've taken Jacopo's text for the control description, and added a second patch that makes use of the control in the Raspberry Pi IPAs to add the digital gain to the metadata. This second patch doesn't really tell you very much. In our libcamera version of raspistill (still image capture app) there will be a line like this: iso *= metadata.get<float>(controls::DigitalGain); and that's about it! Best regards David David Plowman (2): libcamera: controls: Add DigitalGain control src: ipa: raspberrypi: Add digital gain to libcamera metadata src/ipa/raspberrypi/raspberrypi.cpp | 4 +++- src/libcamera/control_ids.yaml | 17 +++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-)