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

docs and support for player and library events #241

Open
wants to merge 3 commits into
base: feat/library-and-player-resources
Choose a base branch
from

Conversation

dexter21767-dev
Copy link
Member

No description provided.

Copy link
Member

@elpiel elpiel left a comment

Choose a reason for hiding this comment

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

LGTM so far!
We still need to define the request of routes in protocol.md:

  • define library route and params
  • define player route and params

Copy link
Member

@elpiel elpiel left a comment

Choose a reason for hiding this comment

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

Let me know if I've missed something @dexter21767-dev

@@ -11,7 +11,7 @@ This allows Stremio or other similar applications to aggregate content seamlessl

To define a minimal addon, you only need an HTTP server/endpoint serving a `/manifest.json` file and responding to resource requests at `/{resource}/{type}/{id}.json`.

Currently used resources are: `catalog`, `meta`, `stream`, `subtitles`, `watchStatus`.
Currently used resources are: `catalog`, `meta`, `stream`, `subtitles`, `player`, `library`.
Copy link
Member

Choose a reason for hiding this comment

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

  • add playback

docs/api/requests/definePlayerHandler.md Show resolved Hide resolved
Comment on lines 29 to 32
- Play: `action=play&currentTime={milliseconds}&duration={milliseconds}`
- Start: `action=start&currentTime={milliseconds}&duration={milliseconds}`
- End: `action=end&currentTime={milliseconds}&duration={milliseconds}`
- Pause: `action=pause&currentTime={milliseconds}&duration={milliseconds}`
Copy link
Member

Choose a reason for hiding this comment

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

  • Play and Pause should be separated into the playback events resource
  • As we've discussed in Slack, duration and currentTime might not be available and are not required for each of these actions and must be made optional. Only Play and Pause make sense with those arguments.

Comment on lines +37 to +38
- Add to library: `action=libraryAdd`
- Remove from library: `action=libraryRemove`
Copy link
Member

Choose a reason for hiding this comment

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

I suggest to rename these to add and remove respectively.
We are already under the library resource and a simple action name would be sufficient

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.

2 participants