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

Controllers creation modifications #1871

Conversation

jeremco
Copy link
Contributor

@jeremco jeremco commented Apr 11, 2017

Hello

The goal of this PR is to propose some changes in controllers creation sequences.
Some classes declared as singleton are now base classes (ManifestUpdater and EventController), because they don't need to be singletons (owned only by one class).
I have made some changes in streamProcessor in creation of buffer and schedule controller, to standardize the creation

Jérémie

@jeremco jeremco force-pushed the controllers_creation_modifications branch from f07f680 to 9ddaebd Compare April 12, 2017 07:08
@nicosang nicosang force-pushed the controllers_creation_modifications branch from 421de0e to 6c7e4d1 Compare April 25, 2017 12:32
@nicosang nicosang mentioned this pull request Apr 25, 2017
@nicosang nicosang force-pushed the controllers_creation_modifications branch from 0934758 to f3fc83d Compare April 27, 2017 08:09
@nicosang nicosang force-pushed the controllers_creation_modifications branch from 015e175 to 9056a9f Compare May 11, 2017 09:12
@nicosang nicosang force-pushed the controllers_creation_modifications branch 3 times, most recently from 1a57ff6 to fda3b5a Compare May 22, 2017 12:28
@nicosang nicosang force-pushed the controllers_creation_modifications branch from f129773 to a7cdaca Compare May 29, 2017 09:10
@jeremco jeremco force-pushed the controllers_creation_modifications branch 2 times, most recently from a5cae5e to 28c08e1 Compare June 9, 2017 09:47
@jeremco jeremco force-pushed the controllers_creation_modifications branch from 28c08e1 to 3f8378a Compare June 15, 2017 09:42
nicosang and others added 18 commits June 19, 2017 09:25
…ereby, tests can be executed with a mock loader.
- Unformize the way PlaybackController and StreamController are instanciated
- ManifestUpdater and EventController doesn't need to be singletons (getIsntance is called by only once controller for both classes)
- Update StreamProcessor creation : attributes are now passed in config and not in initialize methods -> uniformize with other attributes
- Rename AbrController initialize method, because it is not a initialization but a registration
- Stream Processor - change the way controllers are created: the parameters are passed in create method, not in initialize method
nicosang and others added 20 commits June 19, 2017 09:57
…ream is read, move getInstance call in initialize method.
@jeremco jeremco force-pushed the controllers_creation_modifications branch from 3f8378a to 3873ff3 Compare June 19, 2017 08:52
@nicosang
Copy link
Contributor

@AkamaiDASH could you, please, give us your feedback on this big PR? ;-)
Each time an other PR is merged, the rebase round is, for us, an important task. If you and other people are disagree we can discuss naturally...

@dsparacio
Copy link
Contributor

It is way too large for me to review and have any meaning. If you are willing to repair any broken references, if any, and support the large PR from regression I do not have any issue. Seems like changing stuff for consistency so that is great.

Copy link
Contributor

@dsparacio dsparacio left a comment

Choose a reason for hiding this comment

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

It is way too large for me to review and have any meaning. If you are willing to repair any broken references, if any, and support the large PR from regression I do not have any issue. Seems like changing stuff for consistency so that is great.

@@ -36,7 +36,7 @@ describe("RepresentationController", function () {
manifestModel.setValue(mpd);

const abrController = AbrController(context).getInstance();
abrController.registerStreamProcessor(testType, streamProcessor);
abrController.registerStreamType(testType, streamProcessor);
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure I agree with this naming. What is your justification?

@dsparacio dsparacio merged commit f16baba into Dash-Industry-Forum:development Jun 19, 2017
@jeremco jeremco deleted the controllers_creation_modifications branch June 21, 2017 08:55
spiterikevin added a commit to spiterikevin/dash.js that referenced this pull request Jul 7, 2017
dsparacio pushed a commit that referenced this pull request Jul 10, 2017
fix abandonment in AbrController (broken since #1871)
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