From: Carl Hetherington <cth@carlh.net>
Date: Fri, 12 Sep 2014 22:16:31 +0000 (+0100)
Subject: Merge master.
X-Git-Tag: v2.0.48~591
X-Git-Url: https://git.carlh.net/gitweb/?p=dcpomatic.git;a=commitdiff_plain;h=3e12c68dc0451e73b5bc1a84d1d70f4999f7b4b5

Merge master.
---

3e12c68dc0451e73b5bc1a84d1d70f4999f7b4b5
diff --cc ChangeLog
index 183339f61,6803d355c..222a0eea1
--- a/ChangeLog
+++ b/ChangeLog
@@@ -1,18 -1,7 +1,20 @@@
  2014-09-12  Carl Hetherington  <cth@carlh.net>
  
 +	* Version 2.0.9 released.
 +
 +2014-09-12  Carl Hetherington  <cth@carlh.net>
 +
 +	* Add "re-examine" option to content context menu (#339).
 +
 +2014-09-11  Carl Hetherington  <cth@carlh.net>
 +
 +	* Restore encoding optimisations for still-image sources.
 +
 +	* Add option to re-make signing chain with specified organisation,
 +	common names etc. (#354)
 +
+ 	* Allow separate X and Y scale for subtitles (#337).
+ 
  2014-09-10  Carl Hetherington  <cth@carlh.net>
  
  	* Allow DCP names to be created using the ISDCF template and then
diff --cc src/lib/film.cc
index 475dd6844,54503ef72..268109921
--- a/src/lib/film.cc
+++ b/src/lib/film.cc
@@@ -91,12 -91,11 +91,14 @@@ using dcp::raw_convert
   * 7 -> 8
   * Use <Scale> tag in <VideoContent> rather than <Ratio>.
   * 8 -> 9
 - * DCI -> ISDCF.
 + * DCI -> ISDCF
+  * 9 -> 10
+  * Subtitle X and Y scale.
 + *
 + * Bumped to 32 for 2.0 branch; some times are expressed in Times rather
 + * than frames now.
   */
 -int const Film::current_state_version = 10;
 +int const Film::current_state_version = 32;
  
  /** Construct a Film object in a given directory.
   *
diff --cc src/lib/player.cc
index e46d539f8,8063d1212..f83c9563b
--- a/src/lib/player.cc
+++ b/src/lib/player.cc
@@@ -110,179 -110,244 +110,180 @@@ Player::setup_pieces (
  			}
  		}
  
 -		if (_audio && ad && ad->has_audio ()) {
 -			if ((*i)->audio_position < earliest_t) {
 -				earliest_t = (*i)->audio_position;
 -				earliest = *i;
 -				type = AUDIO;
 -			}
 +		optional<FrameRateChange> best_overlap_frc;
 +		if (best_overlap) {
 +			best_overlap_frc = FrameRateChange (best_overlap->video_frame_rate(), _film->video_frame_rate ());
 +		} else {
 +			/* No video overlap; e.g. if the DCP is just audio */
 +			best_overlap_frc = FrameRateChange (_film->video_frame_rate(), _film->video_frame_rate ());
  		}
 -	}
  
 -	if (!earliest) {
 -		flush ();
 -		return true;
 -	}
 +		/* FFmpeg */
 +		shared_ptr<const FFmpegContent> fc = dynamic_pointer_cast<const FFmpegContent> (*i);
 +		if (fc) {
 +			decoder.reset (new FFmpegDecoder (fc, _film->log()));
 +			frc = FrameRateChange (fc->video_frame_rate(), _film->video_frame_rate());
 +		}
  
 -	switch (type) {
 -	case VIDEO:
 -		if (earliest_t > _video_position) {
 -			emit_black ();
 -		} else {
 -			if (earliest->repeating ()) {
 -				earliest->repeat (this);
 -			} else {
 -				earliest->decoder->pass ();
 -			}
 +		shared_ptr<const DCPContent> dc = dynamic_pointer_cast<const DCPContent> (*i);
 +		if (dc) {
 +			decoder.reset (new DCPDecoder (dc, _film->log ()));
 +			frc = FrameRateChange (dc->video_frame_rate(), _film->video_frame_rate());
  		}
 -		break;
  
 -	case AUDIO:
 -		if (earliest_t > _audio_position) {
 -			emit_silence (_film->time_to_audio_frames (earliest_t - _audio_position));
 -		} else {
 -			earliest->decoder->pass ();
 -
 -			if (earliest->decoder->done()) {
 -				shared_ptr<AudioContent> ac = dynamic_pointer_cast<AudioContent> (earliest->content);
 -				assert (ac);
 -				shared_ptr<Resampler> re = resampler (ac, false);
 -				if (re) {
 -					shared_ptr<const AudioBuffers> b = re->flush ();
 -					if (b->frames ()) {
 -						process_audio (
 -							earliest,
 -							b,
 -							ac->audio_length() * ac->output_audio_frame_rate() / ac->content_audio_frame_rate(),
 -							true
 -							);
 -					}
 +		/* ImageContent */
 +		shared_ptr<const ImageContent> ic = dynamic_pointer_cast<const ImageContent> (*i);
 +		if (ic) {
 +			/* See if we can re-use an old ImageDecoder */
 +			for (list<shared_ptr<Piece> >::const_iterator j = old_pieces.begin(); j != old_pieces.end(); ++j) {
 +				shared_ptr<ImageDecoder> imd = dynamic_pointer_cast<ImageDecoder> ((*j)->decoder);
 +				if (imd && imd->content() == ic) {
 +					decoder = imd;
  				}
  			}
 -		}
 -		break;
 -	}
  
 -	if (_audio) {
 -		boost::optional<Time> audio_done_up_to;
 -		for (list<shared_ptr<Piece> >::iterator i = _pieces.begin(); i != _pieces.end(); ++i) {
 -			if ((*i)->decoder->done ()) {
 -				continue;
 +			if (!decoder) {
 +				decoder.reset (new ImageDecoder (ic));
  			}
  
 -			shared_ptr<AudioDecoder> ad = dynamic_pointer_cast<AudioDecoder> ((*i)->decoder);
 -			if (ad && ad->has_audio ()) {
 -				audio_done_up_to = min (audio_done_up_to.get_value_or (TIME_MAX), (*i)->audio_position);
 -			}
 +			frc = FrameRateChange (ic->video_frame_rate(), _film->video_frame_rate());
  		}
  
 -		if (audio_done_up_to) {
 -			TimedAudioBuffers<Time> tb = _audio_merger.pull (audio_done_up_to.get ());
 -			Audio (tb.audio, tb.time);
 -			_audio_position += _film->audio_frames_to_time (tb.audio->frames ());
 +		/* SndfileContent */
 +		shared_ptr<const SndfileContent> sc = dynamic_pointer_cast<const SndfileContent> (*i);
 +		if (sc) {
 +			decoder.reset (new SndfileDecoder (sc));
 +			frc = best_overlap_frc;
  		}
 -	}
 -		
 -	return false;
 -}
 -
 -/** @param extra Amount of extra time to add to the content frame's time (for repeat) */
 -void
 -Player::process_video (weak_ptr<Piece> weak_piece, shared_ptr<const ImageProxy> image, Eyes eyes, Part part, bool same, VideoContent::Frame frame, Time extra)
 -{
 -	/* Keep a note of what came in so that we can repeat it if required */
 -	_last_incoming_video.weak_piece = weak_piece;
 -	_last_incoming_video.image = image;
 -	_last_incoming_video.eyes = eyes;
 -	_last_incoming_video.part = part;
 -	_last_incoming_video.same = same;
 -	_last_incoming_video.frame = frame;
 -	_last_incoming_video.extra = extra;
 -	
 -	shared_ptr<Piece> piece = weak_piece.lock ();
 -	if (!piece) {
 -		return;
 -	}
  
 -	shared_ptr<VideoContent> content = dynamic_pointer_cast<VideoContent> (piece->content);
 -	assert (content);
 -
 -	FrameRateChange frc (content->video_frame_rate(), _film->video_frame_rate());
 -	if (frc.skip && (frame % 2) == 1) {
 -		return;
 -	}
 -
 -	Time const relative_time = (frame * frc.factor() * TIME_HZ / _film->video_frame_rate());
 -	if (content->trimmed (relative_time)) {
 -		return;
 -	}
 -
 -	Time const time = content->position() + relative_time + extra - content->trim_start ();
 -	libdcp::Size const image_size = content->scale().size (content, _video_container_size, _film->frame_size ());
 -
 -	shared_ptr<PlayerVideoFrame> pi (
 -		new PlayerVideoFrame (
 -			image,
 -			content->crop(),
 -			image_size,
 -			_video_container_size,
 -			_film->scaler(),
 -			eyes,
 -			part,
 -			content->colour_conversion()
 -			)
 -		);
 -	
 -	if (_film->with_subtitles ()) {
 -		for (list<Subtitle>::const_iterator i = _subtitles.begin(); i != _subtitles.end(); ++i) {
 -			if (i->covers (time)) {
 -				/* This may be true for more than one of _subtitles, but the last (latest-starting)
 -				   one is the one we want to use, so that's ok.
 -				*/
 -				Position<int> const container_offset (
 -					(_video_container_size.width - image_size.width) / 2,
 -					(_video_container_size.height - image_size.width) / 2
 -					);
 -				
 -				pi->set_subtitle (i->out_image(), i->out_position() + container_offset);
 -			}
 +		/* SubRipContent */
 +		shared_ptr<const SubRipContent> rc = dynamic_pointer_cast<const SubRipContent> (*i);
 +		if (rc) {
 +			decoder.reset (new SubRipDecoder (rc));
 +			frc = best_overlap_frc;
  		}
 -	}
  
 -	/* Clear out old subtitles */
 -	for (list<Subtitle>::iterator i = _subtitles.begin(); i != _subtitles.end(); ) {
 -		list<Subtitle>::iterator j = i;
 -		++j;
 -		
 -		if (i->ends_before (time)) {
 -			_subtitles.erase (i);
 +		/* DCPSubtitleContent */
 +		shared_ptr<const DCPSubtitleContent> dsc = dynamic_pointer_cast<const DCPSubtitleContent> (*i);
 +		if (dsc) {
 +			decoder.reset (new DCPSubtitleDecoder (dsc));
 +			frc = best_overlap_frc;
  		}
  
 -		i = j;
 +		_pieces.push_back (shared_ptr<Piece> (new Piece (*i, decoder, frc.get ())));
  	}
  
 -#ifdef DCPOMATIC_DEBUG
 -	_last_video = piece->content;
 -#endif
 +	_have_valid_pieces = true;
 +}
  
 -	Video (pi, same, time);
 +void
 +Player::content_changed (weak_ptr<Content> w, int property, bool frequent)
 +{
 +	shared_ptr<Content> c = w.lock ();
 +	if (!c) {
 +		return;
 +	}
  
 -	_last_emit_was_black = false;
 -	_video_position = piece->video_position = (time + TIME_HZ / _film->video_frame_rate());
 +	if (
 +		property == ContentProperty::POSITION ||
 +		property == ContentProperty::LENGTH ||
 +		property == ContentProperty::TRIM_START ||
 +		property == ContentProperty::TRIM_END ||
 +		property == ContentProperty::PATH ||
 +		property == VideoContentProperty::VIDEO_FRAME_TYPE ||
 +		property == DCPContentProperty::CAN_BE_PLAYED
 +		) {
 +		
 +		_have_valid_pieces = false;
 +		Changed (frequent);
  
 -	if (frc.repeat > 1 && !piece->repeating ()) {
 -		piece->set_repeat (_last_incoming_video, frc.repeat - 1);
 +	} else if (
 +		property == SubtitleContentProperty::USE_SUBTITLES ||
 +		property == SubtitleContentProperty::SUBTITLE_X_OFFSET ||
 +		property == SubtitleContentProperty::SUBTITLE_Y_OFFSET ||
- 		property == SubtitleContentProperty::SUBTITLE_SCALE ||
++		property == SubtitleContentProperty::SUBTITLE_X_SCALE ||
++		property == SubtitleContentProperty::SUBTITLE_Y_SCALE ||
 +		property == VideoContentProperty::VIDEO_CROP ||
 +		property == VideoContentProperty::VIDEO_SCALE ||
 +		property == VideoContentProperty::VIDEO_FRAME_RATE
 +		) {
 +		
 +		Changed (frequent);
  	}
  }
  
  /** @param already_resampled true if this data has already been through the chain up to the resampler */
  void
 -Player::process_audio (weak_ptr<Piece> weak_piece, shared_ptr<const AudioBuffers> audio, AudioContent::Frame frame, bool already_resampled)
 +Player::playlist_changed ()
  {
 -	shared_ptr<Piece> piece = weak_piece.lock ();
 -	if (!piece) {
 -		return;
 -	}
 +	_have_valid_pieces = false;
 +	Changed (false);
 +}
  
 -	shared_ptr<AudioContent> content = dynamic_pointer_cast<AudioContent> (piece->content);
 -	assert (content);
 +void
 +Player::set_video_container_size (dcp::Size s)
 +{
 +	_video_container_size = s;
  
 -	if (!already_resampled) {
 -		/* Gain */
 -		if (content->audio_gain() != 0) {
 -			shared_ptr<AudioBuffers> gain (new AudioBuffers (audio));
 -			gain->apply_gain (content->audio_gain ());
 -			audio = gain;
 -		}
 -		
 -		/* Resample */
 -		if (content->content_audio_frame_rate() != content->output_audio_frame_rate()) {
 -			shared_ptr<Resampler> r = resampler (content, true);
 -			pair<shared_ptr<const AudioBuffers>, AudioContent::Frame> ro = r->run (audio, frame);
 -			audio = ro.first;
 -			frame = ro.second;
 -		}
 -	}
 -	
 -	Time const relative_time = _film->audio_frames_to_time (frame);
 +	_black_image.reset (new Image (PIX_FMT_RGB24, _video_container_size, true));
 +	_black_image->make_black ();
 +}
  
 -	if (content->trimmed (relative_time)) {
 -		return;
 -	}
 +void
 +Player::film_changed (Film::Property p)
 +{
 +	/* Here we should notice Film properties that affect our output, and
 +	   alert listeners that our output now would be different to how it was
 +	   last time we were run.
 +	*/
  
 -	Time time = content->position() + (content->audio_delay() * TIME_HZ / 1000) + relative_time - content->trim_start ();
 -	
 -	/* Remap channels */
 -	shared_ptr<AudioBuffers> dcp_mapped (new AudioBuffers (_film->audio_channels(), audio->frames()));
 -	dcp_mapped->make_silent ();
 -
 -	AudioMapping map = content->audio_mapping ();
 -	for (int i = 0; i < map.content_channels(); ++i) {
 -		for (int j = 0; j < _film->audio_channels(); ++j) {
 -			if (map.get (i, static_cast<libdcp::Channel> (j)) > 0) {
 -				dcp_mapped->accumulate_channel (
 -					audio.get(),
 -					i,
 -					static_cast<libdcp::Channel> (j),
 -					map.get (i, static_cast<libdcp::Channel> (j))
 -					);
 -			}
 -		}
 +	if (p == Film::SCALER || p == Film::CONTAINER || p == Film::VIDEO_FRAME_RATE) {
 +		Changed (false);
  	}
 +}
  
 -	audio = dcp_mapped;
 -
 -	/* We must cut off anything that comes before the start of all time */
 -	if (time < 0) {
 -		int const frames = - time * _film->audio_frame_rate() / TIME_HZ;
 -		if (frames >= audio->frames ()) {
 -			return;
 +list<PositionImage>
 +Player::transform_image_subtitles (list<ImageSubtitle> subs) const
 +{
 +	list<PositionImage> all;
 +	
 +	for (list<ImageSubtitle>::const_iterator i = subs.begin(); i != subs.end(); ++i) {
 +		if (!i->image) {
 +			continue;
  		}
  
 -		shared_ptr<AudioBuffers> trimmed (new AudioBuffers (audio->channels(), audio->frames() - frames));
 -		trimmed->copy_from (audio.get(), audio->frames() - frames, frames, 0);
 -
 -		audio = trimmed;
 -		time = 0;
 +		/* We will scale the subtitle up to fit _video_container_size */
 +		dcp::Size scaled_size (i->rectangle.width * _video_container_size.width, i->rectangle.height * _video_container_size.height);
 +		
 +		/* Then we need a corrective translation, consisting of two parts:
 +		 *
 +		 * 1.  that which is the result of the scaling of the subtitle by _video_container_size; this will be
 +		 *     rect.x * _video_container_size.width and rect.y * _video_container_size.height.
 +		 *
 +		 * 2.  that to shift the origin of the scale by subtitle_scale to the centre of the subtitle; this will be
- 		 *     (width_before_subtitle_scale * (1 - subtitle_scale) / 2) and
- 		 *     (height_before_subtitle_scale * (1 - subtitle_scale) / 2).
++		 *     (width_before_subtitle_scale * (1 - subtitle_x_scale) / 2) and
++		 *     (height_before_subtitle_scale * (1 - subtitle_y_scale) / 2).
 +		 *
 +		 * Combining these two translations gives these expressions.
 +		 */
 +
 +		all.push_back (
 +			PositionImage (
 +				i->image->scale (
 +					scaled_size,
 +					Scaler::from_id ("bicubic"),
 +					i->image->pixel_format (),
 +					true
 +					),
 +				Position<int> (
 +					rint (_video_container_size.width * i->rectangle.x),
 +					rint (_video_container_size.height * i->rectangle.y)
 +					)
 +				)
 +			);
  	}
  
 -	_audio_merger.push (audio, time);
 -	piece->audio_position += _film->audio_frames_to_time (audio->frames ());
 +	return all;
  }
  
  void
@@@ -520,59 -572,76 +521,59 @@@ Player::dcp_to_content_subtitle (shared
  }
  
  void
 -Player::emit_black ()
 +PlayerStatistics::dump (shared_ptr<Log> log) const
  {
 -#ifdef DCPOMATIC_DEBUG
 -	_last_video.reset ();
 -#endif
 -
 -	Video (_black_frame, _last_emit_was_black, _video_position);
 -	_video_position += _film->video_frames_to_time (1);
 -	_last_emit_was_black = true;
 +	log->log (String::compose ("Video: %1 good %2 skipped %3 black %4 repeat", video.good, video.skip, video.black, video.repeat), Log::TYPE_GENERAL);
 +	log->log (String::compose ("Audio: %1 good %2 skipped %3 silence", audio.good, audio.skip, audio.silence.seconds()), Log::TYPE_GENERAL);
  }
  
 -void
 -Player::emit_silence (OutputAudioFrame most)
 +PlayerStatistics const &
 +Player::statistics () const
  {
 -	if (most == 0) {
 -		return;
 -	}
 -	
 -	OutputAudioFrame N = min (most, _film->audio_frame_rate() / 2);
 -	shared_ptr<AudioBuffers> silence (new AudioBuffers (_film->audio_channels(), N));
 -	silence->make_silent ();
 -	Audio (silence, _audio_position);
 -	_audio_position += _film->audio_frames_to_time (N);
 +	return _statistics;
  }
  
 -void
 -Player::film_changed (Film::Property p)
 +PlayerSubtitles
 +Player::get_subtitles (DCPTime time, DCPTime length, bool starting)
  {
 -	/* Here we should notice Film properties that affect our output, and
 -	   alert listeners that our output now would be different to how it was
 -	   last time we were run.
 -	*/
 +	list<shared_ptr<Piece> > subs = overlaps<SubtitleContent> (time, time + length);
  
 -	if (p == Film::SCALER || p == Film::WITH_SUBTITLES || p == Film::CONTAINER || p == Film::VIDEO_FRAME_RATE) {
 -		Changed (false);
 -	}
 -}
 +	PlayerSubtitles ps (time, length);
  
 -void
 -Player::process_subtitle (weak_ptr<Piece> weak_piece, shared_ptr<Image> image, dcpomatic::Rect<double> rect, Time from, Time to)
 -{
 -	if (!image) {
 -		/* A null image means that we should stop any current subtitles at `from' */
 -		for (list<Subtitle>::iterator i = _subtitles.begin(); i != _subtitles.end(); ++i) {
 -			i->set_stop (from);
 +	for (list<shared_ptr<Piece> >::const_iterator j = subs.begin(); j != subs.end(); ++j) {
 +		shared_ptr<SubtitleContent> subtitle_content = dynamic_pointer_cast<SubtitleContent> ((*j)->content);
 +		if (!subtitle_content->use_subtitles ()) {
 +			continue;
  		}
 -	} else {
 -		_subtitles.push_back (Subtitle (_film, _video_container_size, weak_piece, image, rect, from, to));
 -	}
 -}
  
 -/** Re-emit the last frame that was emitted, using current settings for crop, ratio, scaler and subtitles.
 - *  @return false if this could not be done.
 - */
 -bool
 -Player::repeat_last_video ()
 -{
 -	if (!_last_incoming_video.image || !_have_valid_pieces) {
 -		return false;
 -	}
 +		shared_ptr<SubtitleDecoder> subtitle_decoder = dynamic_pointer_cast<SubtitleDecoder> ((*j)->decoder);
 +		ContentTime const from = dcp_to_content_subtitle (*j, time);
 +		/* XXX: this video_frame_rate() should be the rate that the subtitle content has been prepared for */
 +		ContentTime const to = from + ContentTime::from_frames (1, _film->video_frame_rate ());
  
 -	process_video (
 -		_last_incoming_video.weak_piece,
 -		_last_incoming_video.image,
 -		_last_incoming_video.eyes,
 -		_last_incoming_video.part,
 -		_last_incoming_video.same,
 -		_last_incoming_video.frame,
 -		_last_incoming_video.extra
 -		);
 +		list<ContentImageSubtitle> image = subtitle_decoder->get_image_subtitles (ContentTimePeriod (from, to), starting);
 +		for (list<ContentImageSubtitle>::iterator i = image.begin(); i != image.end(); ++i) {
 +			
 +			/* Apply content's subtitle offsets */
 +			i->sub.rectangle.x += subtitle_content->subtitle_x_offset ();
 +			i->sub.rectangle.y += subtitle_content->subtitle_y_offset ();
 +
 +			/* Apply content's subtitle scale */
- 			i->sub.rectangle.width *= subtitle_content->subtitle_scale ();
- 			i->sub.rectangle.height *= subtitle_content->subtitle_scale ();
++			i->sub.rectangle.width *= subtitle_content->subtitle_x_scale ();
++			i->sub.rectangle.height *= subtitle_content->subtitle_y_scale ();
 +
 +			/* Apply a corrective translation to keep the subtitle centred after that scale */
- 			i->sub.rectangle.x -= i->sub.rectangle.width * (subtitle_content->subtitle_scale() - 1);
- 			i->sub.rectangle.y -= i->sub.rectangle.height * (subtitle_content->subtitle_scale() - 1);
++			i->sub.rectangle.x -= i->sub.rectangle.width * (subtitle_content->subtitle_x_scale() - 1);
++			i->sub.rectangle.y -= i->sub.rectangle.height * (subtitle_content->subtitle_y_scale() - 1);
 +			
 +			ps.image.push_back (i->sub);
 +		}
 +
 +		list<ContentTextSubtitle> text = subtitle_decoder->get_text_subtitles (ContentTimePeriod (from, to), starting);
 +		for (list<ContentTextSubtitle>::const_iterator i = text.begin(); i != text.end(); ++i) {
 +			copy (i->subs.begin(), i->subs.end(), back_inserter (ps.text));
 +		}
 +	}
  
 -	return true;
 +	return ps;
  }
diff --cc src/lib/subtitle_content.cc
index 0e5661945,3702eef41..5b370847b
--- a/src/lib/subtitle_content.cc
+++ b/src/lib/subtitle_content.cc
@@@ -35,42 -33,26 +35,46 @@@ using dcp::raw_convert
  
  int const SubtitleContentProperty::SUBTITLE_X_OFFSET = 500;
  int const SubtitleContentProperty::SUBTITLE_Y_OFFSET = 501;
- int const SubtitleContentProperty::SUBTITLE_SCALE = 502;
- int const SubtitleContentProperty::USE_SUBTITLES = 503;
+ int const SubtitleContentProperty::SUBTITLE_X_SCALE = 502;
+ int const SubtitleContentProperty::SUBTITLE_Y_SCALE = 503;
++int const SubtitleContentProperty::USE_SUBTITLES = 504;
 +
 +SubtitleContent::SubtitleContent (shared_ptr<const Film> f)
 +	: Content (f)
 +	, _use_subtitles (false)
 +	, _subtitle_x_offset (0)
 +	, _subtitle_y_offset (0)
- 	, _subtitle_scale (1)
++	, _subtitle_x_scale (1)
++	, _subtitle_y_scale (1)
 +{
 +
 +}
  
  SubtitleContent::SubtitleContent (shared_ptr<const Film> f, boost::filesystem::path p)
  	: Content (f, p)
 +	, _use_subtitles (false)
  	, _subtitle_x_offset (0)
  	, _subtitle_y_offset (0)
- 	, _subtitle_scale (1)
+ 	, _subtitle_x_scale (1)
+ 	, _subtitle_y_scale (1)
  {
  
  }
  
 -SubtitleContent::SubtitleContent (shared_ptr<const Film> f, shared_ptr<const cxml::Node> node, int version)
 +SubtitleContent::SubtitleContent (shared_ptr<const Film> f, cxml::ConstNodePtr node, int version)
  	: Content (f, node)
 +	, _use_subtitles (false)
  	, _subtitle_x_offset (0)
  	, _subtitle_y_offset (0)
- 	, _subtitle_scale (1)
+ 	, _subtitle_x_scale (1)
+ 	, _subtitle_y_scale (1)
  {
 +	if (version >= 32) {
 +		_use_subtitles = node->bool_child ("UseSubtitles");
 +	} else {
 +		_use_subtitles = false;
 +	}
 +	
  	if (version >= 7) {
  		_subtitle_x_offset = node->number_child<float> ("SubtitleXOffset");
  		_subtitle_y_offset = node->number_child<float> ("SubtitleYOffset");
@@@ -107,31 -94,21 +120,33 @@@ SubtitleContent::SubtitleContent (share
  		}
  	}
  
 +	_use_subtitles = ref->use_subtitles ();
  	_subtitle_x_offset = ref->subtitle_x_offset ();
  	_subtitle_y_offset = ref->subtitle_y_offset ();
- 	_subtitle_scale = ref->subtitle_scale ();
+ 	_subtitle_x_scale = ref->subtitle_x_scale ();
+ 	_subtitle_y_scale = ref->subtitle_y_scale ();
  }
  
  void
  SubtitleContent::as_xml (xmlpp::Node* root) const
  {
 +	root->add_child("UseSubtitles")->add_child_text (raw_convert<string> (_use_subtitles));
  	root->add_child("SubtitleXOffset")->add_child_text (raw_convert<string> (_subtitle_x_offset));
  	root->add_child("SubtitleYOffset")->add_child_text (raw_convert<string> (_subtitle_y_offset));
- 	root->add_child("SubtitleScale")->add_child_text (raw_convert<string> (_subtitle_scale));
+ 	root->add_child("SubtitleXScale")->add_child_text (raw_convert<string> (_subtitle_x_scale));
+ 	root->add_child("SubtitleYScale")->add_child_text (raw_convert<string> (_subtitle_y_scale));
  }
  
 +void
 +SubtitleContent::set_use_subtitles (bool u)
 +{
 +	{
 +		boost::mutex::scoped_lock lm (_mutex);
 +		_use_subtitles = u;
 +	}
 +	signal_changed (SubtitleContentProperty::USE_SUBTITLES);
 +}
 +	
  void
  SubtitleContent::set_subtitle_x_offset (double o)
  {
@@@ -157,19 -144,7 +182,20 @@@ SubtitleContent::set_subtitle_y_scale (
  {
  	{
  		boost::mutex::scoped_lock lm (_mutex);
- 		_subtitle_scale = s;
+ 		_subtitle_y_scale = s;
  	}
- 	signal_changed (SubtitleContentProperty::SUBTITLE_SCALE);
+ 	signal_changed (SubtitleContentProperty::SUBTITLE_Y_SCALE);
  }
 +
 +string
 +SubtitleContent::identifier () const
 +{
 +	SafeStringStream s;
 +	s << Content::identifier()
- 	  << "_" << raw_convert<string> (subtitle_scale())
++	  << "_" << raw_convert<string> (subtitle_x_scale())
++	  << "_" << raw_convert<string> (subtitle_y_scale())
 +	  << "_" << raw_convert<string> (subtitle_x_offset())
 +	  << "_" << raw_convert<string> (subtitle_y_offset());
 +
 +	return s.str ();
 +}
diff --cc src/lib/subtitle_content.h
index 97649f4d5,329368e44..c3c25232f
--- a/src/lib/subtitle_content.h
+++ b/src/lib/subtitle_content.h
@@@ -27,39 -27,24 +27,41 @@@ class SubtitleContentPropert
  public:
  	static int const SUBTITLE_X_OFFSET;
  	static int const SUBTITLE_Y_OFFSET;
- 	static int const SUBTITLE_SCALE;
+ 	static int const SUBTITLE_X_SCALE;
+ 	static int const SUBTITLE_Y_SCALE;
 +	static int const USE_SUBTITLES;
  };
  
 +/** @class SubtitleContent
 + *  @brief Parent for content which has the potential to include subtitles.
 + *
 + *  Although inheriting from this class indicates that the content could
 + *  have subtitles, it may not.  ::has_subtitles() will tell you.
 + */
  class SubtitleContent : public virtual Content
  {
  public:
 +	SubtitleContent (boost::shared_ptr<const Film>);
  	SubtitleContent (boost::shared_ptr<const Film>, boost::filesystem::path);
 -	SubtitleContent (boost::shared_ptr<const Film>, boost::shared_ptr<const cxml::Node>, int version);
 +	SubtitleContent (boost::shared_ptr<const Film>, cxml::ConstNodePtr, int version);
  	SubtitleContent (boost::shared_ptr<const Film>, std::vector<boost::shared_ptr<Content> >);
 -	
 +
  	void as_xml (xmlpp::Node *) const;
 +	std::string identifier () const;
 +
 +	virtual bool has_subtitles () const = 0;
  
 +	void set_use_subtitles (bool);
  	void set_subtitle_x_offset (double);
  	void set_subtitle_y_offset (double);
- 	void set_subtitle_scale (double);
+ 	void set_subtitle_x_scale (double);
+ 	void set_subtitle_y_scale (double);
  
 +	bool use_subtitles () const {
 +		boost::mutex::scoped_lock lm (_mutex);
 +		return _use_subtitles;
 +	}
 +
  	double subtitle_x_offset () const {
  		boost::mutex::scoped_lock lm (_mutex);
  		return _subtitle_x_offset;
@@@ -70,15 -55,19 +72,20 @@@
  		return _subtitle_y_offset;
  	}
  
- 	double subtitle_scale () const {
+ 	double subtitle_x_scale () const {
  		boost::mutex::scoped_lock lm (_mutex);
- 		return _subtitle_scale;
+ 		return _subtitle_x_scale;
+ 	}
+ 
+ 	double subtitle_y_scale () const {
+ 		boost::mutex::scoped_lock lm (_mutex);
+ 		return _subtitle_y_scale;
  	}
 -	
 +
  private:
 -	friend class ffmpeg_pts_offset_test;
 +	friend struct ffmpeg_pts_offset_test;
  
 +	bool _use_subtitles;
  	/** x offset for placing subtitles, as a proportion of the container width;
  	 * +ve is further right, -ve is further left.
  	 */
diff --cc src/wx/subtitle_panel.cc
index fecc85106,7953682fc..21d6f8e5b
--- a/src/wx/subtitle_panel.cc
+++ b/src/wx/subtitle_panel.cc
@@@ -84,15 -82,15 +93,16 @@@ SubtitlePanel::SubtitlePanel (ContentPa
  	
  	_x_offset->SetRange (-100, 100);
  	_y_offset->SetRange (-100, 100);
- 	_scale->SetRange (1, 1000);
- 	_scale->SetValue (100);
+ 	_x_scale->SetRange (10, 1000);
+ 	_y_scale->SetRange (10, 1000);
  
 -	_with_subtitles->Bind (wxEVT_COMMAND_CHECKBOX_CLICKED, boost::bind (&SubtitlePanel::with_subtitles_toggled, this));
 -	_x_offset->Bind       (wxEVT_COMMAND_SPINCTRL_UPDATED, boost::bind (&SubtitlePanel::x_offset_changed, this));
 -	_y_offset->Bind       (wxEVT_COMMAND_SPINCTRL_UPDATED, boost::bind (&SubtitlePanel::y_offset_changed, this));
 -	_x_scale->Bind        (wxEVT_COMMAND_SPINCTRL_UPDATED, boost::bind (&SubtitlePanel::x_scale_changed, this));
 -	_y_scale->Bind        (wxEVT_COMMAND_SPINCTRL_UPDATED, boost::bind (&SubtitlePanel::y_scale_changed, this));
 -	_stream->Bind         (wxEVT_COMMAND_CHOICE_SELECTED,  boost::bind (&SubtitlePanel::stream_changed, this));
 +	_use->Bind         (wxEVT_COMMAND_CHECKBOX_CLICKED, boost::bind (&SubtitlePanel::use_toggled, this));
 +	_x_offset->Bind    (wxEVT_COMMAND_SPINCTRL_UPDATED, boost::bind (&SubtitlePanel::x_offset_changed, this));
 +	_y_offset->Bind    (wxEVT_COMMAND_SPINCTRL_UPDATED, boost::bind (&SubtitlePanel::y_offset_changed, this));
- 	_scale->Bind       (wxEVT_COMMAND_SPINCTRL_UPDATED, boost::bind (&SubtitlePanel::scale_changed, this));
++	_x_scale->Bind       (wxEVT_COMMAND_SPINCTRL_UPDATED, boost::bind (&SubtitlePanel::x_scale_changed, this));
++	_y_scale->Bind       (wxEVT_COMMAND_SPINCTRL_UPDATED, boost::bind (&SubtitlePanel::y_scale_changed, this));
 +	_stream->Bind      (wxEVT_COMMAND_CHOICE_SELECTED,  boost::bind (&SubtitlePanel::stream_changed, this));
 +	_view_button->Bind (wxEVT_COMMAND_BUTTON_CLICKED,   boost::bind (&SubtitlePanel::view_clicked, this));
  }
  
  void
@@@ -158,35 -164,19 +170,36 @@@ SubtitlePanel::use_toggled (
  void
  SubtitlePanel::setup_sensitivity ()
  {
 -	bool h = false;
 -	bool j = false;
 -	if (_editor->film()) {
 -		h = _editor->film()->has_subtitles ();
 -		j = _editor->film()->with_subtitles ();
 +	int any_subs = 0;
 +	int ffmpeg_subs = 0;
 +	int subrip_or_dcp_subs = 0;
 +	SubtitleContentList c = _parent->selected_subtitle ();
 +	for (SubtitleContentList::const_iterator i = c.begin(); i != c.end(); ++i) {
 +		shared_ptr<const FFmpegContent> fc = boost::dynamic_pointer_cast<const FFmpegContent> (*i);
 +		shared_ptr<const SubRipContent> sc = boost::dynamic_pointer_cast<const SubRipContent> (*i);
 +		shared_ptr<const DCPSubtitleContent> dsc = boost::dynamic_pointer_cast<const DCPSubtitleContent> (*i);
 +		if (fc) {
 +			if (fc->has_subtitles ()) {
 +				++ffmpeg_subs;
 +				++any_subs;
 +			}
 +		} else if (sc || dsc) {
 +			++subrip_or_dcp_subs;
 +			++any_subs;
 +		} else {
 +			++any_subs;
 +		}
  	}
 +		
 +	_use->Enable (any_subs > 0);
 +	bool const use = _use->GetValue ();
  	
 -	_with_subtitles->Enable (h);
 -	_x_offset->Enable (j);
 -	_y_offset->Enable (j);
 -	_x_scale->Enable (j);
 -	_y_scale->Enable (j);
 -	_stream->Enable (j);
 +	_x_offset->Enable (any_subs > 0 && use);
 +	_y_offset->Enable (any_subs > 0 && use);
- 	_scale->Enable (any_subs > 0 && use);
++	_x_scale->Enable (any_subs > 0 && use);
++	_y_scale->Enable (any_subs > 0 && use);
 +	_stream->Enable (ffmpeg_subs == 1);
 +	_view_button->Enable (subrip_or_dcp_subs == 1);
  }
  
  void
@@@ -230,11 -220,20 +243,20 @@@ SubtitlePanel::y_offset_changed (
  }
  
  void
- SubtitlePanel::scale_changed ()
+ SubtitlePanel::x_scale_changed ()
+ {
 -	SubtitleContentList c = _editor->selected_subtitle_content ();
++	SubtitleContentList c = _parent->selected_subtitle ();
+ 	if (c.size() == 1) {
+ 		c.front()->set_subtitle_x_scale (_x_scale->GetValue() / 100.0);
+ 	}
+ }
+ 
+ void
+ SubtitlePanel::y_scale_changed ()
  {
 -	SubtitleContentList c = _editor->selected_subtitle_content ();
 -	if (c.size() == 1) {
 -		c.front()->set_subtitle_y_scale (_y_scale->GetValue() / 100.0);
 +	SubtitleContentList c = _parent->selected_subtitle ();
 +	for (SubtitleContentList::iterator i = c.begin(); i != c.end(); ++i) {
- 		(*i)->set_subtitle_scale (_scale->GetValue() / 100.0);
++		(*i)->set_subtitle_y_scale (_y_scale->GetValue() / 100.0);
  	}
  }
  
@@@ -242,37 -241,8 +264,38 @@@ voi
  SubtitlePanel::content_selection_changed ()
  {
  	film_content_changed (FFmpegContentProperty::SUBTITLE_STREAMS);
 +	film_content_changed (SubtitleContentProperty::USE_SUBTITLES);
  	film_content_changed (SubtitleContentProperty::SUBTITLE_X_OFFSET);
  	film_content_changed (SubtitleContentProperty::SUBTITLE_Y_OFFSET);
- 	film_content_changed (SubtitleContentProperty::SUBTITLE_SCALE);
+ 	film_content_changed (SubtitleContentProperty::SUBTITLE_X_SCALE);
+ 	film_content_changed (SubtitleContentProperty::SUBTITLE_Y_SCALE);
  }
 +
 +void
 +SubtitlePanel::view_clicked ()
 +{
 +	if (_view) {
 +		_view->Destroy ();
 +		_view = 0;
 +	}
 +
 +	SubtitleContentList c = _parent->selected_subtitle ();
 +	assert (c.size() == 1);
 +
 +	shared_ptr<SubtitleDecoder> decoder;
 +	
 +	shared_ptr<SubRipContent> sr = dynamic_pointer_cast<SubRipContent> (c.front ());
 +	if (sr) {
 +		decoder.reset (new SubRipDecoder (sr));
 +	}
 +	
 +	shared_ptr<DCPSubtitleContent> dc = dynamic_pointer_cast<DCPSubtitleContent> (c.front ());
 +	if (dc) {
 +		decoder.reset (new DCPSubtitleDecoder (dc));
 +	}
 +	
 +	if (decoder) {
 +		_view = new SubtitleView (this, _parent->film(), decoder, c.front()->position ());
 +		_view->Show ();
 +	}
 +}
diff --cc src/wx/subtitle_panel.h
index 9e60db34b,7f5d9239d..bcff995a0
--- a/src/wx/subtitle_panel.h
+++ b/src/wx/subtitle_panel.h
@@@ -33,20 -32,19 +33,22 @@@ public
  	void content_selection_changed ();
  	
  private:
 -	void with_subtitles_toggled ();
 +	void use_toggled ();
  	void x_offset_changed ();
  	void y_offset_changed ();
- 	void scale_changed ();
+ 	void x_scale_changed ();
+ 	void y_scale_changed ();
  	void stream_changed ();
 +	void view_clicked ();
  
  	void setup_sensitivity ();
  	
 -	wxCheckBox* _with_subtitles;
 +	wxCheckBox* _use;
  	wxSpinCtrl* _x_offset;
  	wxSpinCtrl* _y_offset;
- 	wxSpinCtrl* _scale;
+ 	wxSpinCtrl* _x_scale;
+ 	wxSpinCtrl* _y_scale;
  	wxChoice* _stream;
 +	wxButton* _view_button;
 +	SubtitleView* _view;
  };