Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add dearrow support for thumbnails #4520

Merged

Conversation

ChunkyProgrammer
Copy link
Member

Add dearrow support for thumbnails

Pull Request Type

  • Feature Implementation

Related issue

closes #4146

Description

This PR allows for using DeArrow for thumbnails. This PR also stores the duration of a video in the dearrow cache if available and will fallback to the dearrow duration for shorts, etc.

Screenshots

DeArrowed Screenshot:
image

Non DeArrowed Screenshot:
image

Testing

test 1

  • enable DeArrow Thumbnails
  • enable DeArrow Titles
  • go to subscription feed
  • see DeArrowed titles when available
  • See dearrowed thumbnails when available

test 2

  • enable DeArrow Thumbnails
  • disable DeArrow Titles
  • go to subscription feed
  • see some thumbnails update (if available). Thumbnails should update next time the page is viewed

test 3

  • disable DeArrow Thumbnails
  • enable DeArrow Titles
  • see dearrowed titles when available
  • see original thumbnail

Desktop

  • OS: Solus
  • OS Version: Solus 4.5 Resilience
  • FreeTube version: 0.19.1

@github-actions github-actions bot added the PR: waiting for review For PRs that are complete, tested, and ready for review label Jan 4, 2024
@FreeTubeBot FreeTubeBot enabled auto-merge (squash) January 4, 2024 03:50
@kommunarr
Copy link
Collaborator

Is missing the tooltip label, otherwise is looking good behavior-wise.

@efb4f5ff-1298-471a-8973-3d47447115dc
Copy link
Member

efb4f5ff-1298-471a-8973-3d47447115dc commented Jan 5, 2024

All Thumbnail Preferences are overwritten except for Blur.

Q: Should we even overwrite thumbnail preference Hidden and Blur

  • If answer is yes then we should make this clear in the tooltip messaging

  • Im leaning towards no because when Display Titles Without Excessive Capitalization is enabled and DeArrow Titles are enabled, It will adjust the DeArrow titles to not use Excessive Capitalization. So if we continue this thought Hidden and Blur should overwrite DeArrow Thumbnails

@@ -367,6 +376,13 @@ export default defineComponent({
}
},

displayDuration: function () {
if (this.useDeArrowTitles && (this.duration === '' || this.duration === '0:00') && this.deArrowCache?.videoDuration) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How to test this

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. In the "Subscription Settings" turn on "Fetch Feeds from RSS".
  2. Subscribe to a channel that has dearrow titles e.g. https://www.youtube.com/@MrBeast
  3. On the subscription page any unwatched videos from that channel should now have durations shown (for watched videos the duration comes from the watch history).

Copy link
Member

@absidue absidue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code works, just a few small suggestions concerning performance and an outdated code comment.

src/renderer/components/ft-list-video/ft-list-video.js Outdated Show resolved Hide resolved
src/renderer/components/ft-list-video/ft-list-video.js Outdated Show resolved Hide resolved
src/renderer/components/ft-list-video/ft-list-video.js Outdated Show resolved Hide resolved
src/renderer/helpers/sponsorblock.js Outdated Show resolved Hide resolved
@absidue absidue added PR: changes requested and removed PR: waiting for review For PRs that are complete, tested, and ready for review labels Jan 14, 2024
Co-authored-by: absidue <48293849+absidue@users.noreply.github.com>
@absidue absidue added PR: waiting for review For PRs that are complete, tested, and ready for review and removed PR: changes requested labels Jan 14, 2024
Copy link
Collaborator

@PikachuEXE PikachuEXE left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested fetch sub via RSS + duration
image

@FreeTubeBot FreeTubeBot merged commit ae7a2fa into FreeTubeApp:development Jan 15, 2024
5 checks passed
@github-actions github-actions bot removed the PR: waiting for review For PRs that are complete, tested, and ready for review label Jan 15, 2024
PikachuEXE added a commit to PikachuEXE/FreeTube that referenced this pull request Jan 15, 2024
…mark

* development: (53 commits)
  Add dearrow support for thumbnails (FreeTubeApp#4520)
  Translated using Weblate (French)
  Fix hardware acceleration flag for Linux (FreeTubeApp#4532)
  Translated using Weblate (Bengali)
  Translated using Weblate (Czech)
  Translated using Weblate (Hungarian)
  Translated using Weblate (Turkish)
  Translated using Weblate (Spanish)
  Translated using Weblate (Arabic)
  Translated using Weblate (Italian)
  Translated using Weblate (Polish)
  Translated using Weblate (Russian)
  Translated using Weblate (French)
  Translated using Weblate (Chinese (Simplified))
  Add toggle to suppress sending additional args to external players (FreeTubeApp#4515)
  Translated using Weblate (English (United Kingdom))
  Translated using Weblate (Italian)
  Translated using Weblate (French)
  Translated using Weblate (Finnish)
  Translated using Weblate (Polish)
  ...

# Conflicts:
#	src/renderer/store/modules/settings.js
PikachuEXE added a commit to PikachuEXE/FreeTube that referenced this pull request Jan 15, 2024
…m-builds/current

* feature/consistent-sharable-video-url-local: (30 commits)
  * Update places generating sharable YT video URLs to always return prefix https://youtu.be/
  Translated using Weblate (Chinese (Traditional))
  Translated using Weblate (Chinese (Simplified))
  Add dearrow support for thumbnails (FreeTubeApp#4520)
  Translated using Weblate (French)
  Fix hardware acceleration flag for Linux (FreeTubeApp#4532)
  Translated using Weblate (Bengali)
  Translated using Weblate (Czech)
  Translated using Weblate (Hungarian)
  Translated using Weblate (Turkish)
  Translated using Weblate (Spanish)
  Translated using Weblate (Arabic)
  Translated using Weblate (Italian)
  Translated using Weblate (Polish)
  Translated using Weblate (Russian)
  Translated using Weblate (French)
  Translated using Weblate (Chinese (Simplified))
  Add toggle to suppress sending additional args to external players (FreeTubeApp#4515)
  Translated using Weblate (English (United Kingdom))
  Translated using Weblate (Italian)
  ...
@ChunkyProgrammer ChunkyProgrammer deleted the dearrow-thumbnails branch January 30, 2024 00:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request]: Improved Thumbnails using DeArrow
6 participants