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

Add custom loaders structure #307

Merged
merged 8 commits into from
Apr 29, 2018
Merged

Add custom loaders structure #307

merged 8 commits into from
Apr 29, 2018

Conversation

progre
Copy link
Contributor

@progre progre commented Apr 26, 2018

ref: #294

Usage:

  1. Create your own loader (example)

  2. Create player;

flvJS.createPlayer(
  {
    type: 'flv',
    isLive: true,
  },
  {
    loader: new ObservableLoader(source),
  },
);

@xqq
Copy link
Contributor

xqq commented Apr 27, 2018

Very glad to receive this pull request from you.

The custom loader should be passed with a class (aka. Constructor), because in VOD mode:

  • IOController will re-create a new loader to recover loading if detected a network Early-EOF
  • IOController will re-create a new loader when need to resume loading from a paused state
  • For multipart playback, IOController will create a new loader for every MediaSegment

Rename the field name in Config loader to be customLoader will be better.

@progre
Copy link
Contributor Author

progre commented Apr 28, 2018

OK. I replaced a instance of custom loader with a constructor.

So, it's usage was changed like this;

flvJS.createPlayer(
  {
    type: 'flv',
    isLive: true,
  },
  {
    customLoader: ObservableLoader,
    customLoaderParameters: myCustomParams,
  },
);

@xqq
Copy link
Contributor

xqq commented Apr 28, 2018

If extra parameters are needed for custom loader's constructor, I think using closure to wrap the constructor may solve it.

But why customLoaderParameters is not yet used?

@progre
Copy link
Contributor Author

progre commented Apr 28, 2018

hmm... I don't know how to wrap constructor using closure.
Can you tell me how to do it?

@xqq
Copy link
Contributor

xqq commented Apr 28, 2018

function init() {
    let extraParam = null;

    function createObservableLoader(seekHandler, config) {
        return new ObservableLoader(extraParam, seekHandler, config);
    }
    
    flvjs.createPlayer({
        type: 'flv',
        isLive: true
    }, {
        customLoader: createObservableLoader
    });
}

このようにできるかも

@progre
Copy link
Contributor Author

progre commented Apr 28, 2018

I see. Certainly, It seems to be possible.
I removed the parameter in type definition.

@xqq
Copy link
Contributor

xqq commented Apr 28, 2018

@progre
Copy link
Contributor Author

progre commented Apr 28, 2018

Range can be used to extend BaseLoader#open;

import flvJS, { MediaSegment, Range } from 'flv.js';
...
class MyCustomLoader extends flvJS.BaseLoader {
  open(dataSource: MediaSegment, range: Range) {
...

(but I don't use range parameter yet. So I don't need it.)

@xqq
Copy link
Contributor

xqq commented Apr 28, 2018

I think it is accessible through the namespace FlvJs... aka FlvJs.MediaSegment / FlvJs.Range.
I'm not familiar with TypeScript so probably wrong.

@progre
Copy link
Contributor Author

progre commented Apr 28, 2018

hmm.. It can this way;

export declare namespace FlvJs {
...

but, This is need import like this. I think this is a little ugly.

import flvJS, { FlvJs } from 'flv.js';

@xqq xqq merged commit d4657bb into bilibili:master Apr 29, 2018
@xqq
Copy link
Contributor

xqq commented Apr 29, 2018

Merged, thanks!

I will manually remove the exported type until found other solutions to refactor the d.ts.

@progre progre deleted the custom-loader-pr branch April 29, 2018 03:06
@xqq
Copy link
Contributor

xqq commented Apr 30, 2018

#310

@xqq
Copy link
Contributor

xqq commented May 22, 2018

WHY USE CHINESE IN SUCH A PURE ENGLISH PULL REQUEST?

@melodysummer
Copy link

a new learner... ok, I'll explain it in English.
flv.js supports the customloader config? @xqq

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants