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 MPEG-DASH support to playback #27

Merged
merged 4 commits into from
Dec 6, 2023
Merged

Add MPEG-DASH support to playback #27

merged 4 commits into from
Dec 6, 2023

Conversation

kschults
Copy link
Contributor

@kschults kschults commented Dec 6, 2023

Adds support for playback. Create new MediaSourceGenerator. Refactors some common code into a header aware helper.

@BarryFruitman BarryFruitman self-requested a review December 6, 2023 15:44
@BarryFruitman BarryFruitman self-assigned this Dec 6, 2023
@BarryFruitman BarryFruitman added the Medium Medium complexity/size label Dec 6, 2023
@@ -96,6 +96,8 @@ internal class ExoplayerDownloadEngine @Inject constructor(private val context:
return when (@C.ContentType val type = Util.inferContentType(uri)) {
C.TYPE_HLS ->
DownloadHelper.forMediaItem(context, mediaItem, renderersFactory, DefaultDataSource.Factory(context, dataSourceFactory))
C.TYPE_DASH ->
DownloadHelper.forMediaItem(context, mediaItem, renderersFactory, DefaultDataSource.Factory(context, dataSourceFactory))

Choose a reason for hiding this comment

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

Is DASH likely to diverge from HLS? If not, you should combine these cases.

import javax.inject.Inject
import javax.inject.Singleton

@Singleton

Choose a reason for hiding this comment

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

Just curious what @Singleton does here? I thought it only needs to be on the provides method.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're probably right, but I think it's helpful to see it here to see that it's a singleton, otherwise it's hidden.

@aferrariScribd aferrariScribd self-assigned this Dec 6, 2023
Copy link
Contributor

@aferrariScribd aferrariScribd left a comment

Choose a reason for hiding this comment

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

lgtm

@kschults kschults merged commit 4abdf58 into main Dec 6, 2023
4 checks passed
@kschults kschults deleted the karl/APT-9543-dash branch December 6, 2023 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Medium Medium complexity/size
Development

Successfully merging this pull request may close these issues.

3 participants