Skip to content

Add support to pre-initialize the DRM#725

Merged
glennguy merged 1 commit intoxbmc:Matrixfrom
CastagnaIT:licensed_manifest
Jul 13, 2021
Merged

Add support to pre-initialize the DRM#725
glennguy merged 1 commit intoxbmc:Matrixfrom
CastagnaIT:licensed_manifest

Conversation

@CastagnaIT
Copy link
Collaborator

@CastagnaIT CastagnaIT commented Jun 28, 2021

This PR add the support to pre-initialize the DRM with a PSSH+KID provided by an add-on via ListItem property.

The purpose is to allow add-ons to make licensed manifest requests via ISA manifest proxy callback.
In order to do licensed manifest requests is needed two data:

  • The challenge
  • The session id

As discussed previously on #666 and CastagnaIT/plugin.video.netflix#1129 (comment) i have implemented all the things.

In brief:

  • The add-on add in the play callback this property (to the list item passed to xbmcplugin.setResolvedUrl) :
    setProperty('inputstream.adaptive.pre_init_data', 'pssh|kid')
    by providing the PSSH and the KID values both encoded as base64 and splitted by a |
  • When ISA see the pre_init_data property data, pre initialize the DRM with the PSSH+KID provided
    At this point we have the DRM challenge and the session id generated
  • When ISA do the manifest proxy callback to the add-on, attach to the request these two headers:
    challengeB64 (encoded as base64) and sessionId
  • The add-on when receive the manifest proxy callback can read the challenge and the session id from the headers

NOTE: This not change in any way the current use of the manifest request

The tests were conducted with ntfx add-on, where if the challenge/sid were wrong, video playback would not take place,
and on ARM devices it is the only way to achieve 1080P resolution.

Tested on:

  • ARM device with last CoreELEC nightly with Kodi 19 ✅
  • Windows 10 64 bit with latest Kodi 19 release ✅
  • Android Widevine L3 with latest Kodi 19 release ✅
  • Android Widevine L1 with latest Kodi 19 release ✅
  • Linux Mint with latest Kodi 19 release ✅

@matthuisman
Copy link
Contributor

matthuisman commented Jun 29, 2021

awesome work.

one thing I noticed after a quick look is no need to make changes to wvdecrypter_android.cpp
That actually isn't used anymore.

wvdecrypter_android_jni.cpp is the one used

I have a PR open to clean that up #697
One of the main reasons because it isn't clear which is actually used :)

oh, and I think you need to bump the ssd version number here too if changes are made to the decrypter:
https://github.com/xbmc/inputstream.adaptive/blob/Matrix/src/SSD_dll.h#L24

@CastagnaIT
Copy link
Collaborator Author

wvdecrypter_android_jni.cpp is the one used

each time i did not understand which one was used.. i will move the changes to the other file

@CastagnaIT CastagnaIT marked this pull request as ready for review June 30, 2021 16:25
@CastagnaIT
Copy link
Collaborator Author

I have done the suggested changes, now is ready for review,
please wait to merge until i will get a feedback from a WV L1 android device with 4K

@CastagnaIT
Copy link
Collaborator Author

CastagnaIT commented Jul 2, 2021

i have a problem discovered only now (and reported also by others) because i was not aware that was needed reboot kodi every time i update ISA to make libreelec works with the updated binary

the problem in my theory after a lot of tests, is that in the challenge data sometimes there are some special chars that i am not able to send to the http headers, results that in the data added in the "challengeB64" header is ignored?! and leaves the value blank

i have tried also update the b64_encode helper with new code, with no improvement.
i have tried also enclose the challenge two times as b64_encode, with no improvement.
i have tried send the challenge data by making a manifest Post request by add the param:
file.CURLAddOption(ADDON_CURL_OPTION_PROTOCOL, "postdata", postData.c_str());
in the manifest download method, in this way seem works better, but after 1 or 2 playback the problem happen again, blank challenge value...

and the challenge data there is always, this is the data size reported of
challenge_ in wvdecrypter.cpp
when works and also when not works (blank data sent):
GetChallengeB64Data CHALLENGE DATA SIZE: (1773)

I ran out of ideas on how to proceed

UPDATE: in the
file.CURLAddOption(ADDON_CURL_OPTION_PROTOCOL, "postdata", postData.c_str());
the data is passed then the problem could be in curl

@CastagnaIT
Copy link
Collaborator Author

found a way to keep Ap4CommonEncryption.h unchanged
and in this way i have cast with the WV_CencSingleSampleDecrypter class the decripter to get the value
but unfurnately the problem is not resolved sometimes works other no

this is an example of the log:

2021-07-03 15:18:31.744 T:5297    DEBUG <general>: AddOnLog: inputstream.adaptive: PreInitializeDRM - Entering encryption section
2021-07-03 15:18:31.786 T:5297    DEBUG <general>: AddOnLog: inputstream.adaptive: CDM version: 4.10.2252.0
2021-07-03 15:18:31.967 T:5297    DEBUG <general>: AddOnLog: inputstream.adaptive: cdm::OnInitialized: true
2021-07-03 15:18:32.063 T:5297    DEBUG <general>: AddOnLog: inputstream.adaptive: PreInitializeDRM - Initializing session with KID: 0000000003D267490000000000000000
2021-07-03 15:18:32.404 T:5105    DEBUG <general>: ------ Window Deinit (DialogNotification.xml) ------
2021-07-03 15:18:32.786 T:5297    DEBUG <general>: AddOnLog: inputstream.adaptive: CDMMessage: 1 arrived!
2021-07-03 15:18:32.823 T:5297    ERROR <general>: AddOnLog: inputstream.adaptive: GetChallengeB64Data: is executed
2021-07-03 15:18:32.823 T:5297    ERROR <general>: AddOnLog: inputstream.adaptive: etChallengeB64Data: converted data: ¤F5à�¦�Ú�ÃÿÐ0ºaêí�
2021-07-03 15:18:32.823 T:5297    DEBUG <general>: AddOnLog: inputstream.adaptive: GetChallengeB64Data: data set x&Ò
2021-07-03 15:18:32.848 T:5297    DEBUG <general>: AddOnLog: inputstream.adaptive: CDMMessage: 4 arrived!
2021-07-03 15:18:32.871 T:5297    DEBUG <general>: CurlFile::ParseAndCorrectUrl() adding custom header option 'challengeB64: '
2021-07-03 15:18:32.871 T:5297    DEBUG <general>: CurlFile::ParseAndCorrectUrl() adding custom header option 'sessionId: CB1CB479F8DCDF56CC2F3EF54744E32B'
2021-07-03 15:18:32.871 T:5297    DEBUG <general>: CurlFile::Open(0xea61b430) http://127.0.0.1:53821/msl/get_manifest?videoid=80230399
2021-07-03 15:18:32.871 T:5297    DEBUG <general>: easy_acquire - Created session to http://127.0.0.1
2021-07-03 15:18:32.873 T:5299    DEBUG <general>: [plugin.video.netflix (0)] HTTP Server: received GET request /msl/get_manifest?videoid=80230399
2021-07-03 15:18:32.873 T:5299    ERROR <general>: ----------------------------------------
2021-07-03 15:18:32.873 T:5299    ERROR <general>: 
                                                   
2021-07-03 15:18:32.873 T:5299    ERROR <general>: Exception happened during processing of request from
###cutted addon raised exception due to None value###

@glennguy
Copy link
Contributor

glennguy commented Jul 3, 2021

In GetChallengeB64() you're returning a c_str() of a local variable, the local variable is destroyed soon after the function exits so sometimes you're able to read the information in time, other times it's too late and the pointer is junk.
One way would be to get it to return std::string instead then call c_str() where you need it in main.cpp

@CastagnaIT
Copy link
Collaborator Author

@glennguy it seems to have worked!!! I played 6 videos without any problems, great 🤩
I'll have the other guys run some tests to confirm

@CastagnaIT CastagnaIT force-pushed the licensed_manifest branch from 73fdaab to 5d61d89 Compare July 4, 2021 11:59
@CastagnaIT
Copy link
Collaborator Author

CastagnaIT commented Jul 6, 2021

i have done all tests with success
this PR is done you can review

@CastagnaIT
Copy link
Collaborator Author

I need to do some changes but can only be done after the freeze fix PR will be merged

@glennguy glennguy requested a review from phunkyfish July 8, 2021 11:46
@phunkyfish
Copy link
Contributor

I need to do some changes but can only be done after the freeze fix PR will be merged

Please post when this is the case and I will review.

@phunkyfish phunkyfish closed this Jul 9, 2021
@phunkyfish phunkyfish reopened this Jul 9, 2021
@phunkyfish
Copy link
Contributor

phunkyfish commented Jul 9, 2021

Apologies, hit wrong button. No intention to close this.

@CastagnaIT CastagnaIT force-pushed the licensed_manifest branch from 5d61d89 to 0488770 Compare July 9, 2021 12:38
@CastagnaIT
Copy link
Collaborator Author

@phunkyfish i have rebased and done the last changes

@CastagnaIT CastagnaIT force-pushed the licensed_manifest branch 3 times, most recently from d0112a9 to 7d8c48a Compare July 9, 2021 14:01
@CastagnaIT CastagnaIT force-pushed the licensed_manifest branch from 7d8c48a to 02b3bb2 Compare July 9, 2021 14:04
Comment on lines +3541 to +3548
m_session = std::shared_ptr<Session>(new Session(
manifest, url.c_str(), mfup, lt, lk, ld, lsc, manh, medh, props.GetProfileFolder(), m_width,
m_height, ov_audio, m_playTimeshiftBuffer, force_secure_decoder, drmPreInitData));
Copy link
Contributor

Choose a reason for hiding this comment

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

For a future refactor. This function call makes little to no sense at all.


int retrycount=0;
while (session_.empty() && ++retrycount < 100)
while (!drm.GetCdmAdapter()->IsSessionActive() && ++retrycount < 100)
Copy link
Contributor

@phunkyfish phunkyfish Jul 9, 2021

Choose a reason for hiding this comment

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

Also not for this PR but this is not a good way to do this. A wait for condition would be much better. Then no need to thread::sleep_for which is expensive.

Allow add-ons to make licensed manifest request via proxy callback
@glennguy glennguy merged commit 474f41b into xbmc:Matrix Jul 13, 2021
@CastagnaIT CastagnaIT deleted the licensed_manifest branch August 26, 2021 12:47
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.

6 participants

Comments