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

WIP: timeline server supporting features #466

Closed
wants to merge 4 commits into from
Closed

WIP: timeline server supporting features #466

wants to merge 4 commits into from

Conversation

dassio
Copy link
Contributor

@dassio dassio commented Oct 6, 2020

add metadata to photos own database for fast timeline info query
following changes are added :

  1. add migration to store the metadata
MariaDB [nextcloud]> describe oc_photosmetadata;
+--------------------+--------------+------+-----+---------+----------------+
| Field              | Type         | Null | Key | Default | Extra          |
+--------------------+--------------+------+-----+---------+----------------+
| id                 | int(11)      | NO   | PRI | NULL    | auto_increment |
| file_id            | int(11)      | YES  |     | NULL    |                |
| date_time_original | varchar(200) | YES  |     | NULL    |                |
| gps_latitude       | varchar(255) | YES  |     | NULL    |                |
| gps_longitude      | varchar(255) | YES  |     | NULL    |                |
| gps_latitude_ref   | varchar(255) | YES  |     | NULL    |                |
| gps_longitude_ref  | varchar(255) | YES  |     | NULL    |                |
| path               | varchar(255) | YES  |     | NULL    |                |
+--------------------+--------------+------+-----+---------+----------------+
  1. add command to extract existing file metadata
root@1bd5f6fc3e6b:/var/www/html# sudo  -u www-data ./occ photos: -h
Description:
  extract metadata from image files: date taken, location...

Usage:
  photos:extractmetadata [options] [--] [<user_id>]

Arguments:
  user_id               extract photo metadata for the given user

Options:
  -p, --path[=PATH]     limit extraction to this photo folder(album), eg. --path="/alice/files/Photos/2020-3/", the user_id is determined by the path and the user_id parameter is ignored
  -h, --help            Display this help message
  -q, --quiet           Do not output any message
  -V, --version         Display this application version
      --ansi            Force ANSI output
      --no-ansi         Disable ANSI output
  -n, --no-interaction  Do not ask any interactive question
      --no-warnings     Skip global warnings, show command output only
  -v|vv|vvv, --verbose  Increase the verbosity of messages: 1 for normal output, 2 for more verbose output and 3 for debug
  1. get user all photos number by month
curl -k -i -H "Accept: application/json" -H "Content-Type: application/json" -H "Cookie: __Host-nc_sameSiteCookielax=true; __Host-nc_sameSiteCookiestrict=true; nc_sameSiteCookielax=true; nc_sameSiteCookiestrict=true; nc_username=danny; nc_token=3%2BCLyVRqpUGwCbany%2FU7odzUSbTF9dC6; nc_session_id=qdlk3gnb3muuugbjblcemn1kks; XDEBUG_SESSION=XDEBUG_ECLIPSE; ocf6hkhrls8q=d630f3ea256da534b4100395a049482c; oc_sessionPassphrase=jhoszO87UjcudUSvNi3%2F9NgNTz7qApJKS%2FFFYehpMknbInUPKT0Peh4VcISB%2BxsKTr4ctsa%2FxlnF8q4c9xXBy6IgmX4fTIdDHfssFp%2FfJRY8EMe6MH4zdocLhxfm8mF8; ocandbbmzjpq=7eecd010120f174d305855aee443a227; JSESSIONID=B57788520C0C2995DA0C3A28FCAD7DCC; ocjjjzdb39no=qdlk3gnb3muuugbjblcemn1kks" -H "requesttoken: Jr7wkdepIYQSGSNpBfy4aE9pf6zL5LA6E7e5P8GztZ8=:bfampJPsd6tZQGcPU8jqJjUPUNapjMFXSYDTZarS2dc=" https://localhost/index.php/apps/photos/api/v1/numberbymonth
HTTP/1.1 200 OK
Server: nginx
Date: Tue, 06 Oct 2020 16:41:06 GMT
Content-Type: application/json; charset=utf-8
Content-Length: 378
Connection: keep-alive
Vary: Accept-Encoding
Expires: Thu, 19 Nov 1981 08:52:00 GMT
Cache-Control: no-cache, no-store, must-revalidate
Pragma: no-cache
Content-Security-Policy: default-src 'none';base-uri 'none';manifest-src 'self'
Referrer-Policy: no-referrer
X-Content-Type-Options: nosniff
X-Download-Options: noopen
X-Frame-Options: SAMEORIGIN
X-Permitted-Cross-Domain-Policies: none
X-Robots-Tag: none
X-XSS-Protection: 1; mode=block
Feature-Policy: autoplay 'none';camera 'none';fullscreen 'none';geolocation 'none';microphone 'none';payment 'none'
Strict-Transport-Security: max-age=31536000; includeSubDomains

{"unknown":15,"2014-Sep":11,"2015-Oct":7,"2016-Jun":2,"2016-Jul":20,"2016-Aug":26,"2016-Oct":29,"2017-Mar":40,"2017-Apr":34,"2017-Jul":1,"2015-Mar":14,"2015-May":12,"2018-Aug":4,"2018-Oct":2,"2015-Jul":1,"2015-Aug":1,"2015-Sep":2,"2015-Dec":1,"2016-Feb":3,"2016-Mar":5,"2016-May":1,"2016-Sep":44,"2016-Nov":1,"2016-Dec":16,"2017-Jan":38,"2017-Feb":13,"2017-May":24,"2018-Sep":2}

4 add API to get list of file ids of one specific month

 curl -k -i -H "Accept: application/json" -H "Content-Type: application/json" -H "Cookie: __Host-nc_sameSiteCookielax=true; __Host-nc_sameSiteCookiestrict=true; nc_sameSiteCookielax=true; nc_sameSiteCookiestrict=true; nc_username=danny; nc_token=3%2BCLyVRqpUGwCbany%2FU7odzUSbTF9dC6; nc_session_id=qdlk3gnb3muuugbjblcemn1kks; XDEBUG_SESSION=XDEBUG_ECLIPSE; ocf6hkhrls8q=d630f3ea256da534b4100395a049482c; oc_sessionPassphrase=jhoszO87UjcudUSvNi3%2F9NgNTz7qApJKS%2FFFYehpMknbInUPKT0Peh4VcISB%2BxsKTr4ctsa%2FxlnF8q4c9xXBy6IgmX4fTIdDHfssFp%2FfJRY8EMe6MH4zdocLhxfm8mF8; ocandbbmzjpq=7eecd010120f174d305855aee443a227; JSESSIONID=B57788520C0C2995DA0C3A28FCAD7DCC; ocjjjzdb39no=qdlk3gnb3muuugbjblcemn1kks" -H "requesttoken: Jr7wkdepIYQSGSNpBfy4aE9pf6zL5LA6E7e5P8GztZ8=:bfampJPsd6tZQGcPU8jqJjUPUNapjMFXSYDTZarS2dc=" https://localhost/index.php/apps/photos/api/v1/photosofmonth/2016-08
HTTP/1.1 200 OK
Server: nginx
Date: Tue, 06 Oct 2020 16:42:19 GMT
Content-Type: application/json; charset=utf-8
Content-Length: 157
Connection: keep-alive
Expires: Thu, 19 Nov 1981 08:52:00 GMT
Cache-Control: no-cache, no-store, must-revalidate
Pragma: no-cache
Content-Security-Policy: default-src 'none';base-uri 'none';manifest-src 'self'
Referrer-Policy: no-referrer
X-Content-Type-Options: nosniff
X-Download-Options: noopen
X-Frame-Options: SAMEORIGIN
X-Permitted-Cross-Domain-Policies: none
X-Robots-Tag: none
X-XSS-Protection: 1; mode=block
Feature-Policy: autoplay 'none';camera 'none';fullscreen 'none';geolocation 'none';microphone 'none';payment 'none'
Strict-Transport-Security: max-age=31536000; includeSubDomains

[14380,14271,14388,14282,14681,14688,14690,14698,14691,14755,14733,14710,14753,14736,14756,14757,14759,14758,14761,14760,14764,14762,14763,14765,14769,14768]

TODO:
extract photo metadata during upload, pending server pull request : 23230

Signed-off-by: xiangbin.li <dassio@icloud.com>
Signed-off-by: xiangbin.li <dassio@icloud.com>
Signed-off-by: xiangbin.li <dassio@icloud.com>
Signed-off-by: xiangbin.li <dassio@icloud.com>
@dassio dassio changed the title WIP: save meta data to datbase for easy timeline WIP: timeline server features Oct 6, 2020
@dassio dassio changed the title WIP: timeline server features WIP: timeline server supporting features Oct 6, 2020
@skjnldsv
Copy link
Member

skjnldsv commented Oct 6, 2020

Hey!

Thanks a lot for addressing this!
Before you put additional work into this, read this and the various other comments I made about metadata: #226

Proper metadata integration should NOT go into photos. It should be straight into server so that another apps can benefit from this! ⚠️

Are you on our community chat https://cloud.nextcloud.com/call/xs25tz5y?
It's always best to tell the maintainers before addressing such big issues, that way we could have discussed it beforehand! 🤗

Hope you didn't put that many hours into this yet! Let's see what we can do! 💪

@skjnldsv
Copy link
Member

skjnldsv commented Oct 6, 2020

Are you on our community chat cloud.nextcloud.com/call/xs25tz5y?

I realised you're already on the Photos public room as well 🚀

@tacruc
Copy link

tacruc commented Oct 6, 2020

This in Server would solve a bunch if problems for nextcloud/maps, too. CC @eneiluj

@dassio
Copy link
Contributor Author

dassio commented Oct 6, 2020

Proper metadata integration should NOT go into photos. It should be straight into server so that another apps can benefit from this! warning

not sure about this one, this is not general metadata, it is metadata specific for photos and it can be exposed as API.

I think the community has been back and forth with the issue, and there should not be only one way of doing it right , for a simple photos metadata reader, we should move forward

also changing the server app will bring more people with conflicting ideas, if we have a seperate repo for photos, we should improve on ourself before contributing to the server app

Hope you didn't put that many hours into this yet! Let's see what we can do! muscle

it is a good learning experience

It's always best to tell the maintainers before addressing such big issues, that way we could have discussed it beforehand! hugs

several days goes by without any reply to my design, I though that the public photos group is dead

@dassio
Copy link
Contributor Author

dassio commented Oct 6, 2020

at least the numberbymonth and photosofmonth should be here.

working the database part on server repo and api part on photos repo will be an horrifying dev experience

i think we should fix the functionality here before moving the code to server app

@kesselb
Copy link
Contributor

kesselb commented Oct 6, 2020

Thanks for your work 👍

working the database part on server repo and api part on photos repo will be an horrifying dev experience

Database and Api should be server (to make this functionality available to other apps as well). Api in this context means a php component to fetch meta data for a give file. A rest api will be done by the apps itself.

If we implement it here other apps (like maps) have to maintain a dependency to photos. If photos app is installed and enabled use meta data. That's bad to maintain.

Keep the good work up 🚀

@dassio
Copy link
Contributor Author

dassio commented Oct 6, 2020

Database and Api should be server (to make this functionality available to other apps as well). Api in this context means a php component to fetch meta data for a give file. A rest api will be done by the apps itself.

but the design of the API will be highly influenced by the app who is using it, maybe we can come up with a general API but i don't see how can that come to life without any app using it, we have to iterate to get the final perfect API that fits all app.

some API are only meant to be used by specific photos app, like: photos app does not care about the pdf metadata. we should have the api matrue enough before thinking about making it general in the core server app

@dassio
Copy link
Contributor Author

dassio commented Oct 6, 2020

when I say horrifying I mean we will have seperate PRs and maybe in the server PR people will say this have to kept in a seperate app , and in the photos PR people will say this have to be in the server app, and it will get stuck like all the other issues years ago

i want to move forward first here

@skjnldsv
Copy link
Member

skjnldsv commented Oct 6, 2020

but the design of the API will be highly influenced by the app who is using it, maybe we can come up with a general API but i don't see how can that come to life without any app using it, we have to iterate to get the final perfect API that fits all app.

Not if it's generic. Metadatas are metadata, whatever info the app is using, the database should not care. Taken date, geolocation, focale, aperture, brand... etc etc
All of this should be stored in a db.

Then the photos app can request specific data from it.
After that, the design and the data processing can and will happen in the photos :)

@dassio
Copy link
Contributor Author

dassio commented Oct 6, 2020

but the design of the API will be highly influenced by the app who is using it, maybe we can come up with a general API but i don't see how can that come to life without any app using it, we have to iterate to get the final perfect API that fits all app.

Not if it's generic. Metadatas are metadata, whatever info the app is using, the database should not care. Taken date, geolocation, focale, aperture, brand... etc etc
All of this should be stored in a db.

Then the photos app can request specific data from it.
After that, the design and the data processing can and will happen in the photos :)

yes, metadata is generic , but the API is not , photos will request diffrent meta API than an ebook app , i am just saying that from the start the development , we don't have to seperate the DB and API , we can move the DB later to the core serve app

@skjnldsv
Copy link
Member

skjnldsv commented Oct 7, 2020

yes, metadata is generic , but the API is not , photos will request diffrent meta API than an ebook app , i am just saying that from the start the development , we don't have to seperate the DB and API , we can move the DB later to the core serve app

No, the API should be generic too.
The data to request to the api are not generic and up to the developer.
Like said, this will not be merged into photos.

@simonspa
Copy link

simonspa commented Oct 7, 2020

I'm following this discussion and I have the feeling that we are not all talking about the same things, so let me try to provide my point of view here - because in the end I believe we are talking about very similar things, and the feature would be really beneficial not only for bringing Photos forward but also for many other apps.

All below should be appended with "AFAIK"...

There are two APIs being talked about.

  • A server API for metadata: This is supposed to be a PHP server API that can be used by apps to request metadata of individual files, e.g. via their fileid. Any app wishing to process metadata could ask the central server component for this - without having the need to process files themselves and registering a background job for this. This is the API I believe @skjnldsv is talking about.
  • A public web API for photos: This API would expose the albums and photos of the Photos app to the "outside" (e.g. the Photos app js components) by providing queryable endpoints for e.g. a list of photos for a given month, just as @dassio suggests. Internally (in PHP that is) however, the code would use the server API to fetch metadata from files of type image. How or if there would still be another (app-local) database table needed to cache things is a different question I believe.

Now, the question is how to move forward from here.

  • Directly implementing a server component for metadata will likely take time and will not be released before NC21 in ~4 months. Also, it would require all interested metadata parties to come together and define common specs and some technical details of the API and the implementation, which might be tricky. It would however put the whole development effort on sturdy technical footing from the start.
  • First implementing this functionality in the Photos app could be quicker to accomplish since it involves fewer people and less technical depth owing to the more stringent requirements of image metadata only. However, it would be likely that a major rework would be necessary when moving things central later on and - as far as I understand - the functionality couldn't be shipped earlier than NC21 anyway because Photos is a bundled app delivered together with the server.

@skjnldsv
Copy link
Member

skjnldsv commented Oct 7, 2020

Thank for clarifying, I agree with what you said

  • First implementing this functionality in the Photos app could be quicker to accomplish since it involves fewer people and less technical depth owing to the more stringent requirements of image metadata only. However, it would be likely that a major rework would be necessary when moving things central later on and - as far as I understand - the functionality couldn't be shipped earlier than NC21 anyway because Photos is a bundled app delivered together with the server.

I'd rather have this done properly once than having to come back to old code and clean/migrate it.
It's mistake that was done multiple times in the past and I'd rather have things done once and for all, not in a two-steps process :)

PS; @simonspa, wanna be part of the discussion to design and define goals on this metadata api?


EDIT:

  • Directly implementing a server component for metadata will likely take time and will not be released before NC21 in ~4 months.

Well, photos is tight to the server schedule, so this pr won't see the light before 21 anyway 😉
We don't backport features for stability purposes. Unless they're very tiny!

@simonspa
Copy link

simonspa commented Oct 7, 2020

PS; @simonspa, wanna be part of the discussion to design and define goals on this metadata api?

Sure, if I can be of any help? :)

Well, photos is tight to the server schedule, so this pr won't see the light before 21 anyway wink
We don't backport features for stability purposes. Unless they're very tiny!

yes, as I said:

and - as far as I understand - the functionality couldn't be shipped earlier than NC21 anyway because Photos is a bundled app delivered together with the server.

So basically if we want this in the next feature release we have 4 months minus feature freeze deadline anyway and can use this time to make a proper dent. What do you think @dassio - would you be in for defining and implementing the metadata API? The second API is almost there already thanks to your great work!

@dassio
Copy link
Contributor Author

dassio commented Oct 7, 2020

we already have an converstaion on the server : https://cloud.nextcloud.com/call/vqrbn4qp ,

i would be really glade see how can we implement

  • maybe part of partI (generic server metadata processing )
  • finalize the partII (photos API)

@blizzz blizzz modified the milestones: Nextcloud 25, Nextcloud 26 Sep 22, 2022
@dassio dassio closed this by deleting the head repository Oct 17, 2022
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

Successfully merging this pull request may close these issues.

6 participants