From 2450fc24e30121863403517b44dfdfa7cb25e33a Mon Sep 17 00:00:00 2001 From: George Claghorn Date: Fri, 19 Jan 2018 10:47:18 -0500 Subject: Preserve display aspect ratio for videos with rectangular samples --- activestorage/CHANGELOG.md | 9 ++++ .../lib/active_storage/analyzer/video_analyzer.rb | 50 ++++++++++++++------- activestorage/test/analyzer/video_analyzer_test.rb | 22 +++++++-- .../files/video_with_rectangular_samples.mp4 | Bin 0 -> 361535 bytes 4 files changed, 62 insertions(+), 19 deletions(-) create mode 100644 activestorage/test/fixtures/files/video_with_rectangular_samples.mp4 (limited to 'activestorage') diff --git a/activestorage/CHANGELOG.md b/activestorage/CHANGELOG.md index 5e01297fc1..061898d143 100644 --- a/activestorage/CHANGELOG.md +++ b/activestorage/CHANGELOG.md @@ -1,3 +1,12 @@ +* Preserve display aspect ratio when extracting width and height from videos + with rectangular samples in `ActiveStorage::Analyzer::VideoAnalyzer`. + + When a video contains a display aspect ratio, emit it in metadata as + `:display_aspect_ratio` rather than the ambiguous `:aspect_ratio`. Compute + its height by scaling its encoded frame width according to the DAR. + + *George Claghorn* + * Use `after_destroy_commit` instead of `before_destroy` for purging attachments when a record is destroyed. diff --git a/activestorage/lib/active_storage/analyzer/video_analyzer.rb b/activestorage/lib/active_storage/analyzer/video_analyzer.rb index aa532da201..f0d9baa199 100644 --- a/activestorage/lib/active_storage/analyzer/video_analyzer.rb +++ b/activestorage/lib/active_storage/analyzer/video_analyzer.rb @@ -9,12 +9,12 @@ module ActiveStorage # * Height (pixels) # * Duration (seconds) # * Angle (degrees) - # * Aspect ratio + # * Display aspect ratio # # Example: # # ActiveStorage::VideoAnalyzer.new(blob).metadata - # # => { width: 640, height: 480, duration: 5.0, angle: 0, aspect_ratio: [4, 3] } + # # => { width: 640.0, height: 480.0, duration: 5.0, angle: 0, display_aspect_ratio: [4, 3] } # # When a video's angle is 90 or 270 degrees, its width and height are automatically swapped for convenience. # @@ -25,24 +25,24 @@ module ActiveStorage end def metadata - { width: width, height: height, duration: duration, angle: angle, aspect_ratio: aspect_ratio }.compact + { width: width, height: height, duration: duration, angle: angle, display_aspect_ratio: display_aspect_ratio }.compact end private def width - rotated? ? raw_height : raw_width + if rotated? + computed_height || encoded_height + else + encoded_width + end end def height - rotated? ? raw_width : raw_height - end - - def raw_width - Integer(video_stream["width"]) if video_stream["width"] - end - - def raw_height - Integer(video_stream["height"]) if video_stream["height"] + if rotated? + encoded_width + else + computed_height || encoded_height + end end def duration @@ -53,16 +53,36 @@ module ActiveStorage Integer(tags["rotate"]) if tags["rotate"] end - def aspect_ratio + def display_aspect_ratio if descriptor = video_stream["display_aspect_ratio"] - descriptor.split(":", 2).collect(&:to_i) + terms = descriptor.split(":", 2).collect(&:to_i) + terms if terms.count == 2 && terms.min >= 0 end end + def rotated? angle == 90 || angle == 270 end + def computed_height + if encoded_width && display_height_scale + encoded_width * display_height_scale + end + end + + def encoded_width + @encoded_width ||= Float(video_stream["width"]) if video_stream["width"] + end + + def encoded_height + @encoded_height ||= Float(video_stream["height"]) if video_stream["height"] + end + + def display_height_scale + @display_height_scale ||= Float(display_aspect_ratio.last) / display_aspect_ratio.first if display_aspect_ratio + end + def tags @tags ||= video_stream["tags"] || {} diff --git a/activestorage/test/analyzer/video_analyzer_test.rb b/activestorage/test/analyzer/video_analyzer_test.rb index 7fc1cfce06..03fbd72735 100644 --- a/activestorage/test/analyzer/video_analyzer_test.rb +++ b/activestorage/test/analyzer/video_analyzer_test.rb @@ -8,28 +8,42 @@ require "active_storage/analyzer/video_analyzer" class ActiveStorage::Analyzer::VideoAnalyzerTest < ActiveSupport::TestCase test "analyzing a video" do blob = create_file_blob(filename: "video.mp4", content_type: "video/mp4") - metadata = blob.tap(&:analyze).metadata + metadata = extract_metadata_from(blob) assert_equal 640, metadata[:width] assert_equal 480, metadata[:height] - assert_equal [4, 3], metadata[:aspect_ratio] + assert_equal [4, 3], metadata[:display_aspect_ratio] assert_equal 5.166648, metadata[:duration] assert_not_includes metadata, :angle end test "analyzing a rotated video" do blob = create_file_blob(filename: "rotated_video.mp4", content_type: "video/mp4") - metadata = blob.tap(&:analyze).metadata + metadata = extract_metadata_from(blob) assert_equal 480, metadata[:width] assert_equal 640, metadata[:height] - assert_equal [4, 3], metadata[:aspect_ratio] + assert_equal [4, 3], metadata[:display_aspect_ratio] assert_equal 5.227975, metadata[:duration] assert_equal 90, metadata[:angle] end + test "analyzing a video with rectangular samples" do + blob = create_file_blob(filename: "video_with_rectangular_samples.mp4", content_type: "video/mp4") + metadata = extract_metadata_from(blob) + + assert_equal 1280, metadata[:width] + assert_equal 720, metadata[:height] + assert_equal [16, 9], metadata[:display_aspect_ratio] + end + test "analyzing a video without a video stream" do blob = create_file_blob(filename: "video_without_video_stream.mp4", content_type: "video/mp4") assert_equal({ "analyzed" => true, "identified" => true }, blob.tap(&:analyze).metadata) end + + private + def extract_metadata_from(blob) + blob.tap(&:analyze).metadata + end end diff --git a/activestorage/test/fixtures/files/video_with_rectangular_samples.mp4 b/activestorage/test/fixtures/files/video_with_rectangular_samples.mp4 new file mode 100644 index 0000000000..12b04afc87 Binary files /dev/null and b/activestorage/test/fixtures/files/video_with_rectangular_samples.mp4 differ -- cgit v1.2.3