| Message ID | 20201209102630.5562-1-naush@raspberrypi.com |
|---|---|
| Headers | show |
| Series |
|
| Related | show |
Hi all, I should note one thing with these changes; currently the VBLANK control gets set together with the EXPOSURE control. This means that the EXPOSURE value may be considered invalid/out-of-range by the V4L2 framework as it does not adjust the limit of exposure times based on the new VBLANK values. This is wrong, but we sort-of get away with it because we adapt exposure slowly. I can make a quick change to our staggered write component to fix this. If we mark VBLANK as an "immediate update" control, we can set VBLANK separately and before any other control. This will then mean that the V4L2 framework will accept the exposure control based on the new limits of exposure time. Obviously, the staggered control is going to be superseded by the DelayedControl work of Niklas, so it will be a low effort change for the short term. Do you think it is worth including in the current patch set? Thanks, Nauhs On Wed, 9 Dec 2020 at 10:26, Naushir Patuck <naush@raspberrypi.com> wrote: > Hi all, > > Here is v4 of the framerate control work. Apologies that this has been > sent some months after the the last version, but other things took > priority. I think most of the changes here are uncontroversial after the > last round of discussions on the same topic. > > Regards, > Naush > > > Naushir Patuck (3): > libcamera: controls: Add frame duration control > libcamera: raspberrypi: Add control of sensor vblanking > ipa: raspberrypi: config: Update shutter speeds for imx219/477 and > ov5647 > > include/libcamera/ipa/raspberrypi.h | 1 + > src/ipa/raspberrypi/cam_helper.cpp | 37 ++++++++++++++++- > src/ipa/raspberrypi/cam_helper.hpp | 15 ++++++- > src/ipa/raspberrypi/cam_helper_imx219.cpp | 13 +++++- > src/ipa/raspberrypi/cam_helper_imx477.cpp | 11 ++++- > src/ipa/raspberrypi/cam_helper_ov5647.cpp | 11 ++++- > src/ipa/raspberrypi/data/imx219.json | 15 ++++++- > src/ipa/raspberrypi/data/imx477.json | 15 ++++++- > src/ipa/raspberrypi/data/ov5647.json | 15 ++++++- > src/ipa/raspberrypi/raspberrypi.cpp | 41 ++++++++++++++++--- > src/libcamera/control_ids.yaml | 13 ++++++ > .../pipeline/raspberrypi/raspberrypi.cpp | 3 +- > 12 files changed, 171 insertions(+), 19 deletions(-) > > -- > 2.25.1 > >
Hi Naush Would this have a bad effect when the application has taken over control of the exposure time and might be setting it in large jumps between values of its choosing? It's supposed to change instantaneously... Ta David On Wed, 9 Dec 2020 at 14:25, Naushir Patuck <naush@raspberrypi.com> wrote: > > Hi all, > > I should note one thing with these changes; currently the VBLANK control gets set together with the EXPOSURE control. This means that the EXPOSURE value may be considered invalid/out-of-range by the V4L2 framework as it does not adjust the limit of exposure times based on the new VBLANK values. This is wrong, but we sort-of get away with it because we adapt exposure slowly. > > I can make a quick change to our staggered write component to fix this. If we mark VBLANK as an "immediate update" control, we can set VBLANK separately and before any other control. This will then mean that the V4L2 framework will accept the exposure control based on the new limits of exposure time. Obviously, the staggered control is going to be superseded by the DelayedControl work of Niklas, so it will be a low effort change for the short term. Do you think it is worth including in the current patch set? > > Thanks, > Nauhs > > > > > On Wed, 9 Dec 2020 at 10:26, Naushir Patuck <naush@raspberrypi.com> wrote: >> >> Hi all, >> >> Here is v4 of the framerate control work. Apologies that this has been sent some months after the the last version, but other things took priority. I think most of the changes here are uncontroversial after the last round of discussions on the same topic. >> >> Regards, >> Naush >> >> >> Naushir Patuck (3): >> libcamera: controls: Add frame duration control >> libcamera: raspberrypi: Add control of sensor vblanking >> ipa: raspberrypi: config: Update shutter speeds for imx219/477 and >> ov5647 >> >> include/libcamera/ipa/raspberrypi.h | 1 + >> src/ipa/raspberrypi/cam_helper.cpp | 37 ++++++++++++++++- >> src/ipa/raspberrypi/cam_helper.hpp | 15 ++++++- >> src/ipa/raspberrypi/cam_helper_imx219.cpp | 13 +++++- >> src/ipa/raspberrypi/cam_helper_imx477.cpp | 11 ++++- >> src/ipa/raspberrypi/cam_helper_ov5647.cpp | 11 ++++- >> src/ipa/raspberrypi/data/imx219.json | 15 ++++++- >> src/ipa/raspberrypi/data/imx477.json | 15 ++++++- >> src/ipa/raspberrypi/data/ov5647.json | 15 ++++++- >> src/ipa/raspberrypi/raspberrypi.cpp | 41 ++++++++++++++++--- >> src/libcamera/control_ids.yaml | 13 ++++++ >> .../pipeline/raspberrypi/raspberrypi.cpp | 3 +- >> 12 files changed, 171 insertions(+), 19 deletions(-) >> >> -- >> 2.25.1 >> > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi David, On Wed, 9 Dec 2020 at 14:46, David Plowman <david.plowman@raspberrypi.com> wrote: > Hi Naush > > Would this have a bad effect when the application has taken over > control of the exposure time and might be setting it in large jumps > between values of its choosing? It's supposed to change > instantaneously... > Yes it might have an effect in the cases where we may be dealing with much larger jumps in values. The only real way would be to set the VBLANK before other controls - not a difficult change to make. Perhaps I should prototype something in staggered write and share with the mailing list. Regards, Naush > > Ta > David > > On Wed, 9 Dec 2020 at 14:25, Naushir Patuck <naush@raspberrypi.com> wrote: > > > > Hi all, > > > > I should note one thing with these changes; currently the VBLANK control > gets set together with the EXPOSURE control. This means that the EXPOSURE > value may be considered invalid/out-of-range by the V4L2 framework as it > does not adjust the limit of exposure times based on the new VBLANK > values. This is wrong, but we sort-of get away with it because we adapt > exposure slowly. > > > > I can make a quick change to our staggered write component to fix this. > If we mark VBLANK as an "immediate update" control, we can set VBLANK > separately and before any other control. This will then mean that the V4L2 > framework will accept the exposure control based on the new limits of > exposure time. Obviously, the staggered control is going to be superseded > by the DelayedControl work of Niklas, so it will be a low effort change for > the short term. Do you think it is worth including in the current patch > set? > > > > Thanks, > > Nauhs > > > > > > > > > > On Wed, 9 Dec 2020 at 10:26, Naushir Patuck <naush@raspberrypi.com> > wrote: > >> > >> Hi all, > >> > >> Here is v4 of the framerate control work. Apologies that this has been > sent some months after the the last version, but other things took > priority. I think most of the changes here are uncontroversial after the > last round of discussions on the same topic. > >> > >> Regards, > >> Naush > >> > >> > >> Naushir Patuck (3): > >> libcamera: controls: Add frame duration control > >> libcamera: raspberrypi: Add control of sensor vblanking > >> ipa: raspberrypi: config: Update shutter speeds for imx219/477 and > >> ov5647 > >> > >> include/libcamera/ipa/raspberrypi.h | 1 + > >> src/ipa/raspberrypi/cam_helper.cpp | 37 ++++++++++++++++- > >> src/ipa/raspberrypi/cam_helper.hpp | 15 ++++++- > >> src/ipa/raspberrypi/cam_helper_imx219.cpp | 13 +++++- > >> src/ipa/raspberrypi/cam_helper_imx477.cpp | 11 ++++- > >> src/ipa/raspberrypi/cam_helper_ov5647.cpp | 11 ++++- > >> src/ipa/raspberrypi/data/imx219.json | 15 ++++++- > >> src/ipa/raspberrypi/data/imx477.json | 15 ++++++- > >> src/ipa/raspberrypi/data/ov5647.json | 15 ++++++- > >> src/ipa/raspberrypi/raspberrypi.cpp | 41 ++++++++++++++++--- > >> src/libcamera/control_ids.yaml | 13 ++++++ > >> .../pipeline/raspberrypi/raspberrypi.cpp | 3 +- > >> 12 files changed, 171 insertions(+), 19 deletions(-) > >> > >> -- > >> 2.25.1 > >> > > _______________________________________________ > > libcamera-devel mailing list > > libcamera-devel@lists.libcamera.org > > https://lists.libcamera.org/listinfo/libcamera-devel >
Hi all, Here is v4 of the framerate control work. Apologies that this has been sent some months after the the last version, but other things took priority. I think most of the changes here are uncontroversial after the last round of discussions on the same topic. Regards, Naush Naushir Patuck (3): libcamera: controls: Add frame duration control libcamera: raspberrypi: Add control of sensor vblanking ipa: raspberrypi: config: Update shutter speeds for imx219/477 and ov5647 include/libcamera/ipa/raspberrypi.h | 1 + src/ipa/raspberrypi/cam_helper.cpp | 37 ++++++++++++++++- src/ipa/raspberrypi/cam_helper.hpp | 15 ++++++- src/ipa/raspberrypi/cam_helper_imx219.cpp | 13 +++++- src/ipa/raspberrypi/cam_helper_imx477.cpp | 11 ++++- src/ipa/raspberrypi/cam_helper_ov5647.cpp | 11 ++++- src/ipa/raspberrypi/data/imx219.json | 15 ++++++- src/ipa/raspberrypi/data/imx477.json | 15 ++++++- src/ipa/raspberrypi/data/ov5647.json | 15 ++++++- src/ipa/raspberrypi/raspberrypi.cpp | 41 ++++++++++++++++--- src/libcamera/control_ids.yaml | 13 ++++++ .../pipeline/raspberrypi/raspberrypi.cpp | 3 +- 12 files changed, 171 insertions(+), 19 deletions(-)