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

NIP-54: Inline Image Metadata #478

Closed
wants to merge 3 commits into from
Closed

Conversation

jb55
Copy link
Contributor

@jb55 jb55 commented Apr 27, 2023

Inline Image metadata is additional information associated with urls in notes that
clients can use to provide a better experience when loading images.

@vitorpamplona
Copy link
Collaborator

vitorpamplona commented Apr 27, 2023

I think this is a terrible hack that duplicates hashes and metadata across all kind-1s that cite the same URL.

How many tag properties can be added? We need space for 5 at the least, but the default will have more once we add license attributes.

{
  "content": "More image metadata tests don’t mind me https://nostr.build/i/863321bb1ae68830ebcf9a343ca0a5e0ce2323d0a55b7fbe86f7a886cf61db8d.jpg ",
  "kind": 1,
  "tags": [
    [
      "imeta",
      "url https://nostr.build/i/863321bb1ae68830ebcf9a343ca0a5e0ce2323d0a55b7fbe86f7a886cf61db8d.jpg",
      "blurhash eVF$^OI:${M{o#*0-nNFxakD-?xVM}WEWB%iNKxvR-oetmo#R-aen$",
      "dim 3024x4032",
      "description <accessibility descriptor with spaces>"
      "decryptionKeys <secret> <nonce>"
      "license <CC, CC-atribution, etc>"
    ]
  ]
}

Needless to say, the encoding of <label><space><info> is extremely weak. A simple percent-encoded queryString would have been much better.

A quoted NIP-94 event inside a regular kind-1 is a superior engineering solution, IMHO.

@jb55
Copy link
Contributor Author

jb55 commented Apr 27, 2023

I think this is a terrible hack that duplicates hashes and metadata across all kind-1s that cite the same URL.

disagree. clients might not care to re-use urls across posts .

How many tag properties can be added? We need space for 5 at the least, but the default will have more once we add license attributes.

nip01 doesn't specify a limit but, I think some relays do something like 256 ? lots of room.

Needless to say, the encoding of <label><space><info> is extremely weak. A simple percent-encoded queryString would have been much better.

let parts = field.split(" ", max: 1)
if parts.count != 2 return null
let name = parts[0]
let data = parts[1]

A quoted NIP-94 event inside a regular kind-1 is a superior engineering solution, IMHO.

This is for clients that are not planning on implementing nip-94 like damus

@staab
Copy link
Member

staab commented Apr 27, 2023

You know, I think I'm with Vitor on this one. Composition is amazingly powerful. If clients can make rendering generic you get all kinds of neat emergent behavior (e.g. quoted kind-1's in DMs).

The main objection to NIP-94 was latency, right? Maybe we could do something similar to 9735's where the kind-1063 can be optionally provided in a tag, e.g. ["context", <kind-1063 JSON>]. This obviously has a bandwidth cost, but a client need not publish the 1063 directly, just adding it to the text event's tags directly.

This could also be a useful mechanism for reducing flakiness of quotes — if you're quoting an event, the client could provide the entire quoted event using context, reducing latency and the chance that the note won't be found.

@jb55
Copy link
Contributor Author

jb55 commented Apr 27, 2023

damus is not implementing nip-94, this is for clients who want a simpler self-contained option. this is unrelated to nip94, and as that is starting to be used to enable storing data on relays (nip95) I want nothing to do with it. not to mention it is backwards incompatible with urls and doing something like an embedded nip94 is very bloaty and carries extra signatures for no reason.

@staab
Copy link
Member

staab commented Apr 27, 2023

My suggestion is as self-contained as the new NIP, if a little more verbose, and it re-uses an existing part of the protocol, decreasing implementation surface area, and enabling new emergent behavior. More options = less simplicity.

@alexgleason
Copy link
Member

I'm a huge fan of this idea.

In my perfect world attachments wouldn't be present in the content at all, and we'd include tags like this instead:

{
  "content": "More image metadata tests don’t mind me",
  "kind": 1,
  "tags": [
    [
      "media",
      "image/jpeg",
      "https://nostr.build/i/863321bb1ae68830ebcf9a343ca0a5e0ce2323d0a55b7fbe86f7a886cf61db8d.jpg",
      "blurhash eVF$^OI:${M{o#*0-nNFxakD-?xVM}WEWB%iNKxvR-oetmo#R-aen$",
      "dim 3024x4032"
    ]
  ]
}

(mime type and url are positional and required, other arguments are named and optional)

However if we're set on including the URL in the content, I think this is a decent solution. I would just try to generalize it to other kinds of attachments such as videos, and ideally be able to handle whatever else you could throw at it like PDFs etc.

{
  "content": "More image metadata tests don’t mind me https://nostr.build/i/863321bb1ae68830ebcf9a343ca0a5e0ce2323d0a55b7fbe86f7a886cf61db8d.jpg ",
  "kind": 1,
  "tags": [
    [
      "mmeta",
      "mime image/jpeg",
      "url https://nostr.build/i/863321bb1ae68830ebcf9a343ca0a5e0ce2323d0a55b7fbe86f7a886cf61db8d.jpg",
      "blurhash eVF$^OI:${M{o#*0-nNFxakD-?xVM}WEWB%iNKxvR-oetmo#R-aen$",
      "dim 3024x4032"
    ]
  ]
}

mmeta = media meta, and mime type is added so it can be used for videos etc.

@jb55
Copy link
Contributor Author

jb55 commented Apr 27, 2023

good call on mime, will add

@jb55
Copy link
Contributor Author

jb55 commented Apr 27, 2023

mmeta too, this makes sense

@jb55
Copy link
Contributor Author

jb55 commented Apr 27, 2023

hmm well mime+mmeta start to feel a lot like nip94 at that point.. maybe it's worth keeping imeta small and concise for images...

@Giszmo
Copy link
Member

Giszmo commented Apr 27, 2023

How about skipping tags if you want it contained?

{
  "content": "More image metadata tests don’t mind me https://nostr.build/i/863321bb1ae68830ebcf9a343ca0a5e0ce2323d0a55b7fbe86f7a886cf61db8d.jpg#blurhash=eVF$^OI:${M{o#*0-nNFxakD-?xVM}WEWB%iNKxvR-oetmo#R-aen$&dim=3024x4032 ",
  "kind": 1,
  "tags": []
}

The URL got the meta data after a # which should not get sent to the server anyway but should be interpreted by the client.

Edit: In this event I added the #key=value&key2=value2 part with some url encoding and snort was only slightly confused and added a non-intended tag:

{
  "content": "More image metadata tests don’t mind me https://nostr.build/i/863321bb1ae68830ebcf9a343ca0a5e0ce2323d0a55b7fbe86f7a886cf61db8d.jpg#blurhash=eVF%24%5EOI%3A%24%7BM%7Bo%23%2A0-nNFxakD-%3FxVM%7DWEWB%25iNKxvR-oetmo%23R-aen%24&dim=3024x4032%20",
  "created_at": 1682620905,
  "id": "7f7c30b59b83ea168831b45b0605f3043d96885ce4a1462837475c20a717b691",
  "kind": 1,
  "pubkey": "46fcbe3065eaf1ae7811465924e48923363ff3f526bd6f73d7c184b16bd8ce4d",
  "sig": "5f7538bb4743055363b0e151ce9e0b8316d27f4c9dc0bcce224fd58e6167f9c6172f192027614839b92138de1cab4ef46903e06a22fbf68ac672643ee7744bc4",
  "tags": [
    [
      "t",
      "blurhash"
    ]
  ]
}

@staab
Copy link
Member

staab commented Apr 27, 2023

How about skipping tags if you want it contained?

I already have enough trouble parsing URLs, I don't want to do more of that.

@jb55
Copy link
Contributor Author

jb55 commented Apr 27, 2023

How about skipping tags if you want it contained?

{
  "content": "More image metadata tests don’t mind me https://nostr.build/i/863321bb1ae68830ebcf9a343ca0a5e0ce2323d0a55b7fbe86f7a886cf61db8d.jpg#blurhash=eVF$^OI:${M{o#*0-nNFxakD-?xVM}WEWB%iNKxvR-oetmo#R-aen$&dim=3024x4032 ",
  "kind": 1,
  "tags": []
}

The URL got the meta data after a # which should not get sent to the server anyway but should be interpreted by the client.

Interesting idea! They would make the url look pretty bad for clients that don't support this. The tag approach seemed friendlier. plus it could get really ugly once we started adding more and more metadata fields 🤔

@Giszmo
Copy link
Member

Giszmo commented Apr 27, 2023

@jb55 @staab Clients that don't support this do not really care. Current day Snort handling https://snort.social/e/note10a7rpdvms04pdzp3k3dsvp0nqs7edzzuujs5v2phgawzpfchk6gs3f92nl without problem. Note that I did url-encode stuff, apparently correctly.

Screenshot from 2023-04-27 14-42-08

@jb55
Copy link
Contributor Author

jb55 commented Apr 27, 2023

@jb55 @staab Clients that don't support this do not really care. Current day Snort handling

I was just thinking of text clients and things, but they could strip out the # in theory

@Giszmo
Copy link
Member

Giszmo commented Apr 27, 2023

Image handling in text clients wasn't really a consideration of mine but wouldn't they ellipse shorten urls anyway? And the more technical would show the tags, so ... I'm not sure what is gained by moving this to tags.

@jb55
Copy link
Contributor Author

jb55 commented Apr 27, 2023

definitely an interesting approach, I would have to change my code and see if its simpler. it likely is 🤔, then you wouldn't have to duplicate the url in tags.

@jb55
Copy link
Contributor Author

jb55 commented Apr 27, 2023

ok I'm leaning toward switching to this approach. I really like it. great idea @Giszmo

@Giszmo
Copy link
Member

Giszmo commented Apr 27, 2023

@jb55 glad I could help. A mention as co-author would be appreciated.

@vitorpamplona
Copy link
Collaborator

vitorpamplona commented Apr 27, 2023

@Giszmo's idea is at least easier for users to copy and paste with all the attached metadata so the user doesn't need to retype accessibility descriptors every time and the app doesn't need to compute hashes again.

The current one requires custom copy paste code to move all tags of one URLs into a new kind 1.

I still think NIP-94 is superior. Just copy nostr:nevent1... and boom. all tags are there. And there is a notification to the author of the image when copy pasting. Win-win.

@jb55
Copy link
Contributor Author

jb55 commented Apr 27, 2023

ah crap there is one issue for me here. The tags approach allows me to start building blurhash images immediately when I receive the note over the wire (a slow operation so I need to queue it immediately). I don't do any content parsing when I receive notes since that could be potentially slow if I do it for every note I receive over the wire, since most won't be seen. ugh!

@mikedilger
Copy link
Contributor

I like @Giszmo 's approach: I was thinking of an 'nimage1' bech encoded tlv thing (to avoid duplicating the URL in the tags) until I read @Giszmo 's note and realized that his suggestion tops that because it is backwards compatible to all the clients. And URL fragments are client-side with no participation from the web server, so this works very well.

URL encoding/decoding libraries are widely available and it is a well known art.

We are already stuck with early content parsing: As for @jb55 's performance issue: Until recently I wasn't doing content parsing immediately on incoming events either, but once embedded 'nevent' links became a thing, I started doing that content parsing on all incoming events (just once, cached) so that I could start fetching these referenced events immediately. So now that I am doing content parsing anyways, I don't mind this data being in the content.

@jb55
Copy link
Contributor Author

jb55 commented Apr 27, 2023

@mikedilger yeah if I were to change this I would have to move content parsing to all events... I would have to test how this affects battery and performance on mobile devices which are more resource constrained than something like gossip :(

@jb55
Copy link
Contributor Author

jb55 commented Apr 27, 2023

We are already stuck with early content parsing

I don't think this is necessarily true. Damus lazily parses and fetches this information when we are about to scroll into view.

@jb55
Copy link
Contributor Author

jb55 commented Apr 27, 2023

I could try this with blurhash but I notice is was pretty slow and didn't want it popping in, but I can experiment with that.

@vitorpamplona
Copy link
Collaborator

Don't forget to percent-encode.

@jb55
Copy link
Contributor Author

jb55 commented Apr 27, 2023

in damus I will just include my old variant and remove it once I've managed to make it work with the new one, but send both 🤔

@mikedilger
Copy link
Contributor

@mikedilger yeah if I were to change this I would have to move content parsing to all events... I would have to test how this affects battery and performance on mobile devices which are more resource constrained than something like gossip :(

I hear you. @bu5hm4nn argued against nevent embedded for that very reason. We didn't even want to parse every note on the desktop and weren't even thinking about limited phone resources.

@fiatjaf
Copy link
Member

fiatjaf commented Apr 28, 2023

I like the tag approach more than the giant URL approach.

@fiatjaf
Copy link
Member

fiatjaf commented Apr 28, 2023

Is it a dumb idea to use an offset index to refer to where in the content the media reference is? #319 (comment)

@v0l
Copy link
Member

v0l commented Apr 28, 2023

I think this is a bit unnecessary, NIP94 can already do this

@jb55
Copy link
Contributor Author

jb55 commented Apr 28, 2023

do we have an alt-text spec ? maybe image optimization is too specific. this would be a great place for alt text

@jb55 jb55 changed the title NIP-54: Image Loading Optimization NIP-54: Image Metadata Apr 28, 2023
@Semisol
Copy link
Collaborator

Semisol commented Apr 28, 2023

damus is not implementing nip-94, this is for clients who want a simpler self-contained option. this is unrelated to nip94, and as that is starting to be used to enable storing data on relays (nip95) I want nothing to do with it. not to mention it is backwards incompatible with urls and doing something like an embedded nip94 is very bloaty and carries extra signatures for no reason.

it also requires extra REQs... by that time you could have already loaded the image most likely

@vitorpamplona
Copy link
Collaborator

it also requires extra REQs... by that time you could have already loaded the image most likely

Not a big deal. We load quoted events all the time. This has already been optimized.

@jb55
Copy link
Contributor Author

jb55 commented Apr 28, 2023

ok all updated, added alt-text

@staab
Copy link
Member

staab commented Apr 28, 2023

do we have an alt-text spec ? maybe image optimization is too specific. this would be a great place for alt text

You mean something like File Metadata?

@vitorpamplona
Copy link
Collaborator

vitorpamplona commented Apr 28, 2023

To be clear, I don't think hash, alt-text, and any other metadata should be in this spec. This is not a replacement for file metadata, otherwise there would be way too many fields in these tags.

@jb55
Copy link
Contributor Author

jb55 commented Apr 28, 2023

I am not implementing nip94 so this makes the most sense to me. clients can choose to ignore those fields if they are using nip94, but I still think nip94 is a separate use case than inline image metadata.

@jb55
Copy link
Contributor Author

jb55 commented Apr 28, 2023

do we have an alt-text spec ? maybe image optimization is too specific. this would be a great place for alt text

You mean something like File Metadata?

do you have anything better to do?

@jb55 jb55 changed the title NIP-54: Image Metadata NIP-54: Inline Image Metadata Apr 28, 2023
Inline image metadata is additional information associated with
urls in notes that clients can use to provide a better experience when
loading images.
@jb55
Copy link
Contributor Author

jb55 commented Apr 28, 2023

Clarified the difference between this and NIP94: 3c5c49b

@jb55
Copy link
Contributor Author

jb55 commented Apr 28, 2023

renamed to "Inline Image Metadata" to clarify this even further

@ChristopherKing42
Copy link

What if we create a NIP that requires future NIPs to backwards compatible? 🤔 Or that a backwards incompatible NIP needs to meet certain criteria (like a vote or something)?

It seems that there a lot of implicit assumptions about backwards compatibility that aren't common and aren't explicit.

@jb55
Copy link
Contributor Author

jb55 commented Apr 28, 2023

It's usually never a problem, urls are a weird edge case where almost all standardized on using image URLs because it made the most sense. It is not really a spec per say, it's just a thing that all clients do. Markdown in kind1 was another one of those things that has subjective rendering which is why damus eventually removed it.

@vitorpamplona
Copy link
Collaborator

So, it's not about Image Loading Optimization thing anymore? Looks like we are back to being a copy cat of NIP-94.

@jb55
Copy link
Contributor Author

jb55 commented Apr 28, 2023

this is for inline image metadata where some fields can be used for loading optimization yes

@jb55
Copy link
Contributor Author

jb55 commented Apr 28, 2023

it has all the same data as before, you can ignore the alt-text if you find it offensive.

@vitorpamplona
Copy link
Collaborator

Then it should not be positional anymore. There are too many inline image metadata options for people to agree on an ordering scheme. It will be a mess.

Keep it simple, solve your optimization needs, and address the rest of the tags outside an optimization context.

@jb55
Copy link
Contributor Author

jb55 commented Apr 28, 2023

this is exhausting. ya'll are wasting my time. I'm closing this and my interest in submitting new NIPs has gone to 0. if anyone else wants to pick this up then so be it. please do not implement this as it is no longer what damus implements.

@jb55 jb55 closed this Apr 28, 2023
@jb55
Copy link
Contributor Author

jb55 commented Apr 28, 2023

I will be uploading the old version of the spec at https://github.com/damus-io/dips if any other clients are interested in using this information in their clients.

@vitorpamplona
Copy link
Collaborator

Well, you folks know we are using NIP94 directly for all kinds of metadata. I am still open to implementing something that makes sense for regular URLs as long as it doesn't end up creating a duplicated structure.

@ChristopherKing42
Copy link

It's usually never a problem, urls are a weird edge case

Are there any other edge cases that should be clarified, so that future NIPs don't mess with them (or everyone agrees that messing with the edge case is fine)?

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.

None yet