Subtitle "to" times used to be stored against their "from" times.

Sadly an example shows that from times are not unique.  This patch
uses a hash of stuff from the first AVSubtitle as the key.

diff --git a/src/lib/ffmpeg.cc b/src/lib/ffmpeg.cc
index 6103ccd50ed98863afeb31f6087cddb251226765..df708d923d278d21d2aaab71c97e7263ef626a62 100644 (file)
--- a/src/lib/ffmpeg.cc
+++ b/src/lib/ffmpeg.cc
@@ -26,6 +26,7 @@
 #include "log.h"
 #include "ffmpeg_subtitle_stream.h"
 #include "ffmpeg_audio_stream.h"
+#include "md5_digester.h"
 #include "compose.hpp"
 extern "C" {
 #include <libavcodec/avcodec.h>
@@ -268,6 +269,27 @@ FFmpeg::subtitle_period (AVSubtitle const & sub)
                );
 }
 
+string
+FFmpeg::subtitle_id (AVSubtitle const & sub)
+{
+       MD5Digester digester;
+       digester.add (sub.start_display_time);
+       digester.add (sub.end_display_time);
+       digester.add (sub.pts);
+       for (unsigned int i = 0; i < sub.num_rects; ++i) {
+               AVSubtitleRect* rect = sub.rects[i];
+               digester.add (rect->x);
+               digester.add (rect->y);
+               digester.add (rect->w);
+               digester.add (rect->h);
+               int const line = rect->pict.linesize[0];
+               for (int j = 0; j < rect->h; ++j) {
+                       digester.add (rect->pict.data[0] + j * line, line);
+               }
+       }
+       return digester.get ();
+}
+
 /** Compute the pts offset to use given a set of audio streams and some video details.
  *  Sometimes these parameters will have just been determined by an Examiner, sometimes
  *  they will have been retrieved from a piece of Content, hence the need for this method

diff --git a/src/lib/ffmpeg.h b/src/lib/ffmpeg.h
index b3bc13e5cc222b9033472bfb43df8f3b70693fd7..0b195268a31cde69cca8fce82872d070390b1983 100644 (file)
--- a/src/lib/ffmpeg.h
+++ b/src/lib/ffmpeg.h
@@ -56,7 +56,8 @@ protected:
                std::vector<boost::shared_ptr<FFmpegAudioStream> > audio_streams, boost::optional<ContentTime> first_video, double video_frame_rate
                ) const;
 
-       static FFmpegSubtitlePeriod subtitle_period (AVSubtitle const &);
+       static FFmpegSubtitlePeriod subtitle_period (AVSubtitle const & sub);
+       static std::string subtitle_id (AVSubtitle const & sub);
 
        boost::shared_ptr<const FFmpegContent> _ffmpeg_content;
 

diff --git a/src/lib/ffmpeg_content.cc b/src/lib/ffmpeg_content.cc
index a317ec35fee91ad4d45c7668b9bca6e06e7dcda0..ba799be72f0aad6f84c852481da897a7c1104a76 100644 (file)
--- a/src/lib/ffmpeg_content.cc
+++ b/src/lib/ffmpeg_content.cc
@@ -75,7 +75,7 @@ FFmpegContent::FFmpegContent (shared_ptr<const Film> film, cxml::ConstNodePtr no
 {
        list<cxml::NodePtr> c = node->node_children ("SubtitleStream");
        for (list<cxml::NodePtr>::const_iterator i = c.begin(); i != c.end(); ++i) {
-               _subtitle_streams.push_back (shared_ptr<FFmpegSubtitleStream> (new FFmpegSubtitleStream (*i)));
+               _subtitle_streams.push_back (shared_ptr<FFmpegSubtitleStream> (new FFmpegSubtitleStream (*i, version)));
                if ((*i)->optional_number_child<int> ("Selected")) {
                        _subtitle_stream = _subtitle_streams.back ();
                }

diff --git a/src/lib/ffmpeg_decoder.cc b/src/lib/ffmpeg_decoder.cc
index c393067ce376fbe11fcd8266e8195dedb7f88b83..c25efa463c8f61121d70a5e6fcac98a88bb63211 100644 (file)
--- a/src/lib/ffmpeg_decoder.cc
+++ b/src/lib/ffmpeg_decoder.cc
@@ -1,5 +1,5 @@
 /*
-    Copyright (C) 2012-2015 Carl Hetherington <cth@carlh.net>
+    Copyright (C) 2012-2016 Carl Hetherington <cth@carlh.net>
 
     This program is free software; you can redistribute it and/or modify
     it under the terms of the GNU General Public License as published by
@@ -34,6 +34,7 @@
 #include "ffmpeg_content.h"
 #include "raw_image_proxy.h"
 #include "film.h"
+#include "md5_digester.h"
 #include "compose.hpp"
 extern "C" {
 #include <libavcodec/avcodec.h>
@@ -426,7 +427,7 @@ FFmpegDecoder::decode_subtitle_packet ()
                period.to = sub_period.to.get() + _pts_offset;
        } else {
                /* We have to look up the `to' time in the stream's records */
-               period.to = ffmpeg_content()->subtitle_stream()->find_subtitle_to (period.from);
+               period.to = ffmpeg_content()->subtitle_stream()->find_subtitle_to (subtitle_id (sub));
        }
 
        for (unsigned int i = 0; i < sub.num_rects; ++i) {

diff --git a/src/lib/ffmpeg_examiner.cc b/src/lib/ffmpeg_examiner.cc
index 719e7a2b0863e085fcb6a0308d73e345c8df1bcb..48738b91725da6cc156f901e6b3494c6d9177d10 100644 (file)
--- a/src/lib/ffmpeg_examiner.cc
+++ b/src/lib/ffmpeg_examiner.cc
@@ -142,11 +142,13 @@ FFmpegExaminer::FFmpegExaminer (shared_ptr<const FFmpegContent> c, shared_ptr<Jo
                }
        }
 
+       /* Finish off any hanging subtitles at the end */
        for (LastSubtitleMap::const_iterator i = _last_subtitle_start.begin(); i != _last_subtitle_start.end(); ++i) {
                if (i->second) {
                        i->first->add_subtitle (
+                               i->second->id,
                                ContentTimePeriod (
-                                       i->second.get (),
+                                       i->second->time,
                                        ContentTime::from_frames (video_length(), video_frame_rate().get_value_or (24))
                                        )
                                );
@@ -203,24 +205,25 @@ FFmpegExaminer::subtitle_packet (AVCodecContext* context, shared_ptr<FFmpegSubti
        int frame_finished;
        AVSubtitle sub;
        if (avcodec_decode_subtitle2 (context, &sub, &frame_finished, &_packet) >= 0 && frame_finished) {
+               string id = subtitle_id (sub);
                FFmpegSubtitlePeriod const period = subtitle_period (sub);
                LastSubtitleMap::iterator last = _last_subtitle_start.find (stream);
                if (last != _last_subtitle_start.end() && last->second) {
                        /* We have seen the start of a subtitle but not yet the end.  Whatever this is
                           finishes the previous subtitle, so add it */
-                       stream->add_subtitle (ContentTimePeriod (last->second.get (), period.from));
+                       stream->add_subtitle (last->second->id, ContentTimePeriod (last->second->time, period.from));
                        if (sub.num_rects == 0) {
                                /* This is a `proper' end-of-subtitle */
-                               _last_subtitle_start[stream] = optional<ContentTime> ();
+                               _last_subtitle_start[stream] = optional<SubtitleStart> ();
                        } else {
                                /* This is just another subtitle, so we start again */
-                               _last_subtitle_start[stream] = period.from;
+                               _last_subtitle_start[stream] = SubtitleStart (id, period.from);
                        }
                } else if (sub.num_rects == 1) {
                        if (period.to) {
-                               stream->add_subtitle (ContentTimePeriod (period.from, period.to.get ()));
+                               stream->add_subtitle (id, ContentTimePeriod (period.from, period.to.get ()));
                        } else {
-                               _last_subtitle_start[stream] = period.from;
+                               _last_subtitle_start[stream] = SubtitleStart (id, period.from);
                        }
                }
                avsubtitle_free (&sub);

diff --git a/src/lib/ffmpeg_examiner.h b/src/lib/ffmpeg_examiner.h
index 6e218512917813a5fe91ddbb7501c1b2ed3345a9..e87e11d1c7db29249e360963057a44a90c1aa647 100644 (file)
--- a/src/lib/ffmpeg_examiner.h
+++ b/src/lib/ffmpeg_examiner.h
@@ -86,6 +86,17 @@ private:
        Frame _video_length;
        bool _need_video_length;
 
-       typedef std::map<boost::shared_ptr<FFmpegSubtitleStream>, boost::optional<ContentTime> > LastSubtitleMap;
+       struct SubtitleStart
+       {
+               SubtitleStart (std::string id_, ContentTime time_)
+                       : id (id_)
+                       , time (time_)
+               {}
+
+               std::string id;
+               ContentTime time;
+       };
+
+       typedef std::map<boost::shared_ptr<FFmpegSubtitleStream>, boost::optional<SubtitleStart> > LastSubtitleMap;
        LastSubtitleMap _last_subtitle_start;
 };

diff --git a/src/lib/ffmpeg_subtitle_stream.cc b/src/lib/ffmpeg_subtitle_stream.cc
index e12075581b12705a71d53153904218a6950b1cce..466032b37235b4a48c744f42fdd7541cc9f77155 100644 (file)
--- a/src/lib/ffmpeg_subtitle_stream.cc
+++ b/src/lib/ffmpeg_subtitle_stream.cc
@@ -1,5 +1,5 @@
 /*
-    Copyright (C) 2013-2014 Carl Hetherington <cth@carlh.net>
+    Copyright (C) 2013-2016 Carl Hetherington <cth@carlh.net>
 
     This program is free software; you can redistribute it and/or modify
     it under the terms of the GNU General Public License as published by
@@ -32,16 +32,33 @@ using std::cout;
  *  @param t String returned from to_string().
  *  @param v State file version.
  */
-FFmpegSubtitleStream::FFmpegSubtitleStream (cxml::ConstNodePtr node)
+FFmpegSubtitleStream::FFmpegSubtitleStream (cxml::ConstNodePtr node, int version)
        : FFmpegStream (node)
 {
-       BOOST_FOREACH (cxml::NodePtr i, node->node_children ("Period")) {
-               add_subtitle (
-                       ContentTimePeriod (
-                               ContentTime (i->number_child<ContentTime::Type> ("From")),
-                               ContentTime (i->number_child<ContentTime::Type> ("To"))
-                               )
-                       );
+       if (version == 32) {
+               BOOST_FOREACH (cxml::NodePtr i, node->node_children ("Period")) {
+                       /* In version 32 we assumed that from times were unique, so they weer
+                          used as identifiers.
+                       */
+                       add_subtitle (
+                               raw_convert<string> (i->string_child ("From")),
+                               ContentTimePeriod (
+                                       ContentTime (i->number_child<ContentTime::Type> ("From")),
+                                       ContentTime (i->number_child<ContentTime::Type> ("To"))
+                                       )
+                               );
+               }
+       } else {
+               /* In version 33 we use a hash of various parts of the subtitle as the id */
+               BOOST_FOREACH (cxml::NodePtr i, node->node_children ("Subtitle")) {
+                       add_subtitle (
+                               raw_convert<string> (i->string_child ("Id")),
+                               ContentTimePeriod (
+                                       ContentTime (i->number_child<ContentTime::Type> ("From")),
+                                       ContentTime (i->number_child<ContentTime::Type> ("To"))
+                                       )
+                               );
+               }
        }
 }
 
@@ -50,18 +67,19 @@ FFmpegSubtitleStream::as_xml (xmlpp::Node* root) const
 {
        FFmpegStream::as_xml (root);
 
-       for (map<ContentTime, ContentTime>::const_iterator i = _subtitles.begin(); i != _subtitles.end(); ++i) {
-               xmlpp::Node* node = root->add_child ("Period");
-               node->add_child("From")->add_child_text (raw_convert<string> (i->first.get ()));
-               node->add_child("To")->add_child_text (raw_convert<string> (i->second.get ()));
+       for (map<string, ContentTimePeriod>::const_iterator i = _subtitles.begin(); i != _subtitles.end(); ++i) {
+               xmlpp::Node* node = root->add_child ("Subtitle");
+               node->add_child("Id")->add_child_text (i->first);
+               node->add_child("From")->add_child_text (raw_convert<string> (i->second.from.get ()));
+               node->add_child("To")->add_child_text (raw_convert<string> (i->second.to.get ()));
        }
 }
 
 void
-FFmpegSubtitleStream::add_subtitle (ContentTimePeriod period)
+FFmpegSubtitleStream::add_subtitle (string id, ContentTimePeriod period)
 {
-       DCPOMATIC_ASSERT (_subtitles.find (period.from) == _subtitles.end ());
-       _subtitles[period.from] = period.to;
+       DCPOMATIC_ASSERT (_subtitles.find (id) == _subtitles.end ());
+       _subtitles[id] = period;
 }
 
 list<ContentTimePeriod>
@@ -70,9 +88,9 @@ FFmpegSubtitleStream::subtitles_during (ContentTimePeriod period, bool starting)
        list<ContentTimePeriod> d;
 
        /* XXX: inefficient */
-       for (map<ContentTime, ContentTime>::const_iterator i = _subtitles.begin(); i != _subtitles.end(); ++i) {
-               if ((starting && period.contains (i->first)) || (!starting && period.overlaps (ContentTimePeriod (i->first, i->second)))) {
-                       d.push_back (ContentTimePeriod (i->first, i->second));
+       for (map<string, ContentTimePeriod>::const_iterator i = _subtitles.begin(); i != _subtitles.end(); ++i) {
+               if ((starting && period.contains (i->second.from)) || (!starting && period.overlaps (i->second))) {
+                       d.push_back (i->second);
                }
        }
 
@@ -80,20 +98,19 @@ FFmpegSubtitleStream::subtitles_during (ContentTimePeriod period, bool starting)
 }
 
 ContentTime
-FFmpegSubtitleStream::find_subtitle_to (ContentTime from) const
+FFmpegSubtitleStream::find_subtitle_to (string id) const
 {
-       map<ContentTime, ContentTime>::const_iterator i = _subtitles.find (from);
+       map<string, ContentTimePeriod>::const_iterator i = _subtitles.find (id);
        DCPOMATIC_ASSERT (i != _subtitles.end ());
-       return i->second;
+       return i->second.to;
 }
 
 /** Add some offset to all the times in the stream */
 void
 FFmpegSubtitleStream::add_offset (ContentTime offset)
 {
-       map<ContentTime, ContentTime> fixed;
-       for (map<ContentTime, ContentTime>::iterator i = _subtitles.begin(); i != _subtitles.end(); ++i) {
-               fixed[i->first + offset] = i->second + offset;
+       for (map<string, ContentTimePeriod>::iterator i = _subtitles.begin(); i != _subtitles.end(); ++i) {
+               i->second.from += offset;
+               i->second.to += offset;
        }
-       _subtitles = fixed;
 }

diff --git a/src/lib/ffmpeg_subtitle_stream.h b/src/lib/ffmpeg_subtitle_stream.h
index 0809b359a1a6daa7b5349455fadab624ec08e418..6cd5318d0d9e124ea87007eb8c7d15aad464c6c5 100644 (file)
--- a/src/lib/ffmpeg_subtitle_stream.h
+++ b/src/lib/ffmpeg_subtitle_stream.h
@@ -1,5 +1,5 @@
 /*
-    Copyright (C) 2013-2015 Carl Hetherington <cth@carlh.net>
+    Copyright (C) 2013-2016 Carl Hetherington <cth@carlh.net>
 
     This program is free software; you can redistribute it and/or modify
     it under the terms of the GNU General Public License as published by
@@ -27,15 +27,15 @@ public:
                : FFmpegStream (n, i)
        {}
 
-       FFmpegSubtitleStream (cxml::ConstNodePtr);
+       FFmpegSubtitleStream (cxml::ConstNodePtr, int version);
 
        void as_xml (xmlpp::Node *) const;
 
-       void add_subtitle (ContentTimePeriod period);
+       void add_subtitle (std::string id, ContentTimePeriod period);
        std::list<ContentTimePeriod> subtitles_during (ContentTimePeriod period, bool starting) const;
-       ContentTime find_subtitle_to (ContentTime from) const;
+       ContentTime find_subtitle_to (std::string id) const;
        void add_offset (ContentTime offset);
 
 private:
-       std::map<ContentTime, ContentTime> _subtitles;
+       std::map<std::string, ContentTimePeriod> _subtitles;
 };

diff --git a/src/lib/film.cc b/src/lib/film.cc
index 20b959dd04c526e215050cf5f8af4b45c0bdc0b6..9320cf5d2c8aaa9625cc4d4ca67695c80e07c64b 100644 (file)
--- a/src/lib/film.cc
+++ b/src/lib/film.cc
@@ -103,8 +103,11 @@ using boost::is_any_of;
  *
  * Bumped to 32 for 2.0 branch; some times are expressed in Times rather
  * than frames now.
+ *
+ * 32 -> 33
+ * Changed <Period> to <Subtitle> in FFmpegSubtitleStream
  */
-int const Film::current_state_version = 32;
+int const Film::current_state_version = 33;
 
 /** Construct a Film object in a given directory.
  *