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

Extract GPS data from EXIF #33511

Merged
merged 2 commits into from
Oct 11, 2022
Merged

Extract GPS data from EXIF #33511

merged 2 commits into from
Oct 11, 2022

Conversation

artonge
Copy link
Contributor

@artonge artonge commented Aug 11, 2022

I had to do a workaround to read EXIF data properly:

Some work has been done by @CarlSchwan here.

But the exif_read_data call does not work, and probably never worked. One possible explanation is that the resource is wrapped. From the README:

> Note: due to php's internal stream buffering the $count passed to the read callback will be equal to php's internal buffer size (8192 on default) an not the number of bytes requested by fopen()

As it works when the stream is not wrapped, it might be an explanation for why the exif_read_data call fails.

To make it work, I copy the stream into a tmp stream without wrappers before passing it to exif_read_data.

✔️ I found a workaround by calling stream_set_chunk_size($fileDescriptor, 1); before we read the stream. No idea why it works, but it does, and I am reverting it right after to prevent any side effect outside our use case.

I also cherry-picked @CarlSchwan PR that adds a CLI command to extract metadata #32309

This was referenced Aug 12, 2022
@artonge artonge force-pushed the feat/extract_gps_from_exif branch 3 times, most recently from b4da031 to c096b1d Compare August 23, 2022 10:38
@artonge artonge added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Aug 23, 2022
@PVince81 PVince81 requested a review from come-nc August 24, 2022 14:47
@blizzz blizzz mentioned this pull request Aug 24, 2022
Copy link
Contributor

@come-nc come-nc left a comment

Choose a reason for hiding this comment

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

A few remarks.

Regarding copying the stream to a temp one, I have no idea if this is avoidable.

lib/private/Metadata/Provider/ExifProvider.php Outdated Show resolved Hide resolved
lib/private/Metadata/Provider/ExifProvider.php Outdated Show resolved Hide resolved
lib/private/Metadata/Provider/ExifProvider.php Outdated Show resolved Hide resolved
lib/private/Metadata/Provider/ExifProvider.php Outdated Show resolved Hide resolved
@felhe
Copy link

felhe commented Aug 25, 2022

Just adding my two cents, but the recent changes by @CarlSchwan also made the size property from the EXIF metadata available as a DAV property (see here). Wouldn't it be useful to expose the GPS coordinates to the WebDAV PROPFIND as well? @CarlSchwan @icewind1991

@PVince81
Copy link
Member

Just adding my two cents, but the recent changes by @CarlSchwan also made the size property from the EXIF metadata available as a DAV property (see here). Wouldn't it be useful to expose the GPS coordinates to the WebDAV PROPFIND as well? @CarlSchwan @icewind1991

that's the plan already, it should be automatically exposed once it exists in the database

@skjnldsv skjnldsv added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Aug 26, 2022
This was referenced Aug 30, 2022
apps/files/lib/Command/Scan.php Outdated Show resolved Hide resolved
apps/files/lib/Command/Scan.php Outdated Show resolved Hide resolved
lib/private/Metadata/FileMetadataMapper.php Outdated Show resolved Hide resolved
lib/private/Metadata/FileMetadataMapper.php Outdated Show resolved Hide resolved
lib/private/Metadata/FileMetadataMapper.php Outdated Show resolved Hide resolved
lib/private/Metadata/Provider/ExifProvider.php Outdated Show resolved Hide resolved
@github-actions
Copy link
Contributor

Possible performance regression detected

Show Output
1 queries added

= /remote.php/dav/files/test
= /remote.php/dav/files/test/test.txt
= /remote.php/dav/files/test/many_files
≠ /remote.php/dav/files/test/new_file.txt with 1 queries added
  - UPDATE "oc_filecache" SET "mtime" = MAX("mtime", :dcValue1), "etag" = :dcValue3, "size" = CASE WHEN "size" > '-1' THEN MAX("size" + :dcValue4, :dcValue5) ELSE "size" END WHERE ("storage" = :dcValue2) AND ("path_hash" IN ('d41d8cd98f00b204e9800998ecf8427e', '45b963397aa40d4a0063e0d85e4fe7a1'))
  + UPDATE "oc_filecache" SET "mtime" = MAX("mtime", :dcValue1), "etag" = :dcValue3 WHERE ("storage" = :dcValue2) AND ("path_hash" IN ('d41d8cd98f00b204e9800998ecf8427e', '45b963397aa40d4a0063e0d85e4fe7a1'))
  + UPDATE "oc_filecache" SET "size" = MAX("size" + :dcValue1, :dcValue2) WHERE ("storage" = :dcValue3) AND ("path_hash" IN ('d41d8cd98f00b204e9800998ecf8427e', '45b963397aa40d4a0063e0d85e4fe7a1')) AND ("size" > '-1')
= /remote.php/dav/files/test/new_file.txt

@github-actions
Copy link
Contributor

Possible performance regression detected

Show Output
1 queries added

= /remote.php/dav/files/test
= /remote.php/dav/files/test/test.txt
= /remote.php/dav/files/test/many_files
≠ /remote.php/dav/files/test/new_file.txt with 1 queries added
  - UPDATE "oc_filecache" SET "mtime" = MAX("mtime", :dcValue1), "etag" = :dcValue3, "size" = CASE WHEN "size" > '-1' THEN MAX("size" + :dcValue4, :dcValue5) ELSE "size" END WHERE ("storage" = :dcValue2) AND ("path_hash" IN ('d41d8cd98f00b204e9800998ecf8427e', '45b963397aa40d4a0063e0d85e4fe7a1'))
  + UPDATE "oc_filecache" SET "mtime" = MAX("mtime", :dcValue1), "etag" = :dcValue3 WHERE ("storage" = :dcValue2) AND ("path_hash" IN ('d41d8cd98f00b204e9800998ecf8427e', '45b963397aa40d4a0063e0d85e4fe7a1'))
  + UPDATE "oc_filecache" SET "size" = MAX("size" + :dcValue1, :dcValue2) WHERE ("storage" = :dcValue3) AND ("path_hash" IN ('d41d8cd98f00b204e9800998ecf8427e', '45b963397aa40d4a0063e0d85e4fe7a1')) AND ("size" > '-1')
= /remote.php/dav/files/test/new_file.txt

@github-actions
Copy link
Contributor

Possible performance regression detected

Show Output
1 queries added

= /remote.php/dav/files/test
= /remote.php/dav/files/test/test.txt
= /remote.php/dav/files/test/many_files
≠ /remote.php/dav/files/test/new_file.txt with 1 queries added
  - UPDATE "oc_filecache" SET "mtime" = MAX("mtime", :dcValue1), "etag" = :dcValue3, "size" = CASE WHEN "size" > '-1' THEN MAX("size" + :dcValue4, :dcValue5) ELSE "size" END WHERE ("storage" = :dcValue2) AND ("path_hash" IN ('d41d8cd98f00b204e9800998ecf8427e', '45b963397aa40d4a0063e0d85e4fe7a1'))
  + UPDATE "oc_filecache" SET "mtime" = MAX("mtime", :dcValue1), "etag" = :dcValue3 WHERE ("storage" = :dcValue2) AND ("path_hash" IN ('d41d8cd98f00b204e9800998ecf8427e', '45b963397aa40d4a0063e0d85e4fe7a1'))
  + UPDATE "oc_filecache" SET "size" = MAX("size" + :dcValue1, :dcValue2) WHERE ("storage" = :dcValue3) AND ("path_hash" IN ('d41d8cd98f00b204e9800998ecf8427e', '45b963397aa40d4a0063e0d85e4fe7a1')) AND ("size" > '-1')
= /remote.php/dav/files/test/new_file.txt

Copy link
Member

@PVince81 PVince81 left a comment

Choose a reason for hiding this comment

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

👍 please double check the perf bot

Copy link
Contributor

@come-nc come-nc left a comment

Choose a reason for hiding this comment

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

Apart from small change on logger, approved

lib/private/Metadata/Provider/ExifProvider.php Outdated Show resolved Hide resolved
@PVince81 PVince81 added 4. to release Ready to be released and/or waiting for tests to finish and removed 2. developing Work in progress labels Oct 11, 2022
artonge and others added 2 commits October 11, 2022 10:41
Signed-off-by: Louis Chemineau <louis@chmn.me>
Signed-off-by: Carl Schwan <carl@carlschwan.eu>
Signed-off-by: Louis Chemineau <louis@chmn.me>
@PVince81 PVince81 added the pending documentation This pull request needs an associated documentation update label Oct 11, 2022
@PVince81
Copy link
Member

@artonge please also make sure the new CLI args are present in the documentation

@artonge artonge merged commit 94ded14 into master Oct 11, 2022
@artonge artonge deleted the feat/extract_gps_from_exif branch October 11, 2022 12:11
@artonge artonge removed the pending documentation This pull request needs an associated documentation update label Feb 7, 2023
@mpivchev
Copy link

mpivchev commented Aug 2, 2023

We want to use this on iOS and Android clients to fetch location data without downloading the full res image + exif. To me it looks like this code only does the process of obtaining the info but we can't request it from client. Is this added somewhere else?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish backport-request feature: files
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants