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

Android: encodingType is not ignored when sourceType is set with PHOTOLIBRARY or SAVEDPHOTOALBUM #769

Open
5r1m opened this issue Oct 12, 2021 · 5 comments

Comments

@5r1m
Copy link

5r1m commented Oct 12, 2021

As per documentation: https://cordova.apache.org/docs/en/10.x/reference/cordova-plugin-camera/#cameraoptions-errata
"encodingType" should be ignored when source type is PHOTOLIBRARY or SAVEDPHOTOALBUM. How ever when "encodingType" is set with "Camera.EncodingType.JPEG" and selecting a png picture from album results in unecessary transformation and is not as per the documentation

Screenshot 2021-10-12 at 1 04 47 PM

The reason seems to be unnecessary check for mime type of the choosen file to match the encodingType in options and when not matched it does the transformation:

getMimetypeForEncodingType().equalsIgnoreCase(mimeTypeOfGalleryFile))

Is this a bug or a known behavior? Atleast there is difference in documentation and implementation and there is no option to avoid transformation with out providing the same encoding type which is not possible to provide before hand

@breautek
Copy link
Contributor

Looks like this might have been a regression introduced from #731

@5r1m
Copy link
Author

5r1m commented Oct 18, 2021

Thank @breautek for the hint. In such case, can the PR be reverted @PieterVanPoyer ? Also we are seeing weird problems in this use case, like file name changing to png.jpg in pixel devices and modified.jpg in samsung devices (same name for every image being picked) etc. by which its overwriting in temp folder by the last selected picture. So apart from unnecessary conversion, it leading to other issues as well.

@PieterVanPoyer
Copy link
Contributor

Hey @5r1m

I think you are correct. The documentation and implementation are not in sync. Something must be done to fix this mismatch.
I checked the iOs version, and that version always transforms to the encodingType.
So, I think it would be better to keep this 2 implementations behave the same, and change the documentation.

At this point there is still a difference between these 2 implementations. On Android only next formats are being transformed -

return JPEG_MIME_TYPE.equalsIgnoreCase(mimeType) || PNG_MIME_TYPE.equalsIgnoreCase(mimeType)
|| HEIC_MIME_TYPE.equalsIgnoreCase(mimeType);

On iOs, a Gif, a BMP, and more types are being transformed to the encodingType.

Kind regards
Pieter

@5r1m
Copy link
Author

5r1m commented Oct 22, 2021

Hi @PieterVanPoyer

Yes, ideally iOS and Android should be in sync and especially when they are not specific to any platform support/limitation. How ever now the question is

  • Should there be transformation done?
    IMHO No. Firstly this is unmanaged break in the library and secondly transformation is not required in all the cases.

  • Should transformation be completely avoided
    Again, No. As in case of HEIC, transformation might be still required.

But I think it's not like one thing fits all. In this new version, there is no way to avoid transformation 100% until we know the mime type which user will be selecting well ahead, which is not possible. So instead of forcing the transformation in such cases, why not extend the API to provide options on doing the transformation for selected types only? This way it gives control on transformation to be done or not to API consumers which would be more optimal.

Thanks.

@igorsantos07
Copy link

I know this is almost a year old and the library seems abandoned, but here are my two cents from a fellow dev which is trying (hard) to use the library:

While keeping the conversion would break compatibility, I'd say bump the minor (maybe even the major version, I know it feels harsh but that's what major versions are for) and KEEP the transform! It makes NO sense on:

  1. having a different behavior on iOS when you're coding on a hybrid platform such as Cordova
  2. setting an option which might be ignored depending on another seemingly unrelated option
  3. Lastly, it MIGHT be required by the developer (such as us) since we might be doing image transformation later and we require a fixed image type.

If it is technically possible to convert the picture and always return a given type, why skip that step? When reading the docs, I thought it was a tech limitation, but it looks like it was by design?

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

No branches or pull requests

4 participants