raspberrypi / picamera2

New libcamera based python library
BSD 2-Clause "Simplified" License
845 stars 180 forks source link

start_and_capture_files() ignores config created via create_still_configuration() and applied with configure() #853

Open cdcformatc opened 10 months ago

cdcformatc commented 10 months ago

Using: picamera2 version 0.3.12 libcamera build v0.1.0+52-a858d20b

Trying to flip the image with the following code:

picam2 = Picamera2(camera)
transform = libcamera.Transform(hflip=True, vflip=True)
still_config = picam2.create_still_configuration(transform=transform)
picam2.configure(still_config)
print(picam2.camera_configuration())

output:

{'use_case': 'still', 'transform': <libcamera.Transform 'hvflip'>, 'colour_space': <libcamera.ColorSpace 'sYCC'>, 'buffer_count': 1, 'queue': True, 'main': {'format': 'BGR888', 'size': (4608, 2592), 'stride': 13824, 'framesize': 35831808}, 'lores': None, 'raw': {'format': 'SRGGB10_CSI2P', 'size': (4608, 2592), 'stride': 5760, 'framesize': 14929920}, 'controls': {'NoiseReductionMode': <NoiseReductionModeEnum.HighQuality: 2>, 'FrameDurationLimits': (100, 1000000000)}, 'sensor': {'bit_depth': 10, 'output_size': (4608, 2592)}, 'display': None, 'encode': None}

However after capturing an image (edit: with start_and_capture_files) it is not flipped, even though the 'transform' member in the output would indicate otherwise.

Applying the workaround mentioned in this issue and changing the code to:

picam2 = Picamera2(camera)
transform = libcamera.Transform(hflip=True, vflip=True)
picam2.still_configuration = picam2.create_still_configuration(transform=transform)

Does result in a properly flipped image.

davidplowman commented 10 months ago

Hi, yes, we unfortunately updated to an upstream version of libcamera where the reporting of transforms is broken. The behaviour of the images should be OK, but it probably reports the wrong transform back to you. For now, you will just have to ignore the reported transform. This is fixed in the latest version of libcamera, so we'll try to get a build out soon with these fixes in.

cdcformatc commented 10 months ago

It is not just a reporting issue, the line picam2.configure(still_config) does not apply the configuration. The workaround is needed to actually apply the configuration.

davidplowman commented 10 months ago

Hi, thanks for your message. I'm afraid I'm still not quite sure I'm understanding what's wrong. I've run the following:

from picamera2 import Picamera2
from libcamera import Transform

picam2 = Picamera2()

transform = Transform()
config = picam2.create_still_configuration(transform=transform)
picam2.configure(config)
print("Configuration:", picam2.camera_configuration()) # reports "identity" transform

picam2.start()
picam2.capture_file("test_0.jpg") # image is right way up
picam2.stop()

transform = Transform(hflip=True, vflip=True)
config = picam2.create_still_configuration(transform=transform)
picam2.configure(config)
print("Configuration:", picam2.camera_configuration()) # reports "hvflip" transform

picam2.start()
picam2.capture_file("test_180.jpg") # image is upside down
picam2.stop()

with the latest released software on a Pi 4 using an HQ cam (imx477). As far as I can tell, everything is behaving as expected.

Could you provide a similar small and self-contained script, showing exactly what is wrong. Please run the script on the latest software as I'm not really in a position to go back and fix superseded versions. (To get the latest software use sudo apt update followed by sudo apt upgrade - please do a backup first or use a spare SD card.) Please also confirm what kind of Pi and what camera you are using.

For reference, I have Picamera2 at version 0.3.14, and libcamera reports v0.1.0+99-4a23664b.

Thanks!

cdcformatc commented 10 months ago

I was unaware there was a more recent Picamera2 release I have been using 0.3.12. I am not sure why my libcamera version is older as well. I will try to fix these issues and get back to you.

For reference: Hardware: Compute Module 3+ with the V3.0 Compute Module IO Board. Cameras: Two imx708, one of them imx708-wide. I have also been using an imx219 in testing but imx708-wide is my target. OS: Raspberry Pi OS Lite (64-bit) bookworm image released on 2020/10/10.

/boot/config.txt default except with some added lines to get the camera GPIOs re-wired Reference for these changes

camera_auto_detect=0
dtoverlay=imx708,cam0
dtoverlay=imx708,cam1
dtparam=cam0_reg
dtparam=cam0_reg_gpio=3
dtparam=cam1_reg
dtparam=cam1_reg_gpio=31

I don't think there is anything wrong with this setup, libcamera complains about the imx708-wide but it still takes a picture. The posted workaround works to flip the image so I really don't think it is the setup.

Minimal version of my script:

from libcamera import Transform
from picamera2 import Picamera2
import time

def get_camera(camera_num, quality=80, flip=True)
    # create camera
    picam2= Picamera2(camera_num)

    # set JPEG quality
    picam2.options['quality'] = quality

    # set up transform for flipping the image
    transform = Transform(hflip=flip, vflip=flip)

    # create the configuration
    still_config = picam2.create_still_configuration(transform=transform)

    # apply the configuration
    picam2.configure(still_config)

    # sleep a little after configure
    time.sleep(1)

    return picam2

def take_picture(camera, filename):
    camera.start_and_capture_files(
        filename,
        initial_delay=0,
        num_files=1,
        delay=0,
        show_preview=False)

if __name__ == '__main__':
    camera = get_camera(flip=True)
    take_picture(camera, 'unflipped.jpg')

I need to get the pictures taken with as little delay as possible so the picam2 object is created when the script begins. Some time later the actual pictures are taken on command with start_and_capture_files with initial_delay=0 and delay=0 to remove any forced delays from the library.

As I said I will update those libraries and post an update. I appreciate you taking the time to investigate.

davidplowman commented 10 months ago

You know, I wonder if the problem is that you're using start_and_capture_files(). That is likely to reconfigure the camera, so I could imagine it might clobber stuff that you've set up. It was put in for educational settings where they wanted to type "one thing" and get some pictures out. You might do better to take just slightly more control of the camera as in the script that I used. There will be no difference in speed of captures, in fact, it might help you to be in control of exactly when the camera gets configured/started. Could you give that a try? Thanks.

If you do want to capture images as quickly as possibly, I would recommend configuring and starting the camera up front. In the configuration, set the buffer_count to 3 as this will stop you from dropping camera frames. At the moment you want a capture, just use capture_array or capture_file. Because of all the pipelining in the camera system, the frames returned can be (partially or even fully) exposed before you actually request the capture. Is your application sensitive to this?

cdcformatc commented 9 months ago

I finally got back to working on the camera scripts. This time I am able to debug! I can confirm that start_and_capture_files() indeed does not use the same configuration as capture_file(). At least when configured this way.

The issue as I see it is within picamera2.configure_() picamera2.py Lines 1090-1097:

self.camera_config = camera_config
# Fill in the embedded configuration structures if those were used.
if initial_config == "preview":
    self.preview_configuration.update(camera_config)
elif initial_config == "still":
    self.still_configuration.update(camera_config)
else:
    self.video_configuration.update(camera_config)

This means if you call configure() with a dict (which is what create_still_configuration() returns), then self.still_configuration is never updated. And of course start_and_capture_files ultimately uses self.still_configuration. As it is now the video_configuration is updated whenever you pass anything other than the literal strings "preview" and "still".

What I would expect to happen is that this code would actually change the still_configuration.

transform = Transform(hflip=True, vflip=True)
still_config = picamera2.create_still_configuration(transform=transform)
picamera2.configure(still_config )
# picamera2.camera_config is updated
# picamera2.video_configuration is updated

This is sidestepped if you directly set picamera2.still_configuration OR pass the config to picamera2.start_and_capture_files() as the capture_mode parameter.

transform = Transform(hflip=True, vflip=True)
still_config = picamera2.create_still_configuration(transform=transform)
picamera2.configure(still_config) #required to set picamera2.camera_config

#workaround 1:
picamera2.still_configuration = still_config # required to set picamera2.still_configuration

#workaround 2:
picamera2.start_and_capture_files(
    full_path,
    initial_delay=0,
    num_files=1,
    delay=0,
    show_preview=False,
    capture_mode=still_config # calls picamera2.configure() directly with this parameter
)

Library versions for reference:

picamera2 version 0.3.16-1
rpicam-apps build: f74361ee6a56 23-11-2023 (16:52:49)
libcamera build: v0.1.0+118-563cd78e
davidplowman commented 8 months ago

Hi, and thanks for the update on this.

Actually, I don't like the behaviour at all where it updates the video_configuration by default!

It seems plausible to update the embedded preview/still/video configuration is the configuration "use_case" has a matching name, so maybe we should do that?

On the other hand, we're soon going to start allowing arbitrary user defined use cases. The idea is that users will be able to associate a particular set of buffers with specific use cases, so this can make multiple mode switches more reliable (no more memory fragmentation) and also faster (buffers are persistent and not reallocated). So part of me also wonders about keeping the dictionary configuration (which I much prefer and use all the time) and the embedded configuration objects (which I was somewhat "bounced" into providing and never use myself) completely separate.

Not sure. But that else: which updates the video configuration should definitely be an elif initial_config == "video":, I reckon.

cdcformatc commented 8 months ago

So part of me also wonders about keeping the dictionary configuration (which I much prefer and use all the time) and the embedded configuration objects (which I was somewhat "bounced" into providing and never use myself) completely separate.

I could tell while reading the code that there was at least two separate competing ideas on how to handle the configuration. If you are interested in my two cents I also prefer the dictionary configurations they are more intuitive personally. I think that you should just choose one or the other as the hybrid approach is more confusing than it ought to be. Completely separating the two competing configuration methods would also be preferable to this hybrid approach.

Not sure. But that else: which updates the video configuration should definitely be an elif initial_config == "video":, I reckon.

Explicit is better than implicit! It was a surprise at least to find out that by passing a dict meant for a still configuration was actually being ignored, and actually applied to the video config?!

Thanks for looking at this issue and also providing some insight into your experience developing the package, and the peek into the future! I appreciate the hard work.