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

diff --git a/app/helpers/statuses_helper.rb b/app/helpers/statuses_helper.rb
index 866a9902c3e4da5bf5e70ec140e692d194773bf4..a51597cf353e013c159ed61c0728c9d017f12150 100644 (file)
--- 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 9da143c96e2562472875ea1218cedb6b9b58450c..5f6132f12a7ced4191f576b2b27cf9728e30803a 100644 (file)
--- 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 (
-      <div className={classNames('audio-player', { editable, 'with-light-background': darkText })} ref={this.setPlayerRef} style={{ width: '100%', height: this.state.height || this.props.height }} onMouseEnter={this.handleMouseEnter} onMouseLeave={this.handleMouseLeave}>
+      <div className={classNames('audio-player', { editable, 'with-light-background': darkText })} ref={this.setPlayerRef} style={{ width: '100%', height: this.props.fullscreen ? '100%' : (this.state.height || this.props.height) }} onMouseEnter={this.handleMouseEnter} onMouseLeave={this.handleMouseLeave}>
         <audio
           src={src}
           ref={this.setAudioRef}
@@ -630,7 +634,6 @@ class Audio extends React.PureComponent {
           onPlay={this.handlePlay}
           onPause={this.handlePause}
           onProgress={this.handleProgress}
-          onTimeUpdate={this.handleTimeUpdate}
           crossOrigin='anonymous'
         />
 
@@ -691,7 +694,7 @@ class Audio extends React.PureComponent {
               </div>
 
               <span className='video-player__time'>
-                <span className='video-player__time-current'>{formatTime(currentTime)}</span>
+                <span className='video-player__time-current'>{formatTime(Math.floor(currentTime))}</span>
                 <span className='video-player__time-sep'>/</span>
                 <span className='video-player__time-total'>{formatTime(this.state.duration || Math.floor(this.props.duration))}</span>
               </span>

diff --git a/app/javascript/mastodon/features/video/index.js b/app/javascript/mastodon/features/video/index.js
index 1f85375ffabeb8fbbb12a4d9a51e06f56e4360a4..135200a3d759a2ec0f09eb707641e618ffbed340 100644 (file)
--- 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) && (
                 <span className='video-player__time'>
-                  <span className='video-player__time-current'>{formatTime(currentTime)}</span>
+                  <span className='video-player__time-current'>{formatTime(Math.floor(currentTime))}</span>
                   <span className='video-player__time-sep'>/</span>
                   <span className='video-player__time-total'>{formatTime(duration)}</span>
                 </span>

diff --git a/app/javascript/styles/mastodon/basics.scss b/app/javascript/styles/mastodon/basics.scss
index a5dbe75fb371071b472df183bbc74ce96fe67354..9e63b1d316fb1142d00cf1991a1c9ede9ae886b5 100644 (file)
--- 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 75ce9fc4f8d5f608f8d068664dfc8358b613030f..d44467009f60fafe53bfa2a81bc10dfee5c7a30c 100644 (file)
--- 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 839576372b1c45d778cb34a0dc8601a9da55c8d6..6350d7ed0f2e15013580b3e03e566826e0ff78ca 100644 (file)
--- 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 ea868b3f627c416cb05cae511c52810d9d45708f..3d308ee6967f7c9e0b44f4df8dc16a9e257b4822 100644 (file)
--- 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 8e409846a707342a92ae44a5bde2a24652d2f1b2..c23733053a13ae0a1e2e631dbc9765d89fda3f3f 100644 (file)
--- 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 67f9274b6ffb016cdd7f5c956f0e7407300e2673..c8b6147ef77d14803b9f7715905544cc5619f244 100644 (file)
--- 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 da7caf166d7e47dfae2456bbafc22b42505fda27..d5950658a68005abd69a7e2e34fdb9d290c27588 100644 (file)
--- 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 148ae5e2be087388ee203c872745cfb9a8fd38b7..73f9ae2bf62d69a820eca59543bdc3d40f777810 100644 (file)
--- 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 aed96e3e1cc0d7de68ba566682f6f3ed662f906c..52692129e146ef50d3f1773f3f5280f27a1c484b 100644 (file)
--- 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"