ExtractorAPI: support Kotlin Uuid#2855
Conversation
I didn't find any extensions using the variables directly, so doesn't keep back compat there, as it was very hard to because of the way CS3IPlayer uses them etc... I decided not to change `DrmMetadata` to use the Kotlin Uuid and just convert all usage in CS3IPlayer to Java UUIDs for now. We could eventually change `DrmMetadata` to Kotlin Uuid though, and just convert to Java in one single place for Media3 but I figured that was out of scope and this accomplish the main goal of supporting Kotlin Uuid in the main API surface. This, combined with some of my other PRs accomplish the majority of the remaining API surface migrations, so I wanted to get it done before next stable also. I added a global OptIn for `kotlin.uuid.ExperimentalUuidApi` in `:app` only because adding it as `@file:OptIn(kotlin.uuid.ExperimentalUuidApi::class)` doesn't work for some reason though it does in library and would for extensions. This also will no longer be necessary at all in Kotlin 2.4, but I just wanted to do this now so that the API surface is updated and we can support the new version for the next stable release. If there is some changes or suggestions needed for this I am happy to make those.
|
It should still be possible to add backwards getters and setters. We can make them DrmExtractorLink ... {
@Deprecated(level = DeprecationLevel.HIDDEN)
fun setUuid(uuid : java.util.UUID) {
this.uuid = uuid.toKotlinUuid()
}
@Deprecated(level = DeprecationLevel.HIDDEN)
fun getUuid() : java.util.UUID = this.uuid.toJavaUuid()
}This code should compile down to the old JVM signature when accessing it by .uuid |
Done (if I did it right) I considered this I just didn't find any extension doing it like that so didn't but no harm doing it I suppose. |
fire-light42
left a comment
There was a problem hiding this comment.
The code looks good!
However, according to the docs this is not recommended, woe is android developers.
https://kotlinlang.org/api/core/kotlin-stdlib/kotlin.uuid/-experimental-uuid-api/
Beware using the annotated API especially if you're developing a library, since your library might become binary incompatible with the future versions of the standard library.
Lets hope they can keep a struct with two longs stable.
there are no binary changes to our current usage of it, and the API is fully stable in upcoming Kotlin 2.4, I just wanted to do this now so we can have an API open for this for extensions. |
I didn't find any extensions using the variables directly, so doesn't keep back compat there, as it was very hard to because of the way CS3IPlayer uses them etc... I decided not to change
DrmMetadatato use the Kotlin Uuid and just convert all usage in CS3IPlayer to Java UUIDs for now. We could eventually changeDrmMetadatato Kotlin Uuid though, and just convert to Java in one single place for Media3 but I figured that was out of scope and this accomplish the main goal of supporting Kotlin Uuid in the main API surface. This, combined with some of my other PRs accomplish the majority of the remaining API surface migrations, so I wanted to get it done before next stable also.I added a global OptIn for
kotlin.uuid.ExperimentalUuidApiin:apponly because adding it as@file:OptIn(kotlin.uuid.ExperimentalUuidApi::class)doesn't work for some reason though it does in library and would for extensions. This also will no longer be necessary at all in Kotlin 2.4, but I just wanted to do this now so that the API surface is updated and we can support the new version for the next stable release.If there is some changes or suggestions needed for this I am happy to make those.
UPDATE: Looking again it seems some extensions might actually use the vals directly, I am not certain of the best way to fix this to be fully back compat.
UPDATE 2: I added new val names for the new Kotlin Uuid vals to maintain back compat. We can deprecate the old ones after next stable. The only thing this doesn't maintain back compat for now is if someone used old constructors directly. But the main constructor is private and the back compat ones have been deprecated as an error level for awhile. Just keeps them for now because the builder seems to call the deprecated versions currently. Fixing this is out of scope of this pull request.