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

Buf fix (ESN parsing): Update xmldialog_esnwidevine.py #1719

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Barri-G
Copy link

@Barri-G Barri-G commented Aug 19, 2024

Thanks for the add-on.
This is my first bug fix pull request, after founding a couple of lines to be changed when investigating current add-on problems (no video playback after 2 mins).

Description:

  1. Please, note that valid Web browser ESN have only 2 '-' (the same format generated when resetting settings).

  2. Also, additional Android patterns are used today (as you can check decompiling APK from https://github.com/aidanmacgregor/Netflix_ATV_L3_DRM_Uncertified_Mod).

Final Note:
This changes will allow users to test additional ESNs on different platforms to see if it affects the playback issue.

Regards!

Thanks for the add-on.
Found a couple of lines to be changed when investigating current add-on problems (no video playback after 2 mins).

1. Please, note that valid Web browser ESN have only 2 '-' (the same format generated when resetting settings).

2. Also, additional Android patterns are used today (as you can check decompiling APK from https://github.com/aidanmacgregor/Netflix_ATV_L3_DRM_Uncertified_Mod).

This changes will allow user to test additional ESNs on different platforms to see if it affects the playback issue.
@CastagnaIT
Copy link
Owner

This changes will allow users to test

it is not a task of users to randomly make attempts without known what they are doing

would have made sense if this PR provide a working use case
or at least proven that new ESNs are shown and used on the original apps
usually different types of ESNs have a specific purpose, and you provide no details nor theory basis

Also, additional Android patterns are used today (as you can check decompiling APK from https://github.com/aidanmacgregor/Netflix_ATV_L3_DRM_Uncertified_Mod).

you base your findings on unofficial code, not a good way to investigate, must be taken official android release and decompile it
also must to consider that the TV and mobile android versions are different, and must be compared both

the fix on "count" not tested but should be ok
anyway i will not merge this PR as is

@CastagnaIT CastagnaIT added the Don't merge PR that should not be merged (yet?) label Aug 23, 2024
@Barri-G
Copy link
Author

Barri-G commented Aug 23, 2024

Good afternoon,

You have the right to merge or not this PR, but I would like to clarify my points in case you reconsider your position. Upon reviewing my previous message, it's clear that I did not express myself correctly or in detail the first time.

This changes will allow users to test ==> This change will allow users to manually write known and valid ESNs

ESNs differ from browser to browser, and it is documented that browsers limit available resolutions for streaming (source: https://help.netflix.com/en/node/23931).

ESNs can be retrieved from the browser using devtools (this includes the Windows app, which is essentially an encapsulated browser). I believe this could be a valid use case.

Regarding Android ESNs, I have checked all my certified devices, and they still use the coded pattern (NFANDROID1-), so I cannot prove that the official app is using a new pattern. However, I use the referenced unofficial APK on an uncertified device (XGIMI projector), and that’s the only way Netflix works on it. I thought this might be sufficient proof to accept the new pattern.

That said, I want to express my admiration for your efforts with this plugin, and I look forward to future releases.

Have a great day.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Don't merge PR that should not be merged (yet?)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants