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

[DRM] Revert to single session decryption (when possible) #1165

Merged
merged 6 commits into from
Apr 12, 2023

Conversation

glennguy
Copy link
Contributor

Description

Many problems are now being observed from changes relating back to the introduction of CBCS decryption. For this change separate sessions were maintained, partly in an effort to avoid having more Bento4 patches. Now we observe the multiple license requests involved appear to inform service providers that there are multiple concurrent streams playing, causing license rejection after some duration of playback.

Another prior change was to force separate audio/video sessions on Android to resolve 'pixelation' issues. This workaround has worked but again as above has side effects. This issue should be investigated separately and a proper solution found.

Motivation and context

In an effort to resolve issues experienced in #1147

How has this been tested?

Runtime tests on CBCS and CENC test streams show no side effects. Artifacts of this PR will be distributed for user testing in particular on Android.

Screenshots (if appropriate):

Types of change

  • Bug fix (non-breaking change which fixes an issue)
  • Clean up (non-breaking change which removes non-working, unmaintained functionality)
  • Improvement (non-breaking change which improves existing functionality)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that will cause existing functionality to change)
  • Cosmetic change (non-breaking change that doesn't touch code)
  • None of the above (please explain below)

Checklist:

  • I have read the Contributing document
  • My code follows the Code Guidelines of this project
  • My change requires a change to the Wiki documentation
  • I have updated the documentation accordingly

@matthuisman
Copy link
Contributor

matthuisman commented Feb 24, 2023

looking at red lines vs green lines - seems this change simplifies the code quite a bit :)

Looking at your bento branch, seems the patch would be quite small?
Just this commit if im not mistaken: glennguy/Bento4@3694e15

@matthuisman
Copy link
Contributor

matthuisman commented Feb 24, 2023

have tested with kayo

device 1) started windows stream - OK
device 2) started android 32bit stream - OK
device 3) started android 64bit stream - OK

once device .3 started - device .1 started getting the DRM errors (but kept playing)
Stopped device .1 and restarted it
Now device .2 starts getting DRM errors (but kept playing)

So seems Kayo just "kicks off" the oldest device when a new stream starts
Suprised device .2 still playing with drm error
Maybe it will stop after more time - will keep it playing

In summary -works really well!! thank you so much @glennguy :)

@glennguy
Copy link
Contributor Author

Thanks for testing @matthuisman !

Yes just changing the function signature is all that's needed in Bento4.

@CastagnaIT
Copy link
Collaborator

CastagnaIT commented Feb 25, 2023

@matthuisman remember to never use # char to refer to a point on a list, because currently you have linked the PR to other issues/PR's ... we prefer to avoid it

@scottydulton
Copy link

@matthuisman which 32 bit device did you test on?

@matthuisman
Copy link
Contributor

Nvidia shield tube

@scottydulton
Copy link

scottydulton commented Feb 27, 2023

@matthuisman still cant wrap my head around what tells the stream to parseandcorrect(from windows log) the url after license fail on all platforms except fire tv haha. and my fire TV log is very bare compared to windows log, far less information, is that normal? ive got all of the same component specific items enabled

@CastagnaIT
Copy link
Collaborator

i tried test this with n€tflix VP9.2 but something broken decrypting
https://paste.kodi.tv/atabitepoz.kodi

@glennguy
Copy link
Contributor Author

glennguy commented Mar 4, 2023

I never checked the preinit code, will have a look today. I notice on the second licence call one of the keys comes back with a bad status code and that's the one that's failing

@glennguy
Copy link
Contributor Author

glennguy commented Mar 5, 2023

@CastagnaIT would it be possible to get a copy of log where it works?

@CastagnaIT
Copy link
Collaborator

CastagnaIT commented Mar 5, 2023

here https://paste.kodi.tv/mokivovitu.kodi

preinit dont have to make license call

@CastagnaIT
Copy link
Collaborator

the bento patch to SetFragmentInfo has also reversed method argument names, crypt vs skip blocks but seem to no affect the problem i found
however the bento patch added is irrilevant, the SetFragmentInfo is a custom method added by us that can be safetely removed
you can add the new virtual method directly to the Adaptive_CencSingleSampleDecrypter

@glennguy glennguy force-pushed the wv-single-instance branch 3 times, most recently from 70d68bd to 5c6fb0f Compare April 8, 2023 05:20
@glennguy
Copy link
Contributor Author

glennguy commented Apr 8, 2023

@CastagnaIT I think this is ready to go if you want to have another look? Working with NF now.

It won't be merged until done with your PR - reviewing that now as I type this. I will redo this to suit afterwards.

@CastagnaIT
Copy link
Collaborator

thanks
yes i will do a new review after that will be adapted with my rework PR

@matthuisman
Copy link
Contributor

I suspect the big rework won't be backported? If this PR is updated for the rework, will this still be backported?

@glennguy
Copy link
Contributor Author

glennguy commented Apr 8, 2023

I suspect the big rework won't be backported?

Correct, Kodi Nexus is at release so we're not doing things of that nature in that branch, only bugfixes and feature backports if suitable

If this PR is updated for the rework, will this still be backported?

Just done :) - #1216

@matthuisman
Copy link
Contributor

oh great :) so i guess you backported the current changes and then update these changes to be compatible with rework PR :)
NICE!

note - decrypting multiple subsamples is possible but we transform the data first so the decrypter sees 1 subsample.

Decrypting with subsamples > 1 seems to be fully broken now, at least as far as I can tell.
Previously turned off to avoid certain pixelation issues. This should be fixed properly rather than this workaround.
Copy link
Collaborator

@CastagnaIT CastagnaIT left a comment

Choose a reason for hiding this comment

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

i made a fast test with n€tflix vp9.2 and works
i will try av1 later i have no more time atm

src/Session.cpp Outdated Show resolved Hide resolved
src/Session.cpp Outdated Show resolved Hide resolved
src/common/AdaptiveCencSampleDecrypter.h Outdated Show resolved Hide resolved
src/common/AdaptiveCencSampleDecrypter.h Show resolved Hide resolved
src/common/AdaptiveCencSampleDecrypter.h Outdated Show resolved Hide resolved
src/common/AdaptiveDecrypter.h Outdated Show resolved Hide resolved
src/common/AdaptiveDecrypter.h Outdated Show resolved Hide resolved
src/common/AdaptiveDecrypter.h Outdated Show resolved Hide resolved
src/common/AdaptiveDecrypter.h Outdated Show resolved Hide resolved
wvdecrypter/wvdecrypter.cpp Outdated Show resolved Hide resolved
@glennguy glennguy force-pushed the wv-single-instance branch 3 times, most recently from d920b94 to 84b95dd Compare April 9, 2023 11:41
@glennguy
Copy link
Contributor Author

glennguy commented Apr 9, 2023

@CastagnaIT I think it's ready to look at again when you're ready

src/CryptoInfo.h Outdated Show resolved Hide resolved
To facilitate CBCS decryption on a single session
Decrypting audio and video can use the same session if both keys are present.
@glennguy glennguy force-pushed the wv-single-instance branch 3 times, most recently from a54870f to 5818750 Compare April 9, 2023 22:27
Copy link
Collaborator

@CastagnaIT CastagnaIT left a comment

Choose a reason for hiding this comment

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

now looks very good
remember to do bento4 release and fix relative commit before merge the PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants