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

Should Media3 use media duration to calculate frame rate rather than track duration? #1531

Closed
ybai001 opened this issue Jul 4, 2024 · 2 comments

Comments

@ybai001
Copy link
Contributor

ybai001 commented Jul 4, 2024

Background

When I contribute this code: #1235, I find there is a potential issue in Mp4Extractor.java.

Technical Details

(1) Using AndroidX Media to parse mp4 file. For example, one Media3 test media: https://github.com/androidx/media/blob/release/libraries/test_data/src/test/assets/media/mp4/dolbyVision-hdr.MOV

(2) This test media includes one video track with 5 samples and frame rate is 30fps (You can check it by MediaInfo or other tools.). The key point here is this mp4 file has 'elst' box with segment duration = 64 (timescale = 600, so it is ~0.107s.) If you check the "TRACK DURATION" (moov->trak->tkhd→duration), it is same. But the "MEDIA DURATION" (moov->trak->mdia->mdhd->duration) is 100 (timescale = 600, so it is ~0.167s.) All of above follow ISO spec and there is NO problem.

(3) In AndroidX Media, Mp4Extractor::processMoovAtom(), https://github.com/androidx/media/blob/release/libraries/extractor/src/main/java/androidx/media3/extractor/mp4/Mp4Extractor.java#L553, it calculates framerate by "TRACK DURATION" rather than "MEDIA DURATION", https://github.com/androidx/media/blob/release/libraries/extractor/src/main/java/androidx/media3/extractor/mp4/Mp4Extractor.java#L622
So the result is framerate = 5 / (106667/1000000) = 46.875fps, which is very different from the file's actual framerate (30fps).

(4) If the "MEDIA DURATION" is used, framerate = 5 / (166667/1000000) = 30fps, which is same as the file's actual framerate.

Thought

I think, as long as there is 'elst' box in MP4 file, the calculated frame rate will be wrong here.

Do you think we should change the behavior here (replace track duration with media duration) in Media3?

@ybai001
Copy link
Contributor Author

ybai001 commented Jul 26, 2024

Hi, could you have a look whether this is an issue? :)

@rohitjoins rohitjoins assigned rohitjoins and unassigned icbaker Sep 17, 2024
copybara-service bot pushed a commit that referenced this issue Sep 18, 2024
- Added logic to parse media duration from the `mdhd` box for accurate frame rate calculation.
- Fallbacks to track duration from `tkhd` when `mdhd` contains invalid or missing data.
- Avoids incorrect frame rate calculations in MP4 files with an edit list (`elst`) box.
- Adds frame rate calculations for partially fragmented MP4 files.
- Verified accuracy with tools like `mediainfo` and `ffprobe`.

Issue: #1531

**Note**: The slight difference in frame rate values in dump files that aren’t MP4s with an edit list or fragmented MP4s isn’t due to differences in `tkhd` and `mdhd` duration values (which should be identical for non-edited or non-fragmented files). Rather, it’s because they are calculated using different timescales. The `mvhd` box defines a global movie timescale, which is used for the track's `tkhd` duration. Meanwhile, each track’s `mdhd` box defines its own timescale specific to its content type, which we now use for more accurate frame rate calculation.

PiperOrigin-RevId: 676046744
@rohitjoins
Copy link
Contributor

Do you think we should change the behavior here (replace track duration with media duration) in Media3?

Yes, you're correct. It is more accurate to derive the frame rate using the media duration stored in the mdhd box of each track. We have added a fix for this. Thank you for raising this issue!

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

No branches or pull requests

3 participants