From bb261094cf7ec8f0baf17bf9313c878a9f6fb05d Mon Sep 17 00:00:00 2001 From: Blake Erickson Date: Wed, 14 Feb 2024 13:43:53 -0700 Subject: [PATCH] FEATURE: Auto generate and display video preview image (#25633) This change will allow auto generated video thumbnails to be used instead of the black video thumbnail that overlays videos. Follow up to: 2443446e62bff9dd8a0493d10e5359a5e946a84e --- .../discourse-markdown-it/src/engine.js | 6 +- .../video-placeholder.js | 9 ++ .../mixins/composer-video-thumbnail-uppy.js | 123 +++++++++++------- .../tests/unit/lib/pretty-text-test.js | 4 +- .../stylesheets/common/base/onebox.scss | 4 + app/models/post.rb | 16 ++- lib/pretty_text.rb | 16 +++ spec/system/composer_uploads_spec.rb | 32 ++++- 8 files changed, 147 insertions(+), 63 deletions(-) diff --git a/app/assets/javascripts/discourse-markdown-it/src/engine.js b/app/assets/javascripts/discourse-markdown-it/src/engine.js index 75f0e3b7f91..8e5deb7bdf0 100644 --- a/app/assets/javascripts/discourse-markdown-it/src/engine.js +++ b/app/assets/javascripts/discourse-markdown-it/src/engine.js @@ -187,7 +187,11 @@ function renderImageOrPlayableMedia(tokens, idx, options, env, slf) { options.discourse.previewing && !options.discourse.limitedSiteSettings.enableDiffhtmlPreview ) { - return `
+ const origSrc = token.attrGet("data-orig-src"); + const origSrcId = origSrc + .substring(origSrc.lastIndexOf("/") + 1) + .split(".")[0]; + return `
`; } else { diff --git a/app/assets/javascripts/discourse/app/instance-initializers/video-placeholder.js b/app/assets/javascripts/discourse/app/instance-initializers/video-placeholder.js index 150dcbc88b0..32201ba9f1d 100644 --- a/app/assets/javascripts/discourse/app/instance-initializers/video-placeholder.js +++ b/app/assets/javascripts/discourse/app/instance-initializers/video-placeholder.js @@ -80,6 +80,15 @@ export default { ); containers.forEach((container) => { + // Add video thumbnail image + if (container.dataset.thumbnailSrc) { + const thumbnail = new Image(); + thumbnail.onload = function () { + container.style.backgroundImage = "url('" + thumbnail.src + "')"; + }; + thumbnail.src = container.dataset.thumbnailSrc; + } + const wrapper = document.createElement("div"), overlay = document.createElement("div"); diff --git a/app/assets/javascripts/discourse/app/mixins/composer-video-thumbnail-uppy.js b/app/assets/javascripts/discourse/app/mixins/composer-video-thumbnail-uppy.js index 612d254830f..8977442e5d5 100644 --- a/app/assets/javascripts/discourse/app/mixins/composer-video-thumbnail-uppy.js +++ b/app/assets/javascripts/discourse/app/mixins/composer-video-thumbnail-uppy.js @@ -31,9 +31,11 @@ export default class ComposerVideoThumbnailUppy extends EmberObject.extend( uploadRootPath = "/uploads"; uploadTargetBound = false; useUploadPlaceholders = true; + capabilities = null; constructor(owner) { super(...arguments); + this.capabilities = owner.lookup("service:capabilities"); setOwner(this, owner); } @@ -55,13 +57,17 @@ export default class ComposerVideoThumbnailUppy extends EmberObject.extend( video.muted = true; video.playsinline = true; - let videoSha1 = uploadUrl + const videoSha1 = uploadUrl .substring(uploadUrl.lastIndexOf("/") + 1) .split(".")[0]; // Wait for the video element to load, otherwise the canvas will be empty. - // iOS Safari prefers onloadedmetadata over oncanplay. - video.onloadedmetadata = () => { + // iOS Safari prefers onloadedmetadata over oncanplay. System tests running in Chrome + // prefer oncanplaythrough. + const eventName = this.capabilities.isIOS + ? "onloadedmetadata" + : "oncanplaythrough"; + video[eventName] = () => { const canvas = document.createElement("canvas"); const ctx = canvas.getContext("2d"); canvas.width = video.videoWidth; @@ -70,62 +76,79 @@ export default class ComposerVideoThumbnailUppy extends EmberObject.extend( // A timeout is needed on mobile. setTimeout(() => { ctx.drawImage(video, 0, 0, video.videoWidth, video.videoHeight); + // Detect Empty Thumbnail + const imageData = ctx.getImageData(0, 0, canvas.width, canvas.height); + const data = imageData.data; - // upload video thumbnail - canvas.toBlob((blob) => { - this._uppyInstance = new Uppy({ - id: "video-thumbnail", - meta: { - videoSha1, - upload_type: "thumbnail", - }, - autoProceed: true, - }); - - if (this.siteSettings.enable_upload_debug_mode) { - this._instrumentUploadTimings(); + let isEmpty = true; + for (let i = 0; i < data.length; i += 4) { + // Check RGB values + if (data[i] !== 0 || data[i + 1] !== 0 || data[i + 2] !== 0) { + isEmpty = false; + break; } + } - if (this.siteSettings.enable_direct_s3_uploads) { - this._useS3MultipartUploads(); - } else { - this._useXHRUploads(); - } + if (!isEmpty) { + // upload video thumbnail + canvas.toBlob((blob) => { + this._uppyInstance = new Uppy({ + id: "video-thumbnail", + meta: { + videoSha1, + upload_type: "thumbnail", + }, + autoProceed: true, + }); - this._uppyInstance.on("upload", () => { - this.uploading = true; - }); - - this._uppyInstance.on("upload-success", () => { - this.uploading = false; - callback(); - }); - - this._uppyInstance.on("upload-error", (file, error, response) => { - let message = I18n.t("wizard.upload_error"); - if (response.body.errors) { - message = response.body.errors.join("\n"); + if (this.siteSettings.enable_upload_debug_mode) { + this._instrumentUploadTimings(); } - // eslint-disable-next-line no-console - console.error(message); - this.uploading = false; - callback(); - }); + if (this.siteSettings.enable_direct_s3_uploads) { + this._useS3MultipartUploads(); + } else { + this._useXHRUploads(); + } - try { - this._uppyInstance.addFile({ - source: `${this.id}-video-thumbnail`, - name: `${videoSha1}`, - type: blob.type, - data: blob, + this._uppyInstance.on("upload", () => { + this.uploading = true; }); - } catch (err) { - warn(`error adding files to uppy: ${err}`, { - id: "discourse.upload.uppy-add-files-error", + + this._uppyInstance.on("upload-success", () => { + this.uploading = false; + callback(); }); - } - }); + + this._uppyInstance.on("upload-error", (file, error, response) => { + let message = I18n.t("wizard.upload_error"); + if (response.body.errors) { + message = response.body.errors.join("\n"); + } + + // eslint-disable-next-line no-console + console.error(message); + this.uploading = false; + callback(); + }); + + try { + this._uppyInstance.addFile({ + source: `${this.id}-video-thumbnail`, + name: `${videoSha1}`, + type: blob.type, + data: blob, + }); + } catch (err) { + warn(`error adding files to uppy: ${err}`, { + id: "discourse.upload.uppy-add-files-error", + }); + } + }); + } else { + this.uploading = false; + callback(); + } }, 100); }; } diff --git a/app/assets/javascripts/discourse/tests/unit/lib/pretty-text-test.js b/app/assets/javascripts/discourse/tests/unit/lib/pretty-text-test.js index 897db54958e..694038f1155 100644 --- a/app/assets/javascripts/discourse/tests/unit/lib/pretty-text-test.js +++ b/app/assets/javascripts/discourse/tests/unit/lib/pretty-text-test.js @@ -1498,8 +1498,8 @@ var bar = 'bar'; assert.cookedOptions( `![baby shark|video](upload://eyPnj7UzkU0AkGkx2dx8G4YM1Jx.mp4)`, { previewing: true }, - `

- + `

+

` ); }); diff --git a/app/assets/stylesheets/common/base/onebox.scss b/app/assets/stylesheets/common/base/onebox.scss index b456992ecfb..ff495c5e914 100644 --- a/app/assets/stylesheets/common/base/onebox.scss +++ b/app/assets/stylesheets/common/base/onebox.scss @@ -935,6 +935,8 @@ aside.onebox.mixcloud-preview { } } .video-placeholder-container { + background-size: cover; + background-position: center; position: relative; padding: 0 0 56.25% 0; width: 100%; @@ -1000,6 +1002,8 @@ iframe.vimeo-onebox { } .onebox-placeholder-container { + background-size: cover; + background-position: center; position: relative; width: 100%; padding: 0 0 48.25% 0; diff --git a/app/models/post.rb b/app/models/post.rb index 30dfb179862..c24d06ca094 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -1048,14 +1048,16 @@ class Post < ActiveRecord::Base .where("original_filename like ?", "#{upload.sha1}.%") .order(id: :desc) .first if upload.sha1.present? - if thumbnail.present? && self.is_first_post? && !self.topic.image_upload_id + if thumbnail.present? upload_ids << thumbnail.id - self.topic.update_column(:image_upload_id, thumbnail.id) - extra_sizes = - ThemeModifierHelper.new( - theme_ids: Theme.user_selectable.pluck(:id), - ).topic_thumbnail_sizes - self.topic.generate_thumbnails!(extra_sizes: extra_sizes) + if self.is_first_post? && !self.topic.image_upload_id + self.topic.update_column(:image_upload_id, thumbnail.id) + extra_sizes = + ThemeModifierHelper.new( + theme_ids: Theme.user_selectable.pluck(:id), + ).topic_thumbnail_sizes + self.topic.generate_thumbnails!(extra_sizes: extra_sizes) + end end end upload_ids << upload.id if upload.present? diff --git a/lib/pretty_text.rb b/lib/pretty_text.rb index 375e8b1b343..cdeeab19746 100644 --- a/lib/pretty_text.rb +++ b/lib/pretty_text.rb @@ -302,6 +302,7 @@ module PrettyText add_rel_attributes_to_user_content(doc, add_nofollow) strip_hidden_unicode_bidirectional_characters(doc) sanitize_hotlinked_media(doc) + add_video_placeholder_image(doc) add_mentions(doc, user_id: opts[:user_id]) if SiteSetting.enable_mentions @@ -442,6 +443,21 @@ module PrettyText links end + def self.add_video_placeholder_image(doc) + doc + .css(".video-placeholder-container") + .each do |video| + video_src = video["data-video-src"] + video_sha1 = File.basename(video_src, File.extname(video_src)) + thumbnail = Upload.where("original_filename LIKE ?", "#{video_sha1}.%").last + if thumbnail + video["data-thumbnail-src"] = UrlHelper.absolute( + GlobalPath.upload_cdn_path(thumbnail.url), + ) + end + end + end + def self.extract_mentions(cooked) mentions = cooked diff --git a/spec/system/composer_uploads_spec.rb b/spec/system/composer_uploads_spec.rb index d7a7574b452..7c081509b90 100644 --- a/spec/system/composer_uploads_spec.rb +++ b/spec/system/composer_uploads_spec.rb @@ -51,7 +51,7 @@ describe "Uploading files in the composer", type: :system do # we need to come back to this in a few months and try again. # # c.f. https://groups.google.com/g/chromedriver-users/c/1SMbByMfO2U - xit "generates a thumbnail for the video" do + xit "generates a topic preview thumbnail from the video" do sign_in(current_user) visit "/new-topic" @@ -62,12 +62,38 @@ describe "Uploading files in the composer", type: :system do attach_file(file_path_1) { composer.click_toolbar_button("upload") } expect(composer).to have_no_in_progress_uploads - expect(composer.preview).to have_css(".video-container") + expect(composer.preview).to have_css(".onebox-placeholder-container") composer.submit expect(find("#topic-title")).to have_content("Video upload test") - expect(topic.image_upload_id).to eq(Upload.last.id) + # I think topic list previews need to be enabled for this? + #expect(topic.image_upload_id).to eq(Upload.last.id) + end + + it "generates a thumbnail from the video" do + sign_in(current_user) + + visit "/new-topic" + expect(composer).to be_opened + topic.fill_in_composer_title("Video upload test") + + file_path_1 = file_from_fixtures("small.mp4", "media").path + attach_file(file_path_1) { composer.click_toolbar_button("upload") } + + expect(composer).to have_no_in_progress_uploads + expect(composer.preview).to have_css(".onebox-placeholder-container") + + composer.submit + + expect(find("#topic-title")).to have_content("Video upload test") + + selector = topic.post_by_number_selector(1) + + expect(page).to have_css(selector) + within(selector) do + expect(page).to have_css(".video-placeholder-container[data-thumbnail-src]") + end end end end