From b155c85aaf69f450931c525c4f4b28a16835861a Mon Sep 17 00:00:00 2001 From: Moritz Bunkus Date: Sun, 9 Oct 2016 20:30:36 +0200 Subject: [PATCH] WAV reader: require 5 consecutive DTS headers during byte swap/14-in-16 detection The original code tried all four possible combinations of byte swapping yes/no and 14-in-16 yes/no and aborted as soon as a single DTS header was found. This can be a false positive, though, and with one test file provided by a user this has indeed been the case. The new code does the same detection but requires five consecutive DTS headers to be found before the detection is deemed to be successful. --- ChangeLog | 5 + src/common/dts_parser.cpp | 153 ++++++++++++++++++ src/common/dts_parser.h | 50 ++++++ src/common/private/Rakefile | 7 + src/common/private/dts_parser.h | 39 +++++ src/input/r_wav.cpp | 84 +++------- tests/results.txt | 1 + ...-568dts_wav_14_in_16_detection_failures.rb | 7 + 8 files changed, 285 insertions(+), 61 deletions(-) create mode 100644 src/common/dts_parser.cpp create mode 100644 src/common/dts_parser.h create mode 100644 src/common/private/Rakefile create mode 100644 src/common/private/dts_parser.h create mode 100755 tests/test-568dts_wav_14_in_16_detection_failures.rb diff --git a/ChangeLog b/ChangeLog index b5aa265b7..f988b7f33 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,8 @@ +2016-10-09 Moritz Bunkus + + * mkvmerge: WAV reader bug fix: fixed detection and merging of DTS + in WAV that uses the 14-bytes-in-16-bytes packing method. + 2016-10-08 Moritz Bunkus * MKVToolNix GUI: merge tool enhancement: added menu entries that diff --git a/src/common/dts_parser.cpp b/src/common/dts_parser.cpp new file mode 100644 index 000000000..1f31e28a4 --- /dev/null +++ b/src/common/dts_parser.cpp @@ -0,0 +1,153 @@ +/* + mkvmerge -- utility for splicing together matroska files + from component media subtypes + + Distributed under the GPL v2 + see the file COPYING for details + or visit http://www.gnu.org/copyleft/gpl.html + + DTS decoder & parser + + Written by Moritz Bunkus . +*/ + +#include "common/common_pch.h" + +#include "common/bswap.h" +#include "common/private/dts_parser.h" + +namespace mtx { namespace dts { + +parser_c::impl_t::~impl_t() { +} + +void +parser_c::impl_t::reset() { + swap_bytes = false; + pack_14_16 = false; + first_header = header_t{}; + + decode_buffer.clear(); + swap_remainder.clear(); + pack_remainder.clear(); +} + +// ---------------------------------------------------------------------- + +parser_c::parser_c() + : m{new parser_c::impl_t{}} +{ + m->reset(); +} + +parser_c::~parser_c() { +} + +void +parser_c::reset() { + m->reset(); +} + +bool +parser_c::detect(unsigned char const *buffer, + std::size_t size, + std::size_t num_required_headers) { + for (auto swap_bytes = 0; swap_bytes < 2; ++swap_bytes) { + for (auto pack_14_16 = 0; pack_14_16 < 2; ++pack_14_16) { + reset(); + + m->decode_buffer.add(buffer, size); + + m->swap_bytes = !!swap_bytes; + m->pack_14_16 = !!pack_14_16; + + decode_buffer(); + + auto offset = mtx::dts::find_consecutive_headers(m->decode_buffer.get_buffer(), m->decode_buffer.get_size(), num_required_headers); + + mxdebug_if(m->debug, boost::format("dts::parser::detect: in: size %1% #req headers %2%; result: swap %3% 14/16 %4% offset %5%\n") % size % num_required_headers % m->swap_bytes % m->pack_14_16 % offset); + + if (offset < 0) + continue; + + if (0 > find_header(m->decode_buffer.get_buffer() + offset, m->decode_buffer.get_size() - offset, m->first_header)) + continue; + + m->decode_buffer.clear(); + m->swap_remainder.clear(); + m->pack_remainder.clear(); + + return true; + } + } + + reset(); + + return false; +} + +bool +parser_c::detect(memory_c &buffer, + std::size_t num_required_headers) { + return detect(buffer.get_buffer(), buffer.get_size(), num_required_headers); +} + +header_t +parser_c::get_first_header() + const { + return m->first_header; +} + +void +parser_c::decode_step(byte_buffer_c &remainder_buffer, + std::size_t multiples_of, + std::function const &worker) { + if (remainder_buffer.get_size()) { + m->decode_buffer.prepend(remainder_buffer.get_buffer(), remainder_buffer.get_size()); + remainder_buffer.clear(); + } + + auto remaining_bytes = m->decode_buffer.get_size() % multiples_of; + if (remaining_bytes) { + remainder_buffer.add(m->decode_buffer.get_buffer() + m->decode_buffer.get_size() - remaining_bytes, remaining_bytes); + m->decode_buffer.remove(remaining_bytes, byte_buffer_c::at_back); + } + + worker(); +} + +void +parser_c::decode_buffer() { + if (m->swap_bytes) + decode_step(m->swap_remainder, 2, [this]() { + mtx::bswap_buffer(m->decode_buffer.get_buffer(), m->decode_buffer.get_buffer(), m->decode_buffer.get_size(), 2); + }); + + if (m->pack_14_16) + decode_step(m->pack_remainder, 16, [this]() { + auto len_16 = m->decode_buffer.get_size(); + auto len_14 = len_16 * 14 / 16; + + convert_14_to_16_bits(reinterpret_cast(m->decode_buffer.get_buffer()), len_16 / 2, reinterpret_cast(m->decode_buffer.get_buffer())); + m->decode_buffer.remove(len_16 - len_14, byte_buffer_c::at_back); + }); +} + +memory_cptr +parser_c::decode(unsigned char const *buffer, + std::size_t size) { + m->decode_buffer.add(buffer, size); + decode_buffer(); + + auto decoded = memory_c::clone(m->decode_buffer.get_buffer(), m->decode_buffer.get_size()); + m->decode_buffer.clear(); + + return decoded; +} + +memory_cptr +parser_c::decode(memory_c &buffer) { + return decode(buffer.get_buffer(), buffer.get_size()); +} + +}} diff --git a/src/common/dts_parser.h b/src/common/dts_parser.h new file mode 100644 index 000000000..99569d9d4 --- /dev/null +++ b/src/common/dts_parser.h @@ -0,0 +1,50 @@ +/* + mkvmerge -- utility for splicing together matroska files + from component media subtypes + + Distributed under the GPL v2 + see the file COPYING for details + or visit http://www.gnu.org/copyleft/gpl.html + + DTS decoder & parser + + Written by Moritz Bunkus . +*/ + +#ifndef MTX_COMMON_DTS_PARSER_H +#define MTX_COMMON_DTS_PARSER_H + +#include "common/common_pch.h" + +class byte_buffer_c; + +namespace mtx { namespace dts { + +struct header_t; + +class parser_c { +protected: + struct impl_t; + std::unique_ptr m; + +public: + parser_c(); + virtual ~parser_c(); + + bool detect(unsigned char const *buffer, std::size_t size, std::size_t num_required_headers); + bool detect(memory_c &buffer, std::size_t num_required_headers); + header_t get_first_header() const; + + memory_cptr decode(unsigned char const *buffer, std::size_t size); + memory_cptr decode(memory_c &buffer); + + void reset(); + +protected: + void decode_buffer(); + void decode_step(byte_buffer_c &remainder_buffer, std::size_t multiples_of, std::function const &worker); +}; + +}} + +#endif // MTX_COMMON_DTS_PARSER_H diff --git a/src/common/private/Rakefile b/src/common/private/Rakefile new file mode 100644 index 000000000..387a4063e --- /dev/null +++ b/src/common/private/Rakefile @@ -0,0 +1,7 @@ +#!/usr/bin/env ruby + +import ['..', '../..', '../../..'].collect { |subdir| FileList[File.dirname(__FILE__) + "/#{subdir}/build-config.in"].to_a }.flatten.compact.first.gsub(/build-config.in/, 'Rakefile') + +# Local Variables: +# mode: ruby +# End: diff --git a/src/common/private/dts_parser.h b/src/common/private/dts_parser.h new file mode 100644 index 000000000..cce9cdaa9 --- /dev/null +++ b/src/common/private/dts_parser.h @@ -0,0 +1,39 @@ +/* + mkvmerge -- utility for splicing together matroska files + from component media subtypes + + Distributed under the GPL v2 + see the file COPYING for details + or visit http://www.gnu.org/copyleft/gpl.html + + DTS decoder & parser (private data) + + Written by Moritz Bunkus . +*/ + +#ifndef MTX_COMMON_PRIVATE_DTS_PARSER_H +#define MTX_COMMON_PRIVATE_DTS_PARSER_H + +#include "common/byte_buffer.h" +#include "common/dts.h" +#include "common/dts_parser.h" + +namespace mtx { namespace dts { + +struct parser_c::impl_t { +public: + debugging_option_c debug{"dts_parser"}; + + bool swap_bytes{}, pack_14_16{}; + byte_buffer_c decode_buffer, swap_remainder, pack_remainder; + header_t first_header; + +public: + virtual ~impl_t(); + + void reset(); +}; + +}} + +#endif // MTX_COMMON_PRIVATE_DTS_PARSER_H diff --git a/src/input/r_wav.cpp b/src/input/r_wav.cpp index 2b7371e0d..3562fdf6f 100644 --- a/src/input/r_wav.cpp +++ b/src/input/r_wav.cpp @@ -16,13 +16,12 @@ #include "common/common_pch.h" #include -#if defined(HAVE_UNISTD_H) -# include -#endif // HAVE_UNISTD_H #include "avilib.h" #include "common/ac3.h" +#include "common/bswap.h" #include "common/dts.h" +#include "common/dts_parser.h" #include "common/endian.h" #include "common/error.h" #include "common/id_info.h" @@ -113,10 +112,8 @@ protected: class wav_dts_demuxer_c: public wav_demuxer_c { private: - bool m_swap_bytes, m_pack_14_16; - mtx::dts::header_t m_dtsheader; - memory_cptr m_buf[2]; - int m_cur_buf; + mtx::dts::parser_c m_parser; + memory_cptr m_read_buffer; public: wav_dts_demuxer_c(wav_reader_c *reader, wave_header *wheader); @@ -130,14 +127,11 @@ public: }; virtual unsigned char *get_buffer() { - return m_buf[m_cur_buf]->get_buffer(); - }; + return m_read_buffer->get_buffer(); + } virtual void process(int64_t len); virtual generic_packetizer_c *create_packetizer(); - -private: - virtual int decode_buffer(int len); }; class wav_pcm_demuxer_c: public wav_demuxer_c { @@ -208,7 +202,7 @@ wav_ac3acm_demuxer_c::probe(mm_io_cptr &io) { int wav_ac3acm_demuxer_c::decode_buffer(int len) { if ((2 < len) && m_swap_bytes) { - swab((char *)m_buf[m_cur_buf]->get_buffer(), (char *)m_buf[m_cur_buf ^ 1]->get_buffer(), len); + mtx::bswap_buffer(m_buf[m_cur_buf]->get_buffer(), m_buf[m_cur_buf ^ 1]->get_buffer(), len, 2); m_cur_buf ^= 1; } @@ -262,8 +256,8 @@ wav_ac3wav_demuxer_c::decode_buffer(int len) { return -1; if (m_swap_bytes) { - memcpy( m_buf[m_cur_buf ^ 1]->get_buffer(), m_buf[m_cur_buf]->get_buffer(), 8); - swab((char *)m_buf[m_cur_buf]->get_buffer() + 8, (char *)m_buf[m_cur_buf ^ 1]->get_buffer() + 8, len - 8); + memcpy( m_buf[m_cur_buf ^ 1]->get_buffer(), m_buf[m_cur_buf]->get_buffer(), 8); + mtx::bswap_buffer(m_buf[m_cur_buf]->get_buffer() + 8, m_buf[m_cur_buf ^ 1]->get_buffer() + 8, len - 8, 2); m_cur_buf ^= 1; } @@ -296,16 +290,11 @@ wav_ac3wav_demuxer_c::process(int64_t size) { // ---------------------------------------------------------- wav_dts_demuxer_c::wav_dts_demuxer_c(wav_reader_c *reader, - wave_header *wheader): - wav_demuxer_c(reader, wheader), - m_swap_bytes(false), - m_pack_14_16(false), - m_cur_buf(0) { - - m_buf[0] = memory_c::alloc(DTS_READ_SIZE); - m_buf[1] = memory_c::alloc(DTS_READ_SIZE); - - m_codec = codec_c::look_up(codec_c::type_e::A_DTS); + wave_header *wheader) + : wav_demuxer_c{reader, wheader} + , m_read_buffer{memory_c::alloc(DTS_READ_SIZE)} +{ + m_codec = codec_c::look_up(codec_c::type_e::A_DTS); } wav_dts_demuxer_c::~wav_dts_demuxer_c() { @@ -314,54 +303,27 @@ wav_dts_demuxer_c::~wav_dts_demuxer_c() { bool wav_dts_demuxer_c::probe(mm_io_cptr &io) { io->save_pos(); - int len = io->read(m_buf[m_cur_buf]->get_buffer(), DTS_READ_SIZE); + auto read_buf = memory_c::alloc(DTS_READ_SIZE); + read_buf->set_size(io->read(read_buf, DTS_READ_SIZE)); io->restore_pos(); - if (mtx::dts::detect(m_buf[m_cur_buf]->get_buffer(), len, m_pack_14_16, m_swap_bytes)) { - len = decode_buffer(len); - int pos = mtx::dts::find_consecutive_headers(m_buf[m_cur_buf]->get_buffer(), len, 5); - if (0 <= pos) { - if (0 > mtx::dts::find_header(m_buf[m_cur_buf]->get_buffer() + pos, len - pos, m_dtsheader)) - return false; + if (!m_parser.detect(*read_buf, 5)) + return false; - m_codec.set_specialization(m_dtsheader.get_codec_specialization()); + m_codec.set_specialization(m_parser.get_first_header().get_codec_specialization()); - mxverb(3, boost::format("DTSinWAV: 14->16 %1% swap %2%\n") % m_pack_14_16 % m_swap_bytes); - return true; - } - } - - return false; -} - -int -wav_dts_demuxer_c::decode_buffer(int len) { - if (m_swap_bytes) { - swab((char *)m_buf[m_cur_buf]->get_buffer(), (char *)m_buf[m_cur_buf ^ 1]->get_buffer(), len); - m_cur_buf ^= 1; - } - - if (m_pack_14_16) { - mtx::dts::convert_14_to_16_bits((unsigned short *)m_buf[m_cur_buf]->get_buffer(), len / 2, (unsigned short *)m_buf[m_cur_buf ^ 1]->get_buffer()); - m_cur_buf ^= 1; - len = len * 7 / 8; - } - - return len; + return true; } generic_packetizer_c * wav_dts_demuxer_c::create_packetizer() { - m_ptzr = new dts_packetizer_c(m_reader, m_ti, m_dtsheader); + m_ptzr = new dts_packetizer_c(m_reader, m_ti, m_parser.get_first_header()); // .wav with DTS are always filled up with other stuff to match the bitrate. static_cast(m_ptzr)->set_skipping_is_normal(true); show_packetizer_info(0, m_ptzr); - if (1 < verbose) - m_dtsheader.print(); - return m_ptzr; } @@ -370,8 +332,8 @@ wav_dts_demuxer_c::process(int64_t size) { if (0 >= size) return; - long dec_len = decode_buffer(size); - m_ptzr->process(new packet_t(new memory_c(m_buf[m_cur_buf]->get_buffer(), dec_len, false))); + auto decoded = m_parser.decode(m_read_buffer->get_buffer(), size); + m_ptzr->process(new packet_t(decoded)); } // ---------------------------------------------------------- diff --git a/tests/results.txt b/tests/results.txt index fc10f5b45..01ba3daf2 100644 --- a/tests/results.txt +++ b/tests/results.txt @@ -413,3 +413,4 @@ T_564segfaults_issue_1780_part_7:error-error:passed:20160907-203352:0.042010598 T_565hevc_detection_failed_due_to_width_height:7485c50c93d36712ac137a4c18266828-fb4f943bf719c5c5cae72a4ee8d24591:passed:20160911-140155:1.81172957 T_566opus_with_comment_header:25c1c6085b5514475656c9ab41cb6f05:passed:20160930-100110:0.020105268 T_567propedit_muxing_writing_application:fdfebfa48bbd5fc21088827b0ad8f616-chunky bacon//a4e5c2807f579c56bbe0f1bb3c2626da:passed:20161006-212023:0.033121037 +T_568dts_wav_14_in_16_detection_failures:a0c91e14fd50a73a6865ae097b219268-6c485c163b240e5794706a184b969bda:passed:20161009-202725:0.202166541 diff --git a/tests/test-568dts_wav_14_in_16_detection_failures.rb b/tests/test-568dts_wav_14_in_16_detection_failures.rb new file mode 100755 index 000000000..0827bb970 --- /dev/null +++ b/tests/test-568dts_wav_14_in_16_detection_failures.rb @@ -0,0 +1,7 @@ +#!/usr/bin/ruby -w + +# T_568dts_wav_14_in_16_detection_failures +describe "mkvmerge / DTS in WAV, failure in proper detection of byte swapping/14-in-16 packing" + +test_merge "data/wav/05-labyrinth-path-13.wav" +test_merge "data/wav/06-labyrinth-path-14.wav"