create watchman video.js plugin #6784

Closed

DispatchCommit wants to merge 1 commit from watchman-plugin-odysee into odysee

DispatchCommit commented

2021-08-04 17:59:16 +02:00

(Migrated from github.com)

No description provided.

mayeaux (Migrated from github.com) reviewed 2021-08-04 17:59:16 +02:00

mayeaux commented

2021-08-05 13:11:14 +02:00

(Migrated from github.com)

Pretty good implementation, but missing a few things/has a few concerns:

Here is the definition of the API.
https://github.com/lbryio/odysee-api/blob/watchman/apps/watchman/gen/http/openapi.yaml

So it is pointed at the wrong endpoint and should be pointed at the /reports/playback route
https://github.com/lbryio/odysee-api/blob/watchman/apps/watchman/gen/http/openapi.yaml#L45

If we look at this example request made to the Watchman service, there are a few issues:

rebuf_count is not being properly reset due to the implementation of the event by the tracking plugin. There should be a global variable that stores the previous rebuf_count from which the value to be sent to the backend is determined through subtraction
device value is expecting an enum not a broad user agent. We discussed sending the raw user agent but it's simple enough to use a simple solution to send the enum value, we should follow the spec Andrey is requesting until we have agreement on changing it.
https://github.com/lbryio/odysee-api/blob/watchman/apps/watchman/gen/http/openapi.yaml#L95
player is unpopulated (sometimes it comes through but other times it does not, something to look into)
https://github.com/lbryio/odysee-api/blob/watchman/apps/watchman/gen/http/openapi.yaml#L105
rel_position is in the wrong format and needs to be multiplied by 100 to be given the proper value
https://github.com/lbryio/odysee-api/blob/watchman/apps/watchman/gen/http/openapi.yaml#L133
url is also wrong format:
https://github.com/lbryio/odysee-api/blob/watchman/apps/watchman/gen/http/openapi.yaml#L139
The example in the docs is wrong but it should be a format such as: lbry://@Kona_and_Suba_Guinea_Pig_Adventures#c/lunaandchip#7

There is also the edge case of what happens when someone has buffer events while seeking around the video. Andrey and I arrived at an implementation where the interval would be reset and the existing data would be sent off, while starting a new interval from the point of the seek event. That should be implemented to capture those data events.

Overall, a good implementation but needs more attention to detail to be brought into line with what was defined in the Open API specification.

Pretty good implementation, but missing a few things/has a few concerns: Here is the definition of the API. https://github.com/lbryio/odysee-api/blob/watchman/apps/watchman/gen/http/openapi.yaml So it is pointed at the wrong endpoint and should be pointed at the `/reports/playback` route https://github.com/lbryio/odysee-api/blob/watchman/apps/watchman/gen/http/openapi.yaml#L45 ![Screen Shot 2021-08-05 at 12 55 15](https://user-images.githubusercontent.com/7200471/128338900-24e87a92-7ad7-44c3-a5ce-b89fdbf2a757.jpg) If we look at this example request made to the Watchman service, there are a few issues: 1) `rebuf_count` is not being properly reset due to the implementation of the event by the tracking plugin. There should be a global variable that stores the previous `rebuf_count` from which the value to be sent to the backend is determined through subtraction 2) `device` value is expecting an enum not a broad user agent. We discussed sending the raw user agent but it's simple enough to use a simple solution to send the `enum` value, we should follow the spec Andrey is requesting until we have agreement on changing it. https://github.com/lbryio/odysee-api/blob/watchman/apps/watchman/gen/http/openapi.yaml#L95 3) `player` is unpopulated (sometimes it comes through but other times it does not, something to look into) https://github.com/lbryio/odysee-api/blob/watchman/apps/watchman/gen/http/openapi.yaml#L105 4) `rel_position` is in the wrong format and needs to be multiplied by 100 to be given the proper value https://github.com/lbryio/odysee-api/blob/watchman/apps/watchman/gen/http/openapi.yaml#L133 5) `url` is also wrong format: https://github.com/lbryio/odysee-api/blob/watchman/apps/watchman/gen/http/openapi.yaml#L139 The example in the docs is wrong but it should be a format such as: lbry://@Kona_and_Suba_Guinea_Pig_Adventures#c/lunaandchip#7 There is also the edge case of what happens when someone has buffer events while seeking around the video. Andrey and I arrived at an implementation where the interval would be reset and the existing data would be sent off, while starting a new interval from the point of the seek event. That should be implemented to capture those data events. Overall, a good implementation but needs more attention to detail to be brought into line with what was defined in the Open API specification.

❤️ 2

tzarebczan commented

2021-08-05 18:57:08 +02:00

(Migrated from github.com)

Thanks @mayeaux! https://watchman.na-backend.dev.odysee.com doesn't seem to exist, don't think there's a dev server for it @andybeletsky ?

Other notes:
missing position field
missing protocol (if it's the mp4 direct stream or hls feed)
device example shows "wb" but we send a whole user agent string. Probably fine?
cache - this should come from video header response for cache hit

x-cache: HIT

player should come from headers:

x-powered-by: use-p2

Posted question about TTFB metric: https://github.com/lbryio/odysee-api/issues/350#issuecomment-893608146

Thanks @mayeaux! https://watchman.na-backend.dev.odysee.com doesn't seem to exist, don't think there's a dev server for it @andybeletsky ? Other notes: missing position field missing protocol (if it's the mp4 direct stream or hls feed) device example shows "wb" but we send a whole user agent string. Probably fine? cache - this should come from video header response for cache hit ``` x-cache: HIT ``` player should come from headers: ``` x-powered-by: use-p2 ``` Posted question about TTFB metric: https://github.com/lbryio/odysee-api/issues/350#issuecomment-893608146