Message ID | 20220314154633.506026-1-tomi.valkeinen@ideasonboard.com |
---|---|
Headers | show |
Series |
|
Related | show |
Hi everyone Thanks for the update! On Mon, 14 Mar 2022 at 15:46, Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> wrote: > > Hi, > > This is v5 of the series. The changes to v4: > > - Changed to pybind11 'smart_holders' branch which allows us to drop the > HACK for Camera public destructor > - Changed Request.set_control() so that we can drop the HACK for > exposing Camera from Request > - Moved Python enum defs to a separate file for clarity > - "Forward" declare Python classes to fix docstring issues. > > The set_control change breaks the current users of that function. > Previously you could do this: > > req.set_control("Brightness", 1) > > Now you need to do: > > cid = camera.find_control("Brightness") > req.set_control(cid, 1) > I see the need for this, and I guess it's fine - obviously we'll hide that under our Picamera2 API as we wouldn't want our users to have to do that! There is one further thing I wanted to raise in relation to the Python bindings for controls, now that a few folks are trying out Picamera2. I've had feedback that they don't like the fact that all the "enum" control values (exposure modes, AWB modes, that kind of thing) are simple integers up in the Python world. They complain that this makes them quite unfriendly to use, and I'm inclined to agree! We could always create Python-level definitions for them, and possibly add code to convert stuff on the fly, but I wonder if Pybind11 can do anything more automatic for us? Thanks! David > Tomi > > Tomi Valkeinen (3): > Add Python bindings > py: add unittests.py > py: Add cam.py > > meson.build | 1 + > meson_options.txt | 5 + > src/meson.build | 1 + > src/py/cam/cam.py | 461 +++++++++++++++++++++++++++++++++++ > src/py/cam/cam_kms.py | 183 ++++++++++++++ > src/py/cam/cam_null.py | 46 ++++ > src/py/cam/cam_qt.py | 355 +++++++++++++++++++++++++++ > src/py/cam/cam_qtgl.py | 386 +++++++++++++++++++++++++++++ > src/py/cam/gl_helpers.py | 67 +++++ > src/py/libcamera/__init__.py | 10 + > src/py/libcamera/meson.build | 43 ++++ > src/py/libcamera/pyenums.cpp | 53 ++++ > src/py/libcamera/pymain.cpp | 453 ++++++++++++++++++++++++++++++++++ > src/py/meson.build | 1 + > src/py/test/unittests.py | 364 +++++++++++++++++++++++++++ > subprojects/.gitignore | 3 +- > subprojects/pybind11.wrap | 6 + > 17 files changed, 2437 insertions(+), 1 deletion(-) > create mode 100755 src/py/cam/cam.py > create mode 100644 src/py/cam/cam_kms.py > create mode 100644 src/py/cam/cam_null.py > create mode 100644 src/py/cam/cam_qt.py > create mode 100644 src/py/cam/cam_qtgl.py > create mode 100644 src/py/cam/gl_helpers.py > create mode 100644 src/py/libcamera/__init__.py > create mode 100644 src/py/libcamera/meson.build > create mode 100644 src/py/libcamera/pyenums.cpp > create mode 100644 src/py/libcamera/pymain.cpp > create mode 100644 src/py/meson.build > create mode 100755 src/py/test/unittests.py > create mode 100644 subprojects/pybind11.wrap > > -- > 2.25.1 >
On 17/03/2022 11:44, David Plowman wrote: > Hi everyone > > Thanks for the update! > > On Mon, 14 Mar 2022 at 15:46, Tomi Valkeinen > <tomi.valkeinen@ideasonboard.com> wrote: >> >> Hi, >> >> This is v5 of the series. The changes to v4: >> >> - Changed to pybind11 'smart_holders' branch which allows us to drop the >> HACK for Camera public destructor >> - Changed Request.set_control() so that we can drop the HACK for >> exposing Camera from Request >> - Moved Python enum defs to a separate file for clarity >> - "Forward" declare Python classes to fix docstring issues. >> >> The set_control change breaks the current users of that function. >> Previously you could do this: >> >> req.set_control("Brightness", 1) >> >> Now you need to do: >> >> cid = camera.find_control("Brightness") >> req.set_control(cid, 1) >> > > I see the need for this, and I guess it's fine - obviously we'll hide > that under our Picamera2 API as we wouldn't want our users to have to > do that! Yes, it's not very nice. Then again, having to do a string search every time we call set_control is not good either, at least if we can avoid it (and we can, as above). But I agree that in a higher level API things should be easy. Of course, it would be nice to have both versions even in this low level API, but... if Request doesn't expose camera, I don't think we can do it in a simple way. > There is one further thing I wanted to raise in relation to the Python > bindings for controls, now that a few folks are trying out Picamera2. > > I've had feedback that they don't like the fact that all the "enum" > control values (exposure modes, AWB modes, that kind of thing) are > simple integers up in the Python world. They complain that this makes > them quite unfriendly to use, and I'm inclined to agree! We could > always create Python-level definitions for them, and possibly add code > to convert stuff on the fly, but I wonder if Pybind11 can do anything > more automatic for us? Can you elaborate? Which enums are these? Tomi
On 17/03/2022 11:55, Tomi Valkeinen wrote: > On 17/03/2022 11:44, David Plowman wrote: >> Hi everyone >> >> Thanks for the update! >> >> On Mon, 14 Mar 2022 at 15:46, Tomi Valkeinen >> <tomi.valkeinen@ideasonboard.com> wrote: >>> >>> Hi, >>> >>> This is v5 of the series. The changes to v4: >>> >>> - Changed to pybind11 'smart_holders' branch which allows us to drop the >>> HACK for Camera public destructor >>> - Changed Request.set_control() so that we can drop the HACK for >>> exposing Camera from Request >>> - Moved Python enum defs to a separate file for clarity >>> - "Forward" declare Python classes to fix docstring issues. >>> >>> The set_control change breaks the current users of that function. >>> Previously you could do this: >>> >>> req.set_control("Brightness", 1) >>> >>> Now you need to do: >>> >>> cid = camera.find_control("Brightness") >>> req.set_control(cid, 1) >>> >> >> I see the need for this, and I guess it's fine - obviously we'll hide >> that under our Picamera2 API as we wouldn't want our users to have to >> do that! I think the core question is whether a Request is tied to a Camera. I thought it is, as it is created from the Camera. And if that's the case, I don't see why the Camera cannot be exposed from the Request. Maybe Laurent can explain the reasons why not to expose it. > Yes, it's not very nice. Then again, having to do a string search every > time we call set_control is not good either, at least if we can avoid it > (and we can, as above). But I agree that in a higher level API things > should be easy. Of course, it would be nice to have both versions even > in this low level API, but... if Request doesn't expose camera, I don't > think we can do it in a simple way. Note that there are multiple ways to approach this. E.g.: req.set_control(camera.controls["Brightness"], 1) or: req.set_control(camera.controls.Brightness, 1) or even (although this could perhaps conflict too easily): req.set_control(camera.Brightness, 1) Or we could go the other way: camera.set_control(req, "Brightness", 1) or: camera.controls.Brightness.set(req, 1) Tomi
Hi Tomi On Thu, 17 Mar 2022 at 10:03, Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> wrote: > > On 17/03/2022 11:55, Tomi Valkeinen wrote: > > On 17/03/2022 11:44, David Plowman wrote: > >> Hi everyone > >> > >> Thanks for the update! > >> > >> On Mon, 14 Mar 2022 at 15:46, Tomi Valkeinen > >> <tomi.valkeinen@ideasonboard.com> wrote: > >>> > >>> Hi, > >>> > >>> This is v5 of the series. The changes to v4: > >>> > >>> - Changed to pybind11 'smart_holders' branch which allows us to drop the > >>> HACK for Camera public destructor > >>> - Changed Request.set_control() so that we can drop the HACK for > >>> exposing Camera from Request > >>> - Moved Python enum defs to a separate file for clarity > >>> - "Forward" declare Python classes to fix docstring issues. > >>> > >>> The set_control change breaks the current users of that function. > >>> Previously you could do this: > >>> > >>> req.set_control("Brightness", 1) > >>> > >>> Now you need to do: > >>> > >>> cid = camera.find_control("Brightness") > >>> req.set_control(cid, 1) > >>> > >> > >> I see the need for this, and I guess it's fine - obviously we'll hide > >> that under our Picamera2 API as we wouldn't want our users to have to > >> do that! > > I think the core question is whether a Request is tied to a Camera. I > thought it is, as it is created from the Camera. And if that's the case, > I don't see why the Camera cannot be exposed from the Request. > > Maybe Laurent can explain the reasons why not to expose it. I wouldn't want to worry too much about this! I'll just hide it under the Picamera2 API and that's fine... David > > > Yes, it's not very nice. Then again, having to do a string search every > > time we call set_control is not good either, at least if we can avoid it > > (and we can, as above). But I agree that in a higher level API things > > should be easy. Of course, it would be nice to have both versions even > > in this low level API, but... if Request doesn't expose camera, I don't > > think we can do it in a simple way. > > Note that there are multiple ways to approach this. E.g.: > > req.set_control(camera.controls["Brightness"], 1) > > or: > > req.set_control(camera.controls.Brightness, 1) > > or even (although this could perhaps conflict too easily): > > req.set_control(camera.Brightness, 1) > > Or we could go the other way: > > camera.set_control(req, "Brightness", 1) > > or: > > camera.controls.Brightness.set(req, 1) > > Tomi
Hi again Just to answer the about about the "enum" type controls: On Thu, 17 Mar 2022 at 09:55, Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> wrote: > > On 17/03/2022 11:44, David Plowman wrote: > > Hi everyone > > > > Thanks for the update! > > > > On Mon, 14 Mar 2022 at 15:46, Tomi Valkeinen > > <tomi.valkeinen@ideasonboard.com> wrote: > >> > >> Hi, > >> > >> This is v5 of the series. The changes to v4: > >> > >> - Changed to pybind11 'smart_holders' branch which allows us to drop the > >> HACK for Camera public destructor > >> - Changed Request.set_control() so that we can drop the HACK for > >> exposing Camera from Request > >> - Moved Python enum defs to a separate file for clarity > >> - "Forward" declare Python classes to fix docstring issues. > >> > >> The set_control change breaks the current users of that function. > >> Previously you could do this: > >> > >> req.set_control("Brightness", 1) > >> > >> Now you need to do: > >> > >> cid = camera.find_control("Brightness") > >> req.set_control(cid, 1) > >> > > > > I see the need for this, and I guess it's fine - obviously we'll hide > > that under our Picamera2 API as we wouldn't want our users to have to > > do that! > > Yes, it's not very nice. Then again, having to do a string search every > time we call set_control is not good either, at least if we can avoid it > (and we can, as above). But I agree that in a higher level API things > should be easy. Of course, it would be nice to have both versions even > in this low level API, but... if Request doesn't expose camera, I don't > think we can do it in a simple way. > > > There is one further thing I wanted to raise in relation to the Python > > bindings for controls, now that a few folks are trying out Picamera2. > > > > I've had feedback that they don't like the fact that all the "enum" > > control values (exposure modes, AWB modes, that kind of thing) are > > simple integers up in the Python world. They complain that this makes > > them quite unfriendly to use, and I'm inclined to agree! We could > > always create Python-level definitions for them, and possibly add code > > to convert stuff on the fly, but I wonder if Pybind11 can do anything > > more automatic for us? > > Can you elaborate? Which enums are these? So users would prefer to write something like picamera2.set_controls({"AwbMode": AwbMode.Indoor}) rather than picamera2.set_controls({"AwbMode": 4}) The same applies to all the controls of this type, there including AeMeteringMode AeConstraintMode AeExposureMode AwbMode NoiseReductionMode there will at some point be an AfMode and probably other AF ones and so on. Basically it looks to be mostly anything with "Mode" on the end, though I think we will encounter some others too. Also anything with "State" on the end is probably similar, though in this case we're talking about reported values, not ones you can set. Does that make sense? Thanks! David > > Tomi
On 17/03/2022 12:12, David Plowman wrote: > Hi again > > Just to answer the about about the "enum" type controls: > > On Thu, 17 Mar 2022 at 09:55, Tomi Valkeinen > <tomi.valkeinen@ideasonboard.com> wrote: >> >> On 17/03/2022 11:44, David Plowman wrote: >>> Hi everyone >>> >>> Thanks for the update! >>> >>> On Mon, 14 Mar 2022 at 15:46, Tomi Valkeinen >>> <tomi.valkeinen@ideasonboard.com> wrote: >>>> >>>> Hi, >>>> >>>> This is v5 of the series. The changes to v4: >>>> >>>> - Changed to pybind11 'smart_holders' branch which allows us to drop the >>>> HACK for Camera public destructor >>>> - Changed Request.set_control() so that we can drop the HACK for >>>> exposing Camera from Request >>>> - Moved Python enum defs to a separate file for clarity >>>> - "Forward" declare Python classes to fix docstring issues. >>>> >>>> The set_control change breaks the current users of that function. >>>> Previously you could do this: >>>> >>>> req.set_control("Brightness", 1) >>>> >>>> Now you need to do: >>>> >>>> cid = camera.find_control("Brightness") >>>> req.set_control(cid, 1) >>>> >>> >>> I see the need for this, and I guess it's fine - obviously we'll hide >>> that under our Picamera2 API as we wouldn't want our users to have to >>> do that! >> >> Yes, it's not very nice. Then again, having to do a string search every >> time we call set_control is not good either, at least if we can avoid it >> (and we can, as above). But I agree that in a higher level API things >> should be easy. Of course, it would be nice to have both versions even >> in this low level API, but... if Request doesn't expose camera, I don't >> think we can do it in a simple way. >> >>> There is one further thing I wanted to raise in relation to the Python >>> bindings for controls, now that a few folks are trying out Picamera2. >>> >>> I've had feedback that they don't like the fact that all the "enum" >>> control values (exposure modes, AWB modes, that kind of thing) are >>> simple integers up in the Python world. They complain that this makes >>> them quite unfriendly to use, and I'm inclined to agree! We could >>> always create Python-level definitions for them, and possibly add code >>> to convert stuff on the fly, but I wonder if Pybind11 can do anything >>> more automatic for us? >> >> Can you elaborate? Which enums are these? > > So users would prefer to write something like > > picamera2.set_controls({"AwbMode": AwbMode.Indoor}) > > rather than > > picamera2.set_controls({"AwbMode": 4}) > > The same applies to all the controls of this type, there including > > AeMeteringMode > AeConstraintMode > AeExposureMode > AwbMode > NoiseReductionMode > there will at some point be an AfMode and probably other AF ones > and so on. > > Basically it looks to be mostly anything with "Mode" on the end, > though I think we will encounter some others too. Also anything with > "State" on the end is probably similar, though in this case we're > talking about reported values, not ones you can set. Looks like these are defined in src/libcamera/control_ids.yaml, so we could have a script that generates pybind11 code to create the enums. Or we could just do it by hand. Is that your question, how and where to define the python enums? Or do you think there's some issue using those? Tomi
Hi again On Thu, 17 Mar 2022 at 10:41, Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> wrote: > > On 17/03/2022 12:12, David Plowman wrote: > > Hi again > > > > Just to answer the about about the "enum" type controls: > > > > On Thu, 17 Mar 2022 at 09:55, Tomi Valkeinen > > <tomi.valkeinen@ideasonboard.com> wrote: > >> > >> On 17/03/2022 11:44, David Plowman wrote: > >>> Hi everyone > >>> > >>> Thanks for the update! > >>> > >>> On Mon, 14 Mar 2022 at 15:46, Tomi Valkeinen > >>> <tomi.valkeinen@ideasonboard.com> wrote: > >>>> > >>>> Hi, > >>>> > >>>> This is v5 of the series. The changes to v4: > >>>> > >>>> - Changed to pybind11 'smart_holders' branch which allows us to drop the > >>>> HACK for Camera public destructor > >>>> - Changed Request.set_control() so that we can drop the HACK for > >>>> exposing Camera from Request > >>>> - Moved Python enum defs to a separate file for clarity > >>>> - "Forward" declare Python classes to fix docstring issues. > >>>> > >>>> The set_control change breaks the current users of that function. > >>>> Previously you could do this: > >>>> > >>>> req.set_control("Brightness", 1) > >>>> > >>>> Now you need to do: > >>>> > >>>> cid = camera.find_control("Brightness") > >>>> req.set_control(cid, 1) > >>>> > >>> > >>> I see the need for this, and I guess it's fine - obviously we'll hide > >>> that under our Picamera2 API as we wouldn't want our users to have to > >>> do that! > >> > >> Yes, it's not very nice. Then again, having to do a string search every > >> time we call set_control is not good either, at least if we can avoid it > >> (and we can, as above). But I agree that in a higher level API things > >> should be easy. Of course, it would be nice to have both versions even > >> in this low level API, but... if Request doesn't expose camera, I don't > >> think we can do it in a simple way. > >> > >>> There is one further thing I wanted to raise in relation to the Python > >>> bindings for controls, now that a few folks are trying out Picamera2. > >>> > >>> I've had feedback that they don't like the fact that all the "enum" > >>> control values (exposure modes, AWB modes, that kind of thing) are > >>> simple integers up in the Python world. They complain that this makes > >>> them quite unfriendly to use, and I'm inclined to agree! We could > >>> always create Python-level definitions for them, and possibly add code > >>> to convert stuff on the fly, but I wonder if Pybind11 can do anything > >>> more automatic for us? > >> > >> Can you elaborate? Which enums are these? > > > > So users would prefer to write something like > > > > picamera2.set_controls({"AwbMode": AwbMode.Indoor}) > > > > rather than > > > > picamera2.set_controls({"AwbMode": 4}) > > > > The same applies to all the controls of this type, there including > > > > AeMeteringMode > > AeConstraintMode > > AeExposureMode > > AwbMode > > NoiseReductionMode > > there will at some point be an AfMode and probably other AF ones > > and so on. > > > > Basically it looks to be mostly anything with "Mode" on the end, > > though I think we will encounter some others too. Also anything with > > "State" on the end is probably similar, though in this case we're > > talking about reported values, not ones you can set. > > Looks like these are defined in src/libcamera/control_ids.yaml, so we > could have a script that generates pybind11 code to create the enums. Or > we could just do it by hand. Defining them by hand is fine with me, at least for the time being. I just wonder whether it would get annoying in the longer run, so some kind of script in the build system might be helpful. > > Is that your question, how and where to define the python enums? Or do > you think there's some issue using those? I don't think there's any problem, I guess I'm just flagging that there's something we probably need to do. I could define them all in Picamera2, but it seems like everyone using these Python bindings would appreciate them, so they probably belong in libcamera somewhere. Thanks! David > > Tomi
On 17/03/2022 13:01, David Plowman wrote: > Hi again > > On Thu, 17 Mar 2022 at 10:41, Tomi Valkeinen > <tomi.valkeinen@ideasonboard.com> wrote: >> >> On 17/03/2022 12:12, David Plowman wrote: >>> Hi again >>> >>> Just to answer the about about the "enum" type controls: >>> >>> On Thu, 17 Mar 2022 at 09:55, Tomi Valkeinen >>> <tomi.valkeinen@ideasonboard.com> wrote: >>>> >>>> On 17/03/2022 11:44, David Plowman wrote: >>>>> Hi everyone >>>>> >>>>> Thanks for the update! >>>>> >>>>> On Mon, 14 Mar 2022 at 15:46, Tomi Valkeinen >>>>> <tomi.valkeinen@ideasonboard.com> wrote: >>>>>> >>>>>> Hi, >>>>>> >>>>>> This is v5 of the series. The changes to v4: >>>>>> >>>>>> - Changed to pybind11 'smart_holders' branch which allows us to drop the >>>>>> HACK for Camera public destructor >>>>>> - Changed Request.set_control() so that we can drop the HACK for >>>>>> exposing Camera from Request >>>>>> - Moved Python enum defs to a separate file for clarity >>>>>> - "Forward" declare Python classes to fix docstring issues. >>>>>> >>>>>> The set_control change breaks the current users of that function. >>>>>> Previously you could do this: >>>>>> >>>>>> req.set_control("Brightness", 1) >>>>>> >>>>>> Now you need to do: >>>>>> >>>>>> cid = camera.find_control("Brightness") >>>>>> req.set_control(cid, 1) >>>>>> >>>>> >>>>> I see the need for this, and I guess it's fine - obviously we'll hide >>>>> that under our Picamera2 API as we wouldn't want our users to have to >>>>> do that! >>>> >>>> Yes, it's not very nice. Then again, having to do a string search every >>>> time we call set_control is not good either, at least if we can avoid it >>>> (and we can, as above). But I agree that in a higher level API things >>>> should be easy. Of course, it would be nice to have both versions even >>>> in this low level API, but... if Request doesn't expose camera, I don't >>>> think we can do it in a simple way. >>>> >>>>> There is one further thing I wanted to raise in relation to the Python >>>>> bindings for controls, now that a few folks are trying out Picamera2. >>>>> >>>>> I've had feedback that they don't like the fact that all the "enum" >>>>> control values (exposure modes, AWB modes, that kind of thing) are >>>>> simple integers up in the Python world. They complain that this makes >>>>> them quite unfriendly to use, and I'm inclined to agree! We could >>>>> always create Python-level definitions for them, and possibly add code >>>>> to convert stuff on the fly, but I wonder if Pybind11 can do anything >>>>> more automatic for us? >>>> >>>> Can you elaborate? Which enums are these? >>> >>> So users would prefer to write something like >>> >>> picamera2.set_controls({"AwbMode": AwbMode.Indoor}) >>> >>> rather than >>> >>> picamera2.set_controls({"AwbMode": 4}) >>> >>> The same applies to all the controls of this type, there including >>> >>> AeMeteringMode >>> AeConstraintMode >>> AeExposureMode >>> AwbMode >>> NoiseReductionMode >>> there will at some point be an AfMode and probably other AF ones >>> and so on. >>> >>> Basically it looks to be mostly anything with "Mode" on the end, >>> though I think we will encounter some others too. Also anything with >>> "State" on the end is probably similar, though in this case we're >>> talking about reported values, not ones you can set. >> >> Looks like these are defined in src/libcamera/control_ids.yaml, so we >> could have a script that generates pybind11 code to create the enums. Or >> we could just do it by hand. > > Defining them by hand is fine with me, at least for the time being. I > just wonder whether it would get annoying in the longer run, so some > kind of script in the build system might be helpful. > >> >> Is that your question, how and where to define the python enums? Or do >> you think there's some issue using those? > > I don't think there's any problem, I guess I'm just flagging that > there's something we probably need to do. I could define them all in > Picamera2, but it seems like everyone using these Python bindings > would appreciate them, so they probably belong in libcamera somewhere. Ok. I hacked a quick script. Here's a snippet to add to the py .c file if you want to try them: py::enum_<libcamera::controls::AeMeteringModeEnum>(m, "AeMeteringMode") .value("CentreWeighted", libcamera::controls::MeteringCentreWeighted) .value("Spot", libcamera::controls::MeteringSpot) .value("Matrix", libcamera::controls::MeteringMatrix) .value("Custom", libcamera::controls::MeteringCustom) ; py::enum_<libcamera::controls::AeConstraintModeEnum>(m, "AeConstraintMode") .value("Normal", libcamera::controls::ConstraintNormal) .value("Highlight", libcamera::controls::ConstraintHighlight) .value("Shadows", libcamera::controls::ConstraintShadows) .value("Custom", libcamera::controls::ConstraintCustom) ; py::enum_<libcamera::controls::AeExposureModeEnum>(m, "AeExposureMode") .value("Normal", libcamera::controls::ExposureNormal) .value("Short", libcamera::controls::ExposureShort) .value("Long", libcamera::controls::ExposureLong) .value("Custom", libcamera::controls::ExposureCustom) ; py::enum_<libcamera::controls::AwbModeEnum>(m, "AwbMode") .value("Auto", libcamera::controls::AwbAuto) .value("Incandescent", libcamera::controls::AwbIncandescent) .value("Tungsten", libcamera::controls::AwbTungsten) .value("Fluorescent", libcamera::controls::AwbFluorescent) .value("Indoor", libcamera::controls::AwbIndoor) .value("Daylight", libcamera::controls::AwbDaylight) .value("Cloudy", libcamera::controls::AwbCloudy) .value("Custom", libcamera::controls::AwbCustom) ; py::enum_<libcamera::controls::draft::AePrecaptureTriggerEnum>(m, "AePrecaptureTrigger") .value("Idle", libcamera::controls::draft::AePrecaptureTriggerIdle) .value("Start", libcamera::controls::draft::AePrecaptureTriggerStart) .value("Cancel", libcamera::controls::draft::AePrecaptureTriggerCancel) ; py::enum_<libcamera::controls::draft::AfTriggerEnum>(m, "AfTrigger") .value("Idle", libcamera::controls::draft::AfTriggerIdle) .value("Start", libcamera::controls::draft::AfTriggerStart) .value("Cancel", libcamera::controls::draft::AfTriggerCancel) ; py::enum_<libcamera::controls::draft::NoiseReductionModeEnum>(m, "NoiseReductionMode") .value("Off", libcamera::controls::draft::NoiseReductionModeOff) .value("Fast", libcamera::controls::draft::NoiseReductionModeFast) .value("HighQuality", libcamera::controls::draft::NoiseReductionModeHighQuality) .value("Minimal", libcamera::controls::draft::NoiseReductionModeMinimal) .value("ZSL", libcamera::controls::draft::NoiseReductionModeZSL) ; py::enum_<libcamera::controls::draft::ColorCorrectionAberrationModeEnum>(m, "ColorCorrectionAberrationMode") .value("Off", libcamera::controls::draft::ColorCorrectionAberrationOff) .value("Fast", libcamera::controls::draft::ColorCorrectionAberrationFast) .value("HighQuality", libcamera::controls::draft::ColorCorrectionAberrationHighQuality) ; py::enum_<libcamera::controls::draft::AeStateEnum>(m, "AeState") .value("Inactive", libcamera::controls::draft::AeStateInactive) .value("Searching", libcamera::controls::draft::AeStateSearching) .value("Converged", libcamera::controls::draft::AeStateConverged) .value("Locked", libcamera::controls::draft::AeStateLocked) .value("FlashRequired", libcamera::controls::draft::AeStateFlashRequired) .value("Precapture", libcamera::controls::draft::AeStatePrecapture) ; py::enum_<libcamera::controls::draft::AfStateEnum>(m, "AfState") .value("Inactive", libcamera::controls::draft::AfStateInactive) .value("PassiveScan", libcamera::controls::draft::AfStatePassiveScan) .value("PassiveFocused", libcamera::controls::draft::AfStatePassiveFocused) .value("ActiveScan", libcamera::controls::draft::AfStateActiveScan) .value("FocusedLock", libcamera::controls::draft::AfStateFocusedLock) .value("NotFocusedLock", libcamera::controls::draft::AfStateNotFocusedLock) .value("PassiveUnfocused", libcamera::controls::draft::AfStatePassiveUnfocused) ; py::enum_<libcamera::controls::draft::AwbStateEnum>(m, "AwbState") .value("StateInactive", libcamera::controls::draft::AwbStateInactive) .value("StateSearching", libcamera::controls::draft::AwbStateSearching) .value("Converged", libcamera::controls::draft::AwbConverged) .value("Locked", libcamera::controls::draft::AwbLocked) ; py::enum_<libcamera::controls::draft::LensShadingMapModeEnum>(m, "LensShadingMapMode") .value("Off", libcamera::controls::draft::LensShadingMapModeOff) .value("On", libcamera::controls::draft::LensShadingMapModeOn) ; py::enum_<libcamera::controls::draft::SceneFlickerEnum>(m, "SceneFlicker") .value("Off", libcamera::controls::draft::SceneFickerOff) .value("50Hz", libcamera::controls::draft::SceneFicker50Hz) .value("60Hz", libcamera::controls::draft::SceneFicker60Hz) ; py::enum_<libcamera::controls::draft::TestPatternModeEnum>(m, "TestPatternMode") .value("Off", libcamera::controls::draft::TestPatternModeOff) .value("SolidColor", libcamera::controls::draft::TestPatternModeSolidColor) .value("ColorBars", libcamera::controls::draft::TestPatternModeColorBars) .value("ColorBarsFadeToGray", libcamera::controls::draft::TestPatternModeColorBarsFadeToGray) .value("Pn9", libcamera::controls::draft::TestPatternModePn9) .value("Custom1", libcamera::controls::draft::TestPatternModeCustom1) ;
Cool, thanks, I'll try those and let you know! David On Thu, 17 Mar 2022 at 11:36, Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> wrote: > > On 17/03/2022 13:01, David Plowman wrote: > > Hi again > > > > On Thu, 17 Mar 2022 at 10:41, Tomi Valkeinen > > <tomi.valkeinen@ideasonboard.com> wrote: > >> > >> On 17/03/2022 12:12, David Plowman wrote: > >>> Hi again > >>> > >>> Just to answer the about about the "enum" type controls: > >>> > >>> On Thu, 17 Mar 2022 at 09:55, Tomi Valkeinen > >>> <tomi.valkeinen@ideasonboard.com> wrote: > >>>> > >>>> On 17/03/2022 11:44, David Plowman wrote: > >>>>> Hi everyone > >>>>> > >>>>> Thanks for the update! > >>>>> > >>>>> On Mon, 14 Mar 2022 at 15:46, Tomi Valkeinen > >>>>> <tomi.valkeinen@ideasonboard.com> wrote: > >>>>>> > >>>>>> Hi, > >>>>>> > >>>>>> This is v5 of the series. The changes to v4: > >>>>>> > >>>>>> - Changed to pybind11 'smart_holders' branch which allows us to drop the > >>>>>> HACK for Camera public destructor > >>>>>> - Changed Request.set_control() so that we can drop the HACK for > >>>>>> exposing Camera from Request > >>>>>> - Moved Python enum defs to a separate file for clarity > >>>>>> - "Forward" declare Python classes to fix docstring issues. > >>>>>> > >>>>>> The set_control change breaks the current users of that function. > >>>>>> Previously you could do this: > >>>>>> > >>>>>> req.set_control("Brightness", 1) > >>>>>> > >>>>>> Now you need to do: > >>>>>> > >>>>>> cid = camera.find_control("Brightness") > >>>>>> req.set_control(cid, 1) > >>>>>> > >>>>> > >>>>> I see the need for this, and I guess it's fine - obviously we'll hide > >>>>> that under our Picamera2 API as we wouldn't want our users to have to > >>>>> do that! > >>>> > >>>> Yes, it's not very nice. Then again, having to do a string search every > >>>> time we call set_control is not good either, at least if we can avoid it > >>>> (and we can, as above). But I agree that in a higher level API things > >>>> should be easy. Of course, it would be nice to have both versions even > >>>> in this low level API, but... if Request doesn't expose camera, I don't > >>>> think we can do it in a simple way. > >>>> > >>>>> There is one further thing I wanted to raise in relation to the Python > >>>>> bindings for controls, now that a few folks are trying out Picamera2. > >>>>> > >>>>> I've had feedback that they don't like the fact that all the "enum" > >>>>> control values (exposure modes, AWB modes, that kind of thing) are > >>>>> simple integers up in the Python world. They complain that this makes > >>>>> them quite unfriendly to use, and I'm inclined to agree! We could > >>>>> always create Python-level definitions for them, and possibly add code > >>>>> to convert stuff on the fly, but I wonder if Pybind11 can do anything > >>>>> more automatic for us? > >>>> > >>>> Can you elaborate? Which enums are these? > >>> > >>> So users would prefer to write something like > >>> > >>> picamera2.set_controls({"AwbMode": AwbMode.Indoor}) > >>> > >>> rather than > >>> > >>> picamera2.set_controls({"AwbMode": 4}) > >>> > >>> The same applies to all the controls of this type, there including > >>> > >>> AeMeteringMode > >>> AeConstraintMode > >>> AeExposureMode > >>> AwbMode > >>> NoiseReductionMode > >>> there will at some point be an AfMode and probably other AF ones > >>> and so on. > >>> > >>> Basically it looks to be mostly anything with "Mode" on the end, > >>> though I think we will encounter some others too. Also anything with > >>> "State" on the end is probably similar, though in this case we're > >>> talking about reported values, not ones you can set. > >> > >> Looks like these are defined in src/libcamera/control_ids.yaml, so we > >> could have a script that generates pybind11 code to create the enums. Or > >> we could just do it by hand. > > > > Defining them by hand is fine with me, at least for the time being. I > > just wonder whether it would get annoying in the longer run, so some > > kind of script in the build system might be helpful. > > > >> > >> Is that your question, how and where to define the python enums? Or do > >> you think there's some issue using those? > > > > I don't think there's any problem, I guess I'm just flagging that > > there's something we probably need to do. I could define them all in > > Picamera2, but it seems like everyone using these Python bindings > > would appreciate them, so they probably belong in libcamera somewhere. > > Ok. I hacked a quick script. Here's a snippet to add to the py .c file if you want to try them: > > > py::enum_<libcamera::controls::AeMeteringModeEnum>(m, "AeMeteringMode") > .value("CentreWeighted", libcamera::controls::MeteringCentreWeighted) > .value("Spot", libcamera::controls::MeteringSpot) > .value("Matrix", libcamera::controls::MeteringMatrix) > .value("Custom", libcamera::controls::MeteringCustom) > ; > py::enum_<libcamera::controls::AeConstraintModeEnum>(m, "AeConstraintMode") > .value("Normal", libcamera::controls::ConstraintNormal) > .value("Highlight", libcamera::controls::ConstraintHighlight) > .value("Shadows", libcamera::controls::ConstraintShadows) > .value("Custom", libcamera::controls::ConstraintCustom) > ; > py::enum_<libcamera::controls::AeExposureModeEnum>(m, "AeExposureMode") > .value("Normal", libcamera::controls::ExposureNormal) > .value("Short", libcamera::controls::ExposureShort) > .value("Long", libcamera::controls::ExposureLong) > .value("Custom", libcamera::controls::ExposureCustom) > ; > py::enum_<libcamera::controls::AwbModeEnum>(m, "AwbMode") > .value("Auto", libcamera::controls::AwbAuto) > .value("Incandescent", libcamera::controls::AwbIncandescent) > .value("Tungsten", libcamera::controls::AwbTungsten) > .value("Fluorescent", libcamera::controls::AwbFluorescent) > .value("Indoor", libcamera::controls::AwbIndoor) > .value("Daylight", libcamera::controls::AwbDaylight) > .value("Cloudy", libcamera::controls::AwbCloudy) > .value("Custom", libcamera::controls::AwbCustom) > ; > py::enum_<libcamera::controls::draft::AePrecaptureTriggerEnum>(m, "AePrecaptureTrigger") > .value("Idle", libcamera::controls::draft::AePrecaptureTriggerIdle) > .value("Start", libcamera::controls::draft::AePrecaptureTriggerStart) > .value("Cancel", libcamera::controls::draft::AePrecaptureTriggerCancel) > ; > py::enum_<libcamera::controls::draft::AfTriggerEnum>(m, "AfTrigger") > .value("Idle", libcamera::controls::draft::AfTriggerIdle) > .value("Start", libcamera::controls::draft::AfTriggerStart) > .value("Cancel", libcamera::controls::draft::AfTriggerCancel) > ; > py::enum_<libcamera::controls::draft::NoiseReductionModeEnum>(m, "NoiseReductionMode") > .value("Off", libcamera::controls::draft::NoiseReductionModeOff) > .value("Fast", libcamera::controls::draft::NoiseReductionModeFast) > .value("HighQuality", libcamera::controls::draft::NoiseReductionModeHighQuality) > .value("Minimal", libcamera::controls::draft::NoiseReductionModeMinimal) > .value("ZSL", libcamera::controls::draft::NoiseReductionModeZSL) > ; > py::enum_<libcamera::controls::draft::ColorCorrectionAberrationModeEnum>(m, "ColorCorrectionAberrationMode") > .value("Off", libcamera::controls::draft::ColorCorrectionAberrationOff) > .value("Fast", libcamera::controls::draft::ColorCorrectionAberrationFast) > .value("HighQuality", libcamera::controls::draft::ColorCorrectionAberrationHighQuality) > ; > py::enum_<libcamera::controls::draft::AeStateEnum>(m, "AeState") > .value("Inactive", libcamera::controls::draft::AeStateInactive) > .value("Searching", libcamera::controls::draft::AeStateSearching) > .value("Converged", libcamera::controls::draft::AeStateConverged) > .value("Locked", libcamera::controls::draft::AeStateLocked) > .value("FlashRequired", libcamera::controls::draft::AeStateFlashRequired) > .value("Precapture", libcamera::controls::draft::AeStatePrecapture) > ; > py::enum_<libcamera::controls::draft::AfStateEnum>(m, "AfState") > .value("Inactive", libcamera::controls::draft::AfStateInactive) > .value("PassiveScan", libcamera::controls::draft::AfStatePassiveScan) > .value("PassiveFocused", libcamera::controls::draft::AfStatePassiveFocused) > .value("ActiveScan", libcamera::controls::draft::AfStateActiveScan) > .value("FocusedLock", libcamera::controls::draft::AfStateFocusedLock) > .value("NotFocusedLock", libcamera::controls::draft::AfStateNotFocusedLock) > .value("PassiveUnfocused", libcamera::controls::draft::AfStatePassiveUnfocused) > ; > py::enum_<libcamera::controls::draft::AwbStateEnum>(m, "AwbState") > .value("StateInactive", libcamera::controls::draft::AwbStateInactive) > .value("StateSearching", libcamera::controls::draft::AwbStateSearching) > .value("Converged", libcamera::controls::draft::AwbConverged) > .value("Locked", libcamera::controls::draft::AwbLocked) > ; > py::enum_<libcamera::controls::draft::LensShadingMapModeEnum>(m, "LensShadingMapMode") > .value("Off", libcamera::controls::draft::LensShadingMapModeOff) > .value("On", libcamera::controls::draft::LensShadingMapModeOn) > ; > py::enum_<libcamera::controls::draft::SceneFlickerEnum>(m, "SceneFlicker") > .value("Off", libcamera::controls::draft::SceneFickerOff) > .value("50Hz", libcamera::controls::draft::SceneFicker50Hz) > .value("60Hz", libcamera::controls::draft::SceneFicker60Hz) > ; > py::enum_<libcamera::controls::draft::TestPatternModeEnum>(m, "TestPatternMode") > .value("Off", libcamera::controls::draft::TestPatternModeOff) > .value("SolidColor", libcamera::controls::draft::TestPatternModeSolidColor) > .value("ColorBars", libcamera::controls::draft::TestPatternModeColorBars) > .value("ColorBarsFadeToGray", libcamera::controls::draft::TestPatternModeColorBarsFadeToGray) > .value("Pn9", libcamera::controls::draft::TestPatternModePn9) > .value("Custom1", libcamera::controls::draft::TestPatternModeCustom1) > ; >
Quoting Tomi Valkeinen (2022-03-17 10:03:25) > On 17/03/2022 11:55, Tomi Valkeinen wrote: > > On 17/03/2022 11:44, David Plowman wrote: > >> Hi everyone > >> > >> Thanks for the update! > >> > >> On Mon, 14 Mar 2022 at 15:46, Tomi Valkeinen > >> <tomi.valkeinen@ideasonboard.com> wrote: > >>> > >>> Hi, > >>> > >>> This is v5 of the series. The changes to v4: > >>> > >>> - Changed to pybind11 'smart_holders' branch which allows us to drop the > >>> HACK for Camera public destructor > >>> - Changed Request.set_control() so that we can drop the HACK for > >>> exposing Camera from Request > >>> - Moved Python enum defs to a separate file for clarity > >>> - "Forward" declare Python classes to fix docstring issues. > >>> > >>> The set_control change breaks the current users of that function. > >>> Previously you could do this: > >>> > >>> req.set_control("Brightness", 1) > >>> > >>> Now you need to do: > >>> > >>> cid = camera.find_control("Brightness") > >>> req.set_control(cid, 1) > >>> > >> > >> I see the need for this, and I guess it's fine - obviously we'll hide > >> that under our Picamera2 API as we wouldn't want our users to have to > >> do that! > > I think the core question is whether a Request is tied to a Camera. I > thought it is, as it is created from the Camera. And if that's the case, > I don't see why the Camera cannot be exposed from the Request. This is how I understand it, and the patch I have/had locally to add and expose this is followed by an addition of an error check to make sure that requests queued to a camera that were not created by that camera get rejected. I could probably still do that using the private class infrastructure now though. (re-sketching up that patch now). > Maybe Laurent can explain the reasons why not to expose it. > > > Yes, it's not very nice. Then again, having to do a string search every > > time we call set_control is not good either, at least if we can avoid it > > (and we can, as above). But I agree that in a higher level API things > > should be easy. Of course, it would be nice to have both versions even > > in this low level API, but... if Request doesn't expose camera, I don't > > think we can do it in a simple way. > > Note that there are multiple ways to approach this. E.g.: > > req.set_control(camera.controls["Brightness"], 1) > > or: > > req.set_control(camera.controls.Brightness, 1) > > or even (although this could perhaps conflict too easily): > > req.set_control(camera.Brightness, 1) > > Or we could go the other way: > > camera.set_control(req, "Brightness", 1) > > or: > > camera.controls.Brightness.set(req, 1) > > Tomi
Hi Tomi Thanks for those py::enum definitions - they work great, for example: "picamera2.set_controls({"AwbMode": libcamera.AwbMode.Indoor})" David On Thu, 17 Mar 2022 at 11:50, Kieran Bingham <kieran.bingham@ideasonboard.com> wrote: > > Quoting Tomi Valkeinen (2022-03-17 10:03:25) > > On 17/03/2022 11:55, Tomi Valkeinen wrote: > > > On 17/03/2022 11:44, David Plowman wrote: > > >> Hi everyone > > >> > > >> Thanks for the update! > > >> > > >> On Mon, 14 Mar 2022 at 15:46, Tomi Valkeinen > > >> <tomi.valkeinen@ideasonboard.com> wrote: > > >>> > > >>> Hi, > > >>> > > >>> This is v5 of the series. The changes to v4: > > >>> > > >>> - Changed to pybind11 'smart_holders' branch which allows us to drop the > > >>> HACK for Camera public destructor > > >>> - Changed Request.set_control() so that we can drop the HACK for > > >>> exposing Camera from Request > > >>> - Moved Python enum defs to a separate file for clarity > > >>> - "Forward" declare Python classes to fix docstring issues. > > >>> > > >>> The set_control change breaks the current users of that function. > > >>> Previously you could do this: > > >>> > > >>> req.set_control("Brightness", 1) > > >>> > > >>> Now you need to do: > > >>> > > >>> cid = camera.find_control("Brightness") > > >>> req.set_control(cid, 1) > > >>> > > >> > > >> I see the need for this, and I guess it's fine - obviously we'll hide > > >> that under our Picamera2 API as we wouldn't want our users to have to > > >> do that! > > > > I think the core question is whether a Request is tied to a Camera. I > > thought it is, as it is created from the Camera. And if that's the case, > > I don't see why the Camera cannot be exposed from the Request. > > This is how I understand it, and the patch I have/had locally to add and > expose this is followed by an addition of an error check to make sure > that requests queued to a camera that were not created by that camera get > rejected. > > I could probably still do that using the private class infrastructure > now though. (re-sketching up that patch now). > > > Maybe Laurent can explain the reasons why not to expose it. > > > > > Yes, it's not very nice. Then again, having to do a string search every > > > time we call set_control is not good either, at least if we can avoid it > > > (and we can, as above). But I agree that in a higher level API things > > > should be easy. Of course, it would be nice to have both versions even > > > in this low level API, but... if Request doesn't expose camera, I don't > > > think we can do it in a simple way. > > > > Note that there are multiple ways to approach this. E.g.: > > > > req.set_control(camera.controls["Brightness"], 1) > > > > or: > > > > req.set_control(camera.controls.Brightness, 1) > > > > or even (although this could perhaps conflict too easily): > > > > req.set_control(camera.Brightness, 1) > > > > Or we could go the other way: > > > > camera.set_control(req, "Brightness", 1) > > > > or: > > > > camera.controls.Brightness.set(req, 1) > > > > Tomi
Hi Laurent, On 17/03/2022 13:50, Kieran Bingham wrote: > Quoting Tomi Valkeinen (2022-03-17 10:03:25) >> On 17/03/2022 11:55, Tomi Valkeinen wrote: >>> On 17/03/2022 11:44, David Plowman wrote: >>>> Hi everyone >>>> >>>> Thanks for the update! >>>> >>>> On Mon, 14 Mar 2022 at 15:46, Tomi Valkeinen >>>> <tomi.valkeinen@ideasonboard.com> wrote: >>>>> >>>>> Hi, >>>>> >>>>> This is v5 of the series. The changes to v4: >>>>> >>>>> - Changed to pybind11 'smart_holders' branch which allows us to drop the >>>>> HACK for Camera public destructor >>>>> - Changed Request.set_control() so that we can drop the HACK for >>>>> exposing Camera from Request >>>>> - Moved Python enum defs to a separate file for clarity >>>>> - "Forward" declare Python classes to fix docstring issues. >>>>> >>>>> The set_control change breaks the current users of that function. >>>>> Previously you could do this: >>>>> >>>>> req.set_control("Brightness", 1) >>>>> >>>>> Now you need to do: >>>>> >>>>> cid = camera.find_control("Brightness") >>>>> req.set_control(cid, 1) >>>>> >>>> >>>> I see the need for this, and I guess it's fine - obviously we'll hide >>>> that under our Picamera2 API as we wouldn't want our users to have to >>>> do that! >> >> I think the core question is whether a Request is tied to a Camera. I >> thought it is, as it is created from the Camera. And if that's the case, >> I don't see why the Camera cannot be exposed from the Request. > > This is how I understand it, and the patch I have/had locally to add and > expose this is followed by an addition of an error check to make sure > that requests queued to a camera that were not created by that camera get > rejected. > > I could probably still do that using the private class infrastructure > now though. (re-sketching up that patch now). > >> Maybe Laurent can explain the reasons why not to expose it. Any comment on this? Tomi
Hi Tomi, On Tue, Apr 05, 2022 at 09:47:16AM +0300, Tomi Valkeinen wrote: > On 17/03/2022 13:50, Kieran Bingham wrote: > > Quoting Tomi Valkeinen (2022-03-17 10:03:25) > >> On 17/03/2022 11:55, Tomi Valkeinen wrote: > >>> On 17/03/2022 11:44, David Plowman wrote: > >>>> Hi everyone > >>>> > >>>> Thanks for the update! > >>>> > >>>> On Mon, 14 Mar 2022 at 15:46, Tomi Valkeinen > >>>> <tomi.valkeinen@ideasonboard.com> wrote: > >>>>> > >>>>> Hi, > >>>>> > >>>>> This is v5 of the series. The changes to v4: > >>>>> > >>>>> - Changed to pybind11 'smart_holders' branch which allows us to drop the > >>>>> HACK for Camera public destructor > >>>>> - Changed Request.set_control() so that we can drop the HACK for > >>>>> exposing Camera from Request > >>>>> - Moved Python enum defs to a separate file for clarity > >>>>> - "Forward" declare Python classes to fix docstring issues. > >>>>> > >>>>> The set_control change breaks the current users of that function. > >>>>> Previously you could do this: > >>>>> > >>>>> req.set_control("Brightness", 1) > >>>>> > >>>>> Now you need to do: > >>>>> > >>>>> cid = camera.find_control("Brightness") > >>>>> req.set_control(cid, 1) > >>>>> > >>>> > >>>> I see the need for this, and I guess it's fine - obviously we'll hide > >>>> that under our Picamera2 API as we wouldn't want our users to have to > >>>> do that! > >> > >> I think the core question is whether a Request is tied to a Camera. I > >> thought it is, as it is created from the Camera. And if that's the case, > >> I don't see why the Camera cannot be exposed from the Request. > > > > This is how I understand it, and the patch I have/had locally to add and > > expose this is followed by an addition of an error check to make sure > > that requests queued to a camera that were not created by that camera get > > rejected. > > > > I could probably still do that using the private class infrastructure > > now though. (re-sketching up that patch now). > > > >> Maybe Laurent can explain the reasons why not to expose it. > > Any comment on this? Added on my todo list, I'll sleep over that and try to reply before the end of the week.
Quoting Laurent Pinchart (2022-04-06 02:55:59) > Hi Tomi, > > On Tue, Apr 05, 2022 at 09:47:16AM +0300, Tomi Valkeinen wrote: > > On 17/03/2022 13:50, Kieran Bingham wrote: > > > Quoting Tomi Valkeinen (2022-03-17 10:03:25) > > >> On 17/03/2022 11:55, Tomi Valkeinen wrote: > > >>> On 17/03/2022 11:44, David Plowman wrote: > > >>>> Hi everyone > > >>>> > > >>>> Thanks for the update! > > >>>> > > >>>> On Mon, 14 Mar 2022 at 15:46, Tomi Valkeinen > > >>>> <tomi.valkeinen@ideasonboard.com> wrote: > > >>>>> > > >>>>> Hi, > > >>>>> > > >>>>> This is v5 of the series. The changes to v4: > > >>>>> > > >>>>> - Changed to pybind11 'smart_holders' branch which allows us to drop the > > >>>>> HACK for Camera public destructor > > >>>>> - Changed Request.set_control() so that we can drop the HACK for > > >>>>> exposing Camera from Request > > >>>>> - Moved Python enum defs to a separate file for clarity > > >>>>> - "Forward" declare Python classes to fix docstring issues. > > >>>>> > > >>>>> The set_control change breaks the current users of that function. > > >>>>> Previously you could do this: > > >>>>> > > >>>>> req.set_control("Brightness", 1) > > >>>>> > > >>>>> Now you need to do: > > >>>>> > > >>>>> cid = camera.find_control("Brightness") > > >>>>> req.set_control(cid, 1) > > >>>>> > > >>>> > > >>>> I see the need for this, and I guess it's fine - obviously we'll hide > > >>>> that under our Picamera2 API as we wouldn't want our users to have to > > >>>> do that! > > >> > > >> I think the core question is whether a Request is tied to a Camera. I > > >> thought it is, as it is created from the Camera. And if that's the case, > > >> I don't see why the Camera cannot be exposed from the Request. > > > > > > This is how I understand it, and the patch I have/had locally to add and > > > expose this is followed by an addition of an error check to make sure > > > that requests queued to a camera that were not created by that camera get > > > rejected. > > > > > > I could probably still do that using the private class infrastructure > > > now though. (re-sketching up that patch now). > > > > > >> Maybe Laurent can explain the reasons why not to expose it. > > > > Any comment on this? > > Added on my todo list, I'll sleep over that and try to reply before the > end of the week. Note that now I've now merged my "Ensure requests belong to the camera" patch, so the libcamera API rejects any Request which is not queued directly to it's camera. I would believe that makes it 'safe' to obtain the camera directly from a request, and trust that it's properties are directly associated with the correct camera. -- Kieran > -- > Regards, > > Laurent Pinchart