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

Decouple setup playback process from the load source process #15

Merged
merged 17 commits into from
May 13, 2021

Conversation

joaopaulovieira
Copy link
Member

@joaopaulovieira joaopaulovieira commented Jan 31, 2021

Summary

The code that creates an instance of HLS.JS also parses the manifest automatically. To avoid this behavior when is necessary, this PR creates a new configuration (options.hlsPlayback.preload) for parsing happens only after a play command is received.

Also, configure Jest to avoid noise logs on test output.

Changes

  • Create new options scope options.hlsPlayback;
  • Create new option options.hlsPlayback.preload;
  • Add docs about the new options;
  • Create defaultOptions getter representing options.hlsPlayback initial values;
  • Merge received options.hlsPlayback with defaultOptions;
  • Create new flag _manifestParsed;
  • Listen MANIFEST_PARSED HLS.JS event to set value true on _manifestParsed flag;
  • Set false on _manifestParsed flag when _setup method is called;
  • Load source at first play when it's not load at _setup process;
  • Configure Jest to run with --silent flag;
  • Delete test case covering logic from HTML5Video playback (it's already covered on @clappr/core);

How to test

With options.hlsPlayback.preload as false:

  • Attatch Clappr with HlsjsPlayback;
  • Check the network tab on your browser developer tools;
    • No one manifest (.m3u8) or video chunk (.ts) should have be download.

With options.hlsPlayback.preload as true:

  • Attatch Clappr with HlsjsPlayback;
  • Check the network tab on your browser developer tools;
    • The manifest (.m3u8) and video chunks (.ts) should have be download.

A picture/video tells a thousand words

Before this PR

Screen.Recording.2021-01-31.at.15.54.22.mov

After this PR

With options.hlsPlayback.preload as true:

Screen.Recording.2021-01-31.at.15.56.58.mov

With options.hlsPlayback.preload as false:

Screen.Recording.2021-01-31.at.15.58.12.mov

Copy link

@hound hound bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some files could not be reviewed due to errors:

�[31mERROR�[0m: Can't find config file: eslint:recommended
�[31mERROR�[0m: Can't find config file: eslint:recommended

@joaopaulovieira joaopaulovieira marked this pull request as draft January 31, 2021 18:46
Copy link

@hound hound bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some files could not be reviewed due to errors:

�[31mERROR�[0m: Can't find config file: eslint:recommended
�[31mERROR�[0m: Can't find config file: eslint:recommended

@joaopaulovieira joaopaulovieira marked this pull request as ready for review January 31, 2021 19:17
@joaopaulovieira joaopaulovieira changed the base branch from update-tests-pattern to master January 31, 2021 22:20
README.md Outdated
});
```

#### `hlsPlayback.loadSourceBeforePlay`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about naming it as preload?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in b90c5ae

@joaopaulovieira joaopaulovieira merged commit d4d3023 into master May 13, 2021
@joaopaulovieira joaopaulovieira deleted the decouple-setup-from-load-source branch May 13, 2021 22:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants