Skip to content

Change exoplayer payload type#7010

Merged
bramkragten merged 8 commits intohome-assistant:devfrom
uvjustin:change-exoplayer-payload-type
Sep 30, 2020
Merged

Change exoplayer payload type#7010
bramkragten merged 8 commits intohome-assistant:devfrom
uvjustin:change-exoplayer-payload-type

Conversation

@uvjustin
Copy link
Contributor

Proposed change

The frontend may need to send additional options to the exoplayer/play_hls interface such as whether to be muted or not: #6857 . With that in mind it is probably better to use a JSONObject instead of a String for the payload.
Have also opened up a corresponding PR on the Android side (home-assistant/android#932). If we make the change it is probably better to do it before 0.115 before any of the exoplayer stuff is officially added.

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New feature (thank you!)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Example configuration

Additional information

Checklist

  • The code change is tested and works locally.
  • There is no commented out code in this PR.
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

@bramkragten
Copy link
Member

This will be breaking, how do we handle an old version of the Android app?

@uvjustin
Copy link
Contributor Author

We could change the External Config tag from something like hasExoPlayer to hasExoPlayerv2.
But this might be unnecessary clutter - if Android side gets pulled and the app gets updated before the frontend change there will be minimal disruption. The ExoPlayer code has only been in Android for ~ 2 weeks and the frontend side hasn't had it in a full release yet. It will be more of a headache if we make the change after 0.115 gets released.

@bramkragten
Copy link
Member

The Android app is out already, how can we be sure users update it before Thursday? That's not much time...

@uvjustin
Copy link
Contributor Author

That's a good point. If the app doesn't get updated in time then we should probably just skip exoplayer for 0.115 and wait until the next version. I don't think it's worth cluttering up anything for a workaround.

@bramkragten
Copy link
Member

Ok, let's do that then

@uvjustin
Copy link
Contributor Author

uvjustin commented Sep 15, 2020

I'll make a small PR to return false for the ExoPlayer check. When the Android App gets updated we can reenable it, even in a patch version.

@uvjustin uvjustin mentioned this pull request Sep 15, 2020
9 tasks
@uvjustin uvjustin force-pushed the change-exoplayer-payload-type branch from b1f1ab3 to 106b37d Compare September 29, 2020 11:08
@uvjustin
Copy link
Contributor Author

@bramkragten The Android app was updated in the store a few days ago, so this can be merged without breaking now.

@uvjustin
Copy link
Contributor Author

In 115.5 we made a change to stream to specify the codecs in a HLS master playlist. Since we now have the codecs available in the master playlist, we are now able to check the master playlist for H.265 before resorting to Exoplayer. I've just pushed the relevant changes.
Note we can also do this through the HLS library but it will be a little more complex, plus there's a change that needs to be pushed to the DefinitelyTyped library to fix one of the event types.

@uvjustin
Copy link
Contributor Author

The changes we made in 0.115.5 also changed the url from a regular playlist to a master playlist. The various players don't need the master playlist to play - we only added it because Google Cast devices needed the codec strings to function properly. Should we just send the regular playlist instead?

@bramkragten
Copy link
Member

The changes we made in 0.115.5 also changed the url from a regular playlist to a master playlist. The various players don't need the master playlist to play - we only added it because Google Cast devices needed the codec strings to function properly. Should we just send the regular playlist instead?

Uuuuh... I would not know? @hunterjm?

@uvjustin
Copy link
Contributor Author

It should be a little faster if we do that because we skip one extra master playlist load. The only concern is that we are coupling tight to the backend and have to remember if we ever change anything there.

Co-authored-by: Bram Kragten <mail@bramkragten.nl>
@bramkragten bramkragten merged commit 11e3503 into home-assistant:dev Sep 30, 2020
@bramkragten bramkragten mentioned this pull request Sep 30, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Jul 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants