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

Add "My programs" menu #213

Merged
merged 1 commit into from May 15, 2019
Merged

Add "My programs" menu #213

merged 1 commit into from May 15, 2019

Conversation

dagwieers
Copy link
Collaborator

@dagwieers dagwieers commented May 11, 2019

This PR adds a way of tracking the programs you follow.

  • Add a context menu to programs or episodes with either "Follow" or "Unfollow"
  • Download and cache favorites from VRT NU
  • Add an A-Z listing of followed programs
  • Add a Recent listing of followed programs
  • Add invisible setting 'usefavorites' to disable "My programs" (for unit tests)
  • Disable brand filter in "My programs" recent listing (should be configurable really)

This fixes #181

@dagwieers dagwieers added the enhancement New feature or request label May 11, 2019
@dagwieers dagwieers added this to the v2.0.0 milestone May 11, 2019
@dagwieers dagwieers force-pushed the favorites branch 9 times, most recently from 9af9b48 to 264f983 Compare May 12, 2019 02:28
@dagwieers dagwieers changed the title WIP: Add "My programs" menu Add "My programs" menu May 12, 2019
@dagwieers dagwieers force-pushed the favorites branch 2 times, most recently from 33c9fc6 to 9e9a07c Compare May 12, 2019 20:42
@dagwieers dagwieers mentioned this pull request May 12, 2019
@dagwieers dagwieers force-pushed the favorites branch 11 times, most recently from 2781d73 to de3f996 Compare May 12, 2019 22:33
@mediaminister
Copy link
Collaborator

mediaminister commented May 14, 2019

I know why the "My Programs" feature doesn't work properly. You need a different "new" X-VRT-Token.

The X-VRT-Token we use now to get the VOD streams that require a login doesn't work with https://video-user-data.vrt.be
But this new X-VRT-Token has disadvantages in comparison with the old X-VRT-Token.
The old X-VRT-Token is one year valid and requesting one takes almost no time, just a couple http requests takes no more than 500 ms. With the cache of one year, it almost never slows down video playback.
The new X-VRT-Token is only one hour valid and it takes sometimes more than 1.5 seconds to request one. VRT uses many requests and callbacks between different pages for a simple login. A very bad design. With the new X-VRT-Token, video playback will definitely start slower, because every hour, you need a new X-VRT-Token.

For testing purposes only, I implemented the new "slow" X-VRT-Token to my test branch: mediaminister@6463b66

But, I don't like the fact that the "My programs" feature will slow down video playback or menu loading. I noticed this feature requests an X-VRT-Token when starting the add-on. This blocks loading the main menu for seconds. I don't like that.

@dagwieers
Copy link
Collaborator Author

dagwieers commented May 14, 2019

@mediaminister Thanks for the investigative work!

However it should not slow down video playback or menu loading. As it only needs to be downloaded once every hour. And we can disable 'My programs' by default if that would be an issue.

BTW One option could be asynchronous downloads for cached objects. More caching (e.g. for A-Z listing and categories) is on my wishlist.

I noticed this feature requests an X-VRT-Token when starting the add-on. This blocks loading the main menu for seconds. I don't like that.

It shouldn't. Unless you enter the "My programs" menu and the favorites cache has expired and the token is invalidated, no token is being downloaded. So we already go to great lengths to make this fast. Improvements are definitely welcome.

I used to have an option to disable "My programs", but I made that invisible since I don't think this negatively impacts our users, instead I think this a great feature. I had my doubts, but I use it now all the time. This also makes it possible for Parental Control as it gives parents the possibility to select the programs children are allowed to watch and keep everything else locked.

@dagwieers
Copy link
Collaborator Author

I noticed your implementation replaced the original XVRTToken code, but I would separate them, and only use the short-lived token when it is needed.

@dagwieers dagwieers force-pushed the favorites branch 4 times, most recently from 1d79a0a to fc48abc Compare May 15, 2019 03:23
@dagwieers
Copy link
Collaborator Author

dagwieers commented May 15, 2019

So I would like to merge this, sooner rather than later.
Once it is merged we can work on improving this code e.g. it doesn't impact users that don't use it.
The different tokens are separated too.

Everything that's unrelated to "My programs" has been removed from this PR and merged in separate PRs.

This PR adds a way of tracking the programs you follow.
- Add a context menu to programs or episodes with either "Follow" or "Unfollow"
- Download and cache favorites from VRT NU
- Add an A-Z listing of followed programs
- Add a Recent listing of followed programs
- Add invisible setting 'usefavorites' to disable "My programs" (for unit tests)
- Disable brand filter in "My programs" recent listing (should be configurable really)
@mediaminister
Copy link
Collaborator

mediaminister commented May 15, 2019

Still one big problem: "My programs" asks the user for credentials and requests an X-VRT-Token when starting the add-on. This blocks loading the main menu for seconds. This impacts all (new) users, especially those who don't use the "my programs" menu.
Don't forget that you don't need a VRT-login to watch the live streams. So it is totally unacceptable that we ask people for a VRT-login when the add-on starts.

@dagwieers
Copy link
Collaborator Author

You are right, and it's easy to fix by only activating "My programs" when we have credentials and an token. These things are fixable and I agree we shouldn't annoy users that only are interested in the live streams, or browsing the programs without ever playing one :-)

So let's fix it, rather than repeat the issues.

@dagwieers dagwieers merged commit eb5b426 into add-ons:master May 15, 2019
@mediaminister
Copy link
Collaborator

mediaminister commented May 15, 2019

That's a workaround. "My Programs" shouldn't make any requests to a server in the constructor. Especially when this is privacy sensitive information and the user has not initiated the request.
These requests should always be initiated by the user. For instance, when a corresponding menu item is clicked or a setting is activated.

Just my point of view. I really don't like the fact that the add-on is downloading all my personal tv viewing data in a json file without me knowing or allowing it.

@dagwieers
Copy link
Collaborator Author

dagwieers commented May 15, 2019

I really don't like the fact that the add-on is downloading all my personal tv viewing data in a json file without me knowing or allowing it.

  1. It is not all your personal TV viewing data, it's only the favorites you selected.
  2. The website is doing the exact same thing without opt-in or opt-out.
  3. If you didn't select favorites, nothing is downloaded (I guess that is the opt-out).

Despite that, if you are concerned people are snooping the stuff your are viewing because of the JSON download, you are not trusting HTTPS. If that concern is warranted, you have a big problem using on-demand streaming, because that would be actual TV viewing data and is also using HTTPS.

If the concern is that these favorites are stored on your system, bad luck again, Kodi is storing even more actual TV viewing data already. And not just from VRT NU only.

So I don't agree with your assessment, your concerns are general issues related to on-demand streaming or maybe the security concerns with TLS. We could add a warning, but that would relate to the whole addon, not just the "My program" feature, but honestly, a warning would not be able to provide the right depth and balance, and scare most people needlessly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

'Most Recent', but only of selected Favorites (Follow-support)
2 participants