| Message ID | 20200925085127.17214-1-david.plowman@raspberrypi.com |
|---|---|
| Headers | show |
| Series |
|
| Related | show |
Hi David, On 25/09/2020 09:51, David Plowman wrote: > Hi everyone > > Here's version 2 of the digital zoom patches. I've applied the various > changes that were suggested - so thanks very much for those. The only > significant alteration is in the final patch where I had previously > added a command-line option to qcam to allow a digital zoom to be > requested. This feature has been moved to cam instead... do we think > it's useful there? > > One last question. That example application code that I wrote in a > previous message - I wonder if we should try and include it in > documentation somewhere? And if so, do we have a natural home for it? I think things like that will be useful indeed. Some how we need to make sure we're building any example code regularly, my 'simple-cam' repository already bitrotted recently, so I've now added that to a daily builder. We should probably do the same for any example code. I don't yet know if that should be in the form of an external repo, which we schedule builds, or if it should be src/examples or such ? src/examples/ could of course quite easily integrate into the existing documentation with Doxygen in place though... so that feels quite useful too at the moment. The corollary of that is it adds more code to maintain of course - but hitting compile failures is better than having documentation go out of date IMO. We could also have src/examples only compiled as a feature, for integration tests or such anyway. > I hope that all makes sense! > > Thanks and best regards > David > > David Plowman (6): > libcamera: Add SensorOutputSize property > libcamera: Initialise the SensorOutputSize property > libcamera: Add IspCrop control > libcamera: Add geometry helper functions > libcamera: pipeline: raspberrypi: Implementation of digital zoom > cam: Add command line option to test IspCrop control > > include/libcamera/geometry.h | 20 +++ > include/libcamera/ipa/raspberrypi.h | 1 + > src/cam/capture.cpp | 25 +++- > src/cam/capture.h | 2 +- > src/cam/main.cpp | 3 + > src/cam/main.h | 1 + > src/ipa/raspberrypi/raspberrypi.cpp | 7 + > src/libcamera/camera_sensor.cpp | 6 + > src/libcamera/control_ids.yaml | 9 ++ > src/libcamera/geometry.cpp | 129 ++++++++++++++++++ > .../pipeline/raspberrypi/raspberrypi.cpp | 47 +++++++ > src/libcamera/property_ids.yaml | 19 +++ > 12 files changed, 266 insertions(+), 3 deletions(-) >
Hello, On Fri, Sep 25, 2020 at 11:39:33AM +0100, Kieran Bingham wrote: > On 25/09/2020 09:51, David Plowman wrote: > > Hi everyone > > > > Here's version 2 of the digital zoom patches. I've applied the various > > changes that were suggested - so thanks very much for those. The only > > significant alteration is in the final patch where I had previously > > added a command-line option to qcam to allow a digital zoom to be > > requested. This feature has been moved to cam instead... do we think > > it's useful there? > > > > One last question. That example application code that I wrote in a > > previous message - I wonder if we should try and include it in > > documentation somewhere? And if so, do we have a natural home for it? > > I think things like that will be useful indeed. Fully agreed. There are two locations I can think of for this documentation. The first one is the application howto guide (Documentation/guides/application-developer.rst). This is based on the simple-cam application, so the code would need to be added there first, as well as the documentation, and then copied to the application howto guide. The second location is a currently missing camera pipeline model document. While reading the history of this patch series (apologies for the delay) I became quite convinced that we will need to document an abstract camera pipeline model. The whole point of libcamera is to offer a consistent API to applications that abstracts hardware differences. We thus have developed an abstract pipeline model, but it's all implicit for now. Documenting it explicitly would be useful, and I think digital zoom will fit nicely there. The challenge will be to figure out a good explicit model that will fit all the hardware we will have to support (thus with enough flexibility) while keeping it simple. But without challenge there would be no fun, right ? :-) The pipeline model documentation isn't here yet, so we could capture this in the application guide for now, and either move it or copy it later. The other option is to leave the documentation unaddressed for now until the pipeline model gets documented. > Some how we need to make sure we're building any example code regularly, > my 'simple-cam' repository already bitrotted recently, so I've now added > that to a daily builder. > > We should probably do the same for any example code. > > I don't yet know if that should be in the form of an external repo, > which we schedule builds, or if it should be src/examples or such ? > > src/examples/ could of course quite easily integrate into the existing > documentation with Doxygen in place though... so that feels quite useful > too at the moment. > > The corollary of that is it adds more code to maintain of course - but > hitting compile failures is better than having documentation go out of > date IMO. We could also have src/examples only compiled as a feature, > for integration tests or such anyway. > > > I hope that all makes sense! > > > > Thanks and best regards > > David > > > > David Plowman (6): > > libcamera: Add SensorOutputSize property > > libcamera: Initialise the SensorOutputSize property > > libcamera: Add IspCrop control > > libcamera: Add geometry helper functions > > libcamera: pipeline: raspberrypi: Implementation of digital zoom > > cam: Add command line option to test IspCrop control > > > > include/libcamera/geometry.h | 20 +++ > > include/libcamera/ipa/raspberrypi.h | 1 + > > src/cam/capture.cpp | 25 +++- > > src/cam/capture.h | 2 +- > > src/cam/main.cpp | 3 + > > src/cam/main.h | 1 + > > src/ipa/raspberrypi/raspberrypi.cpp | 7 + > > src/libcamera/camera_sensor.cpp | 6 + > > src/libcamera/control_ids.yaml | 9 ++ > > src/libcamera/geometry.cpp | 129 ++++++++++++++++++ > > .../pipeline/raspberrypi/raspberrypi.cpp | 47 +++++++ > > src/libcamera/property_ids.yaml | 19 +++ > > 12 files changed, 266 insertions(+), 3 deletions(-)