about summary refs log tree commit diff
diff options
context:
space:
mode:
authorEugen Rochko <eugen@zeonfederated.com>2020-06-25 01:33:01 +0200
committerGitHub <noreply@github.com>2020-06-25 01:33:01 +0200
commit662a49dc3f06749936cedd7349092bbe622f0bc6 (patch)
treee70453a48a765854ca0510412ab48accd0d13e6d
parente9ff61ca0704d9570950875e1b72556c4114424f (diff)
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
-rw-r--r--app/helpers/statuses_helper.rb4
-rw-r--r--app/javascript/mastodon/features/audio/index.js23
-rw-r--r--app/javascript/mastodon/features/video/index.js33
-rw-r--r--app/javascript/styles/mastodon/basics.scss27
-rw-r--r--app/models/media_attachment.rb19
-rw-r--r--app/views/accounts/_og.html.haml4
-rw-r--r--app/views/media/player.html.haml18
-rw-r--r--app/views/statuses/_detailed_status.html.haml2
-rw-r--r--app/views/statuses/_og_image.html.haml17
-rw-r--r--app/views/statuses/_simple_status.html.haml2
-rw-r--r--app/workers/post_process_media_worker.rb6
-rw-r--r--config/locales/en.yml3
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 (
-      <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 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) && (
                 <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 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"