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

SimpleDecoder constructor is incompatible with Kotlin (nullness) #8232

Closed
robin2046 opened this issue Nov 17, 2020 · 7 comments
Closed

SimpleDecoder constructor is incompatible with Kotlin (nullness) #8232

robin2046 opened this issue Nov 17, 2020 · 7 comments
Assignees

Comments

@robin2046
Copy link

robin2046 commented Nov 17, 2020

I am using Exoplayer in my Kotlin app. When I try to make my own decoder which extends the SimpleDecoder class (coming from the ExoPlayer core library), I got build error.

The dependencies of ExoPlayer I am using:

implementation "com.google.android.exoplayer:exoplayer-core:2.12.1"
implementation "com.google.android.exoplayer:exoplayer-common:2.12.1"

The constructor of SimpleDecoder is as following:

public abstract class SimpleDecoder<I extends DecoderInputBuffer, O extends OutputBuffer, E extends DecoderException>
implements Decoder<I, O, E> {
 /**
  * @param inputBuffers An array of nulls that will be used to store references to input buffers.
  * @param outputBuffers An array of nulls that will be used to store references to output buffers.
  */
  protected SimpleDecoder(I[] inputBuffers, O[] outputBuffers)
}

And in my TestDecoder.kt I was trying to passing array of nulls to the super class:

class TestDecoder : SimpleDecoder<DecoderInputBuffer, SimpleOutputBuffer, DecoderException>(arrayOfNulls(16), arrayOfNulls(16)) 

And the error is:

TestDecoder.kt: (8, 93): Type mismatch: inferred type is Array<DecoderInputBuffer?> but Array<(out) DecoderInputBuffer> was expected

However, if I create a bridge class in Java inside my project, like following SimpleDecoderBridge.java:

public abstract class SimpleDecoderBridge<I extends DecoderInputBuffer, O extends OutputBuffer, E extends DecoderException> extends SimpleDecoder<I, O, E> {
    protected SimpleDecoderBridge(I[] inputBuffers, O[] outputBuffers) {
        super(inputBuffers, outputBuffers);
    }
}

Then make my TestDecoder extends from this bridge class, there is no build error.

class TestDecoder : SimpleDecoderBridge<DecoderInputBuffer, SimpleOutputBuffer, DecoderException>(arrayOfNulls(16), arrayOfNulls(16))

Could anyone here point out what is going on here? Why does the error happen and why does it not happen later? Am I doing it correctly(passing arrayOfNulls(size) to super) in Kotlin? If not, what is the correct way to do so? Thanks a lot.

@icbaker
Copy link
Collaborator

icbaker commented Nov 17, 2020

[I'm not a Kotlin expert, you may get better help asking this question in a more Kotlin-focussed context like Stack Overflow]

This error looks like it's related to Kotlin's handling of covariance and contravariance:
https://kotlinlang.org/docs/tutorials/kotlin-for-py/generics.html#declaration-site-covariance-and-contravariance

TestDecoder.kt: (8, 93): Type mismatch: inferred type is Array<DecoderInputBuffer?> but Array<(out) DecoderInputBuffer> was expected

I don't have an easily available setup to test Kotlin, so I don't know if this will work, but can you try explicitly listing the type parameter in your call to arrayOfNulls()? (based on my reading of this: https://kotlinlang.org/docs/reference/inline-functions.html#reified-type-parameters)

class TestDecoder : 
    SimpleDecoder<DecoderInputBuffer, SimpleOutputBuffer, DecoderException>(
        arrayOfNulls<out DecoderInputBuffer>(16), 
        arrayOfNulls(16)) 

I don't think this question is strictly related to ExoPlayer, I'll leave it open for a while in case anyone else has suggestions.

@robin2046
Copy link
Author

Thanks for the reply.
I tried to do as you suggested:

class TestDecoder() : 
    SimpleDecoder<DecoderInputBuffer,
        SimpleOutputBuffer,
        MediaCodecRenderer.DecoderException>(
    arrayOfNulls<out DecoderInputBuffer>(16),
    arrayOfNulls<out SimpleOutputBuffer>(16)
)

Still get the same error.

I also tried to ask the same question in StackOverflow before open a question here, but there was not even a comment.

@ojw28
Copy link
Contributor

ojw28 commented Nov 17, 2020

I don't know if this is correct, but it does at least compile:

abstract class TestDecoder : SimpleDecoder<DecoderInputBuffer, SimpleOutputBuffer, DecoderException>(
  arrayOfNulls<DecoderInputBuffer>(16) as Array<out DecoderInputBuffer>,
  arrayOfNulls<SimpleOutputBuffer>(16) as Array<out SimpleOutputBuffer>)

@robin2046
Copy link
Author

Thanks for the suggestion.
Despite of the warning "Unchecked cast: Array<DecoderInputBuffer?> to Array", it does compile without error. May be I need to file a issue to Kotlin team? But I don't know if this is really an issue. Because as you see if I declare a bridge class in Java extending from the SimpleDecoder class, this issue will not happen. Is there by any chance that this is related to any special build/compile/package flags/methods/tweaks used by the released ExoPlayer library?

@icbaker
Copy link
Collaborator

icbaker commented Nov 18, 2020

Ah I have another theory.

Is there by any chance that this is related to any special build/compile/package flags/methods/tweaks used by the released ExoPlayer library?

We have package-info.java files which mark everything as non-null by default, which includes forcing all collections/arrays to contain no nulls. This is partly to help with Kotlin inter-op (where otherwise everything from Java is maybe-null).

We use @Nullable (and similar) annotations to explicitly mark types that should be allowed to be, or contain, nulls. It looks like we're missing annotations on the constructor parameters in SimpleDecoder (since they're explicitly documented as being an array of nulls). I'll work on fixing that.

(Sorry for the wrong diagnosis above, I didn't spot the crucial ? inside the <> and got distracted with the out keyword instead)

The reason your bridge Java class works (I think) is that it's presumably not in a package with the same package-info.java file, so it allows nulls from Kotlin (and nothing is checking where your Java class passes up to SimpleDecoder).

@icbaker icbaker changed the title Build error while trying to extend SimpleDecoder class in Kotlin SimpleDecoder constructor is incompatible with Kotlin (nullness) Nov 18, 2020
@icbaker icbaker added bug and removed question labels Nov 18, 2020
@icbaker
Copy link
Collaborator

icbaker commented Nov 18, 2020

Ah it's been pointed out to me that this is a duplicate of #6792 - and correctly annotating SimpleDecoder isn't easy/possible at the moment.

I'm going to close this - please follow #6792 and in the meantime you'll need to keep casting I'm afraid.

@icbaker icbaker closed this as completed Nov 18, 2020
@robin2046
Copy link
Author

Thanks a lot for the info. It is OK for me to use casting. But the more important thing is to get to know the root cause. Appreciate it.

@google google locked and limited conversation to collaborators Jan 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants