Ogg Opus: calculate timestamps after gaps from page-after-gap's granulepos

Part of #2280.

NEWS.md

@@ -60,6 +60,9 @@
     calculated from the bitstream is one sample or less and if the packet
     isn't the last one in the stream. This circumvents certain rounding
     errors.
+  * The timestamp of the first packet after a gap in the middle of the stream
+    is now calculated based on the Ogg page the packet belongs to, and not
+    based on the timestamps before the gap.
 
   Fixes #2280.
 

src/input/r_ogm.cpp

@@ -1198,8 +1198,16 @@ ogm_a_opus_demuxer_c::process_page(int64_t granulepos) {
   auto eos_here           = false;
   auto gap_here           = false;
 
+  while (true) {
+    auto result = ogg_stream_packetout(&os, &op);
+    if (result == 0)
+      break;
+
+    if (result == -1) {
+      gap_here = true;
+      continue;
+    }
 
-  while (ogg_stream_packetout(&os, &op) == 1) {
     if (op.e_o_s) {
       eos      = 1;
       eos_here = true;
@@ -1227,11 +1235,17 @@ ogm_a_opus_demuxer_c::process_page(int64_t granulepos) {
   if (packets.empty())
     return;
 
-  if (!eos_here && is_first_packet) {
-    // First page in Ogg stream. Streams must not start at 0. If they
-    // don't, this is signaled by the page containing less samples
-    // than the difference between the page's granulepos-indicated end
-    // timestamp and 0.
+  if (!eos_here && (is_first_packet || gap_here)) {
+    // Two possible situations that require calculating the start
+    // timestamps backwards from the granule position:
+
+    // 1. First page in Ogg stream. Streams must not start at 0. If
+    //    they don't, this is signaled by the page containing less
+    //    samples than the difference between the page's
+    //    granulepos-indicated end timestamp and 0.
+
+    // 2. If there was a gap, the calculated prior end timestamp is
+    //    the one before the gap and must therefore not be used.
     auto current_timestamp = page_end_timestamp;
     auto idx               = packets.size();
 
@@ -1242,7 +1256,13 @@ ogm_a_opus_demuxer_c::process_page(int64_t granulepos) {
       packets[idx].first->timestamp  = (current_timestamp + m_timestamp_shift).to_ns();
     }
 
-    if (current_timestamp != timestamp_c::ns(0)) {
+
+    if (gap_here) {
+      mxdebug_if(ms_debug,
+                 boost::format("Opus packet after gap; Ogg page end timestamp %1% page duration %2% first timestamp %3%\n")
+                 % page_end_timestamp % page_duration % format_timestamp(packets.front().first->timestamp));
+
+    } else if (current_timestamp != timestamp_c::ns(0)) {
       if (current_timestamp < timestamp_c::ns(0)) {
         m_timestamp_shift = current_timestamp.negate();
         for (auto const &pair : packets)
@@ -1256,7 +1276,10 @@ ogm_a_opus_demuxer_c::process_page(int64_t granulepos) {
 
   } else {
     // Calculate discard padding based on previous end timestamp and
-    // number of samples in this page.
+    // number of samples in this page. This is done so that streams
+    // that aren't strictly spec-compliant (gaps in the middle =
+    // discard padding present not just on the very last packet) can
+    // be handled gracefully, too.
 
     auto current_timestamp = m_previous_page_end_timestamp;
 

tests/results.txt

@@ -491,3 +491,4 @@ T_642avc_es_clear_internal_buffers_after_reading_headers:0aaf44ccf543a6a739fd79a
 T_643mpeg_ts_bad_utf8_in_service_names:f967b2bf3fb4265ec723f14eb667bb9a:passed:20180615-172956:0.019394331
 T_644mp3_with_discard_padding:e68074afaedbb820210e31e3cf62febc:passed:20180619-214051:0.038730751
 T_645ogg_opus_first_timestamp_negative:4b513ca85e1cf12b378eedd07c8b4ffa-78778f6067b903cc326bf0f998f9790b:passed:20180620-185732:0.925863056
+T_646ogg_opus_gap_page_missing:c3c8ff65984de89ce37c2f03520ae83f:passed:20180621-154830:0.009743698

tests/test-646ogg_opus_gap_page_missing.rb

@@ -0,0 +1,5 @@
+#!/usr/bin/ruby -w
+
+# T_646ogg_opus_gap_page_missing
+describe "mkvmerge / Ogg Opus gap in stream due to missing page"
+test_merge "data/opus/gap-page-missing.opus"