-
Notifications
You must be signed in to change notification settings - Fork 194
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add user defined exif_data when capturing files #786
Conversation
Hm, I have had time to check some of the functionality. Unfortunately, the Let me know if this is wanted, or whether we just overwrite the existing piexif data. |
Hi, thanks for the PR. It seems like a reasonable feature to add. I'm not quite sure why the automatic tests don't seem to have run, but I can look into that. Can you say why you changed the default value of the exif_data in the second commit? I'm wondering if it would be better to squash the two together, we normally prefer to avoid "fix the previous commit" commits! Thanks. |
e32a407
to
62efbf8
Compare
I have squashed the commits. I changed the default values, because the linter check highlighted it. Apparently only immutable types are recommended for keyword arguments... However, this still doesn't resolve the issue of merging nested dictionaries. |
Thanks for the updates. Unluckily you've caught us at a slight awkward time as everything on the "next" branch is being updated for Pi 5 and Bookworm - hence the conflicts. You'll probably find you can't even run this version without building the "next" branch of raspberrypi/libcamera.git as well. But bear with us, this should all get sorted out in the next few days! Looks like I can approve the automatic tests, so I'll do that. |
When capturing to EXIF-capable files, the user can provide the dictionary `exif_data` which is merged with the auto-generated EXIF-data before writing into the image file. Changes affect the `capture_file_` method and all derivatives. Signed-off-by: Simon Lenz <[email protected]>
a3e45a9
to
fde44eb
Compare
There has been an issue with merging into the updated branch, which should be resolved now. |
Is there an interest to merge this still? |
Yes, sorry. There's been a bit of a run of hectic stuff going on, like Pi 5, but I'll try to get round to this in the next couple of days! |
Hi again, sorry for all the delay with this. I've taken your PR, removed the merge commit and added a test for this functionality (which will stop it getting broken in future by accident!). You can find it here #890 Let me know if that still looks OK to you in which case I'll merge that one and close the original here. If there's anything further you'd like to add in the way of tests, please let me know. Thanks for all your patience! |
I've merged the PR derived from this one, so closing it now. |
When capturing to EXIF-capable files, the user can provide the
dictionary exif_data which is merged with the auto-generated EXIF-data
before writing into the image file.
Changes affect the capture_file_ method and all derivatives.
The commit is based on this old discussion #71 and adds the requested
functionality.
Documentation in docstrings has been provided where applicable.
As I have no access to a working picamera environment right now, I can not test
the functionality.
The original pull request was submitted for the main branch, and is superseded
by this one (/pull/779).
Fixed the linter errors accordingly.