From 2bfd531137f1a4874493186015046e33c5a07c1e Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Tue, 12 May 2015 16:13:48 +0100 Subject: [PATCH] Assorted image subtitle fixes. --- src/lib/dcp_decoder.cc | 8 +++- src/lib/dcp_decoder.h | 3 +- src/lib/dcp_subtitle_decoder.cc | 8 +++- src/lib/dcp_subtitle_decoder.h | 3 +- src/lib/dcpomatic_time.h | 1 + src/lib/ffmpeg_decoder.cc | 66 ++++++++++++++++++++----------- src/lib/ffmpeg_decoder.h | 8 +++- src/lib/ffmpeg_examiner.cc | 15 ++++--- src/lib/ffmpeg_examiner.h | 4 +- src/lib/ffmpeg_subtitle_stream.cc | 20 +++++++++- src/lib/player.cc | 8 ++-- src/lib/render_subtitles.cc | 4 -- src/lib/subrip_decoder.cc | 8 +++- src/lib/subrip_decoder.h | 3 +- src/lib/subtitle_decoder.cc | 14 +++---- src/lib/subtitle_decoder.h | 5 ++- src/lib/types.h | 1 + src/lib/util.cc | 13 +++--- src/lib/util.h | 19 ++++++++- 19 files changed, 153 insertions(+), 58 deletions(-) diff --git a/src/lib/dcp_decoder.cc b/src/lib/dcp_decoder.cc index 51d16b43c..ab0df8a86 100644 --- a/src/lib/dcp_decoder.cc +++ b/src/lib/dcp_decoder.cc @@ -133,7 +133,13 @@ DCPDecoder::seek (ContentTime t, bool accurate) list -DCPDecoder::subtitles_during (ContentTimePeriod, bool) const +DCPDecoder::image_subtitles_during (ContentTimePeriod, bool) const +{ + return list (); +} + +list +DCPDecoder::text_subtitles_during (ContentTimePeriod, bool) const { /* XXX */ return list (); diff --git a/src/lib/dcp_decoder.h b/src/lib/dcp_decoder.h index 8afebff57..20ca2bb67 100644 --- a/src/lib/dcp_decoder.h +++ b/src/lib/dcp_decoder.h @@ -40,7 +40,8 @@ public: private: void seek (ContentTime t, bool accurate); bool pass (); - std::list subtitles_during (ContentTimePeriod, bool starting) const; + std::list image_subtitles_during (ContentTimePeriod, bool starting) const; + std::list text_subtitles_during (ContentTimePeriod, bool starting) const; ContentTime _next; std::list > _reels; diff --git a/src/lib/dcp_subtitle_decoder.cc b/src/lib/dcp_subtitle_decoder.cc index e3c06378b..236e99996 100644 --- a/src/lib/dcp_subtitle_decoder.cc +++ b/src/lib/dcp_subtitle_decoder.cc @@ -61,7 +61,13 @@ DCPSubtitleDecoder::pass () } list -DCPSubtitleDecoder::subtitles_during (ContentTimePeriod p, bool starting) const +DCPSubtitleDecoder::image_subtitles_during (ContentTimePeriod, bool) const +{ + return list (); +} + +list +DCPSubtitleDecoder::text_subtitles_during (ContentTimePeriod p, bool starting) const { /* XXX: inefficient */ diff --git a/src/lib/dcp_subtitle_decoder.h b/src/lib/dcp_subtitle_decoder.h index 2326b31ad..9a16fb7c8 100644 --- a/src/lib/dcp_subtitle_decoder.h +++ b/src/lib/dcp_subtitle_decoder.h @@ -32,7 +32,8 @@ protected: bool pass (); private: - std::list subtitles_during (ContentTimePeriod, bool starting) const; + std::list image_subtitles_during (ContentTimePeriod, bool starting) const; + std::list text_subtitles_during (ContentTimePeriod, bool starting) const; std::list _subtitles; std::list::const_iterator _next; diff --git a/src/lib/dcpomatic_time.h b/src/lib/dcpomatic_time.h index dc9b0cd8a..ae8f25199 100644 --- a/src/lib/dcpomatic_time.h +++ b/src/lib/dcpomatic_time.h @@ -193,6 +193,7 @@ class ContentTimePeriod { public: ContentTimePeriod () {} + ContentTimePeriod (ContentTime f, ContentTime t) : from (f) , to (t) diff --git a/src/lib/ffmpeg_decoder.cc b/src/lib/ffmpeg_decoder.cc index bd01b280b..5dc06fa8e 100644 --- a/src/lib/ffmpeg_decoder.cc +++ b/src/lib/ffmpeg_decoder.cc @@ -301,6 +301,7 @@ FFmpegDecoder::seek (ContentTime time, bool accurate) { VideoDecoder::seek (time, accurate); AudioDecoder::seek (time, accurate); + SubtitleDecoder::seek (time, accurate); /* If we are doing an `accurate' seek, we need to use pre-roll, as we don't really know what the seek will give us. @@ -426,27 +427,38 @@ FFmpegDecoder::decode_subtitle_packet () return; } - /* Sometimes we get an empty AVSubtitle, which is used by some codecs to - indicate that the previous subtitle should stop. + /* Subtitle PTS (within the source, not taking into account any of the + source that we may have chopped off for the DCP) */ + FFmpegSubtitlePeriod period = subtitle_period (sub); + period.from += _pts_offset; + if (period.to) { + period.to = period.to.get() + _pts_offset; + } + if (sub.num_rects <= 0) { - image_subtitle (ContentTimePeriod (), shared_ptr (), dcpomatic::Rect ()); + /* Sometimes we get an empty AVSubtitle, which is used by some codecs to + indicate that the previous subtitle should stop. Emit the pending one. + */ + if (_pending_subtitle_from && _pending_subtitle_image && _pending_subtitle_rect) { + image_subtitle ( + ContentTimePeriod (_pending_subtitle_from.get(), period.from), + _pending_subtitle_image, + _pending_subtitle_rect.get () + ); + _pending_subtitle_from = optional (); + _pending_subtitle_image.reset (); + _pending_subtitle_rect = optional > (); + } return; } else if (sub.num_rects > 1) { throw DecodeError (_("multi-part subtitles not yet supported")); } - /* Subtitle PTS (within the source, not taking into account any of the - source that we may have chopped off for the DCP) - */ - ContentTimePeriod period = subtitle_period (sub) + _pts_offset; - AVSubtitleRect const * rect = sub.rects[0]; if (rect->type != SUBTITLE_BITMAP) { - /* XXX */ - // throw DecodeError (_("non-bitmap subtitles not yet supported")); - return; + throw DecodeError (_("non-bitmap subtitles not yet supported")); } /* Note RGBA is expressed little-endian, so the first byte in the word is R, second @@ -475,23 +487,33 @@ FFmpegDecoder::decode_subtitle_packet () } dcp::Size const vs = _ffmpeg_content->video_size (); - - image_subtitle ( - period, - image, - dcpomatic::Rect ( - static_cast (rect->x) / vs.width, - static_cast (rect->y) / vs.height, - static_cast (rect->w) / vs.width, - static_cast (rect->h) / vs.height - ) + dcpomatic::Rect const scaled_rect ( + static_cast (rect->x) / vs.width, + static_cast (rect->y) / vs.height, + static_cast (rect->w) / vs.width, + static_cast (rect->h) / vs.height ); + + if (period.to) { + image_subtitle (ContentTimePeriod (period.from, period.to.get()), image, scaled_rect); + } else { + /* We don't know when this subtitle stops, so store it until we find out */ + _pending_subtitle_from = period.from; + _pending_subtitle_image = image; + _pending_subtitle_rect = scaled_rect; + } avsubtitle_free (&sub); } list -FFmpegDecoder::subtitles_during (ContentTimePeriod p, bool starting) const +FFmpegDecoder::image_subtitles_during (ContentTimePeriod p, bool starting) const { return _ffmpeg_content->subtitles_during (p, starting); } + +list +FFmpegDecoder::text_subtitles_during (ContentTimePeriod, bool) const +{ + return list (); +} diff --git a/src/lib/ffmpeg_decoder.h b/src/lib/ffmpeg_decoder.h index 0334a30e2..b5bcdd358 100644 --- a/src/lib/ffmpeg_decoder.h +++ b/src/lib/ffmpeg_decoder.h @@ -27,6 +27,7 @@ #include "audio_decoder.h" #include "subtitle_decoder.h" #include "ffmpeg.h" +#include "rect.h" extern "C" { #include } @@ -66,7 +67,12 @@ private: void maybe_add_subtitle (); boost::shared_ptr deinterleave_audio (uint8_t** data, int size); - std::list subtitles_during (ContentTimePeriod, bool starting) const; + boost::optional _pending_subtitle_from; + boost::shared_ptr _pending_subtitle_image; + boost::optional > _pending_subtitle_rect; + + std::list image_subtitles_during (ContentTimePeriod, bool starting) const; + std::list text_subtitles_during (ContentTimePeriod, bool starting) const; boost::shared_ptr _log; diff --git a/src/lib/ffmpeg_examiner.cc b/src/lib/ffmpeg_examiner.cc index 4409526dc..e4f4e6f29 100644 --- a/src/lib/ffmpeg_examiner.cc +++ b/src/lib/ffmpeg_examiner.cc @@ -150,13 +150,18 @@ FFmpegExaminer::subtitle_packet (AVCodecContext* context, shared_ptr= 0 && frame_finished) { - ContentTimePeriod const period = subtitle_period (sub); - if (sub.num_rects == 0 && !stream->periods.empty () && stream->periods.back().to > period.from) { - /* Finish the last subtitle */ - stream->periods.back().to = period.from; + FFmpegSubtitlePeriod const period = subtitle_period (sub); + if (sub.num_rects <= 0 && _last_subtitle_start) { + stream->periods.push_back (ContentTimePeriod (_last_subtitle_start.get (), period.from)); + _last_subtitle_start = optional (); } else if (sub.num_rects == 1) { - stream->periods.push_back (period); + if (period.to) { + stream->periods.push_back (ContentTimePeriod (period.from, period.to.get ())); + } else { + _last_subtitle_start = period.from; + } } + avsubtitle_free (&sub); } } diff --git a/src/lib/ffmpeg_examiner.h b/src/lib/ffmpeg_examiner.h index b873222c1..34d4b1e0d 100644 --- a/src/lib/ffmpeg_examiner.h +++ b/src/lib/ffmpeg_examiner.h @@ -55,7 +55,7 @@ private: std::string audio_stream_name (AVStream* s) const; std::string subtitle_stream_name (AVStream* s) const; boost::optional frame_time (AVStream* s) const; - + std::vector > _subtitle_streams; std::vector > _audio_streams; boost::optional _first_video; @@ -64,4 +64,6 @@ private: */ ContentTime _video_length; bool _need_video_length; + + boost::optional _last_subtitle_start; }; diff --git a/src/lib/ffmpeg_subtitle_stream.cc b/src/lib/ffmpeg_subtitle_stream.cc index 3d8fd4e83..66b587209 100644 --- a/src/lib/ffmpeg_subtitle_stream.cc +++ b/src/lib/ffmpeg_subtitle_stream.cc @@ -18,6 +18,11 @@ */ #include "ffmpeg_subtitle_stream.h" +#include "raw_convert.h" +#include +#include + +using std::string; /** Construct a SubtitleStream from a value returned from to_string(). * @param t String returned from to_string(). @@ -26,11 +31,24 @@ FFmpegSubtitleStream::FFmpegSubtitleStream (cxml::ConstNodePtr node) : FFmpegStream (node) { - + BOOST_FOREACH (cxml::NodePtr i, node->node_children ("Period")) { + periods.push_back ( + ContentTimePeriod ( + ContentTime (node->number_child ("From")), + ContentTime (node->number_child ("To")) + ) + ); + } } void FFmpegSubtitleStream::as_xml (xmlpp::Node* root) const { FFmpegStream::as_xml (root); + + BOOST_FOREACH (ContentTimePeriod const & i, periods) { + xmlpp::Node* node = root->add_child ("Period"); + node->add_child("From")->add_child_text (raw_convert (i.from.get ())); + node->add_child("To")->add_child_text (raw_convert (i.to.get ())); + } } diff --git a/src/lib/player.cc b/src/lib/player.cc index 436ae3fe8..640253c6d 100644 --- a/src/lib/player.cc +++ b/src/lib/player.cc @@ -376,9 +376,11 @@ Player::get_video (DCPTime time, bool accurate) list c = transform_image_subtitles (ps.image); copy (c.begin(), c.end(), back_inserter (sub_images)); - /* Text subtitles (rendered to images) */ - sub_images.push_back (render_subtitles (ps.text, _video_container_size)); - + /* Text subtitles (rendered to an image) */ + if (!ps.text.empty ()) { + sub_images.push_back (render_subtitles (ps.text, _video_container_size)); + } + if (!sub_images.empty ()) { for (list >::const_iterator i = pvf.begin(); i != pvf.end(); ++i) { (*i)->set_subtitle (merge (sub_images)); diff --git a/src/lib/render_subtitles.cc b/src/lib/render_subtitles.cc index bc89fd3f8..9620eacbf 100644 --- a/src/lib/render_subtitles.cc +++ b/src/lib/render_subtitles.cc @@ -50,10 +50,6 @@ calculate_position (dcp::VAlign v_align, double v_position, int target_height, i PositionImage render_subtitles (list subtitles, dcp::Size target) { - if (subtitles.empty ()) { - return PositionImage (); - } - /* Estimate height that the subtitle image needs to be */ optional top; optional bottom; diff --git a/src/lib/subrip_decoder.cc b/src/lib/subrip_decoder.cc index 552a96b8f..bef48ba78 100644 --- a/src/lib/subrip_decoder.cc +++ b/src/lib/subrip_decoder.cc @@ -85,7 +85,13 @@ SubRipDecoder::pass () } list -SubRipDecoder::subtitles_during (ContentTimePeriod p, bool starting) const +SubRipDecoder::image_subtitles_during (ContentTimePeriod, bool) const +{ + return list (); +} + +list +SubRipDecoder::text_subtitles_during (ContentTimePeriod p, bool starting) const { /* XXX: inefficient */ diff --git a/src/lib/subrip_decoder.h b/src/lib/subrip_decoder.h index ad9d04e40..876f763d3 100644 --- a/src/lib/subrip_decoder.h +++ b/src/lib/subrip_decoder.h @@ -35,7 +35,8 @@ protected: bool pass (); private: - std::list subtitles_during (ContentTimePeriod, bool starting) const; + std::list image_subtitles_during (ContentTimePeriod, bool starting) const; + std::list text_subtitles_during (ContentTimePeriod, bool starting) const; size_t _next; }; diff --git a/src/lib/subtitle_decoder.cc b/src/lib/subtitle_decoder.cc index 9b2aa8ab0..ac9a67e33 100644 --- a/src/lib/subtitle_decoder.cc +++ b/src/lib/subtitle_decoder.cc @@ -1,5 +1,5 @@ /* - Copyright (C) 2013-2014 Carl Hetherington + Copyright (C) 2013-2015 Carl Hetherington 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 @@ -33,7 +33,8 @@ SubtitleDecoder::SubtitleDecoder (shared_ptr c) } /** Called by subclasses when an image subtitle is ready. - * Image may be 0 to say that there is no current subtitle. + * @param period Period of the subtitle. + * @param image Subtitle image. * @param rect Area expressed as a fraction of the video frame that this subtitle * is for (e.g. a width of 0.5 means the width of the subtitle is half the width * of the video frame) @@ -50,12 +51,11 @@ SubtitleDecoder::text_subtitle (list s) _decoded_text_subtitles.push_back (ContentTextSubtitle (s)); } +/** @param sp Full periods of subtitles that are showing or starting during the specified period */ template list -SubtitleDecoder::get (list const & subs, ContentTimePeriod period, bool starting) +SubtitleDecoder::get (list const & subs, list const & sp, ContentTimePeriod period, bool starting) { - /* Get the full periods of the subtitles that are showing or starting during the specified period */ - list sp = subtitles_during (period, starting); if (sp.empty ()) { /* Nothing in this period */ return list (); @@ -88,13 +88,13 @@ SubtitleDecoder::get (list const & subs, ContentTimePeriod period, bool start list SubtitleDecoder::get_text_subtitles (ContentTimePeriod period, bool starting) { - return get (_decoded_text_subtitles, period, starting); + return get (_decoded_text_subtitles, text_subtitles_during (period, starting), period, starting); } list SubtitleDecoder::get_image_subtitles (ContentTimePeriod period, bool starting) { - return get (_decoded_image_subtitles, period, starting); + return get (_decoded_image_subtitles, image_subtitles_during (period, starting), period, starting); } void diff --git a/src/lib/subtitle_decoder.h b/src/lib/subtitle_decoder.h index d7faaa014..8ba74404f 100644 --- a/src/lib/subtitle_decoder.h +++ b/src/lib/subtitle_decoder.h @@ -49,12 +49,13 @@ protected: private: template - std::list get (std::list const & subs, ContentTimePeriod period, bool starting); + std::list get (std::list const & subs, std::list const & sp, ContentTimePeriod period, bool starting); /** @param starting true if we want only subtitles that start during the period, otherwise * we want subtitles that overlap the period. */ - virtual std::list subtitles_during (ContentTimePeriod period, bool starting) const = 0; + virtual std::list image_subtitles_during (ContentTimePeriod period, bool starting) const = 0; + virtual std::list text_subtitles_during (ContentTimePeriod period, bool starting) const = 0; boost::shared_ptr _subtitle_content; }; diff --git a/src/lib/types.h b/src/lib/types.h index f3877d0d5..e7017a295 100644 --- a/src/lib/types.h +++ b/src/lib/types.h @@ -22,6 +22,7 @@ #include "dcpomatic_time.h" #include "position.h" +#include "rect.h" #include #include #include diff --git a/src/lib/util.cc b/src/lib/util.cc index bffbe90d4..d8c754607 100644 --- a/src/lib/util.cc +++ b/src/lib/util.cc @@ -564,18 +564,21 @@ wrapped_av_malloc (size_t s) } return p; } - -ContentTimePeriod + +FFmpegSubtitlePeriod subtitle_period (AVSubtitle const & sub) { ContentTime const packet_time = ContentTime::from_seconds (static_cast (sub.pts) / AV_TIME_BASE); - ContentTimePeriod period ( + if (sub.end_display_time == static_cast (-1)) { + /* End time is not known */ + return FFmpegSubtitlePeriod (packet_time + ContentTime::from_seconds (sub.start_display_time / 1e3)); + } + + return FFmpegSubtitlePeriod ( packet_time + ContentTime::from_seconds (sub.start_display_time / 1e3), packet_time + ContentTime::from_seconds (sub.end_display_time / 1e3) ); - - return period; } map diff --git a/src/lib/util.h b/src/lib/util.h index c1f7a78c7..44bd7dced 100644 --- a/src/lib/util.h +++ b/src/lib/util.h @@ -74,7 +74,24 @@ extern int dcp_audio_frame_rate (int); extern int stride_round_up (int, int const *, int); extern int round_to (float n, int r); extern void* wrapped_av_malloc (size_t); -extern ContentTimePeriod subtitle_period (AVSubtitle const &); + +class FFmpegSubtitlePeriod +{ +public: + FFmpegSubtitlePeriod (ContentTime f) + : from (f) + {} + + FFmpegSubtitlePeriod (ContentTime f, ContentTime t) + : from (f) + , to (t) + {} + + ContentTime from; + boost::optional to; +}; + +extern FFmpegSubtitlePeriod subtitle_period (AVSubtitle const &); extern void set_backtrace_file (boost::filesystem::path); extern dcp::FrameInfo read_frame_info (FILE* file, int frame, Eyes eyes); extern void write_frame_info (FILE* file, int frame, Eyes eyes, dcp::FrameInfo info); -- 2.30.2