media: Ensure per application provisioning is always supported [chromium/src : main]
Vikram Pasupathy (Gerrit)
Vikram Pasupathy voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Related details
- Feras Aldahlawi
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Feras Aldahlawi (Gerrit)
Feras Aldahlawi voted and added 3 comments![]( "Open in Gerrit")
Votes added by Feras Aldahlawi
| Code-Review | +1 |
3 comments
lgtm, but need android owners to look over the fact that this can indeed be deprecated.
// |get_origin_id_cb| must be provided and is used to obtain an origin ID.
// |allow_empty_origin_id_cb| is used to determine if an empty origin ID is
// allowed or not. It is called if |get_origin_id_cb| is unable to return an
// origin ID.update comment.
Related details
- Vikram Pasupathy
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Vikram Pasupathy (Gerrit)
Vikram Pasupathy added 2 comments![]( "Open in Gerrit")
Bug: 425948259Vikram Pasupathyuse b: 425948259
From the email, it seems to be only for internal bugs.
// |get_origin_id_cb| must be provided and is used to obtain an origin ID.
// |allow_empty_origin_id_cb| is used to determine if an empty origin ID is
// allowed or not. It is called if |get_origin_id_cb| is unable to return an
// origin ID.- Xiaohan Wang
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Xiaohan Wang (Gerrit)
Xiaohan Wang added 1 comment![]( "Open in Gerrit")
}Thanks for the change! It's so exciting to see these complicated code going away. Unfortunately, this is the tricky part that I'm not totally sure whether we can remove entirely.
The FIRST_API_LEVEL is "SDK version that initially shipped on the device":
https://source.android.com/reference/tradefed/com/android/tradefed/device/DeviceProperties#FIRST_API_LEVEL
For example, a device that's originally shipped on N, but later updated to O+, will still return N here, and it will not support per-app-provisioning. Interesting enough, Media.EME.MediaDrm.PreprovisionedOriginId.NonPerAppProvisioningDevice count is almost always zero: http://shortn/_DGhehijuAX. That might mean that in reality, almost all devices indeed support per-app provisioning.
As you can see, there's a disconnection here. I checked the original design doc http://shortn/_tonWP0thCA, and I am confused with this implementation. It feels like there's a bug to me. Using the same example, if a device is current on O, but originally shipped on N (first_api_level), then we'll return true, which seems to be wrong.
This is my understanding, which might not be correct. I suggest you file an internal bug (the current one is too generic), and add Android owners to discuss this. If there's a bug, we should fix the bug first, and then use the metrics to decide whether we can actually remove these...
Related details
- Vikram Pasupathy
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Vikram Pasupathy (Gerrit)
Vikram Pasupathy added 1 comment![]( "Open in Gerrit")
Thanks for the change! It's so exciting to see these complicated code going away. Unfortunately, this is the tricky part that I'm not totally sure whether we can remove entirely.
The FIRST_API_LEVEL is "SDK version that initially shipped on the device":
https://source.android.com/reference/tradefed/com/android/tradefed/device/DeviceProperties#FIRST_API_LEVELFor example, a device that's originally shipped on N, but later updated to O+, will still return N here, and it will not support per-app-provisioning. Interesting enough, Media.EME.MediaDrm.PreprovisionedOriginId.NonPerAppProvisioningDevice count is almost always zero: http://shortn/_DGhehijuAX. That might mean that in reality, almost all devices indeed support per-app provisioning.
As you can see, there's a disconnection here. I checked the original design doc http://shortn/_tonWP0thCA, and I am confused with this implementation. It feels like there's a bug to me. Using the same example, if a device is current on O, but originally shipped on N (first_api_level), then we'll return true, which seems to be wrong.
This is my understanding, which might not be correct. I suggest you file an internal bug (the current one is too generic), and add Android owners to discuss this. If there's a bug, we should fix the bug first, and then use the metrics to decide whether we can actually remove these...
SG, followed up and created https://chromium-review.googlesource.com/c/chromium/src/+/7146728 for that. I will put this as a WIP until we get statistics from that histogram.
Related details
- Xiaohan Wang
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |