Skip to content

Fix: exclude text processors from Stream.getProcessors when text is disabled

Vinay Rosenberg requested to merge github/fork/03k64/bbc-internship-base into development

Created by: 03k64

This fixes a bug observed with low-latency streams containing one or more text representations. The bug prevented the liveCatchUpPlaybackRate from being correctly applied following a buffer stall, leading to unconstrained drift from the user-specified target latency.

The bug manifested as follows:

  1. a buffer stall event occurs, the playbackStalled field on StreamController is set to true
  2. StreamController.startPlaybackCatchUp is called, it uses StreamController.getBufferLevel to obtain the lowest current audio, video text or fragmented text buffer level
  3. in streams with a text or fragmented text representation, the recent change to disable text by default resulted in an empty buffer being maintained for that representation unless text had been explicitly enabled for the player
  4. the empty buffer prevents StreamController.startPlaybackCatchUp from resetting the playbackStalled field of StreamController to false, further, it assumes a buffer stall is ongoing, and resets the newRate to 1.0, preventing latency from being reduced

This change then checks whether text has been enabled and only adds the stream processors for textual representations to those returned by the Stream.getProcessors if it is. I have added some tests to document/verify the new intended behaviour. I have also included a smaller change, that modifies a function-level variable name to provide clarity due to a class-level variable with the same name.

This is my first PR, however I am submitting as part of my work at the BBC, who have previously signed up to the dash.js feedback agreement.

Merge request reports