Fix problems with subtitles when there is a non-zero PTS offset

in FFmpegDecoder.  This offset was not being taken into account for
subtitles prior to this commit.

diff --git a/ChangeLog b/ChangeLog
index a9af721ebc65f1a6a8099d61e0c5b5a9b0da9136..60b6254420a9f2664a644db49e35c7c698c6ec29 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,8 @@
 2015-09-26  Carl Hetherington  <cth@carlh.net>
 
+       * Fix crash with embedded subtitles with some
+       video files.
+
        * Version 2.3.6 released.
 
 2015-09-25  Carl Hetherington  <cth@carlh.net>

diff --git a/src/lib/ffmpeg.cc b/src/lib/ffmpeg.cc
index 8fa03fdffb552f3ddbd49042df0d41d084d5d6ed..de08e9df9394a5c6bea3b34b2eca4eebc40b9dda 100644 (file)
--- a/src/lib/ffmpeg.cc
+++ b/src/lib/ffmpeg.cc
@@ -25,6 +25,7 @@
 #include "raw_convert.h"
 #include "log.h"
 #include "ffmpeg_subtitle_stream.h"
+#include "ffmpeg_audio_stream.h"
 #include "compose.hpp"
 extern "C" {
 #include <libavcodec/avcodec.h>
@@ -32,6 +33,7 @@ extern "C" {
 #include <libswscale/swscale.h>
 }
 #include <boost/algorithm/string.hpp>
+#include <boost/foreach.hpp>
 #include <iostream>
 
 #include "i18n.h"
@@ -39,7 +41,9 @@ extern "C" {
 using std::string;
 using std::cout;
 using std::cerr;
+using std::vector;
 using boost::shared_ptr;
+using boost::optional;
 
 boost::mutex FFmpeg::_mutex;
 boost::weak_ptr<Log> FFmpeg::_ffmpeg_log;
@@ -263,3 +267,55 @@ FFmpeg::subtitle_period (AVSubtitle const & sub)
                packet_time + ContentTime::from_seconds (sub.end_display_time / 1e3)
                );
 }
+
+/** 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
+ *  in FFmpeg.
+ */
+ContentTime
+FFmpeg::pts_offset (vector<shared_ptr<FFmpegAudioStream> > audio_streams, optional<ContentTime> first_video, double video_frame_rate) const
+{
+       /* Audio and video frame PTS values may not start with 0.  We want
+          to fiddle them so that:
+
+          1.  One of them starts at time 0.
+          2.  The first video PTS value ends up on a frame boundary.
+
+          Then we remove big initial gaps in PTS and we allow our
+          insertion of black frames to work.
+
+          We will do:
+            audio_pts_to_use = audio_pts_from_ffmpeg + pts_offset;
+            video_pts_to_use = video_pts_from_ffmpeg + pts_offset;
+       */
+
+       /* First, make one of them start at 0 */
+
+       ContentTime po = ContentTime::min ();
+
+       if (first_video) {
+               po = - first_video.get ();
+       }
+
+       BOOST_FOREACH (shared_ptr<FFmpegAudioStream> i, audio_streams) {
+               if (i->first_audio) {
+                       po = max (po, - i->first_audio.get ());
+               }
+       }
+
+       /* If the offset is positive we would be pushing things from a -ve PTS to be played.
+          I don't think we ever want to do that, as it seems things at -ve PTS are not meant
+          to be seen (use for alignment bars etc.); see mantis #418.
+       */
+       if (po > ContentTime ()) {
+               po = ContentTime ();
+       }
+
+       /* Now adjust so that the video pts starts on a frame */
+       if (first_video) {
+               po += first_video.get().round_up (video_frame_rate) - first_video.get();
+       }
+
+       return po;
+}

diff --git a/src/lib/ffmpeg.h b/src/lib/ffmpeg.h
index 961f5cbb1e2b259b8d205ceb93e808bc0867e767..b3bc13e5cc222b9033472bfb43df8f3b70693fd7 100644 (file)
--- a/src/lib/ffmpeg.h
+++ b/src/lib/ffmpeg.h
@@ -33,6 +33,7 @@ struct AVFrame;
 struct AVIOContext;
 
 class FFmpegContent;
+class FFmpegAudioStream;
 class Log;
 
 class FFmpeg
@@ -51,6 +52,10 @@ public:
 protected:
        AVCodecContext* video_codec_context () const;
        AVCodecContext* subtitle_codec_context () const;
+       ContentTime pts_offset (
+               std::vector<boost::shared_ptr<FFmpegAudioStream> > audio_streams, boost::optional<ContentTime> first_video, double video_frame_rate
+               ) const;
+
        static FFmpegSubtitlePeriod subtitle_period (AVSubtitle const &);
 
        boost::shared_ptr<const FFmpegContent> _ffmpeg_content;

diff --git a/src/lib/ffmpeg_decoder.cc b/src/lib/ffmpeg_decoder.cc
index d343ec317e14c6a32cc8f23574ff9613f698627b..475418d3d2be34f9d30a65b7cfcfcce5e3089f69 100644 (file)
--- a/src/lib/ffmpeg_decoder.cc
+++ b/src/lib/ffmpeg_decoder.cc
@@ -67,51 +67,9 @@ FFmpegDecoder::FFmpegDecoder (shared_ptr<const FFmpegContent> c, shared_ptr<Log>
        , SubtitleDecoder (c)
        , FFmpeg (c)
        , _log (log)
+       , _pts_offset (pts_offset (c->ffmpeg_audio_streams(), c->first_video(), c->video_frame_rate()))
 {
-       /* Audio and video frame PTS values may not start with 0.  We want
-          to fiddle them so that:
 
-          1.  One of them starts at time 0.
-          2.  The first video PTS value ends up on a frame boundary.
-
-          Then we remove big initial gaps in PTS and we allow our
-          insertion of black frames to work.
-
-          We will do:
-            audio_pts_to_use = audio_pts_from_ffmpeg + pts_offset;
-            video_pts_to_use = video_pts_from_ffmpeg + pts_offset;
-       */
-
-       /* First, make one of them start at 0 */
-
-       vector<shared_ptr<FFmpegAudioStream> > streams = c->ffmpeg_audio_streams ();
-
-       _pts_offset = ContentTime::min ();
-
-       if (c->first_video ()) {
-               _pts_offset = - c->first_video().get ();
-       }
-
-       BOOST_FOREACH (shared_ptr<FFmpegAudioStream> i, streams) {
-               if (i->first_audio) {
-                       _pts_offset = max (_pts_offset, - i->first_audio.get ());
-               }
-       }
-
-       /* If _pts_offset is positive we would be pushing things from a -ve PTS to be played.
-          I don't think we ever want to do that, as it seems things at -ve PTS are not meant
-          to be seen (use for alignment bars etc.); see mantis #418.
-       */
-       if (_pts_offset > ContentTime ()) {
-               _pts_offset = ContentTime ();
-       }
-
-       /* Now adjust so that the video pts starts on a frame */
-       if (c->first_video ()) {
-               ContentTime first_video = c->first_video().get() + _pts_offset;
-               ContentTime const old_first_video = first_video;
-               _pts_offset += first_video.round_up (c->video_frame_rate ()) - old_first_video;
-       }
 }
 
 void
@@ -457,7 +415,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 (sub_period.from);
+               period.to = ffmpeg_content()->subtitle_stream()->find_subtitle_to (period.from);
        }
 
        AVSubtitleRect const * rect = sub.rects[0];

diff --git a/src/lib/ffmpeg_examiner.cc b/src/lib/ffmpeg_examiner.cc
index 6173e041529444ed9e2c6cf9e0c8ec305d554bf5..40fe0d28ec5cae2e6bc89d800dd9ef57bb195402 100644 (file)
--- a/src/lib/ffmpeg_examiner.cc
+++ b/src/lib/ffmpeg_examiner.cc
@@ -30,6 +30,7 @@ extern "C" {
 #include "ffmpeg_subtitle_stream.h"
 #include "util.h"
 #include "safe_stringstream.h"
+#include <boost/foreach.hpp>
 #include <iostream>
 
 #include "i18n.h"
@@ -145,6 +146,16 @@ FFmpegExaminer::FFmpegExaminer (shared_ptr<const FFmpegContent> c, shared_ptr<Jo
                                );
                }
        }
+
+       /* We just added subtitles to our streams without taking the PTS offset into account;
+          this is because we might not know the PTS offset when the first subtitle is seen.
+          Now we know the PTS offset so we can apply it to those subtitles.
+       */
+       if (video_frame_rate()) {
+               BOOST_FOREACH (shared_ptr<FFmpegSubtitleStream> i, _subtitle_streams) {
+                       i->add_offset (pts_offset (_audio_streams, _first_video, video_frame_rate().get()));
+               }
+       }
 }
 
 void

diff --git a/src/lib/ffmpeg_subtitle_stream.cc b/src/lib/ffmpeg_subtitle_stream.cc
index 27099b0f34ccf000b3d93179b4cad40c8f2c63ed..e12075581b12705a71d53153904218a6950b1cce 100644 (file)
--- a/src/lib/ffmpeg_subtitle_stream.cc
+++ b/src/lib/ffmpeg_subtitle_stream.cc
@@ -86,3 +86,14 @@ FFmpegSubtitleStream::find_subtitle_to (ContentTime from) const
        DCPOMATIC_ASSERT (i != _subtitles.end ());
        return i->second;
 }
+
+/** 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;
+       }
+       _subtitles = fixed;
+}

diff --git a/src/lib/ffmpeg_subtitle_stream.h b/src/lib/ffmpeg_subtitle_stream.h
index a39b10ffd6624804baa80916a1cfe1f894d32a9e..0809b359a1a6daa7b5349455fadab624ec08e418 100644 (file)
--- a/src/lib/ffmpeg_subtitle_stream.h
+++ b/src/lib/ffmpeg_subtitle_stream.h
@@ -34,8 +34,8 @@ public:
        void add_subtitle (ContentTimePeriod period);
        std::list<ContentTimePeriod> subtitles_during (ContentTimePeriod period, bool starting) const;
        ContentTime find_subtitle_to (ContentTime from) const;
+       void add_offset (ContentTime offset);
 
 private:
        std::map<ContentTime, ContentTime> _subtitles;
 };
-