Fix hanging/overlapping dvb subtitles (#2792).

This reverts a change made in
8ca6fd6d97e6d42492afddb655fa85130946853c
"Fix doubled subtitles if subtitle stop times are specified."

That change breaks the case where a subtitle _does_ have a stop time,
but it's wrong (30s from the start time) and we want the next subtitle
to clear the previous one.

I can't now see how reverting this could cause doubled subtitles,
so maybe that problem wlil come back.  At least now there's a test
for #2792.

diff --git a/src/lib/ffmpeg_decoder.cc b/src/lib/ffmpeg_decoder.cc
index 7f7a078638b540ba81bbbe38c5ad260970232fa0..6130d8e5fb5c59be98c6aec9c6ed7b78e23eeac1 100644 (file)
--- a/src/lib/ffmpeg_decoder.cc
+++ b/src/lib/ffmpeg_decoder.cc
@@ -668,11 +668,11 @@ FFmpegDecoder::decode_and_process_subtitle_packet (AVPacket* packet)
        */
        ContentTime from;
        from = sub_period.from + _pts_offset;
+       _have_current_subtitle = true;
        if (sub_period.to) {
                _current_subtitle_to = *sub_period.to + _pts_offset;
        } else {
                _current_subtitle_to = optional<ContentTime>();
-               _have_current_subtitle = true;
        }
 
        ContentBitmapText bitmap_text(from);

diff --git a/test/subtitle_timing_test.cc b/test/subtitle_timing_test.cc
index 79cc68849499531dfa339e63bfeacd4f16724b9a..8bb18f304c2d77ea2b2702d2077b44f3ee239e8e 100644 (file)
--- a/test/subtitle_timing_test.cc
+++ b/test/subtitle_timing_test.cc
@@ -21,8 +21,13 @@
 
 #include "lib/content.h"
 #include "lib/content_factory.h"
+#include "lib/content_text.h"
+#include "lib/dcpomatic_time.h"
 #include "lib/film.h"
+#include "lib/ffmpeg_content.h"
+#include "lib/ffmpeg_decoder.h"
 #include "lib/text_content.h"
+#include "lib/text_decoder.h"
 #include "lib/video_content.h"
 #include "test.h"
 #include <dcp/cpl.h>
@@ -33,6 +38,9 @@
 #include <iostream>
 
 
+using std::dynamic_pointer_cast;
+
+
 BOOST_AUTO_TEST_CASE (test_subtitle_timing_with_frame_rate_change)
 {
        Cleanup cl;
@@ -72,3 +80,66 @@ BOOST_AUTO_TEST_CASE (test_subtitle_timing_with_frame_rate_change)
        cl.run();
 }
 
+
+BOOST_AUTO_TEST_CASE(dvb_subtitles_replace_the_last)
+{
+       /* roh.mkv contains subtitles that come out of FFmpeg with incorrect stop times (30s
+        * after the start, which seems to be some kind of DVB "standard" timeout).
+        * Between actual subtitles it contains blanks that are apparently supposed to clear
+        * the previous subtitle.  Make sure that happens.
+        */
+       auto content = content_factory(TestPaths::private_data() / "roh.mkv");
+       BOOST_REQUIRE(!content.empty());
+       auto film = new_test_film2("dvb_subtitles_replace_the_last", { content[0] });
+
+       FFmpegDecoder decoder(film, dynamic_pointer_cast<FFmpegContent>(content[0]), false);
+       BOOST_REQUIRE(!decoder.text.empty());
+
+       struct Event {
+               std::string type;
+               dcpomatic::ContentTime time;
+
+               bool operator==(Event const& other) const {
+                       return type == other.type && time == other.time;
+               }
+       };
+
+       std::vector<Event> events;
+
+       auto start = [&events](ContentBitmapText text) {
+               events.push_back({"start", text.from()});
+       };
+
+       auto stop = [&events](dcpomatic::ContentTime time) {
+               if (!events.empty() && events.back().type == "stop") {
+                       /* We'll get a bad (too-late) stop time, then the correct one
+                        * when the "clearing" subtitle arrives.
+                        */
+                       events.pop_back();
+               }
+               events.push_back({"stop", time});
+       };
+
+       decoder.text.front()->BitmapStart.connect(start);
+       decoder.text.front()->Stop.connect(stop);
+
+       while (!decoder.pass()) {}
+
+       using dcpomatic::ContentTime;
+
+       std::vector<Event> correct = {
+               { "start", ContentTime(439872) },  // 4.582000s     actual subtitle #1
+               { "stop",  ContentTime(998400) },  // 10.400000s    stop caused by incoming blank
+               { "start", ContentTime(998400) },  // 10.400000s    blank
+               { "stop",  ContentTime(1141248) }, // 11.888000s    stop caused by incoming subtitle #2
+               { "start", ContentTime(1141248) }, // 11.888000s    subtitle #2
+               { "stop",  ContentTime(1455936) }, // 15.166000s    ...
+               { "start", ContentTime(1455936) }, // 15.166000s
+               { "stop",  ContentTime(1626816) }, // 16.946000s
+               { "start", ContentTime(1626816) }, // 16.946000s
+       };
+
+       BOOST_REQUIRE(events.size() > correct.size());
+       BOOST_CHECK(std::vector<Event>(events.begin(), events.begin() + correct.size()) == correct);
+}
+