From bdbe925a467f9b7149322ad8d1c090d4c1e6d5c3 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Tue, 22 Sep 2015 16:15:08 +0100 Subject: Use uchardet to guess encoding of subtitle files and reject non-UTF-8. --- src/lib/exceptions.h | 8 ++++++++ src/lib/subrip.cc | 23 +++++++++++++++++++++++ 2 files changed, 31 insertions(+) (limited to 'src/lib') diff --git a/src/lib/exceptions.h b/src/lib/exceptions.h index 7240611ee..6939f81a3 100644 --- a/src/lib/exceptions.h +++ b/src/lib/exceptions.h @@ -263,4 +263,12 @@ public: ProgrammingError (std::string file, int line); }; +class TextEncodingError : public StringError +{ +public: + TextEncodingError (std::string s) + : StringError (s) + {} +}; + #endif diff --git a/src/lib/subrip.cc b/src/lib/subrip.cc index f19867952..d4adee428 100644 --- a/src/lib/subrip.cc +++ b/src/lib/subrip.cc @@ -23,10 +23,14 @@ #include "subrip_content.h" #include #include +#include +#include #include "i18n.h" using std::vector; +using std::cout; +using std::string; using boost::shared_ptr; SubRip::SubRip (shared_ptr content) @@ -36,6 +40,25 @@ SubRip::SubRip (shared_ptr content) throw OpenFileError (content->path (0)); } + /* Guess the encoding */ + uchardet_t det = uchardet_new (); + char buffer[1024]; + while (!feof (f)) { + int const n = fread (buffer, 1, sizeof (buffer), f); + if (uchardet_handle_data (det, buffer, n)) { + break; + } + } + + uchardet_data_end (det); + string charset = uchardet_get_charset (det); + uchardet_delete (det); + + if (charset != "UTF-8") { + throw TextEncodingError (_("unrecognised character set; please use files encoded in UTF-8")); + } + + rewind (f); sub::SubripReader reader (f); _subtitles = sub::collect > (reader.subtitles ()); } -- cgit v1.2.3 From f74d619bf98c6069ecc012c1c34fb543592d6bff Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Wed, 23 Sep 2015 10:30:08 +0100 Subject: Try to fix build. --- src/lib/wscript | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/lib') diff --git a/src/lib/wscript b/src/lib/wscript index a95905578..b615b575d 100644 --- a/src/lib/wscript +++ b/src/lib/wscript @@ -143,7 +143,7 @@ def build(bld): AVCODEC AVUTIL AVFORMAT AVFILTER SWSCALE SWRESAMPLE BOOST_FILESYSTEM BOOST_THREAD BOOST_DATETIME BOOST_SIGNALS2 BOOST_REGEX SNDFILE SAMPLERATE OPENJPEG POSTPROC TIFF MAGICK SSH DCP CXML GLIB LZMA XML++ - CURL ZIP FONTCONFIG PANGOMM CAIROMM XMLSEC SUB + CURL ZIP FONTCONFIG PANGOMM CAIROMM XMLSEC SUB UCHARDET """ if bld.env.TARGET_OSX: -- cgit v1.2.3 From 511f4bbb6dd088cc32163edfe5004761a33a8311 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Thu, 24 Sep 2015 15:52:01 +0100 Subject: Fix possible null pointer dereference. --- src/lib/job.cc | 1 - src/lib/server.cc | 4 +++- 2 files changed, 3 insertions(+), 2 deletions(-) (limited to 'src/lib') diff --git a/src/lib/job.cc b/src/lib/job.cc index c228defc5..439000e58 100644 --- a/src/lib/job.cc +++ b/src/lib/job.cc @@ -447,4 +447,3 @@ Job::when_finished (boost::signals2::connection& connection, function fi connection = Finished.connect (finished); } } - diff --git a/src/lib/server.cc b/src/lib/server.cc index c1db1e6ac..260f2e469 100644 --- a/src/lib/server.cc +++ b/src/lib/server.cc @@ -89,7 +89,9 @@ Server::~Server () _io_service.stop (); _broadcast.io_service.stop (); - _broadcast.thread->join (); + if (_broadcast.thread) { + _broadcast.thread->join (); + } } /** @param after_read Filled in with gettimeofday() after reading the input from the network. -- cgit v1.2.3 From b7ef3e79b9eb4f245556e69c68ee624c5b43e126 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Thu, 24 Sep 2015 16:03:06 +0100 Subject: assert (joinable) before joining threads; fix possible crash after cancelling jobs. --- src/lib/job.cc | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'src/lib') diff --git a/src/lib/job.cc b/src/lib/job.cc index 439000e58..7aaac748c 100644 --- a/src/lib/job.cc +++ b/src/lib/job.cc @@ -59,6 +59,7 @@ Job::~Job () { if (_thread) { _thread->interrupt (); + DCPOMATIC_ASSERT (_thread->joinable ()); _thread->join (); } @@ -416,7 +417,10 @@ Job::cancel () } _thread->interrupt (); + DCPOMATIC_ASSERT (_thread->joinable ()); _thread->join (); + delete _thread; + _thread = 0; } void -- cgit v1.2.3 From e975b14d2c962eab149f56a79c35b68b608226d4 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Thu, 24 Sep 2015 16:03:21 +0100 Subject: assert (joinable) before joining threads. --- src/lib/job_manager.cc | 1 + src/lib/server.cc | 8 +++++--- src/lib/server_finder.cc | 2 ++ src/lib/update_checker.cc | 1 + src/lib/writer.cc | 1 + 5 files changed, 10 insertions(+), 3 deletions(-) (limited to 'src/lib') diff --git a/src/lib/job_manager.cc b/src/lib/job_manager.cc index 545fd956c..86e010c10 100644 --- a/src/lib/job_manager.cc +++ b/src/lib/job_manager.cc @@ -62,6 +62,7 @@ JobManager::~JobManager () } if (_scheduler) { + DCPOMATIC_ASSERT (_scheduler->joinable ()); _scheduler->join (); } diff --git a/src/lib/server.cc b/src/lib/server.cc index 260f2e469..dd076a049 100644 --- a/src/lib/server.cc +++ b/src/lib/server.cc @@ -81,15 +81,17 @@ Server::~Server () _full_condition.notify_all (); } - for (vector::iterator i = _worker_threads.begin(); i != _worker_threads.end(); ++i) { - (*i)->join (); - delete *i; + BOOST_FOREACH (boost::thread* i, _worker_threads) { + DCPOMATIC_ASSERT (i->joinable ()); + i->join (); + delete i; } _io_service.stop (); _broadcast.io_service.stop (); if (_broadcast.thread) { + DCPOMATIC_ASSERT (_broadcast.thread->join ()); _broadcast.thread->join (); } } diff --git a/src/lib/server_finder.cc b/src/lib/server_finder.cc index 4b532f981..642767e8b 100644 --- a/src/lib/server_finder.cc +++ b/src/lib/server_finder.cc @@ -63,11 +63,13 @@ ServerFinder::~ServerFinder () _search_condition.notify_all (); if (_search_thread) { + DCPOMATIC_ASSERT (_search_thread->joinable ()); _search_thread->join (); } _listen_io_service.stop (); if (_listen_thread) { + DCPOMATIC_ASSERT (_listen_thread->joinable ()); _listen_thread->join (); } } diff --git a/src/lib/update_checker.cc b/src/lib/update_checker.cc index 4ee728fde..4c5075e20 100644 --- a/src/lib/update_checker.cc +++ b/src/lib/update_checker.cc @@ -86,6 +86,7 @@ UpdateChecker::~UpdateChecker () _condition.notify_all (); if (_thread) { + DCPOMATIC_ASSERT (_thread->joinable ()); _thread->join (); } delete _thread; diff --git a/src/lib/writer.cc b/src/lib/writer.cc index 1318cc20f..bace6602d 100644 --- a/src/lib/writer.cc +++ b/src/lib/writer.cc @@ -477,6 +477,7 @@ Writer::terminate_thread (bool can_throw) _full_condition.notify_all (); lock.unlock (); + DCPOMATIC_ASSERT (_thread->joinable ()); _thread->join (); if (can_throw) { rethrow (); -- cgit v1.2.3 From 86b6bee8957d6ec010d235f1ae83f1ec33a646c1 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Fri, 25 Sep 2015 12:40:19 +0100 Subject: Typo in joinable asserts. --- src/lib/server.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'src/lib') diff --git a/src/lib/server.cc b/src/lib/server.cc index dd076a049..f0b2d9816 100644 --- a/src/lib/server.cc +++ b/src/lib/server.cc @@ -39,6 +39,7 @@ #include #include #include +#include #include #include #include @@ -91,7 +92,7 @@ Server::~Server () _broadcast.io_service.stop (); if (_broadcast.thread) { - DCPOMATIC_ASSERT (_broadcast.thread->join ()); + DCPOMATIC_ASSERT (_broadcast.thread->joinable ()); _broadcast.thread->join (); } } -- cgit v1.2.3 From effc88be7dcf3e0848ed9dab8010e8c20cf4bb38 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Fri, 25 Sep 2015 12:45:48 +0100 Subject: Use libicu instead of uchardet and convert subrip files to UTF-8. --- ChangeLog | 5 +++++ cscript | 12 ++++++------ src/lib/subrip.cc | 46 ++++++++++++++++++++++++---------------------- wscript | 3 --- 4 files changed, 35 insertions(+), 31 deletions(-) (limited to 'src/lib') diff --git a/ChangeLog b/ChangeLog index aeb563d52..b8e85cc87 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,8 @@ +2015-09-25 Carl Hetherington + + * Detect and convert from non-UTF-8 + subtitle encodings. + 2015-09-21 Carl Hetherington * Version 2.3.5 released. diff --git a/cscript b/cscript index 6e2a62dcc..ff1a1582e 100644 --- a/cscript +++ b/cscript @@ -46,7 +46,7 @@ deb_depends['12.04'] = {'libc6': '2.15', 'libcurl3': '7.22.0-3ubuntu4', 'libzip2': '0.10-1ubuntu1', 'libsamplerate0': '0.1.8-4', - 'libuchardet0': '0.0.1-1'} + 'libicu48': '4.8.1.1-3'} deb_depends['14.04'] = {'libc6': '2.19-0ubuntu6', 'libssh-4': '0.6.1-0ubuntu3', @@ -63,7 +63,7 @@ deb_depends['14.04'] = {'libc6': '2.19-0ubuntu6', 'libcurl3': '7.35.0-1ubuntu2', 'libzip2': '0.10.1-1.2', 'libsamplerate0': '0.1.8-7', - 'libuchardet0': '0.0.1-1ubuntu1'} + 'libicu52': '52.1-3'} deb_depends['15.04'] = {'libc6': '2.21-0ubuntu4', 'libssh-4': '0.6.3-3ubuntu3', @@ -81,7 +81,7 @@ deb_depends['15.04'] = {'libc6': '2.21-0ubuntu4', 'libzip2': '0.11.2-1.2', 'libwxgtk3.0-0': '3.0.2-1', 'libsamplerate0': '0.1.8-8', - 'libuchardet0': '0.0.1-1ubuntu1'} + 'libicu52': '52.1-8'} deb_depends['7'] = {'libc6': '2.13', 'libssh-4': '0.5.4', @@ -100,7 +100,7 @@ deb_depends['7'] = {'libc6': '2.13', 'libcairomm-1.0-1': '1.10.0-1', 'libpangomm-1.4-1': '2.28.4-1', 'libsamplerate0': '0.1.8-5', - 'libuchardet': '0.0.1-1'} + 'libicu48': '4.8.1.1-12+deb7u3'} deb_depends['8'] = {'libc6': '2.19-18', 'libssh-4': '0.6.3-4', @@ -120,7 +120,7 @@ deb_depends['8'] = {'libc6': '2.19-18', 'libxcb-xfixes0': '1.10', 'libxcb-shape0': '1.10', 'libsamplerate0': '0.1.8-8', - 'libuchardet': '0.0.1-1'} + 'libicu52': '52.1-8+deb8u2'} deb_depends['unstable'] = {'libc6': '2.13', 'libssh-4': '0.5.4', @@ -137,7 +137,7 @@ deb_depends['unstable'] = {'libc6': '2.13', 'libcurl3': '7.26.0', 'libzip2': '0.10.1', 'libsamplerate0': '0.1.8-8', - 'libuchardet': '0.0.1-1'} + 'libicu52': '52.1-9'} def packages(name, packages, f): s = '%s: ' % name diff --git a/src/lib/subrip.cc b/src/lib/subrip.cc index d4adee428..6df8b236b 100644 --- a/src/lib/subrip.cc +++ b/src/lib/subrip.cc @@ -21,9 +21,11 @@ #include "cross.h" #include "exceptions.h" #include "subrip_content.h" +#include "data.h" #include #include -#include +#include +#include #include #include "i18n.h" @@ -32,34 +34,34 @@ using std::vector; using std::cout; using std::string; using boost::shared_ptr; +using boost::scoped_array; SubRip::SubRip (shared_ptr content) { - FILE* f = fopen_boost (content->path (0), "r"); - if (!f) { - throw OpenFileError (content->path (0)); - } + Data in (content->path (0)); - /* Guess the encoding */ - uchardet_t det = uchardet_new (); - char buffer[1024]; - while (!feof (f)) { - int const n = fread (buffer, 1, sizeof (buffer), f); - if (uchardet_handle_data (det, buffer, n)) { - break; - } - } + UErrorCode status = U_ZERO_ERROR; + UCharsetDetector* detector = ucsdet_open (&status); + ucsdet_setText (detector, reinterpret_cast (in.data().get()), in.size(), &status); - uchardet_data_end (det); - string charset = uchardet_get_charset (det); - uchardet_delete (det); + UCharsetMatch const * match = ucsdet_detect (detector, &status); + char const * in_charset = ucsdet_getName (match, &status); - if (charset != "UTF-8") { - throw TextEncodingError (_("unrecognised character set; please use files encoded in UTF-8")); - } + UConverter* to_utf16 = ucnv_open (in_charset, &status); + /* This is a guess; I think we should be able to encode any input in 4 times its input size */ + scoped_array utf16 (new uint16_t[in.size() * 2]); + int const utf16_len = ucnv_toUChars (to_utf16, utf16.get(), in.size() * 2, reinterpret_cast (in.data().get()), in.size(), &status); + + UConverter* to_utf8 = ucnv_open ("UTF-8", &status); + /* Another guess */ + scoped_array utf8 (new char[utf16_len * 2]); + ucnv_fromUChars (to_utf8, utf8.get(), utf16_len * 2, utf16.get(), utf16_len, &status); + + ucsdet_close (detector); + ucnv_close (to_utf16); + ucnv_close (to_utf8); - rewind (f); - sub::SubripReader reader (f); + sub::SubripReader reader (utf8.get()); _subtitles = sub::collect > (reader.subtitles ()); } diff --git a/wscript b/wscript index 8f6c487ee..245f3353c 100644 --- a/wscript +++ b/wscript @@ -159,9 +159,6 @@ def configure(conf): else: conf.check_cfg(package='libcurl', args='--cflags --libs', uselib_store='CURL', mandatory=True) - # uchardet - conf.check_cfg(package='uchardet', args='--cflags --libs', uselib_store='UCHARDET', mandatory=True) - # libsndfile conf.check_cfg(package='sndfile', args='--cflags --libs', uselib_store='SNDFILE', mandatory=True) -- cgit v1.2.3 From c982ff2fe7a956a79c4e935d78ed6b2c37752d75 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Fri, 25 Sep 2015 14:08:59 +0100 Subject: More build fixes. --- src/lib/wscript | 2 +- src/tools/wscript | 2 +- test/wscript | 2 +- wscript | 3 +++ 4 files changed, 6 insertions(+), 3 deletions(-) (limited to 'src/lib') diff --git a/src/lib/wscript b/src/lib/wscript index b615b575d..3daf3ecf4 100644 --- a/src/lib/wscript +++ b/src/lib/wscript @@ -143,7 +143,7 @@ def build(bld): AVCODEC AVUTIL AVFORMAT AVFILTER SWSCALE SWRESAMPLE BOOST_FILESYSTEM BOOST_THREAD BOOST_DATETIME BOOST_SIGNALS2 BOOST_REGEX SNDFILE SAMPLERATE OPENJPEG POSTPROC TIFF MAGICK SSH DCP CXML GLIB LZMA XML++ - CURL ZIP FONTCONFIG PANGOMM CAIROMM XMLSEC SUB UCHARDET + CURL ZIP FONTCONFIG PANGOMM CAIROMM XMLSEC SUB ICU """ if bld.env.TARGET_OSX: diff --git a/src/tools/wscript b/src/tools/wscript index b01eee7ca..72d8ac9a8 100644 --- a/src/tools/wscript +++ b/src/tools/wscript @@ -29,7 +29,7 @@ def configure(conf): def build(bld): uselib = 'BOOST_THREAD BOOST_DATETIME OPENJPEG DCP XMLSEC CXML XMLPP AVFORMAT AVFILTER AVCODEC ' uselib += 'AVUTIL SWSCALE POSTPROC CURL BOOST_FILESYSTEM SSH ZIP CAIROMM FONTCONFIG PANGOMM SUB MAGICK SNDFILE SAMPLERATE BOOST_REGEX ' - uselib += 'UCHARDET ' + uselib += 'ICU ' if bld.env.TARGET_WINDOWS: uselib += 'WINSOCK2' diff --git a/test/wscript b/test/wscript index a92e344eb..6ee995e36 100644 --- a/test/wscript +++ b/test/wscript @@ -31,7 +31,7 @@ def build(bld): obj = bld(features='cxx cxxprogram') obj.name = 'unit-tests' obj.uselib = 'BOOST_TEST BOOST_THREAD BOOST_FILESYSTEM BOOST_DATETIME SNDFILE SAMPLERATE DCP OPENJPEG FONTCONFIG CAIROMM PANGOMM XMLPP ' - obj.uselib += 'AVFORMAT AVFILTER AVCODEC AVUTIL SWSCALE POSTPROC CXML MAGICK SUB GLIB CURL SSH XMLSEC BOOST_REGEX UCHARDET ' + obj.uselib += 'AVFORMAT AVFILTER AVCODEC AVUTIL SWSCALE POSTPROC CXML MAGICK SUB GLIB CURL SSH XMLSEC BOOST_REGEX ICU ' if bld.env.TARGET_WINDOWS: obj.uselib += 'WINSOCK2' obj.use = 'libdcpomatic2' diff --git a/wscript b/wscript index 245f3353c..22ec79c26 100644 --- a/wscript +++ b/wscript @@ -159,6 +159,9 @@ def configure(conf): else: conf.check_cfg(package='libcurl', args='--cflags --libs', uselib_store='CURL', mandatory=True) + # libicu + conf.check_cfg(package='icu-io', args='--cflags --libs', uselib_store='ICU', mandatory=True) + # libsndfile conf.check_cfg(package='sndfile', args='--cflags --libs', uselib_store='SNDFILE', mandatory=True) -- cgit v1.2.3 From 526715ef11b35258737e7c367cec760a1dbd5047 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Fri, 25 Sep 2015 15:47:02 +0100 Subject: Try to fix Windows build. --- src/lib/subrip.cc | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'src/lib') diff --git a/src/lib/subrip.cc b/src/lib/subrip.cc index 6df8b236b..a707d1f9f 100644 --- a/src/lib/subrip.cc +++ b/src/lib/subrip.cc @@ -50,12 +50,16 @@ SubRip::SubRip (shared_ptr content) UConverter* to_utf16 = ucnv_open (in_charset, &status); /* This is a guess; I think we should be able to encode any input in 4 times its input size */ scoped_array utf16 (new uint16_t[in.size() * 2]); - int const utf16_len = ucnv_toUChars (to_utf16, utf16.get(), in.size() * 2, reinterpret_cast (in.data().get()), in.size(), &status); + int const utf16_len = ucnv_toUChars ( + to_utf16, reinterpret_cast(utf16.get()), in.size() * 2, + reinterpret_cast (in.data().get()), in.size(), + &status + ); UConverter* to_utf8 = ucnv_open ("UTF-8", &status); /* Another guess */ scoped_array utf8 (new char[utf16_len * 2]); - ucnv_fromUChars (to_utf8, utf8.get(), utf16_len * 2, utf16.get(), utf16_len, &status); + ucnv_fromUChars (to_utf8, utf8.get(), utf16_len * 2, reinterpret_cast(utf16.get()), utf16_len, &status); ucsdet_close (detector); ucnv_close (to_utf16); -- cgit v1.2.3 From 19483a961b274befaff263d8646a78a56c102c65 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Fri, 25 Sep 2015 22:10:35 +0100 Subject: Fix assertion failure with .MTS files. These files have subs which start but are never officially finished; this means there are no `to' times to find in find_subtitle_to. Cope with this by stopping the previous sub when a new one arrives if there hasn't been a proper "stop" in the mean time. --- ChangeLog | 2 ++ src/lib/ffmpeg_examiner.cc | 28 ++++++++++++++++++++++++---- src/lib/ffmpeg_examiner.h | 3 ++- 3 files changed, 28 insertions(+), 5 deletions(-) (limited to 'src/lib') diff --git a/ChangeLog b/ChangeLog index 2680c1793..b81fc19a5 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,7 @@ 2015-09-25 Carl Hetherington + * Fix assertion failure when loading .MTS files (#702). + * Fix incorrect hint about 3D content in a 2D DCP. * Detect and convert from non-UTF-8 diff --git a/src/lib/ffmpeg_examiner.cc b/src/lib/ffmpeg_examiner.cc index 576782d0b..6173e0415 100644 --- a/src/lib/ffmpeg_examiner.cc +++ b/src/lib/ffmpeg_examiner.cc @@ -134,6 +134,17 @@ FFmpegExaminer::FFmpegExaminer (shared_ptr c, shared_ptrsecond) { + i->first->add_subtitle ( + ContentTimePeriod ( + i->second.get (), + ContentTime::from_frames (video_length(), video_frame_rate().get_value_or (24)) + ) + ); + } + } } void @@ -176,14 +187,23 @@ FFmpegExaminer::subtitle_packet (AVCodecContext* context, shared_ptr= 0 && frame_finished) { FFmpegSubtitlePeriod const period = subtitle_period (sub); - if (sub.num_rects <= 0 && _last_subtitle_start) { - stream->add_subtitle (ContentTimePeriod (_last_subtitle_start.get (), period.from)); - _last_subtitle_start = optional (); + 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)); + if (sub.num_rects == 0) { + /* This is a `proper' end-of-subtitle */ + _last_subtitle_start[stream] = optional (); + } else { + /* This is just another subtitle, so we start again */ + _last_subtitle_start[stream] = period.from; + } } else if (sub.num_rects == 1) { if (period.to) { stream->add_subtitle (ContentTimePeriod (period.from, period.to.get ())); } else { - _last_subtitle_start = period.from; + _last_subtitle_start[stream] = period.from; } } avsubtitle_free (&sub); diff --git a/src/lib/ffmpeg_examiner.h b/src/lib/ffmpeg_examiner.h index 6fd3220d4..27bff08b4 100644 --- a/src/lib/ffmpeg_examiner.h +++ b/src/lib/ffmpeg_examiner.h @@ -85,5 +85,6 @@ private: Frame _video_length; bool _need_video_length; - boost::optional _last_subtitle_start; + typedef std::map, boost::optional > LastSubtitleMap; + LastSubtitleMap _last_subtitle_start; }; -- cgit v1.2.3 From 924dd6e5356f401c325a42ccb43607b79027bb59 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Sat, 26 Sep 2015 23:28:06 +0100 Subject: const fix. --- src/lib/dcpomatic_time.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/lib') diff --git a/src/lib/dcpomatic_time.h b/src/lib/dcpomatic_time.h index 792eb2c97..7d755a46c 100644 --- a/src/lib/dcpomatic_time.h +++ b/src/lib/dcpomatic_time.h @@ -116,7 +116,7 @@ public: * at some sampling rate. * @param r Sampling rate. */ - Time round_up (float r) { + Time round_up (float r) const { Type const n = llrintf (HZ / r); Type const a = _t + n - 1; return Time (a - (a % n)); -- cgit v1.2.3 From 9d4678143d1fa992059c90a2b0997fa5cae81e93 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Sat, 26 Sep 2015 23:33:28 +0100 Subject: 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. --- ChangeLog | 3 +++ src/lib/ffmpeg.cc | 56 +++++++++++++++++++++++++++++++++++++++ src/lib/ffmpeg.h | 5 ++++ src/lib/ffmpeg_decoder.cc | 46 ++------------------------------ src/lib/ffmpeg_examiner.cc | 11 ++++++++ src/lib/ffmpeg_subtitle_stream.cc | 11 ++++++++ src/lib/ffmpeg_subtitle_stream.h | 2 +- 7 files changed, 89 insertions(+), 45 deletions(-) (limited to 'src/lib') diff --git a/ChangeLog b/ChangeLog index a9af721eb..60b625442 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,8 @@ 2015-09-26 Carl Hetherington + * Fix crash with embedded subtitles with some + video files. + * Version 2.3.6 released. 2015-09-25 Carl Hetherington diff --git a/src/lib/ffmpeg.cc b/src/lib/ffmpeg.cc index 8fa03fdff..de08e9df9 100644 --- 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 @@ -32,6 +33,7 @@ extern "C" { #include } #include +#include #include #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 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 > audio_streams, optional 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 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 961f5cbb1..b3bc13e5c 100644 --- 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 > audio_streams, boost::optional first_video, double video_frame_rate + ) const; + static FFmpegSubtitlePeriod subtitle_period (AVSubtitle const &); boost::shared_ptr _ffmpeg_content; diff --git a/src/lib/ffmpeg_decoder.cc b/src/lib/ffmpeg_decoder.cc index d343ec317..475418d3d 100644 --- a/src/lib/ffmpeg_decoder.cc +++ b/src/lib/ffmpeg_decoder.cc @@ -67,51 +67,9 @@ FFmpegDecoder::FFmpegDecoder (shared_ptr c, shared_ptr , 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 > streams = c->ffmpeg_audio_streams (); - - _pts_offset = ContentTime::min (); - - if (c->first_video ()) { - _pts_offset = - c->first_video().get (); - } - - BOOST_FOREACH (shared_ptr 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 6173e0415..40fe0d28e 100644 --- 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 #include #include "i18n.h" @@ -145,6 +146,16 @@ FFmpegExaminer::FFmpegExaminer (shared_ptr c, shared_ptr 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 27099b0f3..e12075581 100644 --- 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 fixed; + for (map::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 a39b10ffd..0809b359a 100644 --- 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 subtitles_during (ContentTimePeriod period, bool starting) const; ContentTime find_subtitle_to (ContentTime from) const; + void add_offset (ContentTime offset); private: std::map _subtitles; }; - -- cgit v1.2.3 From 9e171f83c256d2386d6ad4ce48bc06a1827ed724 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Sun, 27 Sep 2015 00:40:52 +0100 Subject: Fix error in previous. --- src/lib/ffmpeg.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'src/lib') diff --git a/src/lib/ffmpeg.cc b/src/lib/ffmpeg.cc index de08e9df9..9d5aa92ec 100644 --- a/src/lib/ffmpeg.cc +++ b/src/lib/ffmpeg.cc @@ -314,7 +314,8 @@ FFmpeg::pts_offset (vector > audio_streams, option /* 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(); + ContentTime fvc = first_video.get() + po; + po += fvc.round_up (video_frame_rate) - fvc; } return po; -- cgit v1.2.3 From c0ab72986401e62f51bc1ae22c2c2385a94a887d Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Sun, 27 Sep 2015 00:41:45 +0100 Subject: const fix. --- src/lib/ffmpeg.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/lib') diff --git a/src/lib/ffmpeg.cc b/src/lib/ffmpeg.cc index 9d5aa92ec..29dee2c86 100644 --- a/src/lib/ffmpeg.cc +++ b/src/lib/ffmpeg.cc @@ -314,7 +314,7 @@ FFmpeg::pts_offset (vector > audio_streams, option /* Now adjust so that the video pts starts on a frame */ if (first_video) { - ContentTime fvc = first_video.get() + po; + ContentTime const fvc = first_video.get() + po; po += fvc.round_up (video_frame_rate) - fvc; } -- cgit v1.2.3