[WIP] proto3 proposal #17

lyoshenka · 2019-01-25T16:20:30Z

My proposed schema changes. this is very much WIP. I just wanted to see what it might look like and get your opinions on it.

Goals:

upgrade to proto3
drop Version from all protobuf definitions. One of the core ideas of protobufs is that versioning is not necessary
rename and rearrange some of the data to better match our current terminology and understanding of the protocol
add new fields that we know we need (releaseTime, file fields, tags, etc)

Keep in mind that in proto3, every field is optional. Also keep in mind that I'm trying to match this stuff to https://spec.lbry.io/#specification (e.g. Stream.hash vs Stream.sd_hash)

When you look at the field numbers, anything numbered 1-15 means we think that field will be set in most claims. Numbers 16+ means we think that field will not be set in most claims. There's no functional difference other than a small space savings if we predict it correctly.

I think the biggest questions are about the structure of the stream.proto file.

Closes #3, closes #16, closes #8, closes #13, closes #6, closes lbryio/internal-issues#205.

kauffj

Some comments, but I'm pretty much a protobuf idiot.

proto/stream.proto

kauffj · 2019-01-25T21:35:11Z

proto/stream.proto

+
+    Fee fee = 16;
+    string license_url = 17;
+    string thumbnail = 18;


This seems very safe to include in the top 15, I think.

proto/stream.proto

proto/claim.proto

kauffj · 2019-01-25T21:37:34Z

proto/claim.proto

-    optional Certificate certificate = 4;
-    optional Signature publisherSignature = 5;
+
+    Type type = 1; // do we need this?


It does seem like this could be inferred, though I could also see the metadata switching on type, which might be a reason to keep it?

kauffj · 2019-01-25T21:38:46Z

Is there anything else I should do to test or run this? We also have a semi-long list of fields to add. Would it make sense to add those before merging in case some should go in the top 15?

lyoshenka · 2019-01-28T18:02:06Z

putting this here so i don't forget: #8

lyoshenka · 2019-01-28T18:29:00Z

related: #15

lyoshenka · 2019-01-28T21:36:39Z

@kauffj yes, link any fields that need to be added here. i found a few PRs but not sure i got em all

shyba · 2019-01-29T14:13:27Z

great, maybe make a backup of the current one like /proto_legacy/? The current proto2 one is still needed for decoding claims.

lyoshenka · 2019-02-13T23:28:20Z

Questions re: channel metadata

what does Name mean? don't channels already have a name? is this like the difference between username and display name?
should cover and thumbnail be web URLs or LBRY URLs or claim IDs?

kauffj · 2019-02-14T15:46:05Z

proto/channel.proto

@@ -4,4 +4,8 @@ package pb;

 message Channel {
    bytes public_key = 1;
+    string name = 2;
+    string description = 3;


we may want a short description, limited to a reasonable number of characters and no markup, as well as an unbounded (well, bounded by claim size) field that would be shown on the full profile view

regarding: #17 (comment) - I think Name could be the channels real name, or whatever they want to nickname their channel...i.e. my channel is @iloveLBRY, but the name would be "I Love LBRY!"

How does everyone feel about adding some of my other suggestions from #8 -at least Language (and/or Location? - that's what YT has) and Featured Video (lbry URL/claimid).

should cover and thumbnail be web URLs or LBRY URLs or claim IDs? - I think this should be similar to our current thumbnail system, and if we decide to support LBRY urls/hashes there, then we should here too. For now, http urls / spee.ch links should work fine.

kauffj · 2019-02-14T15:53:49Z

proto/channel.proto

+    string name = 2;
+    string description = 3;
+    string thumbnail_url = 4; // url or claim ID?
+    string cover_url = 5; // url or claim ID?


whatever is determined by lbryio/lbry-sdk#1842

it may be better to have them directly as blob or stream hashes?

lyoshenka · 2019-02-15T15:08:57Z

Question for reviewers:

are any fields still missing?
are any fields unnecessary?
should the fields be structured differently?

tzarebczan · 2019-02-15T16:49:59Z

proto/stream.proto

+    Fee fee = 16;
+    string license_url = 17;
+    string thumbnail_url = 18;
+    uint32 duration = 19;


content height/length and orientation would be helpful for spee.ch and other apps too - see lbryio/spee.ch#435 / lbryio/spee.ch#527. We wouldn't have to run ffmpeg on new files anymore...

per @lyoshenka, we should add these under a dimensions subfield

what are the possible values for orientation?

@lyoshenka I would say horizontal/vertical, but we can expand that to 0/90/180/270 (https://stackoverflow.com/a/47676129/8308000). I asked Sean and he suggested a boolean for a Portrait setting.

tzarebczan · 2019-02-15T17:06:58Z

proto/stream.proto

+    string license_url = 17;
+    string thumbnail_url = 18;
+    uint32 duration = 19;
+    repeated string tags = 20;


Can tags have a "type" property the way it's defined currently? i.e. Category/topic/genre/other/etc.

i'm not sure what you mean. can you give me some examples?

Per the discussion this afternoon, this would pose UX challenges when users tagged content.

The idea was that a top level tag might be bound to some type, i.e. category, with predefined values. Then users could easily reference the category tag when searching/selecting based on tags, as opposed to more generic tags.

neb-b · 2019-02-18T22:04:39Z

What about a website/contact field? Most sites have something for you to add your email address or link to a website. I'm not sure if both would be valuable, or if you could just merge it into one field.

kauffj · 2019-02-18T22:43:26Z

👍 on adding both a website and an email field to channels @lyoshenka

eukreign · 2019-02-19T02:38:26Z

Question Responses:

I think name is the display name. Slightly less ambigious (not sure by how much) might be to use title insead of name, but I think name is fine too.
cover and thumbnail should be claim IDs

Feedback:

thumbnail_url -> thumbnail
cover_url -> cover
using float for the Fee is a bad idea. It should be an integer and then the precision is 2 decimals for fiat currencies (USD) and 8 decimals for crypto currencies (LBC/BTC). ($1USD would be 100 and 1LBC would be 100000000).

New Questions:

Does Channel make sense for certificates if we're planning to also use them for a users identity? I think the old Certificate is more generic when applying to both channels and identities.
Can they all go in one file? They are pretty short and it's easier to get the full picture with just one file open.
Stream vs. File is confusing. Are there Streams which aren't Files?
Insead of having Claim container type with Channel and Stream inside of it, can we just have Channel and Stream as root containers (and get rid of Claim)? (And perhaps add other root objects like Comment in the future?)

tzarebczan · 2019-02-19T04:39:27Z

I don't think I should have to download thumbnails/covers over DHT, unless the long term plan is to allow that for claim thumnails as well (lbryio/lbry-sdk#1842)

neb-b · 2019-02-19T06:33:49Z

I agree. I do not think thumbnail/cover should be a claim id.

I can see a case where it is a url or a claim id (or something else), but since thumbnail is already a url for a claim, this should follow the same behavior.

kauffj · 2019-02-20T18:12:14Z

@seanyesmunt note that as long as we encourage usage of spee.ch for images, we are encouraging thumbnail/cover to be a claim id just with extra steps ;)

kauffj · 2019-02-22T17:31:40Z

Another feature that just struck me as missing is creator identity verification (where verification equals proving I own a certain Twitter account, YouTube account, domain name, or ??). You would prove you own relevant property by issuing a tweet, publishing a video, adding a DNS entry / uploading a file, etc.

This is more complicated than basically everything else we're proposing to add, so I don't want to block on this, but if we can come up with a generic format or nail down a few of them specifically, it'd be good to include.

jackrobison · 2019-02-26T21:45:11Z

proto/channel.proto

+
+message Channel {
+    bytes public_key = 1;
+    string name = 2;


this is different from the name used to make the claim? is this like a title?

kauffj · 2019-02-27T15:00:47Z

@lyoshenka is anything blocking this?

lyoshenka · 2019-02-27T16:54:38Z

i'm dropping the ability to list payments in BTC. we're just keeping usd and lbc.

first attempt at converting to proto3 and some housekeeping

4223fd2

lyoshenka requested review from eukreign, kauffj, shyba and jackrobison January 25, 2019 16:20

lbry-bot assigned eukreign, shyba, jackrobison and kauffj and unassigned shyba and kauffj Jan 25, 2019

kauffj reviewed Jan 25, 2019

View reviewed changes

lyoshenka mentioned this pull request Jan 28, 2019

Rename content_type to media_type #6

Closed

lyoshenka mentioned this pull request Jan 31, 2019

Add releaseTime to schema #13

Closed

shyba mentioned this pull request Feb 6, 2019

Adapt code to handle proto3 and legacy proto2 lbryio/lbry-sdk#1870

Closed

7 tasks

lyoshenka mentioned this pull request Feb 13, 2019

Add channel related metadata columns #8

Closed

10 tasks

more changes

69807aa

kauffj reviewed Feb 14, 2019

View reviewed changes

lyoshenka requested a review from kauffj February 15, 2019 15:07

lbry-bot assigned kauffj Feb 15, 2019

lyoshenka requested a review from tzarebczan February 15, 2019 15:08

lbry-bot assigned tzarebczan Feb 15, 2019

lbry-bot unassigned tzarebczan Feb 15, 2019

tzarebczan reviewed Feb 15, 2019

View reviewed changes

jackrobison reviewed Feb 26, 2019

View reviewed changes

lbry-bot unassigned jackrobison and kauffj Feb 26, 2019

release time

5640bec

lyoshenka force-pushed the proto3 branch from 969afed to 5640bec Compare February 27, 2019 16:40

lyoshenka mentioned this pull request Feb 27, 2019

new proto3 definitions #21

Merged

shyba closed this in #21 Feb 27, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] proto3 proposal #17

[WIP] proto3 proposal #17

lyoshenka commented Jan 25, 2019 •

edited

Loading

kauffj left a comment

kauffj Jan 25, 2019

kauffj Jan 25, 2019

kauffj commented Jan 25, 2019

lyoshenka commented Jan 28, 2019

lyoshenka commented Jan 28, 2019

lyoshenka commented Jan 28, 2019

shyba commented Jan 29, 2019

lyoshenka commented Feb 13, 2019

kauffj Feb 14, 2019

tzarebczan Feb 15, 2019

tzarebczan Feb 16, 2019

kauffj Feb 14, 2019

lyoshenka commented Feb 15, 2019

tzarebczan Feb 15, 2019

tzarebczan Feb 19, 2019

lyoshenka Feb 27, 2019

tzarebczan Feb 28, 2019

tzarebczan Feb 15, 2019

lyoshenka Feb 18, 2019

tzarebczan Feb 18, 2019

neb-b commented Feb 18, 2019

kauffj commented Feb 18, 2019

eukreign commented Feb 19, 2019 •

edited

Loading

tzarebczan commented Feb 19, 2019

neb-b commented Feb 19, 2019

kauffj commented Feb 20, 2019

kauffj commented Feb 22, 2019

jackrobison Feb 26, 2019

kauffj Feb 27, 2019

kauffj commented Feb 27, 2019

lyoshenka commented Feb 27, 2019

[WIP] proto3 proposal #17

[WIP] proto3 proposal #17

Conversation

lyoshenka commented Jan 25, 2019 • edited Loading

kauffj left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kauffj commented Jan 25, 2019

lyoshenka commented Jan 28, 2019

lyoshenka commented Jan 28, 2019

lyoshenka commented Jan 28, 2019

shyba commented Jan 29, 2019

lyoshenka commented Feb 13, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lyoshenka commented Feb 15, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

neb-b commented Feb 18, 2019

kauffj commented Feb 18, 2019

eukreign commented Feb 19, 2019 • edited Loading

tzarebczan commented Feb 19, 2019

neb-b commented Feb 19, 2019

kauffj commented Feb 20, 2019

kauffj commented Feb 22, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kauffj commented Feb 27, 2019

lyoshenka commented Feb 27, 2019

lyoshenka commented Jan 25, 2019 •

edited

Loading

eukreign commented Feb 19, 2019 •

edited

Loading