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

@@ -1,3 +1,8 @@
+2016-10-09  Moritz Bunkus  <moritz@bunkus.org>
+
+        * 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  <moritz@bunkus.org>
 
         * MKVToolNix GUI: merge tool enhancement: added menu entries that

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 <moritz@bunkus.org>.
+*/
+
+#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<void()> 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<unsigned short *>(m->decode_buffer.get_buffer()), len_16 / 2, reinterpret_cast<unsigned short *>(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());
+}
+
+}}

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 <moritz@bunkus.org>.
+*/
+
+#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<impl_t> 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<void()> const &worker);
+};
+
+}}
+
+#endif // MTX_COMMON_DTS_PARSER_H

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:

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 <moritz@bunkus.org>.
+*/
+
+#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

src/input/r_wav.cpp

@@ -16,13 +16,12 @@
 #include "common/common_pch.h"
 
 #include <algorithm>
-#if defined(HAVE_UNISTD_H)
-# include <unistd.h>
-#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<dts_packetizer_c *>(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));
 }
 
 // ----------------------------------------------------------

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

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"