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

Video length not handled for TS videos longer than 13 hr 16 min #855

Closed
1 task done
bennettpeter opened this issue Dec 2, 2023 · 13 comments
Closed
1 task done
Assignees
Labels

Comments

@bennettpeter
Copy link
Contributor

Version

Media3 1.2.0

More version details

No response

Devices that reproduce the issue

Android TV all devices

Devices that do not reproduce the issue

None

Reproducible in the demo app?

Yes

Reproduction steps

This occurs if you play a TS file longer that 13 hrs 16 minutes

How to get a 14-hour ts file:

  • Get a 1 hour h264 (AVC) ts (transport stream) file
  • Create a file called 14_hr.txt with 14 lines containing file 'TS_1_hr.ts'
  • ffmpeg -f concat -safe 0 -i 14_hr.txt -c copy TS_14_hr.ts

Play the 14 hour TS with the demo app
Try to skip forward

Expected result

  • Display the video length as 14 hours
  • Skip forward as requested

Actual result

  • Video length does not show
  • Skip forward does not do anything

The bug is in the TimestampAdjuster.adjustTsTimestamp method. At the beginning of playback the code determines the length of the video by using this to get the end time stamp. When the length of the video is more than 13 hrs 16 minutes, this returns a large negative number and the result is a negative value for video length. Without a video length, skipping is disabled.

In my case the video starts with a zero pts. I am concerned that if the pts started at a higher value, say 13 hours, that it may fail if the video length is more than 16 minutes. ALternatively it may fail if the video has a PTS wrap around point.

I tried to understand the logic in order to provide a fix but I was unsuccessful. I had difficulty with the logic around the wrap around.

Media

Any TS file longer than 13 hours 16 minutes

Bug Report

@tonihei
Copy link
Collaborator

tonihei commented Dec 11, 2023

Thanks for reporting! I'll submit a fix for this problem. Note that the duration estimation will still be broken for files longer than 26.5 hours, but this duration is increasingly unlikely to occur in practice.

copybara-service bot pushed a commit that referenced this issue Dec 14, 2023
The timestamp adjuster also estimates the number of wraparounds
of the 90Khz TS timestamp. It does that by assuming that a new
timestamp is always close to the previous one (in either direction).

This logic doesn't always work for duration estimates because the
timestamp at the end of the media is not close to the one at the
beginning and it may also never be less than the one at the beginning.

This can be fixed by introducing a new estimation model that assumes
the new timestamp is strictly greater than the previous one without
making the assumption that it has to be close to it.

Issue: #855

#minor-release

PiperOrigin-RevId: 590936953
copybara-service bot pushed a commit to google/ExoPlayer that referenced this issue Dec 14, 2023
The timestamp adjuster also estimates the number of wraparounds
of the 90Khz TS timestamp. It does that by assuming that a new
timestamp is always close to the previous one (in either direction).

This logic doesn't always work for duration estimates because the
timestamp at the end of the media is not close to the one at the
beginning and it may also never be less than the one at the beginning.

This can be fixed by introducing a new estimation model that assumes
the new timestamp is strictly greater than the previous one without
making the assumption that it has to be close to it.

Issue: androidx/media#855

#minor-release

PiperOrigin-RevId: 590936953
microkatz pushed a commit that referenced this issue Jan 11, 2024
The timestamp adjuster also estimates the number of wraparounds
of the 90Khz TS timestamp. It does that by assuming that a new
timestamp is always close to the previous one (in either direction).

This logic doesn't always work for duration estimates because the
timestamp at the end of the media is not close to the one at the
beginning and it may also never be less than the one at the beginning.

This can be fixed by introducing a new estimation model that assumes
the new timestamp is strictly greater than the previous one without
making the assumption that it has to be close to it.

Issue: #855

#minor-release

PiperOrigin-RevId: 590936953
(cherry picked from commit 0157878)
@bennettpeter
Copy link
Contributor Author

I have upgraded to version 1.2.1. The fix has made things worse. On a 14 hour ts file the video length is shown correctly, but seeks are extremely slow. Seek forward 1 minute takes 30 seconds or more while the screen shows a spinning progress bar. A seek 1 hour into the 14 hour video showed a spinning progress bar for 5 minutes before I gave up waiting.

With a 13 hour ts file, seeks are fine. Seek forward 1 minute takes 1 second and seek forward 1 hour takes about 1.5 seconds.

@tonihei
Copy link
Collaborator

tonihei commented Jan 31, 2024

When I test this with the 14 hour file I created, seeking behaves normally. Are you able to share the file you have? You can send a Google Drive link or similar to android-media-github@google.com and report back here if you've done that.

@bennettpeter
Copy link
Contributor Author

I sent the email.

To create the 13 hr ts file run this
ffmpeg -f concat -safe 0 -i 13_hr.txt -c copy TS_13_hr.ts
To create the 14 hr ts file run this
ffmpeg -f concat -safe 0 -i 14_hr.txt -c copy TS_14_hr.ts
Everything works with the 13 hr file, but with the 14 hr file seeks do not work.

When attempting seek with the 14 hr file, logcat shows an unending stream of messages "Target buffer size reached with less than 500ms of buffered media data.".

2024-01-31 09:07:57.235 22506-22650 DefaultLoadControl      org.mythtv.leanfront                 W  Target buffer size reached with less than 500ms of buffered media data.
2024-01-31 09:07:57.246 22506-22650 DefaultLoadControl      org.mythtv.leanfront                 W  Target buffer size reached with less than 500ms of buffered media data.
2024-01-31 09:07:57.257 22506-22650 DefaultLoadControl      org.mythtv.leanfront                 W  Target buffer size reached with less than 500ms of buffered media data.

@tonihei
Copy link
Collaborator

tonihei commented Feb 5, 2024

I can't reproduce this problem I'm afraid. And I also don't see the mentioned Target buffer size reached with less than 500ms of buffered media data. error message while playing the 14 hour file.

Given the error message, could it be related to how your DefaultLoadControl is configured? This usually only happens with a large back buffer and a very small configured maximum buffer size that isn't sufficient to hold enough data.

@bennettpeter
Copy link
Contributor Author

I tried the demo application of media with the latest version from main. It shows the length of all of the TS files as 0, even the 1 hour file, and does not allow seeking on any of them. I don't understand this, it was working before on files up to 13 hours. On my own application, using v 1.2.1 it gets the correct length and seeks on files up to 13 hours.

@bennettpeter
Copy link
Contributor Author

This is not an important bug to me. Recordings of longer than 6 hours are not normally supported, so I am happy to leave this as is. If you want to pursue it further I can give you any assistance you need.

@tonihei
Copy link
Collaborator

tonihei commented Feb 5, 2024

I tried the demo application of media with the latest version from main. It shows the length of all of the TS files as 0, even the 1 hour file, and does not allow seeking on any of them. I don't understand this, it was working before on files up to 13 hours.

I'm also testing on the main branch and everything seems to work just fine, both for files below and above 13 h duration... Are you sure this is an unmodified checkout of the library demo app? Or is there is anything else that is special about how you built the demo app for testing?

@bennettpeter
Copy link
Contributor Author

I cloned from github the androidx/media repository. I did a pull of the main branch and I made 2 changes to make it work. One change to add my sample videos to the list, and one change to allow Clear Text Traffic, as I do not have https on my server. I an using Android Studio Giraffe 2022.3.1. I am testing with an "onn 4K streaming Box", latest version with Google TV.
I also tried with checking out the 1.2.1 tag (that is the version I am using in my app). I have attached a screenshot that shows where it has zero length and forward and back skip disabled, with the 1 hour ts file.

diff --git a/demos/main/src/main/AndroidManifest.xml b/demos/main/src/main/AndroidManifest.xml
index da13a42ca4..9bd126a8a7 100644
--- a/demos/main/src/main/AndroidManifest.xml
+++ b/demos/main/src/main/AndroidManifest.xml
@@ -41,6 +41,7 @@
       android:allowBackup="false"
       android:supportsRtl="true"
       android:name="androidx.multidex.MultiDexApplication"
+      android:usesCleartextTraffic="true"
       tools:targetApi="29">
 
     <activity android:name=".SampleChooserActivity"
diff --git a/demos/main/src/main/assets/media.exolist.json b/demos/main/src/main/assets/media.exolist.json
index 17e3c5ef82..bcb99cabd0 100644
--- a/demos/main/src/main/assets/media.exolist.json
+++ b/demos/main/src/main/assets/media.exolist.json
@@ -737,6 +737,18 @@
       {
         "name": "One hour frame counter (MP4)",
         "uri": "https://storage.googleapis.com/exoplayer-test-media-1/mp4/frame-counter-one-hour.mp4"
+      },
+      {
+        "name": "1 hour (TS)",
+        "uri": "http://rocinante:6544/Content/GetVideo?Id=7790"
+      },
+      {
+        "name": "13 hours (TS)",
+        "uri": "http://rocinante:6544/Content/GetVideo?Id=7792"
+      },
+      {
+        "name": "14 hours (TS)",
+        "uri": "http://rocinante:6544/Content/GetVideo?Id=7789"
       }
     ]
   }

vlcsnap-2024-02-05-20h09m29s658

@bennettpeter
Copy link
Contributor Author

The reason that skips work on my application and not the demo application may be that I have implemented my own Extractors factory so that I can supply a larger value to the timestampSearchBytes parameter of the TsExtractor constructor. I am using a value of 488800 instead of the default 112800.

@tonihei
Copy link
Collaborator

tonihei commented Feb 6, 2024

I checked again on a clean checkout of the main branch that the 1 hour TS file you provided plays perfectly fine and has a known duration. The only difference I see is that I copied the file to a local path instead of loading it from a remote server, but that shouldn't change any of the parsing logic.

@bennettpeter
Copy link
Contributor Author

I found what is causing the problem with the demo application. The demo is now working correctly, as you have said, on 1 hour, 13 hour and 14 hour files. Clearly the problem is on my side with the web server. Sorry for the trouble.

@tonihei
Copy link
Collaborator

tonihei commented Feb 6, 2024

Thanks for the confirmation!

@tonihei tonihei closed this as completed Feb 6, 2024
@androidx androidx locked and limited conversation to collaborators Apr 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants