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

Default SingleSampleMediaSource.treatLoadErrorsAsEndOfStream to true #8430

Closed
GouravSna opened this issue Jan 5, 2021 · 9 comments
Closed
Assignees

Comments

@GouravSna
Copy link

GouravSna commented Jan 5, 2021

  • ExoPlayer version number : 2.12.2 and 2.12.1
  • Android version : Android 10 (not OS specific)
  • Android device : Not device specific

Steps to reproduce:

  1. Pass the external subtitle url to media item. Can replace this method in source code with the following method.

I have appended _xyz in the vtt url.

private static List<MediaItem.Subtitle> createSubtitlesFromIntent(
      Intent intent, String extrasKeySuffix) {
//    if (!intent.hasExtra(SUBTITLE_URI_EXTRA + extrasKeySuffix)) {
//      return Collections.emptyList();
//    }
    String url = "https://storage.googleapis.com/exoplayer-test-media-1_xyz/webvtt/numeric-lines.vtt";
    MediaItem.Subtitle subtitleMediaItem = new MediaItem.Subtitle(Uri.parse(url), MimeTypes.TEXT_VTT,"en", C.SELECTION_FLAG_DEFAULT, C.ROLE_FLAG_SUBTITLE, "ENGLISH-ZEE");

    return Collections.singletonList(subtitleMediaItem);
  }
  1. Run any content and playback will fail.

Logcat

2021-01-05 19:14:08.615 31453-31570/com.google.android.exoplayer2.demo E/ExoPlayerImplInternal: Playback error
      com.google.android.exoplayer2.ExoPlaybackException: Source error
        at com.google.android.exoplayer2.ExoPlayerImplInternal.handleMessage(ExoPlayerImplInternal.java:554)
        at android.os.Handler.dispatchMessage(Handler.java:98)
        at android.os.Looper.loop(Looper.java:165)
        at android.os.HandlerThread.run(HandlerThread.java:61)
     Caused by: com.google.android.exoplayer2.upstream.HttpDataSource$InvalidResponseCodeException: Response code: 404
        at com.google.android.exoplayer2.ext.cronet.CronetDataSource.open(CronetDataSource.java:486)
        at com.google.android.exoplayer2.upstream.DefaultDataSource.open(DefaultDataSource.java:199)
        at com.google.android.exoplayer2.upstream.cache.CacheDataSource.openNextSource(CacheDataSource.java:764)
        at com.google.android.exoplayer2.upstream.cache.CacheDataSource.open(CacheDataSource.java:579)
        at com.google.android.exoplayer2.upstream.StatsDataSource.open(StatsDataSource.java:84)
        at com.google.android.exoplayer2.source.SingleSampleMediaPeriod$SourceLoadable.load(SingleSampleMediaPeriod.java:428)
        at com.google.android.exoplayer2.upstream.Loader$LoadTask.run(Loader.java:415)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1133)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:607)
        at java.lang.Thread.run(Thread.java:760)

Some Background

Prior exoplayer 2.12.1, we were passing DefaultLoadErrorHandlingPolicy and here we were listening to Exception but after 2.12.1 this API does not seem to be working. We tried it but we are not getting any callback.

Following class we have added to listen the callback before 2.12.1 and it was fine. It means if the subtitle does not work, we listen to it and it does not hamper the playback.

public class ExternalTextTrackLoadErrorPolicy extends DefaultLoadErrorHandlingPolicy {

    private static final PKLog log = PKLog.get("ExternalTextTrackLoadError");

    private ExternalTextTrackLoadErrorPolicy.OnTextTrackLoadErrorListener textTrackLoadErrorListener;

    public interface OnTextTrackLoadErrorListener {
        void onTextTrackLoadError(PKError currentError);
    }

    public void setOnTextTrackErrorListener(ExternalTextTrackLoadErrorPolicy.OnTextTrackLoadErrorListener onTextTrackErrorListener) {
        this.textTrackLoadErrorListener = onTextTrackErrorListener;
    }

    //public void onTextTrackLoadError(PKError currentError) {
    //    textTrackLoadErrorListener.onTextTrackLoadError(currentError);
    //} TODO // need to test with vtt errored file

    @Override
    public long getRetryDelayMsFor(LoadErrorInfo loadErrorInfo) {
        if (loadErrorInfo == null) {
            return Consts.TIME_UNSET;
        }

        IOException exception = loadErrorInfo.exception;
        Uri pathSegment = getPathSegmentUri(exception);
        if (pathSegment == null || !(exception instanceof HttpDataSource.HttpDataSourceException)) {
            return super.getRetryDelayMsFor(loadErrorInfo);
        }


        String lastPathSegment = pathSegment.getLastPathSegment();
        if (lastPathSegment != null && (lastPathSegment.endsWith(PKSubtitleFormat.vtt.pathExt) || lastPathSegment.endsWith(PKSubtitleFormat.srt.pathExt))) {
            PKError currentError = new PKError(PKPlayerErrorType.SOURCE_ERROR, PKError.Severity.Recoverable, "TextTrack is invalid url=" + pathSegment, exception);
            if (textTrackLoadErrorListener != null) {
                log.e("Error-Event sent, type = " + PKPlayerErrorType.SOURCE_ERROR);
                textTrackLoadErrorListener.onTextTrackLoadError(currentError);
            }
            return Consts.TIME_UNSET;
        } else {
            return super.getRetryDelayMsFor(loadErrorInfo);
        }
    }

    private Uri getPathSegmentUri(IOException ioException) {
        DataSpec dataSpec = null;
        if (ioException instanceof HttpDataSource.InvalidResponseCodeException) {
            dataSpec = ((HttpDataSource.InvalidResponseCodeException) ioException).dataSpec;
        } else if (ioException instanceof HttpDataSource.HttpDataSourceException) {
            dataSpec = ((HttpDataSource.HttpDataSourceException) ioException).dataSpec;
        }

        if (dataSpec != null) {
            return dataSpec.uri;
        }
        return null;
    }
}
@icbaker
Copy link
Collaborator

icbaker commented Jan 5, 2021

Prior exoplayer 2.12.1, we were passing DefaultLoadErrorHandlingPolicy and here we were listening to Exception but after 2.12.1 this API does not seem to be working. We tried it but we are not getting any callback.

Can you provide the info about specifically how you tried to use this in 2.12.1?

@GouravSna
Copy link
Author

It was working in v2.11.7 and we started facing this issue after MediaItem introduction in v2.12.x.

In 2.11.7, we were using SingleSampleMediaSource to set the external subtitles and then apply the error policy on it using ExternalTextTrackLoadErrorPolicy.java class and were using MergingMediaSource.

Now with MediaItem, this flow is not working and after setting the error policy on DefaultMediaSourceFactory; this is not giving us the callback for error.

Please let me know if you want any demo sample code or any more info.

@icbaker
Copy link
Collaborator

icbaker commented Jan 6, 2021

Everything you describe in the original comment is expected until you get to the part about the LoadErrorHandlingPolicy registration not working correctly in 2.12.1, so that's what I'd like to dig into.

Please can you provide enough info to reproduce this part of the problem? You haven't made it clear exactly how you're registering this policy in using 2.12.1.

Please provide a minimal reproducible example that demonstrates the problem in a way that we can build locally. This could be an Android Studio project on GitHub, or zipped up and sent to dev.exoplayer@gmail.com using a subject in the format "Issue #1234", where "#1234" should be replaced with your issue number. Please also update this issue to indicate you’ve done this.

@icbaker icbaker changed the title Playback fails if subtitle file is not valid LoadErrorHandlingPolicy registration in 2.12.1 Jan 6, 2021
@GouravSna
Copy link
Author

GouravSna commented Jan 6, 2021

Please check Sample for Subtitle Not Working branch and Sample for LoadErrorPolicy branch.

You can see this is how we are adding load error policy.

But we are not getting callback in getRetryDelayMsFor method.
Here in this comment you gave this solution earlier as well. But now seems that it is broken.

What we can think of that the object which we are giving to exoplayer for error policy is not being retained and internally another object is being created that's why we are not getting the callback in our class.

@icbaker
Copy link
Collaborator

icbaker commented Jan 8, 2021

Thanks for the branch - using that I'm able to reproduce what you're seeing, and using the debugger I pinned down the problem - you're right that basically your policy is being accidentally dropped internally. Specifically here, it needs to be propagated down to the SingleSampleMediaSource.Factory:

SingleSampleMediaSource.Factory singleSampleSourceFactory =
new SingleSampleMediaSource.Factory(dataSourceFactory);

I've sent a fix for this - it should be included in 2.12.3 which will be released before the end of the month.

Note that even with this fix, your content doesn't play in your LoadErrorHandlingPolicy branch - but I think that's due to the implementation of your policy - the execution does now reach the getRetryDelayMsFor method.

icbaker added a commit that referenced this issue Jan 8, 2021
I think this was missed when integrating DefaultMediaSourceFactory with
SingleSampleMediaSource.Factory in
315ba6f

Issue: #8430
#minor-release
PiperOrigin-RevId: 350759580
@icbaker icbaker closed this as completed Jan 8, 2021
@GouravSna
Copy link
Author

GouravSna commented Jan 11, 2021

Hi @icbaker , now the code is reaching to getRetryDelayMsFor of my custom class and there I am getting the uri from dataSpec and checking if the error is for vtt or srt and if yes then we are returning C.TIME_UNSET.

But there is issue that still the media is failing to play because treatLoadErrorsAsEndOfStream is not set to true, it is false and here in this condition, it goes to else part and eventually it retries and fails.

In ExoPlayer v2.11.7, there was an api to pass treatLoadErrorsAsEndOfStream value, Example is this

I tried setting treatLoadErrorsAsEndOfStream value to true in SingleSampleMediaPeriod constructor then i can see the media plays back with the load policy.

Can you help to let us know how to pass this value now ?

@icbaker
Copy link
Collaborator

icbaker commented Jan 11, 2021

Ah I see - yeah that isn't currently possible using only the MediaItem API.

I will add this as a configuration option to MediaItem.Subtitle.

Until that's done you'll need to either implement your own MediaSource.Factory or skip using MediaItem and use the MediaSource APIs:
https://exoplayer.dev/media-sources.html#media-source-based-playlist-api

@icbaker icbaker reopened this Jan 11, 2021
@icbaker icbaker changed the title LoadErrorHandlingPolicy registration in 2.12.1 Add treatLoadErrorsAsEndOfStream option to MediaItem.Subtitle Jan 11, 2021
@icbaker icbaker changed the title Add treatLoadErrorsAsEndOfStream option to MediaItem.Subtitle Add treatLoadErrorsAsEndOfStream option to MediaItem.Subtitle Jan 11, 2021
@icbaker icbaker added enhancement and removed bug labels Jan 11, 2021
icbaker added a commit that referenced this issue Jan 11, 2021
I think this was missed when integrating DefaultMediaSourceFactory with
SingleSampleMediaSource.Factory in
315ba6f

Issue: #8430
PiperOrigin-RevId: 350759580
@GouravSna
Copy link
Author

Hi @icbaker
In v2.12.3, only setLoadErrorHandlingPolicy is being handled. Please let me know if there is any plan to add treatLoadErrorsAsEndOfStream.
Because I think this fix will not help us.

@icbaker
Copy link
Collaborator

icbaker commented Jan 13, 2021

We plan to handle treatLoadErrorsAsEndOfStream in some way (possibly by adding it to MediaItem.Subtitle, possibly by defaulting to it to true, or possibly by other means).

We haven't made this change yet (hence this issue is still open), so yes it's not included in 2.12.3 released today.

icbaker added a commit that referenced this issue Jan 15, 2021
@icbaker icbaker changed the title Add treatLoadErrorsAsEndOfStream option to MediaItem.Subtitle Default SingleSampleMediaSource.treatLoadErrorsAsEndOfStream to true Jan 15, 2021
@icbaker icbaker closed this as completed Jan 15, 2021
@google google locked and limited conversation to collaborators Mar 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants