Skip to content

Conversation

@peaBerberian
Copy link
Collaborator

We had kind of a dumb mistake where an error arising at license-communication time was falsely categorized as an error at license-fetching time.

This is because we inadvertently also catched errors from the MediaKeySession.prototype.update EME call where we only intended to catch errors at the higher-up getLicense (the RxPlayer API) level.

The fix to that issue is relatively straigtforward.

…ROR`

We had kind of a dumb mistake where an error arising at
license-communication time was falsely categorized as an error at
license-fetching time.

This is because we inadvertently also catched errors from the
`MediaKeySession.prototype.update` EME call where we only intended to
catch errors at the higher-up `getLicense` (the RxPlayer API) level.

The fix to that issue is relatively straigtforward.
@peaBerberian peaBerberian added bug This is an RxPlayer issue (unexpected result when comparing to the API) DRM Relative to DRM (EncryptedMediaExtensions) labels Mar 26, 2025
@peaBerberian peaBerberian added this to the 4.3.0 milestone Mar 26, 2025
async (licenseObject) => {
if (manualCanceller.isUsed()) {
return Promise.resolve();
return;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

NOTE: removed because unnecessary

} else {
try {
return updateSessionWithMessage(session, licenseObject);
await updateSessionWithMessage(session, licenseObject);
Copy link
Collaborator Author

@peaBerberian peaBerberian Mar 26, 2025

Choose a reason for hiding this comment

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

NOTE: awaited to make the try / catch catch rejections, not just errors thrown synchronously. Also, we don't actually care about the returned value

}
}
})
.catch((err: unknown) => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The idea is to now do licenseFetchingPromise.then(onSucceed, onFailure) instead of: licenseFetchingPromise.then(onSucceed).catch(onFailure).

@github-actions
Copy link

✅ Automated performance checks have passed on commit 13e2171fa683ead5e625d646770e7d26798909ac with the base branch dev.

Details

Performance tests 1st run output

No significative change in performance for tests:

Name Mean Median
loading 21.47ms -> 21.58ms (-0.110ms, z: 0.82756) 29.40ms -> 29.40ms
seeking 22.20ms -> 22.19ms (0.010ms, z: 0.04610) 11.55ms -> 11.55ms
audio-track-reload 28.76ms -> 28.64ms (0.118ms, z: 0.29440) 41.70ms -> 41.70ms
cold loading multithread 51.84ms -> 50.84ms (1.003ms, z: 13.91286) 75.75ms -> 74.25ms
seeking multithread 29.61ms -> 34.86ms (-5.257ms, z: 0.74605) 10.65ms -> 10.65ms
audio-track-reload multithread 28.16ms -> 28.05ms (0.114ms, z: 1.61702) 41.40ms -> 41.10ms
hot loading multithread 18.96ms -> 18.80ms (0.158ms, z: 2.64329) 27.30ms -> 27.00ms

@peaBerberian peaBerberian merged commit e55f8a8 into dev Mar 27, 2025
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug This is an RxPlayer issue (unexpected result when comparing to the API) DRM Relative to DRM (EncryptedMediaExtensions)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants