From 8b3c3ab14639f81a5020d2bcb2afc26d1f4cce1e Mon Sep 17 00:00:00 2001 From: Moritz Bunkus Date: Sun, 27 Jul 2014 12:47:25 +0200 Subject: [PATCH] common: fix seek relative end of file offset handling fseeko() etc. require negative offsets if seeking relative to the end of the file should result in the file pointer being inside the file. Therefore MKVToolNix' APIs require that, too, but some calculations treated it as if it were the other war around. Fixes #1035. --- ChangeLog | 3 +++ src/common/mm_io.cpp | 9 ++------- src/common/mm_multi_file_io.cpp | 2 +- src/common/mm_read_buffer_io.cpp | 7 ++----- src/common/mm_write_buffer_io.cpp | 2 +- tests/results.txt | 1 + tests/test-432concatenate_two_ac3_files.rb | 5 +++++ 7 files changed, 15 insertions(+), 14 deletions(-) create mode 100755 tests/test-432concatenate_two_ac3_files.rb diff --git a/ChangeLog b/ChangeLog index b5f5992c2..6aabd5363 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,8 @@ 2014-07-27 Moritz Bunkus + * all: bug fix: Fixed file seeking code for »seek relative to end + of file« case. Fixes #1035. + * mmg: bug fix: Selecting the root of the chapter editor tree will disable the language/country inputs properly as changing those fields doesn't make sense for the root. diff --git a/src/common/mm_io.cpp b/src/common/mm_io.cpp index 1829ddbf5..16ac059ce 100644 --- a/src/common/mm_io.cpp +++ b/src/common/mm_io.cpp @@ -87,12 +87,7 @@ mm_file_io_c::setFilePointer(int64 offset, if (fseeko((FILE *)m_file, offset, whence) != 0) throw mtx::mm_io::seek_x{mtx::mm_io::make_error_code()}; - if (mode == seek_beginning) - m_current_position = offset; - else if (mode == seek_end) - m_current_position = ftello((FILE *)m_file); - else - m_current_position += offset; + m_current_position = ftello((FILE *)m_file); } size_t @@ -765,7 +760,7 @@ mm_mem_io_c::setFilePointer(int64 offset, int64_t new_pos = seek_beginning == mode ? offset - : seek_end == mode ? m_mem_size - offset + : seek_end == mode ? m_mem_size + offset // offsets from the end are negative already : m_pos + offset; if ((0 <= new_pos) && (static_cast(m_mem_size) >= new_pos)) diff --git a/src/common/mm_multi_file_io.cpp b/src/common/mm_multi_file_io.cpp index bda0619ca..c4a91d683 100644 --- a/src/common/mm_multi_file_io.cpp +++ b/src/common/mm_multi_file_io.cpp @@ -61,7 +61,7 @@ mm_multi_file_io_c::setFilePointer(int64 offset, seek_mode mode) { int64_t new_pos = seek_beginning == mode ? offset - : seek_end == mode ? m_total_size - offset + : seek_end == mode ? m_total_size + offset // offsets from the end are negative already : m_current_pos + offset; if ((0 > new_pos) || (static_cast(m_total_size) < new_pos)) diff --git a/src/common/mm_read_buffer_io.cpp b/src/common/mm_read_buffer_io.cpp index 43e9908fe..aad7b7386 100644 --- a/src/common/mm_read_buffer_io.cpp +++ b/src/common/mm_read_buffer_io.cpp @@ -70,7 +70,7 @@ mm_read_buffer_io_c::setFilePointer(int64 offset, break; case seek_end: - new_pos = -1; + new_pos = static_cast(get_size()) + offset; // offsets from the end are negative already break; default: @@ -87,10 +87,7 @@ mm_read_buffer_io_c::setFilePointer(int64 offset, int64_t previous_pos = m_proxy_io->getFilePointer(); // Actual seeking - if (new_pos < 0) - m_proxy_io->setFilePointer(offset, seek_end); - else - m_proxy_io->setFilePointer(std::min(new_pos, get_size()), seek_beginning); + m_proxy_io->setFilePointer(std::min(new_pos, get_size()), seek_beginning); // Get the actual offset from the underlying stream // Better be safe than sorry and use this instead of just taking diff --git a/src/common/mm_write_buffer_io.cpp b/src/common/mm_write_buffer_io.cpp index ced9d4a34..5b2ab3c93 100644 --- a/src/common/mm_write_buffer_io.cpp +++ b/src/common/mm_write_buffer_io.cpp @@ -50,7 +50,7 @@ mm_write_buffer_io_c::setFilePointer(int64 offset, seek_mode mode) { int64_t new_pos = seek_beginning == mode ? offset - : seek_end == mode ? m_proxy_io->get_size() - offset + : seek_end == mode ? m_proxy_io->get_size() + offset // offsets from the end are negative already : getFilePointer() + offset; if (new_pos == static_cast(getFilePointer())) diff --git a/tests/results.txt b/tests/results.txt index 5d2f521c8..3c70fe193 100644 --- a/tests/results.txt +++ b/tests/results.txt @@ -277,3 +277,4 @@ T_428mkv_misdetected_as_ass:e93bf556dd2814f52c44e523ae5b8721:passed:20140518-155 T_429track_statistics_tags:022578a22c45c06ab23dc453df71f7c0-7601976ec2210efb7867cb5067acd319-130e8f42158c1d0fa925724927ff84c5-bbda48963dde87a008a3c11bcdc8421b:passed:20140524-194544:0.635343822 T_430cues_multiple_blocks_same_timecode:f1ab5c927064537eb59ab0f5195d6a1d:passed:20140525-173642:0.033316759 T_431ssa_comments_exclamation_mark:3caa9ad1716134cc1f3e229b88ff94ea:passed:20140618-232324:0.072735677 +T_432concatenate_two_ac3_files:4f4b7c58d4557d52849866e1a3ad1b05:passed:20140727-124637:0.15390456 diff --git a/tests/test-432concatenate_two_ac3_files.rb b/tests/test-432concatenate_two_ac3_files.rb new file mode 100755 index 000000000..c86bcf5d7 --- /dev/null +++ b/tests/test-432concatenate_two_ac3_files.rb @@ -0,0 +1,5 @@ +#!/usr/bin/ruby -w + +# T_432concatenate_two_ac3_files +describe "mkvmerge / read from same file twice via concatenation" +test_merge "'(' data/ac3/misdetected_as_mp2.ac3 data/ac3/misdetected_as_mp2.ac3 ')'"