From 83388269631f377e9853858916aa8c3897f90bb4 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Tue, 22 Feb 2022 06:20:04 +0100 Subject: [PATCH 1/4] Fix wrong styles on strike page (#17615) --- app/javascript/styles/mastodon/admin.scss | 19 +++++++++++++++++++ app/views/disputes/strikes/show.html.haml | 7 ++++--- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/app/javascript/styles/mastodon/admin.scss b/app/javascript/styles/mastodon/admin.scss index 1921eb1466..d3ac4d9952 100644 --- a/app/javascript/styles/mastodon/admin.scss +++ b/app/javascript/styles/mastodon/admin.scss @@ -1497,6 +1497,25 @@ a.sparkline { &:last-child { margin-bottom: 0; } + + strong { + font-weight: 700; + } + } + + &__rules { + list-style: disc; + padding-left: 15px; + margin-bottom: 20px; + color: $darker-text-color; + + &:last-child { + margin-bottom: 0; + } + + &__text { + color: $primary-text-color; + } } &__statuses-list { diff --git a/app/views/disputes/strikes/show.html.haml b/app/views/disputes/strikes/show.html.haml index 3dcb19016e..20a43ff98b 100644 --- a/app/views/disputes/strikes/show.html.haml +++ b/app/views/disputes/strikes/show.html.haml @@ -23,7 +23,7 @@ .report-header__card .strike-card - unless @strike.none_action? - %p= t "user_mailer.warning.explanation.#{@strike.action}" + %p= t "user_mailer.warning.explanation.#{@strike.action}", instance: Rails.configuration.x.local_domain - unless @strike.text.blank? = Formatter.instance.linkify(@strike.text) @@ -34,9 +34,10 @@ = t("user_mailer.warning.categories.#{@strike.report.category}") - if @strike.report.violation? && @strike.report.rule_ids.present? - %ul.rules-list + %ul.strike-card__rules - @strike.report.rules.each do |rule| - %li= rule.text + %li + %span.strike-card__rules__text= rule.text - if @strike.status_ids.present? && !@strike.status_ids.empty? %p From b377022cf9dfac4d98d0d10b511aeb65e540e0a3 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Tue, 22 Feb 2022 15:27:08 +0100 Subject: [PATCH 2/4] Add caching layer to metrics (#17617) --- .../admin/metrics/dimension/base_dimension.rb | 32 +++++++++++- .../metrics/dimension/languages_dimension.rb | 4 +- .../metrics/dimension/servers_dimension.rb | 4 +- .../dimension/software_versions_dimension.rb | 6 +-- .../metrics/dimension/sources_dimension.rb | 4 +- .../dimension/space_usage_dimension.rb | 6 +-- .../dimension/tag_languages_dimension.rb | 6 +-- .../dimension/tag_servers_dimension.rb | 6 +-- .../metrics/measure/active_users_measure.rb | 10 ++-- app/lib/admin/metrics/measure/base_measure.rb | 52 +++++++++++++++++-- .../metrics/measure/interactions_measure.rb | 10 ++-- .../metrics/measure/new_users_measure.rb | 8 +-- .../metrics/measure/opened_reports_measure.rb | 8 +-- .../measure/resolved_reports_measure.rb | 8 +-- .../metrics/measure/tag_accounts_measure.rb | 10 ++-- .../metrics/measure/tag_servers_measure.rb | 10 ++-- .../admin/metrics/measure/tag_uses_measure.rb | 10 ++-- app/lib/admin/metrics/retention.rb | 26 ++++++++++ 18 files changed, 165 insertions(+), 55 deletions(-) diff --git a/app/lib/admin/metrics/dimension/base_dimension.rb b/app/lib/admin/metrics/dimension/base_dimension.rb index 5872c22cbc..bd2e4ececb 100644 --- a/app/lib/admin/metrics/dimension/base_dimension.rb +++ b/app/lib/admin/metrics/dimension/base_dimension.rb @@ -1,23 +1,34 @@ # frozen_string_literal: true class Admin::Metrics::Dimension::BaseDimension + CACHE_TTL = 5.minutes.freeze + def self.with_params? false end + attr_reader :loaded + + alias loaded? loaded + def initialize(start_at, end_at, limit, params) @start_at = start_at&.to_datetime @end_at = end_at&.to_datetime @limit = limit&.to_i @params = params + @loaded = false end def key raise NotImplementedError end + def cache_key + ["metrics/dimension/#{key}", @start_at, @end_at, @limit, canonicalized_params].join(';') + end + def data - raise NotImplementedError + load end def self.model_name @@ -30,11 +41,28 @@ class Admin::Metrics::Dimension::BaseDimension protected + def load + unless loaded? + @values = Rails.cache.fetch(cache_key, expires_in: CACHE_TTL) { perform_query } + @loaded = true + end + + @values + end + + def perform_query + raise NotImplementedError + end + def time_period (@start_at..@end_at) end def params - raise NotImplementedError + {} + end + + def canonicalized_params + params.to_h.to_a.sort_by { |k, _v| k.to_s }.map { |k, v| "#{k}=#{v}" }.join(';') end end diff --git a/app/lib/admin/metrics/dimension/languages_dimension.rb b/app/lib/admin/metrics/dimension/languages_dimension.rb index 1cc5f41207..f1cf82cf27 100644 --- a/app/lib/admin/metrics/dimension/languages_dimension.rb +++ b/app/lib/admin/metrics/dimension/languages_dimension.rb @@ -7,7 +7,9 @@ class Admin::Metrics::Dimension::LanguagesDimension < Admin::Metrics::Dimension: 'languages' end - def data + protected + + def perform_query sql = <<-SQL.squish SELECT locale, count(*) AS value FROM users diff --git a/app/lib/admin/metrics/dimension/servers_dimension.rb b/app/lib/admin/metrics/dimension/servers_dimension.rb index 3e80b66250..91bcce6551 100644 --- a/app/lib/admin/metrics/dimension/servers_dimension.rb +++ b/app/lib/admin/metrics/dimension/servers_dimension.rb @@ -5,7 +5,9 @@ class Admin::Metrics::Dimension::ServersDimension < Admin::Metrics::Dimension::B 'servers' end - def data + protected + + def perform_query sql = <<-SQL.squish SELECT accounts.domain, count(*) AS value FROM statuses diff --git a/app/lib/admin/metrics/dimension/software_versions_dimension.rb b/app/lib/admin/metrics/dimension/software_versions_dimension.rb index 34917404d1..816615f992 100644 --- a/app/lib/admin/metrics/dimension/software_versions_dimension.rb +++ b/app/lib/admin/metrics/dimension/software_versions_dimension.rb @@ -7,12 +7,12 @@ class Admin::Metrics::Dimension::SoftwareVersionsDimension < Admin::Metrics::Dim 'software_versions' end - def data + protected + + def perform_query [mastodon_version, ruby_version, postgresql_version, redis_version] end - private - def mastodon_version value = Mastodon::Version.to_s diff --git a/app/lib/admin/metrics/dimension/sources_dimension.rb b/app/lib/admin/metrics/dimension/sources_dimension.rb index a9f061809c..122807cdcd 100644 --- a/app/lib/admin/metrics/dimension/sources_dimension.rb +++ b/app/lib/admin/metrics/dimension/sources_dimension.rb @@ -5,7 +5,9 @@ class Admin::Metrics::Dimension::SourcesDimension < Admin::Metrics::Dimension::B 'sources' end - def data + protected + + def perform_query sql = <<-SQL.squish SELECT oauth_applications.name, count(*) AS value FROM users diff --git a/app/lib/admin/metrics/dimension/space_usage_dimension.rb b/app/lib/admin/metrics/dimension/space_usage_dimension.rb index aa00a2e18b..5867c5bab6 100644 --- a/app/lib/admin/metrics/dimension/space_usage_dimension.rb +++ b/app/lib/admin/metrics/dimension/space_usage_dimension.rb @@ -8,12 +8,12 @@ class Admin::Metrics::Dimension::SpaceUsageDimension < Admin::Metrics::Dimension 'space_usage' end - def data + protected + + def perform_query [postgresql_size, redis_size, media_size] end - private - def postgresql_size value = ActiveRecord::Base.connection.execute('SELECT pg_database_size(current_database())').first['pg_database_size'] diff --git a/app/lib/admin/metrics/dimension/tag_languages_dimension.rb b/app/lib/admin/metrics/dimension/tag_languages_dimension.rb index afbc8cde8a..e1349c2294 100644 --- a/app/lib/admin/metrics/dimension/tag_languages_dimension.rb +++ b/app/lib/admin/metrics/dimension/tag_languages_dimension.rb @@ -11,7 +11,9 @@ class Admin::Metrics::Dimension::TagLanguagesDimension < Admin::Metrics::Dimensi 'tag_languages' end - def data + protected + + def perform_query sql = <<-SQL.squish SELECT COALESCE(statuses.language, 'und') AS language, count(*) AS value FROM statuses @@ -28,8 +30,6 @@ class Admin::Metrics::Dimension::TagLanguagesDimension < Admin::Metrics::Dimensi rows.map { |row| { key: row['language'], human_key: standard_locale_name(row['language']), value: row['value'].to_s } } end - private - def params @params.permit(:id) end diff --git a/app/lib/admin/metrics/dimension/tag_servers_dimension.rb b/app/lib/admin/metrics/dimension/tag_servers_dimension.rb index 12c5980d72..7ddf3378cd 100644 --- a/app/lib/admin/metrics/dimension/tag_servers_dimension.rb +++ b/app/lib/admin/metrics/dimension/tag_servers_dimension.rb @@ -9,7 +9,9 @@ class Admin::Metrics::Dimension::TagServersDimension < Admin::Metrics::Dimension 'tag_servers' end - def data + protected + + def perform_query sql = <<-SQL.squish SELECT accounts.domain, count(*) AS value FROM statuses @@ -27,8 +29,6 @@ class Admin::Metrics::Dimension::TagServersDimension < Admin::Metrics::Dimension rows.map { |row| { key: row['domain'] || Rails.configuration.x.local_domain, human_key: row['domain'] || Rails.configuration.x.local_domain, value: row['value'].to_s } } end - private - def params @params.permit(:id) end diff --git a/app/lib/admin/metrics/measure/active_users_measure.rb b/app/lib/admin/metrics/measure/active_users_measure.rb index 5131897806..e6f09d4bcf 100644 --- a/app/lib/admin/metrics/measure/active_users_measure.rb +++ b/app/lib/admin/metrics/measure/active_users_measure.rb @@ -5,20 +5,20 @@ class Admin::Metrics::Measure::ActiveUsersMeasure < Admin::Metrics::Measure::Bas 'active_users' end - def total + protected + + def perform_total_query activity_tracker.sum(time_period.first, time_period.last) end - def previous_total + def perform_previous_total_query activity_tracker.sum(previous_time_period.first, previous_time_period.last) end - def data + def perform_data_query activity_tracker.get(time_period.first, time_period.last).map { |date, value| { date: date.to_time(:utc).iso8601, value: value.to_s } } end - protected - def activity_tracker @activity_tracker ||= ActivityTracker.new('activity:logins', :unique) end diff --git a/app/lib/admin/metrics/measure/base_measure.rb b/app/lib/admin/metrics/measure/base_measure.rb index 0107ffd9c1..ed1df9c7d9 100644 --- a/app/lib/admin/metrics/measure/base_measure.rb +++ b/app/lib/admin/metrics/measure/base_measure.rb @@ -1,14 +1,25 @@ # frozen_string_literal: true class Admin::Metrics::Measure::BaseMeasure + CACHE_TTL = 5.minutes.freeze + def self.with_params? false end + attr_reader :loaded + + alias loaded? loaded + def initialize(start_at, end_at, params) @start_at = start_at&.to_datetime @end_at = end_at&.to_datetime @params = params + @loaded = false + end + + def cache_key + ["metrics/measure/#{key}", @start_at, @end_at, canonicalized_params].join(';') end def key @@ -16,15 +27,15 @@ class Admin::Metrics::Measure::BaseMeasure end def total - raise NotImplementedError + load[:total] end def previous_total - raise NotImplementedError + load[:previous_total] end def data - raise NotImplementedError + load[:data] end def self.model_name @@ -37,6 +48,35 @@ class Admin::Metrics::Measure::BaseMeasure protected + def load + unless loaded? + @values = Rails.cache.fetch(cache_key, expires_in: CACHE_TTL) { perform_queries }.with_indifferent_access + @loaded = true + end + + @values + end + + def perform_queries + { + total: perform_total_query, + previous_total: perform_previous_total_query, + data: perform_data_query, + } + end + + def perform_total_query + raise NotImplementedError + end + + def perform_previous_total_query + raise NotImplementedError + end + + def perform_data_query + raise NotImplementedError + end + def time_period (@start_at..@end_at) end @@ -50,6 +90,10 @@ class Admin::Metrics::Measure::BaseMeasure end def params - raise NotImplementedError + {} + end + + def canonicalized_params + params.to_h.to_a.sort_by { |k, _v| k.to_s }.map { |k, v| "#{k}=#{v}" }.join(';') end end diff --git a/app/lib/admin/metrics/measure/interactions_measure.rb b/app/lib/admin/metrics/measure/interactions_measure.rb index b928fdb8fb..7a2b7e0fac 100644 --- a/app/lib/admin/metrics/measure/interactions_measure.rb +++ b/app/lib/admin/metrics/measure/interactions_measure.rb @@ -5,20 +5,20 @@ class Admin::Metrics::Measure::InteractionsMeasure < Admin::Metrics::Measure::Ba 'interactions' end - def total + protected + + def perform_total_query activity_tracker.sum(time_period.first, time_period.last) end - def previous_total + def perform_previous_total_query activity_tracker.sum(previous_time_period.first, previous_time_period.last) end - def data + def perform_data_query activity_tracker.get(time_period.first, time_period.last).map { |date, value| { date: date.to_time(:utc).iso8601, value: value.to_s } } end - protected - def activity_tracker @activity_tracker ||= ActivityTracker.new('activity:interactions', :basic) end diff --git a/app/lib/admin/metrics/measure/new_users_measure.rb b/app/lib/admin/metrics/measure/new_users_measure.rb index b31679ad3a..71191f1a22 100644 --- a/app/lib/admin/metrics/measure/new_users_measure.rb +++ b/app/lib/admin/metrics/measure/new_users_measure.rb @@ -5,15 +5,17 @@ class Admin::Metrics::Measure::NewUsersMeasure < Admin::Metrics::Measure::BaseMe 'new_users' end - def total + protected + + def perform_total_query User.where(created_at: time_period).count end - def previous_total + def perform_previous_total_query User.where(created_at: previous_time_period).count end - def data + def perform_data_query sql = <<-SQL.squish SELECT axis.*, ( WITH new_users AS ( diff --git a/app/lib/admin/metrics/measure/opened_reports_measure.rb b/app/lib/admin/metrics/measure/opened_reports_measure.rb index 9acc2c33db..4b80a0c8c3 100644 --- a/app/lib/admin/metrics/measure/opened_reports_measure.rb +++ b/app/lib/admin/metrics/measure/opened_reports_measure.rb @@ -5,15 +5,17 @@ class Admin::Metrics::Measure::OpenedReportsMeasure < Admin::Metrics::Measure::B 'opened_reports' end - def total + protected + + def perform_total_query Report.where(created_at: time_period).count end - def previous_total + def perform_previous_total_query Report.where(created_at: previous_time_period).count end - def data + def perform_data_query sql = <<-SQL.squish SELECT axis.*, ( WITH new_reports AS ( diff --git a/app/lib/admin/metrics/measure/resolved_reports_measure.rb b/app/lib/admin/metrics/measure/resolved_reports_measure.rb index 00cb24f7e1..4ab746c8fa 100644 --- a/app/lib/admin/metrics/measure/resolved_reports_measure.rb +++ b/app/lib/admin/metrics/measure/resolved_reports_measure.rb @@ -5,15 +5,17 @@ class Admin::Metrics::Measure::ResolvedReportsMeasure < Admin::Metrics::Measure: 'resolved_reports' end - def total + protected + + def perform_total_query Report.resolved.where(action_taken_at: time_period).count end - def previous_total + def perform_previous_total_query Report.resolved.where(action_taken_at: previous_time_period).count end - def data + def perform_data_query sql = <<-SQL.squish SELECT axis.*, ( WITH resolved_reports AS ( diff --git a/app/lib/admin/metrics/measure/tag_accounts_measure.rb b/app/lib/admin/metrics/measure/tag_accounts_measure.rb index ef773081be..8f4512efe7 100644 --- a/app/lib/admin/metrics/measure/tag_accounts_measure.rb +++ b/app/lib/admin/metrics/measure/tag_accounts_measure.rb @@ -9,20 +9,20 @@ class Admin::Metrics::Measure::TagAccountsMeasure < Admin::Metrics::Measure::Bas 'tag_accounts' end - def total + protected + + def perform_total_query tag.history.aggregate(time_period).accounts end - def previous_total + def perform_previous_total_query tag.history.aggregate(previous_time_period).accounts end - def data + def perform_data_query time_period.map { |date| { date: date.to_time(:utc).iso8601, value: tag.history.get(date).accounts.to_s } } end - protected - def tag @tag ||= Tag.find(params[:id]) end diff --git a/app/lib/admin/metrics/measure/tag_servers_measure.rb b/app/lib/admin/metrics/measure/tag_servers_measure.rb index cc064f63f6..11f229602e 100644 --- a/app/lib/admin/metrics/measure/tag_servers_measure.rb +++ b/app/lib/admin/metrics/measure/tag_servers_measure.rb @@ -9,15 +9,17 @@ class Admin::Metrics::Measure::TagServersMeasure < Admin::Metrics::Measure::Base 'tag_servers' end - def total + protected + + def perform_total_query tag.statuses.where('statuses.id BETWEEN ? AND ?', Mastodon::Snowflake.id_at(@start_at, with_random: false), Mastodon::Snowflake.id_at(@end_at, with_random: false)).joins(:account).count('distinct accounts.domain') end - def previous_total + def perform_previous_total_query tag.statuses.where('statuses.id BETWEEN ? AND ?', Mastodon::Snowflake.id_at(@start_at - length_of_period, with_random: false), Mastodon::Snowflake.id_at(@end_at - length_of_period, with_random: false)).joins(:account).count('distinct accounts.domain') end - def data + def perform_data_query sql = <<-SQL.squish SELECT axis.*, ( SELECT count(distinct accounts.domain) AS value @@ -38,8 +40,6 @@ class Admin::Metrics::Measure::TagServersMeasure < Admin::Metrics::Measure::Base rows.map { |row| { date: row['day'], value: row['value'].to_s } } end - protected - def tag @tag ||= Tag.find(params[:id]) end diff --git a/app/lib/admin/metrics/measure/tag_uses_measure.rb b/app/lib/admin/metrics/measure/tag_uses_measure.rb index b7667bc6cf..bce86b89f1 100644 --- a/app/lib/admin/metrics/measure/tag_uses_measure.rb +++ b/app/lib/admin/metrics/measure/tag_uses_measure.rb @@ -9,20 +9,20 @@ class Admin::Metrics::Measure::TagUsesMeasure < Admin::Metrics::Measure::BaseMea 'tag_uses' end - def total + protected + + def perform_total_query tag.history.aggregate(time_period).uses end - def previous_total + def perform_previous_total_query tag.history.aggregate(previous_time_period).uses end - def data + def perform_data_query time_period.map { |date| { date: date.to_time(:utc).iso8601, value: tag.history.get(date).uses.to_s } } end - protected - def tag @tag ||= Tag.find(params[:id]) end diff --git a/app/lib/admin/metrics/retention.rb b/app/lib/admin/metrics/retention.rb index 0179a6e282..f6135ac1ef 100644 --- a/app/lib/admin/metrics/retention.rb +++ b/app/lib/admin/metrics/retention.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class Admin::Metrics::Retention + CACHE_TTL = 5.minutes.freeze + class Cohort < ActiveModelSerializers::Model attributes :period, :frequency, :data end @@ -9,13 +11,37 @@ class Admin::Metrics::Retention attributes :date, :rate, :value end + attr_reader :loaded + + alias loaded? loaded + def initialize(start_at, end_at, frequency) @start_at = start_at&.to_date @end_at = end_at&.to_date @frequency = %w(day month).include?(frequency) ? frequency : 'day' + @loaded = false + end + + def cache_key + ['metrics/retention', @start_at, @end_at, @frequency].join(';') end def cohorts + load + end + + protected + + def load + unless loaded? + @values = Rails.cache.fetch(cache_key, expires_in: CACHE_TTL) { perform_query } + @loaded = true + end + + @values + end + + def perform_query sql = <<-SQL.squish SELECT axis.*, ( WITH new_users AS ( From 51e67f3243d5709361bde722f341b8a5e74d63ca Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Tue, 22 Feb 2022 15:27:25 +0100 Subject: [PATCH 3/4] Fix link colors in report and strike details (#17616) --- app/javascript/styles/mastodon/admin.scss | 47 ++++++++++--------- .../_account_warning.html.haml | 7 +-- app/views/disputes/strikes/show.html.haml | 4 +- 3 files changed, 30 insertions(+), 28 deletions(-) diff --git a/app/javascript/styles/mastodon/admin.scss b/app/javascript/styles/mastodon/admin.scss index d3ac4d9952..ee7b26d076 100644 --- a/app/javascript/styles/mastodon/admin.scss +++ b/app/javascript/styles/mastodon/admin.scss @@ -645,18 +645,6 @@ body, text-decoration: underline; } } - - &--inactive { - .log-entry__title { - text-decoration: line-through; - } - - a, - .username, - .target { - color: $darker-text-color; - } - } } a.name-tag, @@ -1192,17 +1180,6 @@ a.sparkline { font-weight: 600; padding: 4px 0; } - - a { - color: $ui-highlight-color; - text-decoration: none; - - &:hover, - &:focus, - &:active { - text-decoration: underline; - } - } } &--horizontal { @@ -1290,6 +1267,30 @@ a.sparkline { background: linear-gradient(to left, $ui-base-color, transparent); pointer-events: none; } + + a { + color: $secondary-text-color; + text-decoration: none; + unicode-bidi: isolate; + + &:hover { + text-decoration: underline; + + .fa { + color: lighten($dark-text-color, 7%); + } + } + + &.mention { + &:hover { + text-decoration: none; + + span { + text-decoration: underline; + } + } + } + } } &__actions { diff --git a/app/views/admin/account_warnings/_account_warning.html.haml b/app/views/admin/account_warnings/_account_warning.html.haml index ef23c3b776..1462e76d00 100644 --- a/app/views/admin/account_warnings/_account_warning.html.haml +++ b/app/views/admin/account_warnings/_account_warning.html.haml @@ -1,7 +1,8 @@ -= link_to disputes_strike_path(account_warning), class: ['log-entry', account_warning.overruled? && 'log-entry--inactive'] do += link_to disputes_strike_path(account_warning), class: 'log-entry' do .log-entry__header .log-entry__avatar - = image_tag account_warning.target_account.avatar.url(:original), alt: '', width: 40, height: 40, class: 'avatar' + .indicator-icon{ class: account_warning.overruled? ? 'success' : 'failure' } + = fa_icon 'warning' .log-entry__content .log-entry__title = t(account_warning.action, scope: 'admin.strikes.actions', name: content_tag(:span, account_warning.account.username, class: 'username'), target: content_tag(:span, account_warning.target_account.acct, class: 'target')).html_safe @@ -11,7 +12,7 @@ - if account_warning.report_id.present? · - = t('admin.reports.title', id: account_warning.report_id) + = t('admin.reports.report', id: account_warning.report_id) - if account_warning.overruled? · diff --git a/app/views/disputes/strikes/show.html.haml b/app/views/disputes/strikes/show.html.haml index 20a43ff98b..7248b25741 100644 --- a/app/views/disputes/strikes/show.html.haml +++ b/app/views/disputes/strikes/show.html.haml @@ -76,7 +76,7 @@ .report-header__details__item__header %strong= t('disputes.strikes.recipient') .report-header__details__item__content - = admin_account_link_to @strike.target_account, path: can?(:show, @strike.target_account) ? admin_account_path(@strike.target_account_id) : ActivityPub::TagManager.instance.url_for(@strike.target_account) + = link_to @strike.target_account.username, can?(:show, @strike.target_account) ? admin_account_path(@strike.target_account_id) : ActivityPub::TagManager.instance.url_for(@strike.target_account), class: 'table-action-link' .report-header__details__item .report-header__details__item__header %strong= t('disputes.strikes.action_taken') @@ -90,7 +90,7 @@ .report-header__details__item__header %strong= t('disputes.strikes.associated_report') .report-header__details__item__content - = link_to t('admin.reports.report', id: @strike.report.id), admin_report_path(@strike.report) + = link_to t('admin.reports.report', id: @strike.report.id), admin_report_path(@strike.report), class: 'table-action-link' - if @appeal.persisted? .report-header__details__item .report-header__details__item__header From 166f6e4b500dd84eeffdbf887b2dc21e6d8c0aa6 Mon Sep 17 00:00:00 2001 From: Claire Date: Tue, 22 Feb 2022 17:11:22 +0100 Subject: [PATCH 4/4] Fix some media attachments being converted with too high framerates (#17619) Video files with variable framerates are converted to constant framerate videos and the output framerate picked by ffmpeg is based on the original file's container framerate (which can be different from the average framerate). This means that an input video with variable framerate with about 30 frames per second on average, but a maximum of 120 fps will be converted to a constant 120 fps file, which won't be processed by other Mastodon servers. This commit changes it so that input files with VFR and a maximum framerate above the framerate threshold are converted to VFR files with the maximum frame rate enforced. --- app/lib/video_metadata_extractor.rb | 3 ++- app/models/media_attachment.rb | 13 +++++++------ lib/paperclip/transcoder.rb | 16 +++++++++++++++- 3 files changed, 24 insertions(+), 8 deletions(-) diff --git a/app/lib/video_metadata_extractor.rb b/app/lib/video_metadata_extractor.rb index 03e40f923e..2896620cb2 100644 --- a/app/lib/video_metadata_extractor.rb +++ b/app/lib/video_metadata_extractor.rb @@ -2,7 +2,7 @@ class VideoMetadataExtractor attr_reader :duration, :bitrate, :video_codec, :audio_codec, - :colorspace, :width, :height, :frame_rate + :colorspace, :width, :height, :frame_rate, :r_frame_rate def initialize(path) @path = path @@ -42,6 +42,7 @@ class VideoMetadataExtractor @width = video_stream[:width] @height = video_stream[:height] @frame_rate = video_stream[:avg_frame_rate] == '0/0' ? nil : Rational(video_stream[:avg_frame_rate]) + @r_frame_rate = video_stream[:r_frame_rate] == '0/0' ? nil : Rational(video_stream[:r_frame_rate]) end if (audio_stream = audio_streams.first) diff --git a/app/models/media_attachment.rb b/app/models/media_attachment.rb index 0a9d05f1d7..a3115637e5 100644 --- a/app/models/media_attachment.rb +++ b/app/models/media_attachment.rb @@ -38,6 +38,12 @@ class MediaAttachment < ApplicationRecord MAX_DESCRIPTION_LENGTH = 1_500 + IMAGE_LIMIT = 10.megabytes + VIDEO_LIMIT = 40.megabytes + + MAX_VIDEO_MATRIX_LIMIT = 2_304_000 # 1920x1200px + MAX_VIDEO_FRAME_RATE = 60 + IMAGE_FILE_EXTENSIONS = %w(.jpg .jpeg .png .gif).freeze VIDEO_FILE_EXTENSIONS = %w(.webm .mp4 .m4v .mov).freeze AUDIO_FILE_EXTENSIONS = %w(.ogg .oga .mp3 .wav .flac .opus .aac .m4a .3gp .wma).freeze @@ -75,6 +81,7 @@ class MediaAttachment < ApplicationRecord VIDEO_FORMAT = { format: 'mp4', content_type: 'video/mp4', + vfr_frame_rate_threshold: MAX_VIDEO_FRAME_RATE, convert_options: { output: { 'loglevel' => 'fatal', @@ -152,12 +159,6 @@ class MediaAttachment < ApplicationRecord all: '-quality 90 -strip +set modify-date +set create-date', }.freeze - IMAGE_LIMIT = 10.megabytes - VIDEO_LIMIT = 40.megabytes - - MAX_VIDEO_MATRIX_LIMIT = 2_304_000 # 1920x1200px - MAX_VIDEO_FRAME_RATE = 60 - belongs_to :account, inverse_of: :media_attachments, optional: true belongs_to :status, inverse_of: :media_attachments, optional: true belongs_to :scheduled_status, inverse_of: :media_attachments, optional: true diff --git a/lib/paperclip/transcoder.rb b/lib/paperclip/transcoder.rb index ec13050382..afd9f58ff6 100644 --- a/lib/paperclip/transcoder.rb +++ b/lib/paperclip/transcoder.rb @@ -13,6 +13,7 @@ module Paperclip @time = options[:time] || 3 @passthrough_options = options[:passthrough_options] @convert_options = options[:convert_options].dup + @vfr_threshold = options[:vfr_frame_rate_threshold] end def make @@ -41,6 +42,11 @@ module Paperclip when 'mp4' @output_options['acodec'] = 'aac' @output_options['strict'] = 'experimental' + + if high_vfr?(metadata) && !eligible_to_passthrough?(metadata) + @output_options['vsync'] = 'vfr' + @output_options['r'] = @vfr_threshold + end end command_arguments, interpolations = prepare_command(destination) @@ -88,13 +94,21 @@ module Paperclip end def update_options_from_metadata(metadata) - return unless @passthrough_options && @passthrough_options[:video_codecs].include?(metadata.video_codec) && @passthrough_options[:audio_codecs].include?(metadata.audio_codec) && @passthrough_options[:colorspaces].include?(metadata.colorspace) + return unless eligible_to_passthrough?(metadata) @format = @passthrough_options[:options][:format] || @format @time = @passthrough_options[:options][:time] || @time @convert_options = @passthrough_options[:options][:convert_options].dup end + def high_vfr?(metadata) + @vfr_threshold && metadata.r_frame_rate && metadata.r_frame_rate > @vfr_threshold + end + + def eligible_to_passthrough?(metadata) + @passthrough_options && @passthrough_options[:video_codecs].include?(metadata.video_codec) && @passthrough_options[:audio_codecs].include?(metadata.audio_codec) && @passthrough_options[:colorspaces].include?(metadata.colorspace) + end + def update_attachment_type(metadata) @attachment.instance.type = MediaAttachment.types[:gifv] unless metadata.audio_codec end