Open sarayourfriend opened 2 years ago
Sorry I accidentally added multiple commits to this issue instead of #587
@bpatrik Are you open to a PR from me to implement this?
This is my vague idea, at the moment:
diff --git a/src/backend/model/database/enitites/MediaEntity.ts b/src/backend/model/database/enitites/MediaEntity.ts
index 5eb17224..c8a1c6a8 100644
--- a/src/backend/model/database/enitites/MediaEntity.ts
+++ b/src/backend/model/database/enitites/MediaEntity.ts
@@ -106,7 +106,7 @@ export class MediaMetadataEntity implements MediaMetadata {
})
@Index()
creationDate: number;
-
+
@Column('smallint', {
transformer: {
from: (v) => Utils.getOffsetString(v), //from database repr. as smallint (minutes) to string (+/-HH:MM)
@@ -176,6 +176,12 @@ export class MediaMetadataEntity implements MediaMetadata {
@Column('int', {unsigned: true})
duration: number;
+
+ @Column('text')
+ creator?: string;
+
+ @Column('text')
+ rightsStatement?: string;
}
// TODO: fix inheritance once its working in typeorm
diff --git a/src/backend/model/fileaccess/MetadataLoader.ts b/src/backend/model/fileaccess/MetadataLoader.ts
index e0fcb50f..8bb27fc6 100644
--- a/src/backend/model/fileaccess/MetadataLoader.ts
+++ b/src/backend/model/fileaccess/MetadataLoader.ts
@@ -311,6 +311,11 @@ export class MetadataLoader {
if (iptcData.date_time) {
metadata.creationDate = iptcData.date_time.getTime();
}
+
+ if (iptcData.copyright_notice) {
+ // Will be overwritten by dc rights value if present
+ metadata.rightsStatement = iptcData.copyright_notice.replace(/\0/g, '').trim();
+ }
} catch (err) {
// Logger.debug(LOG_TAG, 'Error parsing iptc data', fullPath, err);
}
@@ -320,18 +325,37 @@ export class MetadataLoader {
const exif = await exifr.parse(data, exifrOptions);
//exif is structured in sections, we read the data by section
- //dc-section (subject is the only tag we want from dc)
- if (exif.dc &&
- exif.dc.subject &&
- exif.dc.subject.length > 0) {
- const subj = Array.isArray(exif.dc.subject) ? exif.dc.subject : [exif.dc.subject];
- if (metadata.keywords === undefined) {
- metadata.keywords = [];
- }
- for (const kw of subj) {
- if (metadata.keywords.indexOf(kw) === -1) {
- metadata.keywords.push(kw);
+ //dc-section
+ if (exif.dc) {
+ if (exif.dc.subject &&
+ exif.dc.subject.length > 0) {
+ const subj = Array.isArray(exif.dc.subject) ? exif.dc.subject : [exif.dc.subject];
+ if (metadata.keywords === undefined) {
+ metadata.keywords = [];
}
+ for (const kw of subj) {
+ if (metadata.keywords.indexOf(kw) === -1) {
+ metadata.keywords.push(kw);
+ }
+ }
+ }
+
+ // Always use DC values if they are present
+ // DublinCore is an accessible and widely used standard for these specific values
+ if (exif.dc.creator?.value) {
+ metadata.creator = exif.dc.creator.value;
+ }
+
+ if (exif.dc.rights?.value) {
+ metadata.rightsStatement = exif.dc.rights.value;
+ }
+
+ if (exif.dc.description?.value) {
+ metadata.caption = exif.dc.description.value;
+ }
+
+ if (exif.dc.title?.value) {
+ metadata.title = exif.dc.title.value;
}
}
@@ -358,6 +382,19 @@ export class MetadataLoader {
metadata.cameraData.model = '' + exif.ifd0.Model;
}
//if (exif.ifd0.ModifyDate) {} //Deferred to the exif-section where the other timestamps are
+
+ // Only use ifd0 Copyright and Artist if iptc and DC didn't already provide them
+ if (exif.ifd0.Copyright && !metadata.rightsStatement) {
+ metadata.rightsStatement = exif.ifd0.Copyright;
+ }
+
+ if (exif.ifd0.Artist && !metadata.creator) {
+ metadata.creator = exif.ifd0.Artist;
+ }
+
+ if (exif.ifd0.ImageDescription && !metadata.caption) {
+ metadata.caption = exif.ifd0.ImageDescription;
+ }
}
//exif section starting with the date sectino
diff --git a/src/common/entities/PhotoDTO.ts b/src/common/entities/PhotoDTO.ts
index 6184bbd2..0207db15 100644
--- a/src/common/entities/PhotoDTO.ts
+++ b/src/common/entities/PhotoDTO.ts
@@ -36,6 +36,8 @@ export interface PhotoMetadata extends MediaMetadata {
creationDateOffset?: string;
fileSize: number;
faces?: FaceRegion[];
+ creator?: string;
+ rightsStatement?: string;
}
export interface PositionMetaData {
To be clear, this is just at the "exploratory code" phase, I haven't run it at all and don't expect it to work, and there's no UI yet. I've just run out of time today to continue thinking about this, but wanted to share my idea and get your thoughts in case this is a totally wrong approach from your perspective, or if you had anything else in mind.
I'm particularly interested in making this a possibility as the new backend extension support has my interests piqued to write an extension that exposes an API that someone could use to turn their pigallery2 site into an Openverse provider (https://openverse.org) (I am an Openverse maintainer, and want my own pigallery2 site to be able to do this, so it's wholly self-interested :joy_cat:).
@sarayourfriend I recently did a refactor of metadataloader. You're on the right track. I also opened a PR that fills out metadata.caption
and metadata.title
, which I hope will be merged.
I would recommend adding a private static mapRights(metadata : PhotoMetadata, exif: any)
function in the same style as mapCaption
here, and call it together with the others in mapMetadata
: https://github.com/bpatrik/pigallery2/blob/master/src/backend/model/fileaccess/MetadataLoader.ts#L310-L328
The reasoning being that as requirements evolve and new fields that make sense to map (metadata has loads of redundancy), it will be easy and clear where to do it.
Due to lazy-evaluation, TypeScript / JavaScript makes assigning a variable from the first available of any tag simple. A single line with prioritized tags separated by ||
is all that is needed - as you can see in the mapCaption
function above. You don't even need the clumsy if
-statements.
The test-assets contain a lot of different data for you to refine your functionality with. Embedded data, sidecars with tags written by various tools. I believe I saw 2-3 different tags for "author" and "rights" during my debugging.
Final thing:
When I added creationDateOffset
, @bpatrik showed me ContentWrapper.ts. It's an optimization mechanism, where all metadata tags are mapped to a unique letter for faster transfer. You probably need to add author and rights there too. You can see my addition for creationDateOffset
here if you scroll to the ContentWrapper.
Thanks very much for the advice, @grasdk! Hope to have a PR next weekend :slightly_smiling_face: Nice work with that refactor of the MetadataLoader. It's a lot more pleasant to read and should be more pleasant to work in :rocket:
One thing I ran into playing around with the UI-side of things was the lack of a © icon in ionicons. If you or @bpatrik happen to have ideas or indications of where the best place to bring in a new icon would be, please let me know. Otherwise, I'd be inclined to use this SVG icon, which is public domain, with modifications to round the corners and adjust line weight to make it fit with ionicon's style. If there's a different, preferred approach, let me know.
For icons:
I'm using some angular module to load only those icons that the app uses. No c so far so you won't find it on client side.
I'm loading the icons here: https://github.com/bpatrik/pigallery2/blob/6c53aca7e7d5b65d257c80737647d57854592bbe/src/frontend/app/app.module.ts#L237
This is the full available iconset: https://ionic.io/ionicons Indeed I also can't find any C icon.
The angular module supports loading icons from other provider. You can try looking at bootstrap or font awesome. Please pick a provider that style and margin/padding matches the current style. Also provide a screenshot about the Ui change when you send a PR.
@sarayourfriend I recently did a refactor of metadataloader. You're on the right track. I also opened a PR that fills out
metadata.caption
andmetadata.title
, which I hope will be merged.I would recommend adding a private static
mapRights(metadata : PhotoMetadata, exif: any)
function in the same style asmapCaption
here, and call it together with the others inmapMetadata
: https://github.com/bpatrik/pigallery2/blob/master/src/backend/model/fileaccess/MetadataLoader.ts#L310-L328The reasoning being that as requirements evolve and new fields that make sense to map (metadata has loads of redundancy), it will be easy and clear where to do it.
Due to lazy-evaluation, TypeScript / JavaScript makes assigning a variable from the first available of any tag simple. A single line with prioritized tags separated by
||
is all that is needed - as you can see in themapCaption
function above. You don't even need the clumsyif
-statements.The test-assets contain a lot of different data for you to refine your functionality with. Embedded data, sidecars with tags written by various tools. I believe I saw 2-3 different tags for "author" and "rights" during my debugging.
Final thing:
When I added
creationDateOffset
, @bpatrik showed me ContentWrapper.ts. It's an optimization mechanism, where all metadata tags are mapped to a unique letter for faster transfer. You probably need to add author and rights there too. You can see my addition forcreationDateOffset
here if you scroll to the ContentWrapper.
Adding copyright will probably need more care :
Any extra metadata adtionion linearly slows down the load time of a folder listing or search. Data travels as Json to client. (I guess as utf8, so i 4ish? Byte per every extra character).
If a metadat is different for every photo, in Contentwrapper I only replace the nem (key) of the data (eg: date->d).
If the data probably the same for multiple photos, like keyword, city or now rights, I create a map of those keywords. Only the keys of the map are attached to the photos (eg: #famaliyphoto -> 1).
To better understand: You can see sample code for it in the contentwrapper. Or check content reponose in the browser's dev console.
Thank you! I think it's totally reasonable to expect the majority of creator
and rights
will be shared with other photos :+1:
@bpatrik Wow, that's very cool. Didn't see that it went that deep. Impressive :)
@sarayourfriend A humple request to ensure that if the data is not present also don't display the © symbol and headings. My personal usecase for pigallery2 at the moment is family photos and I never bothered to put copyright into it, though I did put in author on a lot of them as a way to tell who took the photo :)
if the data is not present also don't display the © symbol and headings
:100: absolutely, without a question a requirement for the UI not to show anything if not explicitly available. Maybe even worth putting the whole feature behind a config flag?
@sarayourfriend: I believe this https://github.com/bpatrik/pigallery2/pull/876 influences your planned PR. As we discussed earlier, there is some overlap :)
Great! I haven't had time to start working on this yet, but I'll make sure to pull upstream again before cutting my branch, thanks for the heads up.
Just an update: I wanted to get to this this weekend but I won't have time or energy. I'll let y'all know once I'm getting ready to put a PR up. Hopefully in the next two weeks or so :slightly_smiling_face:
Is your feature request related to a problem? Please describe. Most cameras or photo editing software allows you to embed copyright field(s) in the image metadata. It would be nice to display this in the image metadata when it exists (potentially via a configuration).
Alternatively, probably simper, implement a way insert a blanket copyright statement to show in the metadata of each image.
Describe the solution you'd like Ideally the metadata is pulled from each image so that you can have specific copyright for different media files.
As a stop-gap, implement a new configuration option for a copyright string to present in the metadata view for every media item.
Describe alternatives you've considered (optional) (See above)