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.

Sign up for GitHub

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

Jump to bottom

False positives in demo audio-only asset detection. #794

Closed
joeyparrish opened this issue May 10, 2017 · 3 comments
Closed

False positives in demo audio-only asset detection. #794

joeyparrish opened this issue May 10, 2017 · 3 comments
Assignees
@theodab
Labels
status: archived Archived and locked; will not be updated type: bug Something isn't working correctly
Milestone
v2.2.0

Comments

@joeyparrish
Copy link
Member

  • What version of Shaka Player are you using? master
  • Are you using the demo app or your own custom app? demo app
  • What browser and OS are you using? Chrome on Mac
  • What did you do?

Load any asset.

  • What actually happened?

The audio-only poster was briefly visible between clicking "load" and playback starting.

Could be a race between load() resolution and getVariantTracks().
@joeyparrish joeyparrish added the type: bug Something isn't working correctly label May 10, 2017
@joeyparrish joeyparrish added this to the v2.2.0 milestone May 10, 2017
@theodab theodab self-assigned this May 22, 2017
@theodab
Copy link
Contributor

theodab commented May 22, 2017

This is not actually be a race condition. Instead, it's an issue with the logic for detecting audio-only assets; we currently check if a track is video or audio by the presence of a 'width' variable, but the resolution attribute in HLS is optional so some manifests might not contain in. In such manifests, we are mis-identifying video streams as being audio.

@theodab theodab changed the title Race condition with audio-only poster False positives in demo audio-only asset detection. May 22, 2017
@joeyparrish
Copy link
Member Author

As discussed offline, let's use the newly-added videoCodec field of Track to identify variants with video.

joeyparrish pushed a commit that referenced this issue May 23, 2017
@theodab @joeyparrish
Fixed a bug in how the demo detects audio assets.
82e7306 
In HLS, the resolution property is optional. This means we
cannot assume that a variant with a video stream has a defined
width. Such a check was causing us to incorrectly identify
variants as audio-only, and applying the audio-only poster
incorrectly.
Instead, this change checks for the presence of a video codec.

Closes #794

Change-Id: I80ebd11ebc735468220816c5133a3398436f4530
@joeyparrish
Copy link
Member Author

This fix was just released in v2.1.2.

@shaka-project shaka-project locked and limited conversation to collaborators Mar 22, 2018
@shaka-bot shaka-bot added the status: archived Archived and locked; will not be updated label Apr 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@theodab theodab

Labels
status: archived Archived and locked; will not be updated type: bug Something isn't working correctly
Projects
None yet
Milestone
v2.2.0
Development

No branches or pull requests
3 participants
@joeyparrish @shaka-bot @theodab