-
Notifications
You must be signed in to change notification settings - Fork 242
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
Begin refactor of wvdecrypter, sample-aes-cbc support #883
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice addition! only some code style requests
7b6a206
to
da26852
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i havent tested the feature, just check code style
9c1e139
to
32cc208
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
review second round done,
i have tested it on Windows 10 and no problem,
but when i have tested it on CoreElec (last Kodi 20 nightly) every time i try to play multiple times the first test stream Kodi always crashes (usually after 2 or 3 times), this not happens with the second test stream or others test streams
Crash logs:
kodi_crashlog_20220128145622.log.txt
kodi_crashlog_20220128153636.log.txt
|
||
for (size_t ses(1); ses < cdm_sessions_.size(); ++ses) | ||
{ | ||
AP4_DataBuffer init_data; | ||
const char* optionalKeyParameter(nullptr); | ||
adaptive::AdaptiveTree::Period::PSSH sessionPsshset = | ||
adaptiveTree_->current_period_->psshSets_[ses]; | ||
uint32_t sessionType = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO this is not very clear, here we should make it easier understand that we handle flag operations,
at least we should change all these integer variables to handle a enum, something like this:
enum MediaTypeFlags
{
MEDIA_TYPE_UNDEFINED = 0,
MEDIA_TYPE_VIDEO,
MEDIA_TYPE_AUDIO
...
if it is too much work for this PR, keep it for later cleanup
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for pointing out however let's keep for a later cleanup
32cc208
to
47540ad
Compare
Hmmm very interested in the crash on CoreELEC. Must be some sort of race condition, it seems very similar to other reports we've had from crashes on those platforms. But very good that it seems fairly easy to reproduce. First crash shows that ISA isn't even running still which makes me think it's to do with the wv timer renewal introduced in #589 and modified again in #729 One hint for us to look at is that there's actually a callback that is invoked by the CDM that we're not currently using which may be the key to doing this right. I know that the changes here are seeming to cause crashes on your setup but I feel that the changes merely expose an issue that's already there. One thing I didn't mention in the OP is this introduces using separate CDM sessions for audio and video - you'll notice in the log that the license is retrieved twice. Same behaviour as Android is since #734 Or, alternatively something inside ISA is causing a failure or exit but it's not waiting properly for CDM session to finish closing, therefore crash of all Kodi when it tries to call back.... will have to see. I'll have a dig very soon. |
@CastagnaIT just a thought: would switching to separate audio and video widevine sessions cause issues with the pre-init for NF? Is the platform that you model the plugin from use separate audio/video sessions for playback or just 1? |
i have some questions if the audio track is not DRM protected, is the licence still required/requested?
because i have tried this PR on ntfx but i am not able to see in the log the two license requests, If i use the licese provided with the licensedManifest request, this give me only one license data, not two licenses, For the sake of completeness, after the implementation of thelicensedManifest in website, there is an additional licese request, but this happens only after the video playback is started, which might be the reason why playback fails in my WIP PR after some minuts |
Sorry I should have clarified, my question should have been this condition first. No licence request for audio if not protected. In this case we are good and it makes sense. I haven't had much time this weekend to look further but I'll have a go in the evenings this coming week. My gut feeling is this has to do with the CDM timer functions. I logged the timer getting started and finishing, it seems to run every second. Maybe some sort of race condition.... edit:
I don't think this is right. If the cdm wants to set a timer it should be set without condition... it's possible that SetTimer is being called by CDM before wvdecrypter has a chance to set the flag. |
5de28cd
to
366c5c6
Compare
6177b95
to
781d59e
Compare
@CastagnaIT I've rebased and cleaned up, and retested on LE11 - I couldn't get it to crash on wv 4.10.2252.5 If you get a chance could you try again with the 'good' CDM? |
Also - I reverted a change made to this line
session_active flag would already be true.
|
i will have to check i do not remember anymore the situation on my ARM CoreELEC device |
I have done tests WV 4.10.2391.0 (last): WV 4.10.2252.0 (13904): WV 4.10.2252.0 (13816) and WV 4.10.2252.5 (14150): after these tests on multiple versions i don't know if we can call it as library bug, crash always occurs at playback startup, maybe wait for a new WV version to be released and see if crash still occurs |
Thanks for the detailed testing! I haven't been able to find evidence that it's not unloaded from memory. What I can say is there's the long standing issue in CE/LE that seems to do with their filesystem/caching that usually requires a restart after updating the add-on, maybe that has to do with not 'unloading'? These different versions (13816, 13904, 14150) are coming from different device images right? Are you able do checksums on the two different 4.10.2252.0 versions? Maybe one of these is being optimised in a certain way that causes issues for us, especially when combined with the TLS relocation 'fixes' that are now required in LE/CE? Regarding the black screen, can you check that the changes from d6b3c52 are in your source? It looks to me like the incorrect init segment is downloaded on stream change (480p instead of 1080p). |
i dont know i have done exactly what you need
i have builded from your branch, your branch include that commit
yes ISHelper download from different devices images |
Thanks for md5sums, that was what I was looking for. Both 2252.0 are the same file, interesting that there are crashes on one and not the other. It's making me think about the apparent file caching issue. Ok, so I think the black screen issue is to do with this
my bad, I read the log wrong, all is working as it should in this area. Overall I'd like to just get this in. The feature is unlikely to be used at the moment, and at least it's prepared for the future. If there are issues later on then we can revisit then. When I move my other devices in the house to Kodi 20 I can do some more work. I found with testing that the SetTimer function was the point at which crashing occurred, usually on the 10th (sometimes 7th) call to
Even 4.10.2391.0 seemed to behave nice (although with stuttering/slowness) with this. This observation makes me think we're not doing anything wrong on our side. |
EDIT: same error on last CE nightly, i ask support to CE dev |
i asked to CE dev and seem there is a problem with the decoder that is forced limited to one instance in CE or something related to this, they will let us know if a possible solution could be done, they suspect/theorize a open/close stream overlapping in kodi not allowed in CE, |
CMakeLists.txt
Outdated
@@ -47,6 +47,7 @@ set(ADP_HEADERS | |||
src/codechandler/VP9CodecHandler.h | |||
src/codechandler/WebVTTCodecHandler.h | |||
src/codechandler/ttml/TTML.h | |||
src/common/AdaptiveDecrypter.h |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just can you use same spaces of others
781d59e
to
8889fe9
Compare
…hile CloseSession is active to avoid endless loop. Sometimes when stopping a playback Kodi crashes with a following backtrace: (__m=<optimized out>, this=<optimized out>) at /usr/include/c++/12.1.0/bits/atomic_base.h:486 __b = <optimized out> at /usr/include/c++/12.1.0/atomic:87 (adp=std::shared_ptr<media::CdmAdapter> (use count 1, weak count 1) = {...}, delay=<optimized out>, context=<optimized out>) at /usr/src/debug/kodi-addon-inputstream-adaptive/inputstream.adaptive-20.3.3-Nexus/wvdecrypter/cdm/media/cdm/cdm_adapter.cc:81 at /usr/include/c++/12.1.0/bits/invoke.h:96, unsigned long long, void*), std::shared_ptr<media::CdmAdapter>, long long, void*> >::_M_invoke<0u, 1u, 2u, 3u>(std::_Index_tuple<0u, 1u, 2u, 3u>) (this=<optimized out>) at /usr/include/c++/12.1.0/bits/std_thread.h:252 at /usr/include/c++/12.1.0/bits/std_thread.h:259 at /usr/include/c++/12.1.0/bits/std_thread.h:210 at /usr/src/debug/gcc/libstdc++-v3/src/c++11/thread.cc:82 ret = <optimized out> pd = 0x85cf9080 unwind_buf = {cancel_jmp_buf = {{jmp_buf = {857629366, 66004830, -2049994624, -2008033272, -2008033282, 338, -2008033281, 8387456, -2058383360, -2049995908, 1920, 1080, 3840, 2160, 0, 10000000, -1431355392, 1107558643, 0 <repeats 46 times>}, mask_was_saved = 0}}, priv = {pad = {0x0, 0x0, 0x0, 0x0}, data = {prev = 0x0, cleanup = 0x0, canceltype = 0}}} not_first_call = <optimized out> robust = <optimized out> After searching in the github history it came out that there was an idenctical issue adressed in xbmc#589. For some reason that piece of code got removed in xbmc@8889fe9 (in pull request xbmc#883). The stack trace is from the Raspberry Pi 4 machine. But I can also reproduce the issue on my x86_64 Linux machine by adding a 1s sleep in the beginning of CdmAdapter::SetTimer. In order to reproduce it is necessary to quickly play/stop streams. Following script can be used to ease the reproduction. In my environment when running following script for 1-3 minutes the screen freezes because of the busy loop mentioned in xbmc#589. After applying mentioned commit I'm not able to reproduce the issue anymore. ``` set -x URI='Change it to some uri to play' while true; do curl -s "http://kodi:[email protected]:8080/jsonrpc" -H 'Content-Type: application/json' --data "{\"jsonrpc\":\"2.0\",\"method\":\"Player.Open\",\"params\":{\"item\":{\"file\":\"$URI\"}}}" sleep $[ ($RANDOM % 70 + 30) / 10.0 ] curl -s "http://kodi:[email protected]:8080/jsonrpc" -H 'Content-Type: application/json' --data '{"jsonrpc":"2.0","method":"Player.Stop","params":{"playerid":1}}' sleep 1 done ```
…hile CloseSession is active to avoid endless loop. Sometimes when stopping a playback Kodi crashes with a following backtrace: (__m=<optimized out>, this=<optimized out>) at /usr/include/c++/12.1.0/bits/atomic_base.h:486 __b = <optimized out> at /usr/include/c++/12.1.0/atomic:87 (adp=std::shared_ptr<media::CdmAdapter> (use count 1, weak count 1) = {...}, delay=<optimized out>, context=<optimized out>) at /usr/src/debug/kodi-addon-inputstream-adaptive/inputstream.adaptive-20.3.3-Nexus/wvdecrypter/cdm/media/cdm/cdm_adapter.cc:81 at /usr/include/c++/12.1.0/bits/invoke.h:96, unsigned long long, void*), std::shared_ptr<media::CdmAdapter>, long long, void*> >::_M_invoke<0u, 1u, 2u, 3u>(std::_Index_tuple<0u, 1u, 2u, 3u>) (this=<optimized out>) at /usr/include/c++/12.1.0/bits/std_thread.h:252 at /usr/include/c++/12.1.0/bits/std_thread.h:259 at /usr/include/c++/12.1.0/bits/std_thread.h:210 at /usr/src/debug/gcc/libstdc++-v3/src/c++11/thread.cc:82 ret = <optimized out> pd = 0x85cf9080 unwind_buf = {cancel_jmp_buf = {{jmp_buf = {857629366, 66004830, -2049994624, -2008033272, -2008033282, 338, -2008033281, 8387456, -2058383360, -2049995908, 1920, 1080, 3840, 2160, 0, 10000000, -1431355392, 1107558643, 0 <repeats 46 times>}, mask_was_saved = 0}}, priv = {pad = {0x0, 0x0, 0x0, 0x0}, data = {prev = 0x0, cleanup = 0x0, canceltype = 0}}} not_first_call = <optimized out> robust = <optimized out> After searching in the github history it came out that there was a similar issue adressed in xbmc#728. For some reason that piece of code got removed in xbmc@8889fe9 (in pull request xbmc#883). The stack trace is from the Raspberry Pi 4 machine. But I can also reproduce the issue on my x86_64 Linux machine by adding a 1s sleep in the beginning of CdmAdapter::SetTimer. In order to reproduce it is necessary to quickly play/stop streams. Script attached at the end of the commit message can be used to ease the reproduction. In my environment running the script for 1-3 minutes makes Kodi crash. Although the root cause described in xbmc#729 was a busy loop. Right now address sanitizer reports: ``` AddressSanitizer:DEADLYSIGNAL ================================================================= ==108079==ERROR: AddressSanitizer: SEGV on unknown address 0x7f6e314f7d4a (pc 0x7f6e314f7d4a bp 0x7f6e263fd830 sp 0x7f6e263fd720 T464) ==108079==The signal is caused by a READ memory access. ==108079==Hint: PC is at a non-executable region. Maybe a wild jump? #0 0x7f6e314f7d4a (<unknown module>) xbmc#1 0x7f6e3151c066 (<unknown module>) xbmc#2 0x7f6e31526947 (<unknown module>) xbmc#3 0x7f6e31526729 (<unknown module>) xbmc#4 0x7f6e31526620 (<unknown module>) xbmc#5 0x7f6e315265a1 (<unknown module>) xbmc#6 0x7f6e31526585 (<unknown module>) xbmc#7 0x7f6e46ed72c2 in execute_native_thread_routine /usr/src/debug/gcc/gcc/libstdc++-v3/src/c++11/thread.cc:82 AddressSanitizer can not provide additional info. SUMMARY: AddressSanitizer: SEGV (<unknown module>) Thread T464 created by T51 here: #0 0x7f6e49464207 in __interceptor_pthread_create /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_interceptors.cpp:207 xbmc#1 0x7f6e46ed73a9 in __gthread_create /usr/src/debug/gcc/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/x86_64-pc-linux-gnu/bits/gthr-default.h:663 xbmc#2 0x7f6e46ed73a9 in std::thread::_M_start_thread(std::unique_ptr<std::thread::_State, std::default_delete<std::thread::_State> >, void (*)()) /usr/src/debug/gcc/gcc/libstdc++-v3/src/c++11/thread.cc:147 xbmc#3 0x7f6e31522028 (<unknown module>) xbmc#4 0x7f6e27569d40 (/home/dobo/.kodi/cdm/libwidevinecdm.so+0x969d40) xbmc#5 0x7f6e2757e7fc (/home/dobo/.kodi/cdm/libwidevinecdm.so+0x97e7fc) xbmc#6 0x7f6e27569c36 (/home/dobo/.kodi/cdm/libwidevinecdm.so+0x969c36) xbmc#7 0x7f6e3151fb28 (<unknown module>) xbmc#8 0x7f6e3151c0b1 (<unknown module>) xbmc#9 0x7f6e31526947 (<unknown module>) xbmc#10 0x7f6e31526729 (<unknown module>) xbmc#11 0x7f6e31526620 (<unknown module>) xbmc#12 0x7f6e315265a1 (<unknown module>) xbmc#13 0x7f6e31526585 (<unknown module>) xbmc#14 0x7f6e46ed72c2 in execute_native_thread_routine /usr/src/debug/gcc/gcc/libstdc++-v3/src/c++11/thread.cc:82 Thread T51 created by T45 here: #0 0x7f6e49464207 in __interceptor_pthread_create /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_interceptors.cpp:207 xbmc#1 0x7f6e46ed73a9 in __gthread_create /usr/src/debug/gcc/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/x86_64-pc-linux-gnu/bits/gthr-default.h:663 xbmc#2 0x7f6e46ed73a9 in std::thread::_M_start_thread(std::unique_ptr<std::thread::_State, std::default_delete<std::thread::_State> >, void (*)()) /usr/src/debug/gcc/gcc/libstdc++-v3/src/c++11/thread.cc:147 xbmc#3 0x7f6e31522028 (<unknown module>) xbmc#4 0x7f6e27569d40 (/home/dobo/.kodi/cdm/libwidevinecdm.so+0x969d40) xbmc#5 0x7f6e2757e7fc (/home/dobo/.kodi/cdm/libwidevinecdm.so+0x97e7fc) xbmc#6 0x7f6e2757e70e (/home/dobo/.kodi/cdm/libwidevinecdm.so+0x97e70e) xbmc#7 0x7f6e274e4ecc (/home/dobo/.kodi/cdm/libwidevinecdm.so+0x8e4ecc) xbmc#8 0x7f6e3151f280 (<unknown module>) xbmc#9 0x7f6e314e9ea1 (<unknown module>) xbmc#10 0x7f6e314e49bb (<unknown module>) xbmc#11 0x7f6e314f45ce (<unknown module>) xbmc#12 0x7f6e2889228f in SESSION::CSession::InitializeDRM(bool) /home/dobo/kodi/inputstream.adaptive/src/Session.cpp:643 xbmc#13 0x7f6e28893279 in SESSION::CSession::InitializePeriod(bool) /home/dobo/kodi/inputstream.adaptive/src/Session.cpp:723 xbmc#14 0x7f6e2888e36d in SESSION::CSession::Initialize() /home/dobo/kodi/inputstream.adaptive/src/Session.cpp:266 xbmc#15 0x7f6e28775f4c in CInputStreamAdaptive::Open(kodi::addon::InputstreamProperty const&) /home/dobo/kodi/inputstream.adaptive/src/main.cpp:101 xbmc#16 0x7f6e287871a1 in kodi::addon::CInstanceInputStream::ADDON_Open(AddonInstance_InputStream const*, INPUTSTREAM_PROPERTY*) (/usr/lib/kodi/addons/inputstream.adaptive/inputstream.adaptive.so.20.3.2+0x3871a1) xbmc#17 0x55c2811f4ddc in CInputStreamAddon::Open() (/usr/lib/kodi/kodi.bin+0xb66ddc) Thread T45 created by T0 here: #0 0x7f6e49464207 in __interceptor_pthread_create /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_interceptors.cpp:207 xbmc#1 0x7f6e46ed73a9 in __gthread_create /usr/src/debug/gcc/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/x86_64-pc-linux-gnu/bits/gthr-default.h:663 xbmc#2 0x7f6e46ed73a9 in std::thread::_M_start_thread(std::unique_ptr<std::thread::_State, std::default_delete<std::thread::_State> >, void (*)()) /usr/src/debug/gcc/gcc/libstdc++-v3/src/c++11/thread.cc:147 ``` The root cause of the crash is unknown as it comes directly from libwidevine library. But it crashes when we call cdm->TimerExpired when the session is closed. After applying mentioned commit I'm not able to reproduce the crash anymore. Script to reproduce the issue: ``` set -x URI='Change it to some uri to play' while true; do curl -s "http://kodi:[email protected]:8080/jsonrpc" -H 'Content-Type: application/json' --data "{\"jsonrpc\":\"2.0\",\"method\":\"Player.Open\",\"params\":{\"item\":{\"file\":\"$URI\"}}}" sleep $[ ($RANDOM % 70 + 30) / 10.0 ] curl -s "http://kodi:[email protected]:8080/jsonrpc" -H 'Content-Type: application/json' --data '{"jsonrpc":"2.0","method":"Player.Stop","params":{"playerid":1}}' sleep 1 done ```
This is the result of looking into adding support for CBC encrypted streams. Not very common at the moment but I imagine it would be more popular with 'apple native' services - Fairplay uses this scheme. CBC uses a 'pattern' based encryption, where there is a pattern of clear vs encrypted samples. Typically (and in the case of the test video) there is 1 encrypted sample followed by 9 unencrypted or 'skipped' samples. An advantage of this method can be lower cpu utilization.
main.cpp doesn't want to know the concrete decrypter class, only the generic ap4 cencsinglesampledecrypter. This will provide the flexibility needed in future if there's a Playready or Fairplay decrypter module developed. Therefore for this AES-CBC feature in order to prevent yet more patches to bento4 or to cast into the concrete wv cenc ssd class we have now the 'Adaptive_CencSingleSampleDecrypter' base class which has this extended functionality.
If CBC encryption is found upon stream initialisation we set the decrypter up appropriately.
NOTE: This doesn't include support for the CBCS decoding feature on Android, changes and possibly API bump will be required in Kodi. Follow-up PR to come very soon.
Tested with
for CBC, and for regular CTR tested with:
on Windows 10.
edit: some additional background