More correctly calculate bitmap subtitle scaling (#2670).

This was partially fixed before in
6ac468554c7fea0dfaefde85fb6cdd0fceaf5cad

The last try accounted for cropping, but not for cases where the
source video (after crop) does not precisely fit the DCP container.
In those cases the x scale for the subtitles could be different
to the y scale, squashing or stretching them.

diff --git a/src/lib/bitmap_text.h b/src/lib/bitmap_text.h
index 04cb61e3ea647c0ea1bbc00f7b2e91c2c28abc2b..50f3f507ad6cd9393bd3581e62e6d7f81cfe1446 100644 (file)
--- a/src/lib/bitmap_text.h
+++ b/src/lib/bitmap_text.h
@@ -23,6 +23,7 @@
 
 
 #include "rect.h"
+#include <dcp/types.h>
 #include <memory>
 
 
@@ -31,11 +32,14 @@ class Image;
 class BitmapText
 {
 public:
-       BitmapText(std::shared_ptr<const Image> image_, dcpomatic::Rect<double> rectangle_)
-               : image(image_)
+       BitmapText(dcp::Size parent_size_, std::shared_ptr<const Image> image_, dcpomatic::Rect<double> rectangle_)
+               : parent_size(parent_size_)
+               , image(image_)
                , rectangle(rectangle_)
        {}
 
+       /** Size of the image that this subtitle has come from, with any crop already applied */
+       dcp::Size parent_size;
        std::shared_ptr<const Image> image;
        /** Area that the subtitle covers on its corresponding video, expressed in
         *  proportions of the image size; e.g. rectangle.x = 0.5 would mean that

diff --git a/src/lib/content_text.h b/src/lib/content_text.h
index 8a7f007ef1e1a8d6c2472c510835bbfba58cc23c..d04383c8b669cbc86585f259ab230e0d96d6f955 100644 (file)
--- a/src/lib/content_text.h
+++ b/src/lib/content_text.h
@@ -57,9 +57,9 @@ public:
                : ContentText(from)
        {}
 
-       ContentBitmapText(dcpomatic::ContentTime from, std::shared_ptr<const Image> image, dcpomatic::Rect<double> rect)
+       ContentBitmapText(dcpomatic::ContentTime from, dcp::Size parent_size, std::shared_ptr<const Image> image, dcpomatic::Rect<double> rect)
                : ContentText(from)
-               , subs{ {image, rect} }
+               , subs{ { parent_size, image, rect } }
        {}
 
        /* Our texts, with their rectangles unmodified by any offsets or scales that the content specifies */

diff --git a/src/lib/ffmpeg_decoder.cc b/src/lib/ffmpeg_decoder.cc
index 7f7a078638b540ba81bbbe38c5ad260970232fa0..ab3f0b5be05e817cc0189c539d33407162c89e60 100644 (file)
--- a/src/lib/ffmpeg_decoder.cc
+++ b/src/lib/ffmpeg_decoder.cc
@@ -770,39 +770,41 @@ FFmpegDecoder::process_bitmap_subtitle (AVSubtitleRect const * rect)
                out_p += image->stride()[0];
        }
 
-       int target_width = subtitle_codec_context()->width;
-       if (target_width == 0 && video_codec_context()) {
-               /* subtitle_codec_context()->width == 0 has been seen in the wild but I don't
-                  know if it's supposed to mean something from FFmpeg's point of view.
-               */
-               target_width = video_codec_context()->width;
+       optional<dcp::Size> video_size;
+       if (_ffmpeg_content->video) {
+               video_size = _ffmpeg_content->video->size();
        }
-       int target_height = subtitle_codec_context()->height;
-       if (target_height == 0 && video_codec_context()) {
-               target_height = video_codec_context()->height;
+
+       dcp::Size target_size = { subtitle_codec_context()->width, subtitle_codec_context()->height };
+       if (target_size.width == 0 || target_size.height == 0 || (video_size && *video_size == target_size)) {
+               /* Either the subtitle codec has no specified size, or it's the same as the video.
+                * In either case we'll use the target size once it has been cropped etc. as we
+                * assume that whatever happens to the video should also be done to the subtitles.
+                */
+               if (auto s = ffmpeg_content()->video->scaled_size(film()->frame_size())) {
+                       target_size = *s;
+               }
        }
 
        int x_offset = 0;
        int y_offset = 0;
        if (_ffmpeg_content->video && _ffmpeg_content->video->use()) {
                auto const crop = _ffmpeg_content->video->actual_crop();
-               target_width -= crop.left + crop.right;
-               target_height -= crop.top + crop.bottom;
                x_offset = -crop.left;
                y_offset = -crop.top;
        }
 
-       DCPOMATIC_ASSERT(target_width > 0);
-       DCPOMATIC_ASSERT(target_height > 0);
+       DCPOMATIC_ASSERT(target_size.width > 0);
+       DCPOMATIC_ASSERT(target_size.height > 0);
 
        dcpomatic::Rect<double> const scaled_rect (
-               static_cast<double>(rect->x + x_offset) / target_width,
-               static_cast<double>(rect->y + y_offset) / target_height,
-               static_cast<double>(rect->w) / target_width,
-               static_cast<double>(rect->h) / target_height
+               static_cast<double>(rect->x + x_offset) / target_size.width,
+               static_cast<double>(rect->y + y_offset) / target_size.height,
+               static_cast<double>(rect->w) / target_size.width,
+               static_cast<double>(rect->h) / target_size.height
                );
 
-       return { image, scaled_rect };
+       return { target_size, image, scaled_rect };
 }
 
 

diff --git a/src/lib/player.cc b/src/lib/player.cc
index c03cb97a59f50ad8549f8142279e48a236a801dc..1b5eca65cf379dd08243c884fd8d0e4930b7496a 100644 (file)
--- a/src/lib/player.cc
+++ b/src/lib/player.cc
@@ -1233,7 +1233,8 @@ Player::bitmap_text_start (weak_ptr<Piece> weak_piece, weak_ptr<const TextConten
 
        auto piece = weak_piece.lock ();
        auto content = weak_content.lock ();
-       if (!piece || !content) {
+       auto film = _film.lock();
+       if (!piece || !content || !film) {
                return;
        }
 
@@ -1254,15 +1255,16 @@ Player::bitmap_text_start (weak_ptr<Piece> weak_piece, weak_ptr<const TextConten
 
                auto image = sub.image;
 
-               /* We will scale the subtitle up to fit _video_container_size */
-               int const width = sub.rectangle.width * _video_container_size.load().width;
-               int const height = sub.rectangle.height * _video_container_size.load().height;
+               auto const inter_size = scale_for_display(sub.parent_size, _video_container_size.load(), film->frame_size(), {});
+
+               int const width = sub.rectangle.width * inter_size.width;
+               int const height = sub.rectangle.height * inter_size.height;
                if (width == 0 || height == 0) {
                        return;
                }
 
                dcp::Size scaled_size (width, height);
-               ps.bitmap.push_back (BitmapText(image->scale(scaled_size, dcp::YUVToRGB::REC601, image->pixel_format(), Image::Alignment::PADDED, _fast), sub.rectangle));
+               ps.bitmap.push_back(BitmapText(sub.parent_size, image->scale(scaled_size, dcp::YUVToRGB::REC601, image->pixel_format(), Image::Alignment::PADDED, _fast), sub.rectangle));
        }
 
        DCPTime from(content_time_to_dcp(piece, subtitle.from()));

diff --git a/src/lib/text_decoder.cc b/src/lib/text_decoder.cc
index 75fa33605b0f2f3188f1fc47141a0d609cebd632..894145c355dcf225baf19662dd9bbe5d3cc4b809 100644 (file)
--- a/src/lib/text_decoder.cc
+++ b/src/lib/text_decoder.cc
@@ -352,9 +352,9 @@ TextDecoder::emit_plain (ContentTimePeriod period, sub::Subtitle const& subtitle
  *  of the video frame)
  */
 void
-TextDecoder::emit_bitmap (ContentTimePeriod period, shared_ptr<const Image> image, dcpomatic::Rect<double> rect)
+TextDecoder::emit_bitmap(ContentTimePeriod period, dcp::Size parent_size, shared_ptr<const Image> image, dcpomatic::Rect<double> rect)
 {
-       emit_bitmap_start ({ period.from, image, rect });
+       emit_bitmap_start ({ period.from, parent_size, image, rect });
        emit_stop (period.to);
 }
 

diff --git a/src/lib/text_decoder.h b/src/lib/text_decoder.h
index 1a7632fd82b8448366dddc0676531c8b04fd9a27..47c1be3996b5ebfa8df05fb6ec7d615664e09c7b 100644 (file)
--- a/src/lib/text_decoder.h
+++ b/src/lib/text_decoder.h
@@ -51,7 +51,7 @@ public:
        }
 
        void emit_bitmap_start (ContentBitmapText const& bitmap);
-       void emit_bitmap (dcpomatic::ContentTimePeriod period, std::shared_ptr<const Image> image, dcpomatic::Rect<double> rect);
+       void emit_bitmap(dcpomatic::ContentTimePeriod period, dcp::Size parent_size, std::shared_ptr<const Image> image, dcpomatic::Rect<double> rect);
        void emit_plain_start(dcpomatic::ContentTime from, std::vector<dcp::SubtitleString> s, dcp::SubtitleStandard valign_standard);
        void emit_plain_start (dcpomatic::ContentTime from, sub::Subtitle const & subtitle);
        void emit_plain(dcpomatic::ContentTimePeriod period, std::vector<dcp::SubtitleString> s, dcp::SubtitleStandard valign_standard);