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

[IOS] FIX: Return original filename instead of FullSizeRender #990

Merged
merged 4 commits into from
Oct 2, 2023

Conversation

xick
Copy link
Contributor

@xick xick commented Sep 20, 2023

  • Add method getUntouchedResource to retrieve original photo/video instead of altered one (with adjustments / filters)
  • Fix: When retrieving an asset title ( with entity.title or entity.titleAsync) now returns original file name instead of 'FullSizeRender.*' if this has adjustments.

Related Issues:
Fixes #976
Fixes immich-app/immich#4088

(IN TESTING) Add method to retrieve original photo/video instead of altered one. getTitleAsync now returns original filename instead of 'FullsizeRender'.
@@ -29,6 +29,7 @@ NS_ASSUME_NONNULL_BEGIN
- (nullable NSString*)mimeType;
- (BOOL)isAdjust;
- (PHAssetResource *)getAdjustResource;
- (PHAssetResource *)getUntouchedResource;
Copy link
Member

Choose a reason for hiding this comment

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

This seems to affect the file of Live Photos. What's the difference between them? Any specs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

getAdjustResource return by default media with adjustments ( PHAssetResourceTypeFullSizePhoto/Video) if there are any.
getUntouchedResource will always return original asset (PHAssetResourceTypePhoto/*Video).
LivePhotos are not affected by this, because there are already dedicated methods to deal with them. For example, when retrieving filename of an asset, will check if media type is LivePhoto and use the dedicated method:

- (NSString *)originalFilenameWithSubtype:(int)subtype {
    if (@available(iOS 9.1, *)) {
        if ([self isLivePhoto] && subtype == PHAssetMediaSubtypePhotoLive) {
            return [self getLivePhotosResource].originalFilename;
        }
    }
    PHAssetResource *resource = [self getUntouchedResource];
    if (resource) {
        return resource.originalFilename;
    }

getLivePhotosResource currently will always returns original video asset ( PHAssetResourceTypePairedVideo), and there are no method to retrieve the asset with adjustments, hence we could add this possibility.

@AlexV525
Copy link
Member

AlexV525 commented Oct 2, 2023

@xick Could you add the CHANGELOG entry and update the branch?

@xick
Copy link
Contributor Author

xick commented Oct 2, 2023

Done. I don't know if I should have pushed a single commit instead, sorry but I'm not familiar with PR.

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@AlexV525
Copy link
Member

AlexV525 commented Oct 2, 2023

Done. I don't know if I should have pushed a single commit instead, sorry but I'm not familiar with PR.

You don't have to, we will make a squash merge.

@AlexV525 AlexV525 merged commit e096d5e into fluttercandies:main Oct 2, 2023
10 checks passed
@alextran1502
Copy link

Hello @AlexV525, I apologize for the ping and the forbidden question.

Do you know when we can expect a new release with the library?

Thank you for your work!
Alex

@AlexV525
Copy link
Member

Do you know when we can expect a new release with the library?

Now. :)

@alextran1502
Copy link

@AlexV525 Thanks Alex! I appreciate you so much

@patelnirav48
Copy link

@AlexV525 Using latest version of library still getting FullSizeRender.jpg instead HEIC image and original file name. This happens when i use portrait image in iOS.

@AlexV525
Copy link
Member

AlexV525 commented Mar 5, 2024

@xick Is your fix managed to cover the above case?

@alextran1502
Copy link

@patelnirav48 @AlexV525 it works fine on my end for getting the correct file name for Portrait mode on Iphone 12 Pro

@AlexV525
Copy link
Member

AlexV525 commented Mar 5, 2024

@patelnirav48 Please share details so everyone at here can probably help to sort out the case.

@patelnirav48
Copy link

patelnirav48 commented Mar 5, 2024

Screenshot at Mar 05 17-02-15

@AlexV525 I captured from camera as Portrait as above. Not Portrait / Landscape mode.

Device - iPhone XR
OS - 17.1.2

@patelnirav48
Copy link

@alextran1502 @AlexV525 I'm using latest version of wechat_assets_picker, and that also using latest version of photo manager 3.0.0.. When i pick from gallery its return FullSizeRender.jpg.. even extension get changed with file name.

@maslanypotwor
Copy link

I was watching this PR and got notification so I checked from my side and indeed in Immich I can see that portrait photo I just took and backed up is indeed IMG_0327.HEIC

@AlexV525
Copy link
Member

AlexV525 commented Mar 5, 2024

@alextran1502 @AlexV525 I'm using latest version of wechat_assets_picker, and that also using latest version of photo manager 3.0.0.. When i pick from gallery its return FullSizeRender.jpg.. even extension get changed with file name.

So you're accessing with .file?

@patelnirav48
Copy link

@alextran1502 @AlexV525 I'm using latest version of wechat_assets_picker, and that also using latest version of photo manager 3.0.0.. When i pick from gallery its return FullSizeRender.jpg.. even extension get changed with file name.

So you're accessing with .file?

Accessing as .originFile

@AlexV525
Copy link
Member

AlexV525 commented Mar 5, 2024

I can reproduce this using iPhone 12 with iOS 17.3.1.

@alextran1502
Copy link

@AlexV525 Immich was still using 3.0.0-dev5, I wonder if you can test with that version to see if you can still reproduce this?

@AlexV525
Copy link
Member

AlexV525 commented Mar 5, 2024

@AlexV525 Immich was still using 3.0.0-dev5, I wonder if you can test with that version to see if you can still reproduce this?

I'm using 3.0.0.

@patelnirav48
Copy link

patelnirav48 commented Mar 7, 2024

@alextran1502 I tested with 3.0.0-dev5 its working fine (It's keeping file name and extension as it is). Just it's removing portrait background blur effect from the image.

CC. @AlexV525

@AlexV525
Copy link
Member

AlexV525 commented Mar 7, 2024

@alextran1502 I tested with 3.0.0-dev5 its working fine (It's keeping file name and extension as it is). Just it's removing portrait background blur effect from the image.

So are you running the same issue using the 3.0.0? It includes changes from dev.5 for sure.

@patelnirav48
Copy link

@alextran1502 I tested with 3.0.0-dev5 its working fine (It's keeping file name and extension as it is). Just it's removing portrait background blur effect from the image.

So are you running the same issue using the 3.0.0? It includes changes from dev.5 for sure.

In 3.0.0, It's not working.. but 3.0.0-dev5 its working, but just it's removing portrait background blur effect from the image.

@AlexV525
Copy link
Member

AlexV525 commented Mar 7, 2024

You can use git bisect to find out which commit introduces the regression. AFAIK there are no related changes.

@patelnirav48
Copy link

You can use git bisect to find out which commit introduces the regression. AFAIK there are no related changes.

Ah sorry, you made changes already before few hours, so its working. https://prnt.sc/Gp3PCJlpkfvJ. Thanks

@benmccann
Copy link

If I understand correctly, it seems that there were remaining issues (#976) fixed in #1098, but not yet released. So 3.0.0-dev.5 works, 3.0.0 does not, and when 3.0.1 is released then it should work again.

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