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

Live Photos: Use HEIF to create primary JPEG instead of a MOV still image #926

Closed
alxjsn opened this issue Jan 20, 2021 · 20 comments
Closed
Assignees
Labels
idea Feedback wanted / feature request released Available in the stable release ux Impacts User Experience

Comments

@alxjsn
Copy link

alxjsn commented Jan 20, 2021

When media gets detected as a live photo, the thumbnail that gets generated is based on the video file and not the original photo. The quality tends to be poor and is especially noticeable when there is motion.

Here is an example of a thumbnail that looks blurry in the thumbnail:
1611106685-select

What it should look like:
1611107091-select

Example live photo:
live-photo.zip

Can the thumbnail be based on the original photo instead of the video file?

@lastzero lastzero changed the title Switch live photo thumbnails UX: Automatically select primary JPEG with best resolution Jan 20, 2021
@lastzero
Copy link
Member

Yes, you can manually select a different primary JPEG for live photos that include a HEIC image file:

Screenshot 2021-01-20 at 08 44 31

We don't do this automatically right now as it might overwrite manual changes. Going to take a look at his later.

@lastzero lastzero self-assigned this Jan 20, 2021
@lastzero lastzero added the idea Feedback wanted / feature request label Jan 20, 2021
@alxjsn
Copy link
Author

alxjsn commented Jan 21, 2021

Thanks for adding this to the roadmap! How exactly do I change the the thumbnail manually/main image?

When I go into the files section of an image I don't even see a file with an heic.jpg extension.

1611192700-select

@lastzero
Copy link
Member

Maybe you need to reindex. This was added recently.

@lastzero lastzero changed the title UX: Automatically select primary JPEG with best resolution UX: Automatically select JPEG with best resolution as primary Oct 14, 2021
@lastzero lastzero added the ux Impacts User Experience label Oct 14, 2021
@graciousgrey graciousgrey changed the title UX: Automatically select JPEG with best resolution as primary Stacks: Automatically select JPEG with best resolution as primary Nov 2, 2021
@lastzero lastzero added the please-test Ready for acceptance test label Jan 3, 2022
@lastzero lastzero changed the title Stacks: Automatically select JPEG with best resolution as primary Live Photos: Automatically select HEIF as primary image, if available Jan 3, 2022
@lastzero lastzero changed the title Live Photos: Automatically select HEIF as primary image, if available Live Photos: Select related HEIF files as primary image Jan 3, 2022
@joachimtingvold
Copy link

joachimtingvold commented Jan 6, 2022

I have no <name>.heic.jpg either, and I'm running latest. All my photos have been imported+indexed on 211215-93b26f19. I assume "this was added recently" (from Jan 2021) should be present in that version? I.e. that it should create <name>.heic.jpg, even though it selects <name>.mov.jpg as primary (which is fixed in preview now).

@lastzero
Copy link
Member

lastzero commented Jan 6, 2022

We suggest testing / reindexing with the upcoming UPDATE. Just pushed a few more improvements.

@lastzero
Copy link
Member

lastzero commented Jan 6, 2022

I think what I meant a year ago is that the creation of JPEGs from HEIFs was added recently, not the creation of individual JPEGs for every other file type in a stack. However, I do recall reports from users who said they had multiple JPEGs, but the best one was not used as the primary one.

In January 2021 I may not have known that we also need to change the priority of the main file type from Video to HEIF, which we did 3 days ago. We didn't have a recent iPhone for testing and from what I remember, the test files we received at that time had the same resolution in the related MOV and HEIC files.

We're both exhausted after dealing with all your bug reports and edge cases. Maybe you can search GitHub for related issues and comments? Running the convert command may help, for example.

@lastzero lastzero changed the title Live Photos: Select related HEIF files as primary image Live Photos: Use HEIF as primary file instead of a MOV still image Jan 6, 2022
@lastzero lastzero changed the title Live Photos: Use HEIF as primary file instead of a MOV still image Live Photos: Use HEIF to create the primary JPEG instead of a MOV still image Jan 6, 2022
@joachimtingvold
Copy link

I think what I meant a year ago is that the creation of JPEGs from HEIFs was added recently, not the creation of individual JPEGs for every other file type in a stack.

In other words, "create <file>.heic.jpg from HEIC" should, by that definiton, work in latest? Or am I misunderstanding you? I'm just trying to figure out if I have an edgecase here or not.

I know the "use best stacked photo as primary" is only fixed in preview.

Just trying to understand what would be the expected outcome of HEICs in latest. As far as I understand you, HEICs should generate the following stack using latest:

<name>.heic
<name>.heic.jpg
<name>.mov
<name>.mov.jpg (primary)

While in preview it should generate the following;

<name>.heic
<name>.heic.jpg (primary)
<name>.mov
<name>.mov.jpg

In my case, using latest, I get the following for my HEICs;

<name>.heic
<name>.mov
<name>.mov.jpg (primary)

In January 2021 I may not have known that we also need to change the priority of the main file type from Video to HEIF, which we did 3 days ago. We didn't have a recent iPhone for testing and from what I remember, the test files we received at that time had the same resolution in the related MOV and HEIC files.

Sure. Which is why I'm somewhat confused, since I do not have the <name>.heic.jpg at all (same as OP pointed out in Jan 2021).

We're both exhausted after dealing with all your bug reports and edge cases.

Don't burn yourselves out. As I've already said before; I have no expectation from anyone to fix things. Most issues I've seen here on GitHub has been of the "very nice to have" or "minor annoyance", so I'd say that PP in general is in a good state (i.e. you shouldn't feel like you have to fix everything "yesterday" (-: ).

Maybe you can search GitHub for related issues and comments? Running the convert command may help, for example.

Already done that, which lead me to this issue. convert is running successfully via daily ofelia task. Same with index --cleanup.

@lastzero lastzero changed the title Live Photos: Use HEIF to create the primary JPEG instead of a MOV still image Live Photos: Use HEIF to create primary JPEG instead of a MOV still image Jan 6, 2022
@lastzero
Copy link
Member

lastzero commented Jan 6, 2022

It's possible that import only creates the primary JPEG while index creates both.

@lastzero
Copy link
Member

lastzero commented Jan 6, 2022

In other words, "create <file>.heic.jpg from HEIC" should, by that definiton, work in latest? Or am I misunderstanding you? I'm just trying to figure out if I have an edgecase here or not.

Yes, unless users have a custom runtime environment without HEIF support.

This issue here is specifically for Live Photos with multiple related non-JPEG files.

In my case, using latest, I get the following for my HEICs;

Index or import? Does a full rescan create a JPEG for the other file? You can limit it to specific folders for testing.

Don't burn yourselves out.

No worries... just saying that we can't answer detailed technical questions right now and might also not remember to come back to them at a later time. Aside from that, we appreciate your feedback!

Feel free to create follow up issues if you need additional improvements. If it's just "nice to have" and you don't really need it, I suggest we focus on multi-user without iterating further.

@graciousgrey graciousgrey added released Available in the stable release and removed please-test Ready for acceptance test labels Jan 7, 2022
@joachimtingvold
Copy link

After doing a force re-index, it now seems to correctly create <name>.heic.jpg, and sets that as a primary, for live photos.

However, I've observed two things so far;

  1. Old <name>.mov.jpg files are not removed. A fresh indexed live photo only contains <name>.heic, <name>.heic.jpg and <name>.mov. Maybe purging old <name>.mov.jpg makes sense, since they are no longer used/needed?

  2. The current commits does not seem to resolve stacked live photos. I have several "double" live photos that has been automatically stacked, and they now look like this;

image

... where 20191210_213735_04E22393.mov.jpg is the primary photo. It has successfully created <name>.heic.jpg for the primary stack photo, but it is not set as primary for the stack.

Also, if I at some point wanted to change the primary photo for a stacked live photo, it currently does not seem to create <name>.heic.jpg for the secondary photos. From the screenshot above, 20191210_213735_59BB2523.heic.jpg is missing.

@lastzero
Copy link
Member

lastzero commented Jan 8, 2022

Indexing won't delete files and won't break up existing stacks. You need to do this manually. The potential damage in case of a bug would be far too big.

@lastzero
Copy link
Member

lastzero commented Jan 8, 2022

Also some users may prefer the video still, for example because it reduces flickering on mouse over. It's not up to us to decide this.

@joachimtingvold
Copy link

joachimtingvold commented Jan 8, 2022

Indexing won't delete files and won't break up existing stacks. You need to do this manually. The potential damage in case of a bug would be far too big.

I did not expect it to either. It was just an observation that new live photos only had 3 files, but old ones have 4 (due to the extra <name>.mov.jpg). If that's "working as intended", it's fine by me.

Also some users may prefer the video still, for example because it reduces flickering on mouse over. It's not up to us to decide this.

But it no longer creates <name>.mov.jpg for live photos, so that's not really an option longer anyways?

@joachimtingvold
Copy link

joachimtingvold commented Jan 8, 2022

Actually, I have found several "single" live photos with <name>.heic.jpg created, but it's not set as primary. The old <name>.mov.jpg is still the primary. Are the commited changes meant to address exisiting photos, or just new ones? Does it require more than just index -f to resolve, if it's supposed to fix existing photos as well?

image
(20200715_121819_9D882AA0.mov.jpg is set as the primary for this one)

@lastzero
Copy link
Member

lastzero commented Jan 8, 2022

Nope, just new ones. There's also a rule that assumes videos don't need a still extracted if there already is an image in the stack. But it doesn't mean existing stills will be deleted automatically.

@lastzero
Copy link
Member

lastzero commented Jan 8, 2022

Besides, you could also have created JPEGs manually. It's not feasible to track the origin and change history of all files for now.

@joachimtingvold
Copy link

joachimtingvold commented Jan 8, 2022

And what would be the proper procedure to get <name>.heic.jpg set as primary for live photos for existing photos without nuking the database? I guess I can fix it manually, but.

@lastzero
Copy link
Member

lastzero commented Jan 8, 2022

SQL UPDATE for now 😜

@joachimtingvold
Copy link

joachimtingvold commented Jan 8, 2022

Haha. OK. Maybe one future change would be to differentiate between "automatically set file_primary" and "manually set file_primary"? That way, future changes to logic that dictates "what photo is primary" could be automatically be updated to whatever is default, unless someone has manually overridden the primary for a stack (and in those cases, we could provide a "force" flag to override even manually set primary).

@lastzero
Copy link
Member

lastzero commented Jan 8, 2022

Yep. Requires a schema change, so we didn't implement it this time. Will happen eventually.

lastzero added a commit that referenced this issue Aug 18, 2023
Signed-off-by: Michael Mayer <michael@photoprism.app>
lastzero added a commit that referenced this issue Aug 18, 2023
Signed-off-by: Michael Mayer <michael@photoprism.app>
lastzero added a commit that referenced this issue Aug 18, 2023
Signed-off-by: Michael Mayer <michael@photoprism.app>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
idea Feedback wanted / feature request released Available in the stable release ux Impacts User Experience
Projects
Status: Released 🌈
Development

No branches or pull requests

4 participants