Can't apply defStyleAttr to PlayerView

[JuzTosS]

ExoPlayer/library/ui/src/main/java/com/google/android/exoplayer2/ui/PlayerView.java

Line 375 in 9be5ece

TypedArray a = context.getTheme().obtainStyledAttributes(attrs, R.styleable.PlayerView, 0, 0);

defStyleAttr is not passed to obtainStyledAttributes and a default style can't be applied to PlayerView.

[claincly]

Hi! Please provide complete information as requested in the issue template. The issue template can
be found here.
If you're unable to share bug reports or
test content publicly, please send them to [email protected] using a subject in the format
"Issue #1234" ("#1234" is replaced with your issue number). Please also update this issue to
indicate you've done this.

[JuzTosS]

If you try to call the constructor with default style PlayerView(context, attrs, defStyleAttr) it won't be applied as it's not passed further when retrieving values.

I personally need it for the case to extend PlayerView and have a view that sets TextureView by default instead of SurfaceView. To solve this I can pass a style defStyleAttr with texture view as surface_type=2

So the fixed line will look like this:
TypedArray a = context.getTheme().obtainStyledAttributes(attrs, R.styleable.PlayerView, defStyleAttr, 0);

There's also one missed override with defStyleRes
public FrameLayout(@NonNull Context context, @Nullable AttributeSet attrs, @AttrRes int defStyleAttr, @StyleRes int defStyleRes) {

That also should be used with obtainStyledAttributes, we again should pass defStyleRes instead of 0 to the last parameter.

So the fixed line will look like this:
TypedArray a = context.getTheme().obtainStyledAttributes(attrs, R.styleable.PlayerView, defStyleAttr, defStyleRes);

[ojw28]

There's also one missed override with defStyleRes

It actually looks pretty awkward to support this until our minimum API version has increased to 21. Until then, it's not possible for us to do:

public PlayerView(Context context, @Nullable AttributeSet attrs, int defStyleAttr) {
  this(context, attrs, defStyleAttr, 0);
}

public PlayerView(Context context, @Nullable AttributeSet attrs, int defStyleAttr, int defStyleRes) {
  super(context, attrs, defStyleAttr, defStyleRes);
  // Other initialization
}

because the 4-arg constructor in the super class only exists on API level 21 and higher. I think the implication is that we'd have to either (a) duplicate a lot of initialization code in the 3-arg and 4-arg constructors, or (b) call an initialization method from both constructors and make all of the member variables that are initialized at construction time non-final. Both options seem not that great to me. We could alternatively call 3-arg super class method from our 4-arg method, but that's pretty confusing (and technically wrong, since FrameLayout does make use of the defStyleRes argument).

Do you need the 4-arg constructor, or were you just noting that for completeness? If not, I think we should just fix the problem where defStyleAttr is not being used, and stop there.

[ojw28]

The change referenced above makes use of defStyleAttr. Closing on the assumption that the 4-arg constructor isn't important, but please let us know by replying here if it is.

[JuzTosS]

The case we faced is having your own inheritor of PlayerView with predefined parameters (surface type in our case).

We solved it with composition, but this has a drawback of adding additional layer to the hierarchy.
(OurPlayerClass extends FrameLayout and inflates xml with PlayerView in it)