diff --git a/NEWS.md b/NEWS.md index 340b72c0d..09132d757 100644 --- a/NEWS.md +++ b/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. diff --git a/src/input/r_ogm.cpp b/src/input/r_ogm.cpp index feace7d96..bdc0091b2 100644 --- a/src/input/r_ogm.cpp +++ b/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; diff --git a/tests/results.txt b/tests/results.txt index 251c93ee0..e310081e7 100644 --- a/tests/results.txt +++ b/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 diff --git a/tests/test-646ogg_opus_gap_page_missing.rb b/tests/test-646ogg_opus_gap_page_missing.rb new file mode 100755 index 000000000..6be48aca9 --- /dev/null +++ b/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"