sannies / mp4parser

A Java API to read, write and create MP4 files
Apache License 2.0
2.76k stars 571 forks source link

DefaultMp4Builder produces incorrect EditListBox segment durations #443

Open vlyutskanov opened 3 years ago

vlyutskanov commented 3 years ago

When I use DefaultMp4Builder.build on a movie with h264 video track with edits and aac audio track, the resulting mp4 file has its video reduced to a fraction and only the audio playing.

After some investigation, I noticed that the Movie class has this function defined:

    public long getTimescale() {
        long timescale = this.getTracks().iterator().next().getTrackMetaData().getTimescale();
        for (Track track : this.getTracks()) {
            timescale = gcd(track.getTrackMetaData().getTimescale(), timescale);
        }
        return timescale;
    }

and the DefaultMp4Builder has this one:

    public long getTimescale(Movie movie) {

        long timescale = movie.getTracks().iterator().next().getTrackMetaData().getTimescale();
        for (Track track : movie.getTracks()) {
            timescale = lcm(timescale, track.getTrackMetaData().getTimescale());
        }
        return timescale;
    }

While similar, they differ in logic as the first one uses gdc and the second lcm. DefaultMp4Builder uses in most cases the lcm version, but specifically uses the gcd version in function createEdts:

                entries.add(new EditListBox.Entry(elst,
                        Math.round(edit.getSegmentDuration() * movie.getTimescale()),
                        edit.getMediaTime() * track.getTrackMetaData().getTimescale() / edit.getTimeScale(),
                        edit.getMediaRate()));

If I override createEdts to use the lcm version like this:

                entries.add(new EditListBox.Entry(elst,
                        Math.round(edit.getSegmentDuration() * getTimescale(movie)),
                        edit.getMediaTime() * track.getTrackMetaData().getTimescale() / edit.getTimeScale(),
                        edit.getMediaRate()));

then I get the expected mp4 file that has the complete video.

Shouldn't Movie.getTimescale use lcm too? Or if gcd is the correct behavior there, perhaps DefaultMp4Builder should use consistently the lcm function?