From 29cdba5f295d6f85bbf8ddfbba9961fce8ef6d8f Mon Sep 17 00:00:00 2001 From: Moritz Bunkus Date: Fri, 16 Nov 2018 09:54:52 +0100 Subject: [PATCH] chapters: take smallest timestamp in first file into account When generating chapters mkvmerge has to take into account things such as splitting and file linking. This requires shifting chapter timestamps to match file timestamps. However, for files which don't start at 0 generated chapters would be wrongfully shifted down to below 0 causing invalid timestamps. Fixes #2432. --- NEWS.md | 3 +++ src/merge/output_control.cpp | 7 ++++++- tests/results.txt | 1 + tests/run.rb | 1 + ...nterval_first_video_timestamp_bigger_than_0.rb | 15 +++++++++++++++ 5 files changed, 26 insertions(+), 1 deletion(-) create mode 100755 tests/test-664generate_chapters_interval_first_video_timestamp_bigger_than_0.rb diff --git a/NEWS.md b/NEWS.md index a442a1ae0..008650c25 100644 --- a/NEWS.md +++ b/NEWS.md @@ -15,6 +15,9 @@ so that Windows actually recognizes it. See #2415. * mkvmerge: MP4 reader: fixed handling of atoms whose size exceeds the parent atom's size. Fixes #2431. +* mkvmerge: chapter generation: the start timestamps of chapters generated in + intervals was wrong for files whose smallest video timestamp was bigger than + 0. Fixes #2432. ## Build system changes diff --git a/src/merge/output_control.cpp b/src/merge/output_control.cpp index f5ecfae7b..f3e44554f 100644 --- a/src/merge/output_control.cpp +++ b/src/merge/output_control.cpp @@ -1375,7 +1375,12 @@ prepare_additional_chapter_atoms_for_rendering() { if (!s_chapters_in_this_file) s_chapters_in_this_file = std::make_shared(); - auto offset = timestamp_c::ns(g_no_linking ? g_cluster_helper->get_first_timestamp_in_file() + g_cluster_helper->get_discarded_duration() : 0); + static timestamp_c s_first_file_minimum_timestamp; + + if (!s_first_file_minimum_timestamp.valid()) + s_first_file_minimum_timestamp = timestamp_c::ns(g_no_linking ? g_cluster_helper->get_first_timestamp_in_file() + g_cluster_helper->get_discarded_duration() : 0); + + auto offset = timestamp_c::ns(g_no_linking ? g_cluster_helper->get_first_timestamp_in_file() + g_cluster_helper->get_discarded_duration() : 0) - s_first_file_minimum_timestamp; auto &edition = GetChild(*s_chapters_in_this_file); if (!FindChild(edition)) diff --git a/tests/results.txt b/tests/results.txt index 3ddf2916a..e23a4ea3b 100644 --- a/tests/results.txt +++ b/tests/results.txt @@ -509,3 +509,4 @@ T_660propedit_replace_one_byte_with_ebmlvoid_all_surrounding_elements_coded_size T_661av1_in_matroska_by_mkvmerge_28_0_0:66667a5dd1a2cafb3aa37ce41aedfbf7:passed:20181021-225056:0.008359871 T_662cisco_talos_2018_0694:2bacc50e6e28fb2a92d2abf2583405f5:passed:20181025-215511:0.024040919 T_663mp4_wrong_atom_size_in_track_headers:10655836da56b23c7ba3605ae8de239f:passed:20181115-195646:0.046712794 +T_664generate_chapters_interval_first_video_timestamp_bigger_than_0:0d0a9cb9f4217a364d401fb4000fdba4-true:passed:20181116-094425:0.14444877 diff --git a/tests/run.rb b/tests/run.rb index 3cc2a8de9..12af8dd4d 100755 --- a/tests/run.rb +++ b/tests/run.rb @@ -4,6 +4,7 @@ require "digest/md5" require "json" require "json_schema" require "pp" +require "rexml/document" require "tempfile" require_relative "test.d/controller.rb" diff --git a/tests/test-664generate_chapters_interval_first_video_timestamp_bigger_than_0.rb b/tests/test-664generate_chapters_interval_first_video_timestamp_bigger_than_0.rb new file mode 100755 index 000000000..ae739a540 --- /dev/null +++ b/tests/test-664generate_chapters_interval_first_video_timestamp_bigger_than_0.rb @@ -0,0 +1,15 @@ +#!/usr/bin/ruby -w + +# T_664generate_chapters_interval_first_video_timestamp_bigger_than_0 +describe "mkvmerge / generating chapters in intervals with video tracks which start at timestamp > 0" +test_merge "data/mkv/minimum_video_timestamp_bigger_than_0.mkv", :args => "--generate-chapters interval:60s", :keep_tmp => true + +test "start timestamps" do + extract tmp, :mode => :chapters, :args => "#{tmp}.chapters" + + expected = ["00:00:00.000000000", "00:01:00.000000000", "00:02:00.000000000", "00:03:00.000000000"] + document = REXML::Document.new(File.new("#{tmp}.chapters")) + actual = REXML::XPath.match(document, "/Chapters/EditionEntry/ChapterAtom/ChapterTimeStart").map(&:text) + + expected == actual +end