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

Specification draft #2

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Specification draft #2

wants to merge 8 commits into from

Conversation

SamantazFox
Copy link
Contributor

Very first draft of a specification

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Co-authored-by: ChunkyProgrammer <78101139+ChunkyProgrammer@users.noreply.github.com>
@SamantazFox
Copy link
Contributor Author

@ChunkyProgrammer Thanks ^^

Copy link

@FireMasterK FireMasterK left a comment

Choose a reason for hiding this comment

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

I also feel we should have a decoupled playlist and subscription files.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated
#### > `"thumbnail"`

The URL to the user/channel's thumbnail.

Choose a reason for hiding this comment

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

I feel this should be an optional field, since:

  • It's not really necessary for importing subscriptions.
  • Can change and 404 in the future.

It's also ambiguous whether this should be a direct link to the image or a proxied URL to the media.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

resolved in 5a9c3d9

README.md Outdated Show resolved Hide resolved
README.md Outdated

#### > `"videos"`

An array of `String`.
Copy link
Member

Choose a reason for hiding this comment

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

Should we add thumbnail, duration and published date (utc) optional fields for the video?

Copy link

Choose a reason for hiding this comment

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

Probably not as these can just be fetched by the app, and just bloats the format for no good reason.

Copy link

@Stypox Stypox Apr 26, 2023

Choose a reason for hiding this comment

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

I think it should be consistent across the document, so either an (optional) thumbnail is allowed in all places, or it is not allowed anywhere

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've made the thumbnail present everywhere, but I don't think we should save more metadata in here.
I'm afraid that it'd grew out of control.

@somerandomguy24
Copy link

I also feel we should have a decoupled playlist and subscription files.

Or maybe just be able to choose which you want to export within the app.


The visibility of the playlist.

Accepted values: "public", "unlisted", "private"

Choose a reason for hiding this comment

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

There should also be a value for when the playlist is stored locally rather than on an external server.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This part of the spec only covers user-created playlists.
I think that saved/subscribed playlists (= not created by the user) should be stored differently.

Signed-off-by: Samantaz Fox <coding@samantaz.fr>
Signed-off-by: Samantaz Fox <coding@samantaz.fr>
@SamantazFox
Copy link
Contributor Author

SamantazFox commented May 7, 2023

I've made the changes that were proposed in the various reviews above (may have missed some, sorry):

  • Separated playlists into different files
  • created a new object type (similar to subscriptions) for videos elements
  • Added type (service name) and id (unique id) to subscription to ease parsing
  • Added thumbnails to all objects, but made them optional
  • Added a new file for "saved" (or "bookmarked") playlists

Signed-off-by: Samantaz Fox <coding@samantaz.fr>
#### `"watch_history_present"`

A Boolean.

Indicates whether the special playlist `watch_history` was included in the export.

Choose a reason for hiding this comment

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

Should we really be doing this? Watch history can have additional fields such as the watched till duration, and additional metadata. I would suggest an additional format to store this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This field is only here to indicate if the watch_history file is part of the export.
The format of the watch history can still evolve regardless!

The description of the playlist.
Format: plain text (? TBD)

CAN be nil; In this case, the parser MUST assume an empty `String`.

Choose a reason for hiding this comment

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

Why is it assuming an empty string if the value is nil?

Choose a reason for hiding this comment

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

Agree, I think it's perfectly reasonable to assume null for a description if none was ever set.

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

5 participants