Enable support for image/png

[frlan]

This PR enables support for PNG

Fixes #208

[skjnldsv]

Hey!
Thanks for this, we're still not sure this is the way to go as mentionned by @jancborchardt in the issue 🤔

Let's see

[jancborchardt]

Yeah, let's include PNG and then see how we can mitigate the issue of "sprites showing up" otherwise. 👍

[skjnldsv]

Yeah, let's include PNG and then see how we can mitigate the issue of "sprites showing up" otherwise.

I suggest a min size?

[jancborchardt]

Yeah, let's include PNG and then see how we can mitigate the issue of "sprites showing up" otherwise.

I suggest a min size?

Sure, let's do that! :) I assume image dimensions/pixels, right? Do you want to pick a reasonable size to start with and we can adjust it if needed?

[skjnldsv]

I assume image dimensions/pixels, right? Do you want to pick a reasonable size to start with and we can adjust it if needed?

No we don't store this kind of data, I would pick a min file size, which should be ok up to a certain point.
I mean discarding files under 50kb (random number), would mostl likely ignore what we don't want like sprites and other similar things?

[frlan]

Personally I don't think filtering it on this level (like size) is not a good idea. In past I had e.g. issues with the gallery app not generating thumbnails for IIRC 20MB+ files. Setting a minimum will add similar issues to users but in another part of the show. Also I'm a little woried about performance of huge cloud accounts as this would need more extented parsing on every file.

[skjnldsv]

Well, this goes both way, if we add png we suddenly open a lot more files too. So it might completely make the photos app unusable if someone suddenly have 10k png sprites appearing out of nowhere 😕

[frlan]

Well.... In both cases adding a setting to only search in special folders would be a nice idea I guess. But this is not blocking this PR here (IMHO)

[skjnldsv]

But this is not blocking this PR here (IMHO)

Well, if adding this pr makes the app unusable, it is a blocker ;)
If you had to choose a min size, what would you use?

[frlan]

Puh. Hard question. I did run a short check on my archive I'd like to see at the app:

min size: 125kB
max size: 73457kB
avg size: 10184kB

Having a look into actual sizes of the canvas I came along this values:

min: 352 x 288 px
max: 5040 x 6968 px

Still thinking it's not an optimal idea -- I would set it to something like 256x256px or 100MB

[jancborchardt]

First we are only talking about not having sprites shown, via min size→ so let’s start with that? :)

[frlan]

Right -- So 100kB and/or 256x256 :)

[skjnldsv]

Ah yes, we're not talking about max here, only min :)
Thanks for your help @frlan!

[jancborchardt]

@frlan would you like to add those minimum limitations to your pull request? :)

[frlan]

To be honest -- unlikely. As mentioned above I don't think it's the right approach of solving the issue.

[lpilz]

Any news on this, @frlan? I would really like to see pngs included :)

[frlan]

@lpilz Not from my end. As I said I will not do it as I think the requested solution is not good.

[skjnldsv]

To be honest -- unlikely. As mentioned above I don't think it's the right approach of solving the issue.

What would be the correct solution according to you? :)

[sdalmeida]

@frlan any updates on your proposed solution?

[frlan]

If you are looking for some match-winning-idea from my end -- I'm sorry. I don't have them.

My personal recommendation: Please don't implement any fancy filtering that are not optional -- all of them will have their issues.

Just some examples:

• Filtering by file size will cause the application to only show pictures big enough. What is with good compressed jpeg or png?
• Filtering by size in pixels will touch each and every picture -- maybe again and again.
• Filtering by file type makes actually really sense but will end up in discussions like this one here everytime a new filetype is supported by e.e. imagemagick

IMHO just show PNG (and maybe other media files) and keep the filtering up to the file system hierarchy.

Sorry for not having more constructive idea here.

[sdalmeida]

I do agree with @frlan's proposed solution. Its a photo viewer after all. If you have PNGs (regardless of size) it should get rendered/displayed. You (the user) will decide whether or not you want the image.

If we are saying that merging this PR will cause issues, how come we have Nextcloud 19 Viewer with no support for HEIC 😉. I know a PR was just merged to support it, but then again, it was launched without support for the biggest image format for iOS devices.

In any case, I digress. I'm just a user of the system. It's up to the PO to decide what the project can and can't do. I think its great that we are thinking of different solutions to tackle this issue, but it comes a point where we need to decide what to do (this PR has been opened for 6 months now).

Lets decide so that this can finally get merged :)

[Derkades]

I think a minimum size is a reasonable solution.

What is with good compressed jpeg or png?

I wouldn't call a 100KB file "good compressed". Even whatsapp images, which are compressed quite heavily with bad quality, still end up being 150-500KB. Only very small images like cropped screenshots won't show up but I doubt anyone would want to see those in their timeline anyway.

In my opinion the 'Your albums' view doesn't need this filter because in case of a bunch of sprites they'll be hidden away in a folder. This way, if someone does want to view their folder with tiny screenshots in Photos they still can.

Adding a filter for specific views only is probably outside the scope of this PR though.

[skjnldsv]

Let's take like 50KB then?

[frlan]

Where is the app getting this information from -- didn't found the code for? I've got the bad feeling this will at least double IOPS for generating the gallery as for every file there needs to be an additional syscall.

[skjnldsv]

Where is the app getting this information from -- didn't found the code for?

Should be filterable from the dav request we do.

[rcdailey]

Why doesn't the KISS principle apply here? Include PNG for now and add filters later based on UAT/feedback. Let the user decide what aspects of the library should be included in the gallery or not. For example, a lot of gallery apps on Android will include a lot of PNG files from app data (icons, etc). But I simply add a .nomedia file in these directories to exclude them.

If you start filtering automatically for the user based on file size, etc then you can run into problems where the user wonders why their files aren't showing up. I personally think the diff is fine as-is. I feel like some folks are over-thinking this. Can't this be done iteratively and adjusted based on user feedback?

The immediate issue is that PNG files do not show up at all. Let's solve one problem at a time.

[skjnldsv]

But I simply add a .nomedia file in these directories to exclude them.

This is far from what I call "simple". This is not ok for regular users.
Nonetheless, I'm tired of arguing, I'll merge this and revert if people complains.

[rcdailey]

But I simply add a .nomedia file in these directories to exclude them.

This is far from what I call "simple". This is not ok for regular users.
Nonetheless, I'm tired of arguing, I'll merge this and revert if people complains.

First of all, if you look at this as an "argument" that's a bit counter-productive. I think we're just sharing ideas and perspectives. At the end of the day we all want different things, I think it's a good thing to hear from other people. So please try to have a more positive outlook on the discussion. I'm not bashing on anyone else's opinion; just sharing my own.

Secondly, I feel like you're missing the point of my feedback. The specific implementation of how we allow the user to configure their library is irrelevant in the grander scheme of things. What is important is user choice; I don't think the software should make assumptions or decisions for me about what I want seen in my photo library. This could be implemented any number of ways. The .nomedia is one solution. You could even develop a UI on top of it to make it very easy for users to do this. All of this can be discussed, prioritized, and planned for later.

For now, just don't lose sight of the initial goal: Enable PNG support in libraries. That can be refined and polished over time. As a software engineer, I really see value in the agile approach to this.

Thanks for the continued discussion on this topic. I really appreciate the opportunity to have my opinions read.

EDIT: I also want to specifically address this part of your comment:

...revert if people complains

I strongly suggest you do not revert unless it is creating a critical software issue. I think a better "fail-safe" would be a feature toggle: If you are not confident in this feature, have it behind a feature toggle and make it opt-in. However, I personally think for something this simple that is unnecessary, but I would not object to it.

Ultimately users will never be 100% happy with every feature. If we revert every time that happens it would be chaotic. Please consider another option (I gave examples above).

[skjnldsv]

First of all, if you look at this as an "argument" that's a bit counter-productive. I think we're just sharing ideas and perspectives. At the end of the day we all want different things, I think it's a good thing to hear from other people. So please try to have a more positive outlook on the discussion. I'm not bashing on anyone else's opinion; just sharing my own.

Oh definitely! That was a poor choice of words on my side! Thanks for telling me!
I just feel like this have been going for far too long and needs to move on! 🚀

Ultimately users will never be 100% happy with every feature.

Welcome to my life on this repo

If we revert every time that happens it would be chaotic. Please consider another option (I gave examples above).

Definitely. I won't revert out of the blue for a few voices. But at the same time, people needs to be heard (or not). Cannot have both. So why would adding png support (a few people raised their voices here) have a stronger value than people raising their voices against it?
The issue is we always are biased on issues/features requests. Because the only people that are here are the one that are unhappy, you never see people just passing by and stating they like how it is 😉

In any case, let's merge and see! Thanks for your consideration! 🤗

[Ac1drainn]

I have an interesting idea to mitigate the issue of pngs and sprites showing in photos.
Maybe have configuration or a check mark button for that directory to whitelist / blacklist a directory from being shown in the gallery?

[jancborchardt]

Maybe have configuration or a check mark button for that directory to whitelist / blacklist a directory from being shown in the gallery?

Then we just offload fixing problems to every single user, which is not desirable.

[rhoni]

You just had to call it Camera, not Photos.