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

feat(web): synchronise metadata and album membership between duplicate images #13851

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

EinToni
Copy link

@EinToni EinToni commented Oct 31, 2024

Description

This is my first time contributing here and I hope it is ok to open a PR without creating an issue. This PR arises from the feature request #10262 and in part from #10206. In both more information is wished as well as the possibility to synchronize metadata between the duplicated images.

PR Content

This PR introduces the ability to automatically:

  • Synchronize the albums of duplicated images
    • Meaning: The deduplicated images are all put in all albums any duplicate was part of.
  • Synchronize the favorites
    • Meaning: If any of the duplicates was marked as favorite, the deduplicated images are also marked as favorites.
  • Synchronize the archive
    • Meaning: If any of the duplicates was archived, also archive the deduplicated images.

These three settings can be individually toggled:
image

I also added an indicator for the archive status of any image, same as the favorites icon:
image

And lastly I added the possibility to manually select the date/time, description and location to be synched to the deduplicated kept images. I am not that happy with this solution, but these are all fields that could completely differ from each other so no automatic resolution seemed fitting here.
image
In this example the remaining two images would be marked as favorites, marked as archived, have the location set to Paris, the description to "thisis some test" and the timestamp to 9. Jan. 2023 Mo, 07:15.

Open work

The size of the showed duplicates can now vary and therefore not align with each other. This looks very bad, but I didn't find any nice way to fix this (it's my first time using svelte and I'm not doing web development too often).

@bo0tzz bo0tzz changed the title feat(web): Synchronise metadata and album membership between duplicate images feat(web): synchronise metadata and album membership between duplicate images Oct 31, 2024
@bo0tzz
Copy link
Member

bo0tzz commented Oct 31, 2024

Cool stuff, thank you!

The UI is a little unclear to me at the moment, I'm hoping @alextran1502 has some ideas there. My suggestion would be to separately display another card showing what the deduplicated end result will look like. Just a super quick and dirty browser devtools mockup:
image

@EinToni
Copy link
Author

EinToni commented Nov 4, 2024

Thank you for the suggestion! I like it and it looks great in your example. But where should we put the "preview" card when there are more duplicates (like in my initial example)? The best would probably be to put it below the rest, but I don't know how clean the look then would be.

Also after looking at this again after almost a week, the red and green highlights really clutter up the UI. My idea was to highlight the metadata in red that gets "lost", as it will be overwritten by other data. But I would say removing the red highlighting in favor for proper lineup of the fields will help the readability quite a lot.

I will give the lineup of the data another try and remove the red highlights. Afterwards I can put in your preview card to see how it feels in action.

@EinToni
Copy link
Author

EinToni commented Nov 5, 2024

I decided against a separate preview asset because I saw an example on my regular immich instance, where some duplicates are looking visually slightly different. There I want to keep e.g. 2 of 5 images. Showing this in a single preview would only confuse as I would have to pick some image to diplay.

I therefore opted to display the preview directly in the selected assets (and only in the selected ones). Previously I highlighted all metadata that is different from the selected one. Now only the selected assets reflect the changes that will be done. In my opinion this has improved the usability a lot.
I also adapted the shape of the highlight to match the rest of the UI.
image

Usability feels better:
https://github.com/user-attachments/assets/c5b02bdb-73e8-4d9f-9e89-7f109552fb07

I would be happy about further feedback :)

@bo0tzz
Copy link
Member

bo0tzz commented Nov 5, 2024

Good stuff!

There I want to keep e.g. 2 of 5 images. Showing this in a single preview would only confuse as I would have to pick some image to diplay.

What about displaying both?

@EinToni
Copy link
Author

EinToni commented Nov 5, 2024

Displaying every selected asset is another "preview" card would clutter up the UI quite quickly. And when selecting/deselecting some duplicates the previews would probably dynamically pop up and vanish, dependent on the amount of selected duplicates. This could lead to a lot of confusion and also distinguishing between the previews to tell which preview relates to which duplicate could then become harder than necessary.
Therefore I would say a direct preview of the changes in the already existing displays is better.

@mertalev
Copy link
Contributor

mertalev commented Nov 5, 2024

Wow, awesome stuff! My thought is that the syncing for albums, favorites, etc. is relatively straightforward while metadata is harder to display and edit in an intuitive way. What do you say about handling the former in this PR and the latter in a separate PR?

@EinToni
Copy link
Author

EinToni commented Nov 6, 2024

Thank you!
We can of course split it up, but as the backend behavior is pretty equal, the only real difference is the UI. If we can come to an agreement for the UI, I can also just do it all in this PR.

But if that agreement sounds unrealistic any time soon I'll split it. I would leave that decision up to the immich team.

@NicholasFlamy
Copy link
Member

I decided against a separate preview asset because I saw an example on my regular immich instance, where some duplicates are looking visually slightly different. There I want to keep e.g. 2 of 5 images. Showing this in a single preview would only confuse as I would have to pick some image to diplay.

I therefore opted to display the preview directly in the selected assets (and only in the selected ones). Previously I highlighted all metadata that is different from the selected one. Now only the selected assets reflect the changes that will be done. In my opinion this has improved the usability a lot. I also adapted the shape of the highlight to match the rest of the UI. [image]

Usability feels better: https://github.com/user-attachments/assets/c5b02bdb-73e8-4d9f-9e89-7f109552fb07

I would be happy about further feedback :)

I will comment, the text is touching the edges of the box in some places, if you could put at least like a 2px margin/padding that would probably do it. (Not too much ofc, we still need content, but just enough that it doesn't touch.)

@EinToni
Copy link
Author

EinToni commented Nov 13, 2024

@bo0tzz What will be the further process to continue with this MR?

@bo0tzz bo0tzz requested a review from alextran1502 November 13, 2024 12:45
@alextran1502
Copy link
Contributor

@EinToni hey, sorry for leaving this open. I am currently working on refactoring the code base to the new Svelte 5 syntax. Once that is done I will come back to this

@EinToni EinToni force-pushed the deduplicate-sync-album branch from 855b138 to 3353134 Compare November 27, 2024 17:51
@EinToni
Copy link
Author

EinToni commented Nov 27, 2024

@alextran1502 I rebased onto main and adapted my code for svelte 5. I would be happy to hear your feedback about the feature when you find time for it.

@EinToni EinToni force-pushed the deduplicate-sync-album branch from 024bde9 to 921ae76 Compare December 28, 2024 12:34
@Gorbush
Copy link

Gorbush commented Jan 3, 2025

A with to also sync:

  1. Tags of images
  2. Star rating of images

@@ -80,11 +85,56 @@
});
};

const handleResolve = async (duplicateId: string, duplicateAssetIds: string[], trashIds: string[]) => {
const handleResolve = async (
Copy link

Choose a reason for hiding this comment

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

handleDeduplicateAll isn't updated to respect the new settings. Ideally, both bulk and individual resolutions should behave the same.

Choose a reason for hiding this comment

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

locally hacked up a version that does this. Definitely unpolished and imperfect, but sharing here in case there's anything useful.

I'm not really convinced that 'fetch all the album memberships up front' is the most efficient approach vs requesting per photo. Probably depends on the number of dupes vs overall album size. Might make sense when there are tons of dupes, but otherwise not.

const handleDeduplicateAll = async () => {
    console.log("dedupe all");
    
    const albumIdToAddedAssets = new Map<string, Set<string>>();
    const assetIdToAlbums = new Map<string, Set<string>>();

    {
      const relevantAssetIds = new Set<string>();
      duplicates.forEach((group) => group.assets.forEach((asset) => relevantAssetIds.add(asset.id)));

      const allAlbums1 = await getAllAlbums({});
      const allAlbums = await Promise.all(allAlbums1.map((album) => getAlbumInfo({
          id: album.id,
          withoutAssets: false
        })));
      allAlbums.forEach((album) => {
        albumIdToAddedAssets.set(album.id, new Set<string>());
        album.assets.forEach((asset) => {
          if (relevantAssetIds.has(asset.id)) {
            if (!assetIdToAlbums.has(asset.id)) {
              assetIdToAlbums.set(asset.id, new Set<string>());
            }

            assetIdToAlbums.get(asset.id)?.add(album.id);
          }

        })
      });
    }

    debugger;

    let newFavorites = new Array();
    const idsToDelete = new Set<string>();
    const idsToKeep = new Array();

    duplicates.forEach((group) => {
      
      const assetToKeep = suggestDuplicateByFileSize(group.assets);
      const assetsToDelete = group.assets.filter((asset) => asset.id !== assetToKeep.id);
      
      if (assetToKeep.exifInfo.dateTimeOriginal > '2017-01-01'
         && assetsToDelete.some((asset) => asset.originalFileName !== assetToKeep.originalFileName)
         && assetsToDelete.some((asset) => asset.exifInfo.dateTimeOriginal !== assetToKeep.exifInfo.dateTimeOriginal)) {
        return;
      }

      idsToKeep.push(assetToKeep);

      assetsToDelete.forEach((asset) => idsToDelete.add(asset.id));

      const albumIds = [...new Set(assetsToDelete.filter((asset) => assetIdToAlbums.has(asset.id)).flatMap((asset) => [...assetIdToAlbums.get(asset.id)]))];

      albumIds.forEach((albumId) => {
        if (assetIdToAlbums.has(assetToKeep.id) && assetIdToAlbums.get(assetToKeep.id)?.has(albumId)) {
          return;
        }

        albumIdToAddedAssets.get(albumId).add(assetToKeep.id);
      });

      if (!assetToKeep.isFavorite && assetsToDelete.some(asset => asset.isFavorite)) {
        newFavorites.push(assetToKeep.id);
      }
    });

    debugger;
    await updateAssets({assetBulkUpdateDto: { ids: newFavorites, isFavorite: true}});

    debugger;
    await Promise.all(albumIdToAddedAssets.entries().filter((entry) => entry[1].size > 0).map((entry) => addAssetsToAlbum(entry[0], [...entry[1]], false)));

    debugger;

    await deleteAssets({ assetBulkDeleteDto: { ids: Array.from(idsToDelete), force: !$featureFlags.trash } });
    debugger;
    await updateAssets({
      assetBulkUpdateDto: {
        ids: [...idsToDelete, ...idsToKeep.filter((id): id is string => !!id)],
        duplicateId: null,
      },
    });

    duplicates = [];

    deletedNotification(idsToDelete.length);

    return;
  };

Copy link
Contributor

Choose a reason for hiding this comment

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

Honestly, this should just be handled in the backend. All these queries and set operations can be handled much more efficiently with a single specialized DB query.

Copy link
Author

Choose a reason for hiding this comment

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

But if we automatically sync album membership while deduplicating single images, it should also be done when deduplicating all. Otherwise this would be quite unexpected behavior for the end user.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not saying we shouldn't handle syncing when deduplicating all assets, just that the client is not a great place to be doing this. The DB is much better at doing these operations than JS, can do it without moving all of this data back and forth, and can make the entire process atomic. The bulk dedupe arguably should have been a server endpoint from the start, but with this extra layer of functionality it's just too much to keep in the client.

@EinToni EinToni force-pushed the deduplicate-sync-album branch from 921ae76 to e8b0e03 Compare January 28, 2025 19:34
@EinToni
Copy link
Author

EinToni commented Jan 28, 2025

Hey,
after not doing anything for a month I just rebased onto main (214 commits!! crazy fast development).
Because I don't see this moving forward in a state like this I decided to do what @mertalev already suggested in the beginning. I split it up into the basic synchronization of favorite/archive/album and the more complex UI part. I will move the UI part into its own branch (unless someone contradicts), so this pull request here will only be for the basic automatic sync part.

I hope with this smaller scope and simpler functionality it will be possible to continue and maybe even merge this in the near future.

Edit: If someone searches the original code for the UI-metadata selection, it lives here now: https://github.com/EinToni/immich/tree/deduplicate-UI-metadata

* Added visual indication next to the favorite icon to the images if they are archived.
* Added new settings menu to the the deduplication tab.
* The toggable options in the settings are synchronization of: albums, favorites, archives.
* When synchronizing the albums, all albums will be added to all albums of the duplicates. This way the final deduplicated image is in all albums and also the removed images are in these albums if they may be restored.
* When synchronizing the favorite/archive status, all images will be marked as favorite/archived, if only a single image is in the favorite/archived.
* The selected image to keep will be marked with a red favorite/archive symbol if that status will be applied after the deduplication.
@EinToni EinToni force-pushed the deduplicate-sync-album branch from e8b0e03 to bcd98d7 Compare January 29, 2025 06:36
"synchronize_archives" : "Synchronize Archives",
"synchronize_archives_description" : "If one of the duplicates is a favorite, mark all deduplicated as favorites.",
"synchronize_favorites" : "Synchronize Favorites",
"synchronize_favorites_description" : "If one of the duplicates is already archived, also archive the deduplicated result.",

Choose a reason for hiding this comment

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

Mix up of "synchronize_favorites_description" and "synchronize_archives_description".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants