shaka-project / shaka-player

JavaScript player library / DASH & HLS client / MSE-EME player
Apache License 2.0
7.1k stars 1.33k forks source link

[DASH] TypeError · Cannot read properties of undefined (reading 'audio') #6337

Closed fredrik-telia closed 6 months ago

fredrik-telia commented 6 months ago

Have you read the Tutorials? yes

Have you read the FAQ and checked for duplicate open issues? yes

If the question is related to FairPlay, have you read the tutorial?

What version of Shaka Player are you using? 4.7.9

What browser and OS are you using? Chrome, Edge, Firefox

Please ask your question I'm looking at our production logs and we have a number of errors that I believe come from within Shaka: TypeError · Cannot read properties of undefined (reading 'audio')

(minified code, sorry)

this.l.clientHeight*r),n=Math.min(n,this.l.clientWidth*r)}var s=this.m.filter(function(e){return!(e.audio&&e.audio.fastSwitching||e.video&&e.video.fastSwitching)});if(s.length||(s=this.m),r=s,t&&s.le

Looking at the stacktrace in the log, it seems to point to this line: https://github.com/shaka-project/shaka-player/blob/4ae15c2c6fe2be98a7a6e9960ce48181f45e1324/lib/util/stream_utils.js#L1690

by way of _.chooseVariant

So my question is this: is there a chance that isFastSwitching() could be called with variant being undefined or variant.audio being undefined?

We get these errors quite rarely, and they're hard to debug, currently we're at around 400 errors per month, affecting around 100 unique users.

Additional info: Edge seems overrepresented, accounting for 50% of cases for version 121.0.0, not sure if there's any significance to that. Chrome has more than twice the amount of plays, but less errors.

joeyparrish commented 6 months ago

isFastSwitching() is only called from two places, both in SimpleAbrManager's chooseVariant(). In both cases, we're calling it inside the loop of this.variants_.filter(). So this.variants_ would have to have an undefined in the list. We're never indexing into it with a number.

The list is only ever taken in through setVariants(). This can be called either from Player's updateAbrManagerVariants() or PreloadManager's chooseInitialVariantInner_(). Both cases take the list for setVariants() most immediately from AdaptationSet.

I see one possible path here. AdaptationSet takes a root variant in the constructor, which is never checked. There are two calls to that constructor, and one of them, at the bottom of shaka.media.PreferenceBasedCriteria's create() calls passes the AdaptationSet a root based on current[0]. If the input variants is an empty list, current seems like it would stay empty, and at the end, current[0] would be undefined, which would end up trickling down to AbrManager eventually.

For this to happen, it would appear that playableVariants would have to be empty in Player's updateAbrManagerVariants_(). We don't appear to have a check for that being empty at that stage, though there is an error code for that and there are other checks for "no playable variants" at other points.

joeyparrish commented 6 months ago

I think I found the discrepancy. updateAbrManagerVariants_() actually does have a check, it's just indirect.

  updateAbrManagerVariants_() {
    try {
      goog.asserts.assert(this.manifest_, 'Manifest should exist by now!');
      this.manifestFilterer_.checkRestrictedVariants(this.manifest_);
    } catch (e) {
      this.onError_(e);
      return false;
    }

    const playableVariants = shaka.util.StreamUtils.getPlayableVariants(
        this.manifest_.variants);

    // Update the abr manager with newly filtered variants.
    const adaptationSet = this.currentAdaptationSetCriteria_.create(
        playableVariants);
    this.abrManager_.setVariants(Array.from(adaptationSet.values()));
    return true;
  }

ManifestFilterer.checkRestrictedVariants() is meant to deal with the "nothing is playable" case and generates error code RESTRICTIONS_CANNOT_BE_MET if nothing is playable. Then, separately, StreamUtils.getPlayableVariants() gets a list of those variants. The trouble is caused by those two using different definitions.

ManifestFilterer does not check variant.disabledUntilTime, but StreamUtils does. So if a variant is otherwise playable, but then gets "disabled" by Player's disableStream(), this could happen.

disableStream() checks for an "alternate" stream to be available before disabling the one passed in, but that check could be broken.

That's all the time I have to investigate this today, but I hope that helps. PRs are welcome!

shaka-bot commented 6 months ago

@fredrik-telia Does this answer all your questions? If so, would you please close the issue?

fredrik-telia commented 6 months ago

Thanks for the answer, @joeyparrish, I'll see if I can find some time to better understand what should happen if the variant is undefined, if so I'll make a PR