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

undefined "info" object in DashParser when manifest contains no periods #618

Closed
steve-o opened this issue Dec 2, 2016 · 9 comments
Closed
Labels
status: archived Archived and locked; will not be updated type: bug Something isn't working correctly
Milestone

Comments

@steve-o
Copy link

steve-o commented Dec 2, 2016

  • What version of Shaka Player are you using?

2.0.1-debug and 2.0.0.

  • Can you reproduce the issue with our latest release version?

Yes.

  • Can you reproduce the issue with the latest code from master?

Yes.

  • Are you using the demo app or your own custom app?

Custom app derived from introduction tutorial, i.e.

http://shaka-player-demo.appspot.com/docs/api/tutorial-basic-usage.html

Hosted here against the appspot hosting: https://junko.hk/chome/debug.html

  • What did you do?

Idea was to use tutorial together with DASH manifest and files from YouTube on a new clean HTTPS server.

  • What content did you load?

Simple 40s clip on YouTube: https://www.youtube.com/watch?v=94BNgmNYoH8

  • How did you interact with the content, if at all?

n/a

  • What did you expect to happen?

Video to load without errors or at least report some form of legible error.

  • What actually happened?

Two errors:

  1. Assertion failed: Wrong error type! player.js:449
  2. Error code undefined object TypeError: Cannot read property 'containsInband' of undefined
    at shaka.dash.DashParser.parsePeriods_ (https://shaka-player-demo.appspot.com/lib/dash/dash_parser.js:658:27)
    at shaka.dash.DashParser.parseManifest_ (https://shaka-player-demo.appspot.com/lib/dash/dash_parser.js:517:33)
    at shaka.dash.DashParser. (https://shaka-player-demo.appspot.com/lib/dash/dash_parser.js:410:21)
@joeyparrish
Copy link
Member

@steve-o, are you loading a DASH manifest from YouTube?

@steve-o
Copy link
Author

steve-o commented Dec 2, 2016

@joeyparrish I copied the manifest and retargeted each BaseURL to a local copy of each resource.

I presume there maybe something in there off-standard that may break the player, but it does not look too unusual:

https://junko.hk/chome/dash.mpd

@joeyparrish joeyparrish added the type: question A question from the community label Dec 2, 2016
@joeyparrish
Copy link
Member

There's a typo in your manifest:

<Period,duration

That comma should be a space.

@steve-o
Copy link
Author

steve-o commented Dec 2, 2016

Ooh, thank you. Still looks like a bug in that function though as var info is out of scope.

@steve-o steve-o closed this as completed Dec 2, 2016
@joeyparrish
Copy link
Member

Can you be more specific? I can't reproduce your error in our demo app since your content is not available cross-origin. If there's a bug, we'd love to fix it. Please provide repro or submit a pull request.

@steve-o
Copy link
Author

steve-o commented Dec 2, 2016

Working on trying to fix that, there are two lines matching the undefined object:

https://github.com/google/shaka-player/blob/master/lib/dash/dash_parser.js#L659
https://github.com/google/shaka-player/blob/master/lib/dash/dash_parser.js#L665

The only scope for 'var info' is in the 'for' loop on line 613, unless I am missing something? Is there a globally scoped version?

@joeyparrish joeyparrish added type: bug Something isn't working correctly and removed type: question A question from the community labels Dec 2, 2016
@joeyparrish
Copy link
Member

Odd... that does look wrong. I'm not sure why you get this error and I do not. Looking at the code, you would expect this error to occur on every manifest.

@TheModMaker
Copy link
Contributor

JavaScript has an odd definition of variable scopes, it uses something called hoisting. The variable is actually "defined" at the beginning of the function. So at the end of the method it still has the last value stored (from the last Period). This error only appears when there are no periods and the variable is still set to undefined.

@joeyparrish
Copy link
Member

Logically, containsInband should be the "or" of the property from all periods, not the last period parsed. So the code is subtly wrong, even when it's not causing this TypeError.

If there are no periods, we should throw a specific error that says so, instead of dereferencing null or undefined vars. So that's another small issue on us.

We will fix both of these issues.

All of that said, when your manifest has valid Periods, your problem goes away.

@joeyparrish joeyparrish reopened this Dec 5, 2016
@joeyparrish joeyparrish changed the title undefined "info" object in DashParser undefined "info" object in DashParser when manifest contains no periods Dec 6, 2016
@joeyparrish joeyparrish modified the milestone: v2.1.0 Dec 6, 2016
ismena pushed a commit that referenced this issue Dec 15, 2016
Before this, an empty manifest (no periods) would result in a
TypeError from the containsInband computation.  This corrects the
computation and introduces an explicit error for empty manifests.

Closes #618

Change-Id: Ie9b740dbfa4ffcafbf99541bf03fa68cfae2bf88
@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.
Labels
status: archived Archived and locked; will not be updated type: bug Something isn't working correctly
Projects
None yet
Development

No branches or pull requests

4 participants