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

Allow preload of Dash media without a video element #2274

Merged
merged 37 commits into from Mar 5, 2018

Conversation

robertbryer
Copy link
Contributor

This pull request adds preload() to the player, which initialises the streaming portion of the player without a video element, so implementors can have a media item ready for immediate playback when a view is eventually attached.

Example:

var player = dashjs.MediaPlayer().create();
player.initialize(null, url, true);
player.preload();
//After a delay to load content
player.attachView($('video'))

SourceBufferController is removed, replaced by SourceBufferSink and PreBufferSink, two classes implementing a new FragmentSink interface. There are a few changes elsewhere to not assume the existence of a video element. Some PLAYBACK_NOT_INITIALIZED errors from mediaplayer.js are now STREAMING_NOT_INITIALIZED, but otherwise no other aspect of the API has changed.

import EventBus from '../../core/EventBus';
import Events from '../../core/events/Events';
import FactoryMaker from '../../core/FactoryMaker';
import InitCache from '../utils/InitCache';
import SourceBufferSink from '../SourceBufferSink';
import TextController from '../../Streaming/Text/TextController';
Copy link
Contributor

Choose a reason for hiding this comment

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

Paths are case sensitive on some platforms. This seems to be causing the tests to fails.

@epiclabsDASH
Copy link
Contributor

@robertbryer, sorry, a conflict appeared since one of latest merges.


/**
* The an end place to put fragments after they have been fetched.
* @interface FragmentSink
Copy link
Contributor

Choose a reason for hiding this comment

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

@robertbryer, we could move this interface definition to be in its own file as we do for others like ProtectionController or KeySystem. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure - is the position of the sinks ok in /streaming ? Shall I make /sinks or /controllers/sinks ?

@epiclabsDASH epiclabsDASH added this to the v2.6.5 milestone Nov 27, 2017
@epiclabsDASH epiclabsDASH modified the milestones: v2.6.5, v2.6.6 Jan 3, 2018
@epiclabsDASH epiclabsDASH modified the milestones: v2.6.6, v2.6.7 Jan 30, 2018
…BufferController to SourceBufferSink, make everything work without mediaelement/source
…ome don't like tracks to be added after decoding has started
Fix reset problems

Sort out subtitles and their unit tests
…ce SOURCEBUFFER_APPEND_COMPLETED by local callback, don't store appendedBytesInfo, pass through callback instead
@robertbryer
Copy link
Contributor Author

Hi @epiclabsDASH, I've rebased and improved the documentation, can you review and merge it in for v2.6.7?

@@ -446,12 +412,9 @@ describe('BufferController', function () {
eventBus.trigger(Events.PLAYBACK_SEEKING);
});

/*
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, not sure what to do here; doesn't seem like a valid test when the buffer is getting cleared out on seek?

Copy link
Contributor

Choose a reason for hiding this comment

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

Let me take a look at this part. Structure of the test seems to be ok but I need to take a deeper look.

@epiclabsDASH
Copy link
Contributor

Sure, I have this scheduled for 2.6.7. Thanks for this contribution!

@epiclabsDASH
Copy link
Contributor

@robertbryer, I am doing some tests but am having some problems when attaching the view to the MediaPlayer. If I attach the view a 1-2 seconds after initializing I get CHUNK_DEMUXER_ERROR_APPEND_FAILED errors. If I wait more, everything starts correctly but after playing for a few seconds video starts showing decoding artifacts.

Seems like what dash.js is inserting in the SourceBuffer is not correct.

In the meantime, could you please bring latest dev changes to this branch. There are some changes related with the way the requests are synced that changed recently.

Following with the plan of adding this PR in 2.6.7 I will continue testing it in different scenarios. I will get back to you as soon as I have more feedback.

Thanks!

@robertbryer
Copy link
Contributor Author

Yeah, I'll get on this. It's dropping the init segment if there's no video segments in the prebuffer. I guess my internet is too speedy to notice this...

@robertbryer
Copy link
Contributor Author

@epiclabsDASH - Fixed!

@epiclabsDASH
Copy link
Contributor

@robertbryer, worked like a charm after your latest changes (and before the latest merge brought new conflicts).

@robertbryer, I will merge this after conflicts are resolved. Thanks and sorry for the multiple "resolve conflicts" tasks...

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

Successfully merging this pull request may close these issues.

None yet

4 participants