From 662a49dc3f06749936cedd7349092bbe622f0bc6 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Thu, 25 Jun 2020 01:33:01 +0200 Subject: [PATCH] Fix various issues around OpenGraph representation of media (#14133) - Fix audio attachments not being represented in OpenGraph tags - Fix audio being represented as "1 image" in OpenGraph descriptions - Fix video metadata being overwritten by paperclip-av-transcoder - Fix embedded player not using Mastodon's UI - Fix audio/video progress bars not moving smoothly - Fix audio/video buffered bars not displaying correctly --- app/helpers/statuses_helper.rb | 4 ++- .../mastodon/features/audio/index.js | 23 +++++++------ .../mastodon/features/video/index.js | 33 +++++++++++++------ app/javascript/styles/mastodon/basics.scss | 27 ++++++++++++++- app/models/media_attachment.rb | 19 +++++------ app/views/accounts/_og.html.haml | 4 +-- app/views/media/player.html.haml | 18 ++++++++-- app/views/statuses/_detailed_status.html.haml | 2 +- app/views/statuses/_og_image.html.haml | 17 ++++++++-- app/views/statuses/_simple_status.html.haml | 2 +- app/workers/post_process_media_worker.rb | 6 ++++ config/locales/en.yml | 3 ++ 12 files changed, 117 insertions(+), 41 deletions(-) diff --git a/app/helpers/statuses_helper.rb b/app/helpers/statuses_helper.rb index 866a9902c..a51597cf3 100644 --- a/app/helpers/statuses_helper.rb +++ b/app/helpers/statuses_helper.rb @@ -15,11 +15,13 @@ module StatusesHelper end def media_summary(status) - attachments = { image: 0, video: 0 } + attachments = { image: 0, video: 0, audio: 0 } status.media_attachments.each do |media| if media.video? attachments[:video] += 1 + elsif media.audio? + attachments[:audio] += 1 else attachments[:image] += 1 end diff --git a/app/javascript/mastodon/features/audio/index.js b/app/javascript/mastodon/features/audio/index.js index 9da143c96..5f6132f12 100644 --- a/app/javascript/mastodon/features/audio/index.js +++ b/app/javascript/mastodon/features/audio/index.js @@ -154,6 +154,7 @@ class Audio extends React.PureComponent { width: PropTypes.number, height: PropTypes.number, editable: PropTypes.bool, + fullscreen: PropTypes.bool, intl: PropTypes.object.isRequired, cacheWidth: PropTypes.func, }; @@ -180,7 +181,7 @@ class Audio extends React.PureComponent { _setDimensions () { const width = this.player.offsetWidth; - const height = width / (16/9); + const height = this.props.fullscreen ? this.player.offsetHeight : (width / (16/9)); if (this.props.cacheWidth) { this.props.cacheWidth(width); @@ -291,8 +292,10 @@ class Audio extends React.PureComponent { } handleProgress = () => { - if (this.audio.buffered.length > 0) { - this.setState({ buffer: this.audio.buffered.end(0) / this.audio.duration * 100 }); + const lastTimeRange = this.audio.buffered.length - 1; + + if (lastTimeRange > -1) { + this.setState({ buffer: Math.ceil(this.audio.buffered.end(lastTimeRange) / this.audio.duration * 100) }); } } @@ -349,18 +352,18 @@ class Audio extends React.PureComponent { handleMouseMove = throttle(e => { const { x } = getPointerPosition(this.seek, e); - const currentTime = Math.floor(this.audio.duration * x); + const currentTime = this.audio.duration * x; if (!isNaN(currentTime)) { this.setState({ currentTime }, () => { this.audio.currentTime = currentTime; }); } - }, 60); + }, 15); handleTimeUpdate = () => { this.setState({ - currentTime: Math.floor(this.audio.currentTime), + currentTime: this.audio.currentTime, duration: Math.floor(this.audio.duration), }); } @@ -373,7 +376,7 @@ class Audio extends React.PureComponent { this.audio.volume = x; }); } - }, 60); + }, 15); handleScroll = throttle(() => { if (!this.canvas || !this.audio) { @@ -451,6 +454,7 @@ class Audio extends React.PureComponent { _renderCanvas () { requestAnimationFrame(() => { + this.handleTimeUpdate(); this._clear(); this._draw(); @@ -622,7 +626,7 @@ class Audio extends React.PureComponent { const progress = (currentTime / duration) * 100; return ( -
+
- {formatTime(currentTime)} + {formatTime(Math.floor(currentTime))} / {formatTime(this.state.duration || Math.floor(this.props.duration))} diff --git a/app/javascript/mastodon/features/video/index.js b/app/javascript/mastodon/features/video/index.js index 1f85375ff..135200a3d 100644 --- a/app/javascript/mastodon/features/video/index.js +++ b/app/javascript/mastodon/features/video/index.js @@ -177,15 +177,26 @@ class Video extends React.PureComponent { handlePlay = () => { this.setState({ paused: false }); + this._updateTime(); } handlePause = () => { this.setState({ paused: true }); } + _updateTime () { + requestAnimationFrame(() => { + this.handleTimeUpdate(); + + if (!this.state.paused) { + this._updateTime(); + } + }); + } + handleTimeUpdate = () => { this.setState({ - currentTime: Math.floor(this.video.currentTime), + currentTime: this.video.currentTime, duration: Math.floor(this.video.duration), }); } @@ -217,7 +228,7 @@ class Video extends React.PureComponent { this.video.volume = x; }); } - }, 60); + }, 15); handleMouseDown = e => { document.addEventListener('mousemove', this.handleMouseMove, true); @@ -245,13 +256,14 @@ class Video extends React.PureComponent { handleMouseMove = throttle(e => { const { x } = getPointerPosition(this.seek, e); - const currentTime = Math.floor(this.video.duration * x); + const currentTime = this.video.duration * x; if (!isNaN(currentTime)) { - this.video.currentTime = currentTime; - this.setState({ currentTime }); + this.setState({ currentTime }, () => { + this.video.currentTime = currentTime; + }); } - }, 60); + }, 15); togglePlay = () => { if (this.state.paused) { @@ -387,8 +399,10 @@ class Video extends React.PureComponent { } handleProgress = () => { - if (this.video.buffered.length > 0) { - this.setState({ buffer: this.video.buffered.end(0) / this.video.duration * 100 }); + const lastTimeRange = this.video.buffered.length - 1; + + if (lastTimeRange > -1) { + this.setState({ buffer: Math.ceil(this.video.buffered.end(lastTimeRange) / this.video.duration * 100) }); } } @@ -484,7 +498,6 @@ class Video extends React.PureComponent { onClick={this.togglePlay} onPlay={this.handlePlay} onPause={this.handlePause} - onTimeUpdate={this.handleTimeUpdate} onLoadedData={this.handleLoadedData} onProgress={this.handleProgress} onVolumeChange={this.handleVolumeChange} @@ -525,7 +538,7 @@ class Video extends React.PureComponent { {(detailed || fullscreen) && ( - {formatTime(currentTime)} + {formatTime(Math.floor(currentTime))} / {formatTime(duration)} diff --git a/app/javascript/styles/mastodon/basics.scss b/app/javascript/styles/mastodon/basics.scss index a5dbe75fb..9e63b1d31 100644 --- a/app/javascript/styles/mastodon/basics.scss +++ b/app/javascript/styles/mastodon/basics.scss @@ -68,7 +68,32 @@ body { } &.player { - text-align: center; + padding: 0; + margin: 0; + position: absolute; + width: 100%; + height: 100%; + overflow: hidden; + + & > div { + height: 100%; + } + + .video-player video { + width: 100%; + height: 100%; + max-height: 100vh; + } + + .media-gallery { + margin-top: 0; + height: 100% !important; + border-radius: 0; + } + + .media-gallery__item { + border-radius: 0; + } } &.embed { diff --git a/app/models/media_attachment.rb b/app/models/media_attachment.rb index 75ce9fc4f..d44467009 100644 --- a/app/models/media_attachment.rb +++ b/app/models/media_attachment.rb @@ -194,15 +194,17 @@ class MediaAttachment < ApplicationRecord x, y = (point.is_a?(Enumerable) ? point : point.split(',')).map(&:to_f) - meta = file.instance_read(:meta) || {} + meta = (file.instance_read(:meta) || {}).with_indifferent_access.slice(:focus, :original, :small) meta['focus'] = { 'x' => x, 'y' => y } file.instance_write(:meta, meta) end def focus - x = file.meta['focus']['x'] - y = file.meta['focus']['y'] + x = file.meta&.dig('focus', 'x') + y = file.meta&.dig('focus', 'y') + + return if x.nil? || y.nil? "#{x},#{y}" end @@ -219,12 +221,11 @@ class MediaAttachment < ApplicationRecord before_create :prepare_description, unless: :local? before_create :set_shortcode before_create :set_processing + before_create :set_meta before_post_process :set_type_and_extension before_post_process :check_video_dimensions - before_save :set_meta - class << self def supported_mime_types IMAGE_MIME_TYPES + VIDEO_MIME_TYPES + AUDIO_MIME_TYPES @@ -306,15 +307,11 @@ class MediaAttachment < ApplicationRecord end def set_meta - meta = populate_meta - - return if meta == {} - - file.instance_write :meta, meta + file.instance_write :meta, populate_meta end def populate_meta - meta = file.instance_read(:meta) || {} + meta = (file.instance_read(:meta) || {}).with_indifferent_access.slice(:focus, :original, :small) file.queued_for_write.each do |style, file| meta[style] = style == :small || image? ? image_geometry(file) : video_metadata(file) diff --git a/app/views/accounts/_og.html.haml b/app/views/accounts/_og.html.haml index 839576372..6350d7ed0 100644 --- a/app/views/accounts/_og.html.haml +++ b/app/views/accounts/_og.html.haml @@ -7,7 +7,7 @@ = opengraph 'og:title', yield(:page_title).strip = opengraph 'og:description', description = opengraph 'og:image', full_asset_url(account.avatar.url(:original)) -= opengraph 'og:image:width', '120' -= opengraph 'og:image:height', '120' += opengraph 'og:image:width', '400' += opengraph 'og:image:height', '400' = opengraph 'twitter:card', 'summary' = opengraph 'profile:username', acct(account)[1..-1] diff --git a/app/views/media/player.html.haml b/app/views/media/player.html.haml index ea868b3f6..3d308ee69 100644 --- a/app/views/media/player.html.haml +++ b/app/views/media/player.html.haml @@ -1,2 +1,16 @@ -%video{ poster: @media_attachment.file.url(:small), preload: 'auto', autoplay: 'autoplay', muted: 'muted', loop: 'loop', controls: 'controls', style: "width: #{@media_attachment.file.meta.dig('original', 'width')}px; height: #{@media_attachment.file.meta.dig('original', 'height')}px" } - %source{ src: @media_attachment.file.url(:original), type: @media_attachment.file_content_type } +- content_for :header_tags do + = render_initial_state + = javascript_pack_tag 'public', integrity: true, crossorigin: 'anonymous' + +- if @media_attachment.video? + = react_component :video, src: @media_attachment.file.url(:original), preview: @media_attachment.file.url(:small), blurhash: @media_attachment.blurhash, width: 670, height: 380, editable: true, detailed: true, inline: true, alt: @media_attachment.description do + %video{ controls: 'controls' } + %source{ src: @media_attachment.file.url(:original) } +- elsif @media_attachment.gifv? + = react_component :media_gallery, height: 380, standalone: true, autoplay: true, media: [ActiveModelSerializers::SerializableResource.new(@media_attachment, serializer: REST::MediaAttachmentSerializer).as_json] do + %video{ autoplay: 'autoplay', muted: 'muted', loop: 'loop' } + %source{ src: @media_attachment.file.url(:original) } +- elsif @media_attachment.audio? + = react_component :audio, src: @media_attachment.file.url(:original), poster: full_asset_url(@media_attachment.account.avatar_static_url), width: 670, height: 380, fullscreen: true, alt: @media_attachment.description, duration: @media_attachment.file.meta.dig(:original, :duration) do + %audio{ controls: 'controls' } + %source{ src: @media_attachment.file.url(:original) } diff --git a/app/views/statuses/_detailed_status.html.haml b/app/views/statuses/_detailed_status.html.haml index 8e409846a..c23733053 100644 --- a/app/views/statuses/_detailed_status.html.haml +++ b/app/views/statuses/_detailed_status.html.haml @@ -33,7 +33,7 @@ = render partial: 'statuses/attachment_list', locals: { attachments: status.media_attachments } - elsif status.media_attachments.first.audio? - audio = status.media_attachments.first - = react_component :audio, src: audio.file.url(:original), height: 130, alt: audio.description, preload: true, duration: audio.file.meta.dig(:original, :duration) do + = react_component :audio, src: audio.file.url(:original), poster: full_asset_url(status.account.avatar_static_url), width: 670, height: 380, alt: audio.description, duration: audio.file.meta.dig(:original, :duration) do = render partial: 'statuses/attachment_list', locals: { attachments: status.media_attachments } - else = react_component :media_gallery, height: 380, sensitive: status.sensitive?, standalone: true, autoplay: autoplay, media: status.media_attachments.map { |a| ActiveModelSerializers::SerializableResource.new(a, serializer: REST::MediaAttachmentSerializer).as_json } do diff --git a/app/views/statuses/_og_image.html.haml b/app/views/statuses/_og_image.html.haml index 67f9274b6..c8b6147ef 100644 --- a/app/views/statuses/_og_image.html.haml +++ b/app/views/statuses/_og_image.html.haml @@ -27,12 +27,25 @@ = opengraph 'og:video:height', media.file.meta.dig('original', 'height') = opengraph 'twitter:player:width', media.file.meta.dig('original', 'width') = opengraph 'twitter:player:height', media.file.meta.dig('original', 'height') + - elsif media.audio? + - player_card = true + = opengraph 'og:image', full_asset_url(account.avatar.url(:original)) + = opengraph 'og:image:width', '400' + = opengraph 'og:image:height','400' + = opengraph 'og:audio', full_asset_url(media.file.url(:original)) + = opengraph 'og:audio:secure_url', full_asset_url(media.file.url(:original)) + = opengraph 'og:audio:type', media.file_content_type + = opengraph 'twitter:player', medium_player_url(media) + = opengraph 'twitter:player:stream', full_asset_url(media.file.url(:original)) + = opengraph 'twitter:player:stream:content_type', media.file_content_type + = opengraph 'twitter:player:width', '670' + = opengraph 'twitter:player:height', '380' - if player_card = opengraph 'twitter:card', 'player' - else = opengraph 'twitter:card', 'summary_large_image' - else = opengraph 'og:image', full_asset_url(account.avatar.url(:original)) - = opengraph 'og:image:width', '120' - = opengraph 'og:image:height','120' + = opengraph 'og:image:width', '400' + = opengraph 'og:image:height','400' = opengraph 'twitter:card', 'summary' diff --git a/app/views/statuses/_simple_status.html.haml b/app/views/statuses/_simple_status.html.haml index da7caf166..d5950658a 100644 --- a/app/views/statuses/_simple_status.html.haml +++ b/app/views/statuses/_simple_status.html.haml @@ -37,7 +37,7 @@ = render partial: 'statuses/attachment_list', locals: { attachments: status.media_attachments } - elsif status.media_attachments.first.audio? - audio = status.media_attachments.first - = react_component :audio, src: audio.file.url(:original), height: 110, alt: audio.description, duration: audio.file.meta.dig(:original, :duration) do + = react_component :audio, src: audio.file.url(:original), poster: full_asset_url(status.account.avatar_static_url), width: 610, height: 343, alt: audio.description, duration: audio.file.meta.dig(:original, :duration) do = render partial: 'statuses/attachment_list', locals: { attachments: status.media_attachments } - else = react_component :media_gallery, height: 343, sensitive: status.sensitive?, autoplay: autoplay, media: status.media_attachments.map { |a| ActiveModelSerializers::SerializableResource.new(a, serializer: REST::MediaAttachmentSerializer).as_json } do diff --git a/app/workers/post_process_media_worker.rb b/app/workers/post_process_media_worker.rb index 148ae5e2b..73f9ae2bf 100644 --- a/app/workers/post_process_media_worker.rb +++ b/app/workers/post_process_media_worker.rb @@ -25,8 +25,14 @@ class PostProcessMediaWorker media_attachment = MediaAttachment.find(media_attachment_id) media_attachment.processing = :in_progress media_attachment.save + + # Because paperclip-av-transcover overwrites this attribute + # we will save it here and restore it after reprocess is done + previous_meta = media_attachment.file_meta + media_attachment.file.reprocess!(:original) media_attachment.processing = :complete + media_attachment.file_meta = previous_meta media_attachment.save rescue ActiveRecord::RecordNotFound true diff --git a/config/locales/en.yml b/config/locales/en.yml index aed96e3e1..52692129e 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1117,6 +1117,9 @@ en: spam_detected: This is an automated report. Spam has been detected. statuses: attached: + audio: + one: "%{count} audio" + other: "%{count} audio" description: 'Attached: %{attached}' image: one: "%{count} image"