Level label formatting issue

[kslimani]

i have done some tests with the "angel" dash source url (Star Trek) from demo page and as described in this issue, sometimes the mafinest expose several video representation with the same height but for different language, which result in duplicated labels.

This could be fixed by changing label formatting, maybe add language when available ?

Currently it result in 576p, 480p, ...

Maybe we could change to 576p (en), 480p (en), 576p (de), 480p (de), ... ?

I could push a fix proposition, but i would like first some feedbacks/ideas on potential new label string format. /ping @leandromoreira @jhonatangomes

EDIT: another issue is when levels elements count is high, the select ui is overflow hidden by player container. (maybe find a way to display on higher z-index and maybe display on bottom of button instead of top depending if enough space : not sure how to do that my css is terrible ;-) )

[leandromoreira]

I think your proposed solution is great!!! add (context) language will help! let me bring @joaopaulovieira @towerz to the discussion as well, btw thanks once again man, you're on 🔥 👏 👏 👏 👏 👏 👏 👏 👏

[kslimani]

My proposition is to add these modifications (not added yet to current pr #75) :

_hasDuplicateValue(objArray, key) {
      let hash = Object.create(null)

      for (let i = 0; i < objArray.length; ++i) {
        let v = objArray[i][key]
        if (v && v in hash)
          return true

        hash[v] = true
      }

      return false
  }

  _fillLevels () {
    if (this._levels.length === 0) {
      // Check if some video tracks have same height to avoid duplicated labels
      let hasDupeHeight = this._hasDuplicateValue(this.videoTracks, 'height')

      this._levels = this.videoTracks.map((videoTrack) => {
        return {
          id: videoTrack.id,
          level: videoTrack,
          label: hasDupeHeight ? `${videoTrack.height}p - ${videoTrack.language}` : `${videoTrack.height}p`
        }
      }).reverse()

      this.trigger(Events.PLAYBACK_LEVELS_AVAILABLE, this.levels)
    }
  }

It is a workaround, because level selector plugin (and playbacks) does not have audio language switch mechanism yet.

[leandromoreira]

Cool, what do you think of renaming _hasDuplicateValue to _mightHaveMultipleLanguages ... I'm not sure if it makes it clear or not because the function really just check duplicated values.

One solution to that could be:

let mightHaveMultipleLanguages = this._hasDuplicateValue(this.videoTracks, 'height')

[kslimani]

Maybe rename method _hasDuplicateValue to _hasDuplicateObjectValue (method expects an array of object as first parameter, and second parameter is the name of object property to check for duplicate value)

Yes, we could rename hasDupeHeight variable to mightHaveMultipleLanguages to make it clearer.

[kslimani]

In future version of Clappr i think we could remove completly label formatting logic from playback plugins, and normalize array of object provided by PLAYBACK_LEVELS_AVAILABLE event; and move all the formatting logic inside the level selector plugin ? (still providing formatting callback to allow developper customize labels)

It would also bring consistency because currently, labels for hls is are displaying bitrates and dash are displaying height(p).

[leandromoreira]

100% in favor of creating a new component for that, great idea ! 👍

[kslimani]

awww, the previous provided solution does not work in all case. There are some manifest with multiple time the same height, with the same language, but different bitrate...

I am starting to think that the hls.js playback default approach for label formatting using the bitrates might be a better.

[leandromoreira]

I am starting to think that the hls.js playback default approach for label formatting using the bitrates might be a better.

Yeah and let it open for the custom personalization by the user

[kslimani]

currently, i see 2 solutions :

First solution is to always use bitrate for label and check if video tracks has multiple audio languages. If this is the case, then use bitrate with language code for label.

Second solution is same as previous, but also check if no video track "height" duplication. If no duplication, then use height for label (same as current version), otherwise use bitrate (with/without language code).

Any thoughts ?

[leandromoreira]

First solution is to always use bitrate for label and check if video tracks has multiple audio languages. If this is the case, then use bitrate with language code for label.

I'd like this one... not relying on duplicate height with different bitrates

[kslimani]

@leandromoreira i added a commit in #75 which change the default label formatting to be the same as hls playback (ie: using bitrates).

When multiple languages are available, it append the language code to label.

It also sort the tracks in bandwidth DESC order. (previous code was wrongly assuming that tracks were already sorted and it was only reversing order).