Skip to content
This repository has been archived by the owner on May 3, 2021. It is now read-only.

Shutter speed for long time exposures is displayed as fraction #149

Closed
cryptomilk opened this issue Dec 12, 2018 · 7 comments
Closed

Shutter speed for long time exposures is displayed as fraction #149

cryptomilk opened this issue Dec 12, 2018 · 7 comments
Assignees
Labels
enhancement New feature or request Low Priority Low priority issues
Milestone

Comments

@cryptomilk
Copy link

Detailed description of the problem

A shutter speed of 8s exposure is displayed as 8/1 sec, it should be displayed as just 8 sec.

Example:
https://pixelbook.org/#15392672842961/15415025203602

@ildyria
Copy link
Member

ildyria commented Dec 13, 2018

Quick fix: if the string ends by /1 then we remove the /1.
However I don't know how this will do with times like 5,5 seconds...
@cryptomilk If you have such example can you check?

@cryptomilk
Copy link
Author

There is 16/10 which should be 1.6 sec.

Example: https://pixelbook.org/#15392672842961/15429956116655

@ildyria ildyria reopened this Dec 13, 2018
@ildyria ildyria added bug Something isn't working enhancement New feature or request Low Priority Low priority issues and removed bug Something isn't working labels Dec 13, 2018
@ildyria ildyria self-assigned this Dec 25, 2018
ildyria added a commit that referenced this issue Dec 26, 2018
@ildyria ildyria added this to the v3.2.8 milestone Dec 26, 2018
@dswarbrick
Copy link

dswarbrick commented Dec 27, 2018

This is still broken. I have ExposureTime values of 10/6400 s in images from an old Sony DSC-P72, which are showing in the web UI as "NAN s".

The regex in 3499998 is not capturing the values correctly:

preg_match('/(\d?)\/(\d?) s/', $photo['shutter'], $matches);
var_dump($matches);    // => array(0) { }

This subsequently causes the following line to produce NAN:

$photo['shutter'] = intval($matches[1]) / intval($matches[2]) . ' s';

Incidentally, that line is relying on PHP implicit typecasting, dividing a string by a string, and then explicitly typecasting to int. The individual matches should instead be explicitly typecast to ints before dividing.

I suggest rewriting that section of code to something a bit more straightforward:

                if (preg_match('/(\d+)\/(\d+) s/', $photo['shutter'], $matches)) {
                        $a = intval($matches[1]); 
                        $b = intval($matches[2]);

                        if ($a < $b)
                                $photo['shutter'] = '1/' . ($b / $a) . ' s';
                        else
                                $photo['shutter'] = ($a / $b) . ' s';
                }

@d7415
Copy link
Contributor

d7415 commented Dec 27, 2018

Take this all with a pinch of salt - I don't have time to test.

This is still broken. I have ExposureTime values of 10/6400 s in images from an old Sony DSC-P72, which are showing in the web UI as "NAN s".

The regex in 3499998 is not capturing the values correctly:

Yes, at a glance it looks like it needs '/(\d+)\/(\d+) s/'. I'd be tempted to use '/(\d+)\/(\d+) +s/' to be a little more forgiving, but that's probably not necessary.

Incidentally, that line is relying on PHP implicit typecasting, dividing a string by a string, and then explicitly typecasting to int. The individual matches should instead be explicitly typecast to ints before dividing.

No. Check the brackets.

I suggest rewriting that section of code to something a bit more straightforward:

                if (preg_match('/(\d+)\/(\d+) s/', $photo['shutter'], $matches)) {
                        $a = intval($matches[1]); 
                        $b = intval($matches[2]);

                        if ($a < $b)
                                $photo['shutter'] = '1/' . ($b / $a) . ' s';
                        else
                                $photo['shutter'] = ($a / $b) . ' s';
                }

This would give strange results for many cases, including e.g. 2/3 would be displayed as 1/1.5. Maybe that is your intention, but it seems odd to me. 9/10 would also be strange (1/1.1111111111....).

I'm not sure if this is why you deleted the note about cameras playing nice, but 10/6400 (which would be simplified by your example) is trivially reduced (by a human) to 1/640, but apparently your camera didn't feel up to the task. I'm not sure how much heavy lifting we want to do here in Lychee...

@dswarbrick
Copy link

Incidentally, that line is relying on PHP implicit typecasting, dividing a string by a string, and then explicitly typecasting to int. The individual matches should instead be explicitly typecast to ints before dividing.

No. Check the brackets.

You're right. My tired eyes didn't see the extra brackets.

This would give strange results for many cases, including e.g. 2/3 would be displayed as 1/1.5. Maybe that is your intention, but it seems odd to me. 9/10 would also be strange (1/1.1111111111....).

I'm not sure if this is why you deleted the note about cameras playing nice, but 10/6400 (which would be simplified by your example) is trivially reduced (by a human) to 1/640, but apparently your camera didn't feel up to the task. I'm not sure how much heavy lifting we want to do here in Lychee...

This is why I originally posted in my comment (and later removed) that this should technically use Euclid's algorithm to find the GCD of the fraction's numerator and denominator. I've never seen a camera that offers shutter speeds such as 2/3. I've only ever seen "1/x" fractions, so we might be able to wing it and assume that the EXIF data will only ever contain such ratios whereby the denominator can be evenly divided by the numerator, and the numerator reduced to 1.

Your examples of 2/3 and 9/10 would indeed be handled properly if the code was stricter and implemented Euclid's.

@d7415
Copy link
Contributor

d7415 commented Dec 27, 2018

You're right. My tired eyes didn't see the extra brackets.

Easily done!

This is why I originally posted in my comment (and later removed) that this should technically use Euclid's algorithm to find the GCD of the fraction's numerator and denominator. I've never seen a camera that offers shutter speeds such as 2/3. I've only ever seen "1/x" fractions, so we might be able to wing it and assume that the EXIF data will only ever contain such ratios whereby the denominator can be evenly divided by the numerator, and the numerator reduced to 1.

That's why I'm concerned about doing the heavy lifting here for minimal return. The existing version @ildyria added should show any 1/x as-is, and should (with a regex tweak) convert anything else to a decimal.

I'll have a look myself tomorrow if @ildyria doesn't beat me to it!

@d7415 d7415 reopened this Dec 27, 2018
@d7415 d7415 self-assigned this Dec 27, 2018
@dswarbrick
Copy link

I stumbled across a fairly concise PHP implementation of Euclid's algorithm as a recursive function: https://wpscholar.com/blog/euclidean-algorithm-php/

As far as heavy lifting - maybe it's better to store / cache the calculated "human-friendly" shutter speed in the lychee_photos table, rather than the original EXIF value.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request Low Priority Low priority issues
Projects
None yet
Development

No branches or pull requests

4 participants