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

New subtitle transcoding feature loads all subtitles when initialising #1721

Closed
1 task
TheBeastLT opened this issue Sep 11, 2024 · 3 comments
Closed
1 task
Assignees
Labels

Comments

@TheBeastLT
Copy link

TheBeastLT commented Sep 11, 2024

Version

Media3 1.4.1

More version details

No response

Devices that reproduce the issue

Any device

Devices that do not reproduce the issue

No response

Reproducible in the demo app?

Yes

Reproduction steps

With the version 1.4.1 the experimentalSetTextTrackTranscodingEnabled feature is enabled by default and it introduces behaviour that is not expected and was not there before. With it enabled the player loads each and every subtitle configuration that is defined for the MediaItem even if it is not selected. There can be uses cases where these subtitles configuration are remote urls and there can be many of them for different languages, so loading all of them when initialising the media item is not efficient and introduces delay when the item is played. Ideally only the selected subtitle should be transcoded, as was the behaviour before with remote urls - it would get downloaded only when it was selected.

Expected result

Not all subtitle configuration are loaded when MediaItem is initialized

Actual result

All subtitles configuration are loaded (and in case of remote urls - are downloaded)

Media

Any

Bug Report

@icbaker
Copy link
Collaborator

icbaker commented Sep 23, 2024

This is due to the new subtitle handling using ProgressiveMediaPeriod rather than SingleSampleMediaPeriod, and the different implementations of MediaPeriod.prepare these two classes have.

SingleSampleMediaPeriod does nothing in its prepare method:

public void prepare(Callback callback, long positionUs) {
callback.onPrepared(this);
}

And only loads data inside continueLoading (which will only be called after the track has been selected):

public boolean continueLoading(LoadingInfo loadingInfo) {
if (loadingFinished || loader.isLoading() || loader.hasFatalError()) {
return false;
}
DataSource dataSource = dataSourceFactory.createDataSource();
if (transferListener != null) {
dataSource.addTransferListener(transferListener);
}
SourceLoadable loadable = new SourceLoadable(dataSpec, dataSource);

Whereas ProgressiveMediaPeriod, as a general purpose class for playing progressive media files (like MP4), needs to load data in prepare in order to determine what tracks are available:

public void prepare(Callback callback, long positionUs) {
this.callback = callback;
loadCondition.open();
startLoading();
}

I agree this is unexpected, it's an unintended side-effect of using ProgressiveMediaPeriod.

@TheBeastLT
Copy link
Author

@icbaker Are there any intentions to change and fix this?

@icbaker
Copy link
Collaborator

icbaker commented Oct 31, 2024

Yes, I'm working on this at the moment.

copybara-service bot pushed a commit that referenced this issue Dec 3, 2024
This means we can complete preparation (and trigger track selection)
before opening a `DataSource`, which then means we only end up loading
the data for a selected subtitle track (instead of all tracks as
currently happens).

By making preparation trivial in this case (with no reasonable cause
of error), we can also remove the `suppressPrepareError` option added in
b3290ef.

This change also fixes the implementation of
`ProgressiveMediaPeriod.maybeStartDeferredRetry` to only short-circuit
return `false` if the chosen track is not audio or video **and** there
is at least one audio or video track in this period.

Issue: #1721
PiperOrigin-RevId: 702275968
@icbaker icbaker closed this as completed Dec 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants