Skip to content
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

Batch edit of IPTC tags #6851

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from
Open

Conversation

Bezierr
Copy link
Contributor

@Bezierr Bezierr commented Sep 15, 2023

Hi there,

another proposal. As always, feel free to reject if you don't like it, or don't think it's suitable for general RawTherapee.

This adds the functionality to batch edit IPTC tags -- something that I was used to in Lightroom but missing from RawTherapee.

The fact that IPTC tags are, unlike other parameters, not individual values but contained in a single map, gave me some headache. For everything I have tested, my solution works, but it may well be that, for a more experienced RawTherapee developer, there would have been a better way to do this.

Logic is as follows:

  • For everything except keywords and supplemental categories: A tag value entered for the first selected picture will replace existing values in all other selected pictures. If the new tag value is empty, tag values will be replaced by the empty string, i.e. deleted.
  • For keywords and supplemental categories: An entry added to the first selected picture will be added to all other selected pictures unless it already exists. An entry deleted from the first selected picture will be deleted from all other selected pictures provided it was there. Empty strings will be treated just like any other entries.

This logic is, I believe, in line with the behavior one would expect, and with the way how batch editing works for other parameters. Since values are not numeric, the distinction between "add" and "replace" does not make sense; therefore, no changes to Preferences/Batch Processing.

Tested quite extensively for Linux, not for the other OS.

Side note: When working on this, I found that I needed some automatism for initializing all pictures before batch editing them (setting IPTC tags is usually the very first thing I do), since I kept forgetting to do it manually. Which was the reason for my "initialization" proposal some time ago. Thanks for your comments there; I'll look into that one again as soon as I find the time.

@Lawrence37
Copy link
Collaborator

Nice work!

Since values are not numeric, the distinction between "add" and "replace" does not make sense; therefore, no changes to Preferences/Batch Processing.

For keywords and supplemental categories, a set mode could do a replacement, though I'm not sure how useful that would be.

@Lawrence37 Lawrence37 added type: enhancement Something could be better than it currently is scope: GUI Changes to GUI, not core functionality labels Sep 17, 2023
@Bezierr
Copy link
Contributor Author

Bezierr commented Oct 13, 2023

I had thought about this replacement functionality myself, but given that (a) I personally have never missed it, and (b) all the programs I know don't provide it, I thought it would be better to not unnecessarily complicate things. So I'd propose to leave it as it is (provided nobody finds any bugs, of course) for the time being.

@Bezierr
Copy link
Contributor Author

Bezierr commented Sep 20, 2024

Hi there,

for personal reasons, it's been quite a while since I last looked into RawTherapee (apart from using it for my pictures). Given that you seemed to like this proposal, I was a bit amazed to find that it neither made its way into the dev branch nor into the new releases.

As I always emphasize, it is, of course, entirely up to you what you include into main RawTherapee, and what not. Still, may I ask for the reason?

@Lawrence37
Copy link
Collaborator

For me, it dropped off my radar after waiting for others to comment. Then there were higher priorities for 5.11 so I decided to leave it for 5.12 unless someone else reviewed it sooner.

@Lawrence37 Lawrence37 added this to the v5.12 milestone Sep 21, 2024
Copy link
Collaborator

@Lawrence37 Lawrence37 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed your code uses tabs for indentation, but the surrounding code uses spaces. Could you change the tabs to spaces for consistency? GitHub shows the tabs with a width of 8 characters which makes your code look over-indented. I think your editor is set to display tabs with a width of 4.

Function-wise, this works perfectly.

// determine changes to IPTC tags of first selected picture
originalParams = initialPP[0];

if (pparams.metadata.iptc[CAPTION].at(0).compare(originalParams.metadata.iptc[CAPTION].at(0)) != 0) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This if-else-if chain belongs in IPTCPanel::write. The booleans like captionChangedFlag can be placed in ParamsEdited. The Glib::ustrings like captionChanged are not needed since they can be found in pparams. The exceptions are the added/deleted keyword/category, which can also go in pparams.

newParams = initialPP[i];
newParams = initialPP[i];

if (event == rtengine::EvIPTC) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move this block into ParamsEdited::combine.

@Bezierr
Copy link
Contributor Author

Bezierr commented Oct 7, 2024

Hi,
thanks for the feedback. I'll look into this as soon as I find the time, which unfortunately won't be during the next couple of weeks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope: GUI Changes to GUI, not core functionality type: enhancement Something could be better than it currently is
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants