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

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

Open
wants to merge 4 commits into
base: development
Choose a base branch
from

Commits on Jul 29, 2020

  1. Rename function-level variable for clarity

    - removes potential for ambiguity between liveDelay variable in
      startPlaybackCatchUp function and PlaybackController module when
      reasoning about the code
    - new name intended to convey that the player's liveDelay value is used
      throughout this function
    03k64 committed Jul 29, 2020
    Configuration menu
    Copy the full SHA
    23c9481 View commit details
    Browse the repository at this point in the history
  2. Fix: exclude text processors when text is disabled

    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.
    
    Tested on Firefox 78.02 & Chrome 84.0.414.79 on Ubuntu 20.04.
    03k64 committed Jul 29, 2020
    Configuration menu
    Copy the full SHA
    43e2f43 View commit details
    Browse the repository at this point in the history
  3. Tests: verify behaviour of Stream.getProcessors

    - existing test verifies that empty array of stream processors exists
    when stream is not activated
    - additional tests check that text and fragmented text processors are
    returned by Stream.getProcessors _only_ when text is enabled via the
    TextController
    03k64 committed Jul 29, 2020
    Configuration menu
    Copy the full SHA
    1cb20c6 View commit details
    Browse the repository at this point in the history

Commits on Aug 12, 2020

  1. Fix: exclude text buffer level if text is disabled

    This fix ensure the correct minimum buffer level is reported when text
    is disabled. Previously a buffer level of 0 is reported at all times if
    a text track is present in the manifest but text is disabled in the
    player. This fix checks whether text is enabled and only includes text
    buffer metrics in buffer level reporting if it is.
    03k64 committed Aug 12, 2020
    Configuration menu
    Copy the full SHA
    8353fbe View commit details
    Browse the repository at this point in the history