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

playerPlaybackSpeed patch #1729

Merged
merged 3 commits into from
Aug 29, 2023
Merged

playerPlaybackSpeed patch #1729

merged 3 commits into from
Aug 29, 2023

Conversation

raszpl
Copy link
Contributor

@raszpl raszpl commented Aug 8, 2023

player.js
fixing playerPlaybackSpeed

  • music detection works again
  • clicking "(Force playback speed even for music?)" triggers immediate change
  • disabling "Forced playback speed, speed-watching" correctly restores normal playback speed
  • no longer setting setInterval without any reason on every page load

Update functions.js

  • removed unused parameter

raszpl added 2 commits August 8, 2023 01:41
fixing playerPlaybackSpeed
- music detection works again
- clicking "(Force playback speed even for music?)" triggers immediate change
- no longer setting setInterval without any reason on every page load
removed unused parameter
@raszpl
Copy link
Contributor Author

raszpl commented Aug 8, 2023

as I mentioned in f6d277b#commitcomment-123855032 I fixed music detection, it wasnt doing anything before.

  1. I still dont understand the need for waiting for document.querySelector('div#description') in setInterval. Im sure its there for some (historical?) reason, but I tested it locally and at least in my browser I could get rid of whole setInterval simplifying this function greatly and it still worked as intended.

Without it it becomes just:

	if (this.storage.player_forced_playback_speed === true) {
		if (player.getVideoData().isLive === false) {
			let category = document.querySelector('meta[itemprop=genre]')?.content;
			let titlekeywords = document.getElementsByTagName('meta')?.title?.content + document.getElementsByTagName('meta')?.keywords?.content;
			if (this.storage.player_force_speed_on_music === true // dont care if music, just switch it
				|| (this.storage.player_force_speed_on_music === false // check if NOT music first
					&& document.querySelector('h3#title')?.innerText !== 'Music' // (=buyable/registered music table)
					&& category !== 'Music'
					&& !(/official (music )?video|lyrics|cover[\)\]]|[\(\[]cover|cover version|karaok|(sing|play)[- ]?along|卡拉OK|卡拉OK|الكاريوكيкараоке|カラオケ|노래방/i.test(titlekeywords) && !/do[ck]u|interv[iyj]|back[- ]?stage|インタビュー|entrevista|面试|面試|회견|wawancara|مقابلة|интервью|entretien|기록한 것|记录|記錄|ドキュメンタリ|وثائقي|документальный/i.test(titlekeywords)) // says its music in description, but not backstage/interview? ugly hack!
				// && location.href.indexOf('music') === -1	// (=only running on www.youtube.com anyways)
				)) {
					player.setPlaybackRate(Number(option));
					video.playbackRate = Number(option);
			} else { // Music and we are not overriding speed, set normal
				player.setPlaybackRate(1);
				video.playbackRate = 1;
			}
		}
	} else {
		player.setPlaybackRate(1);
		video.playbackRate = 1;
	}

  1. I dont know what video.playbackRate is used for
  2. I dont know what ImprovedTube.elements.player.setPlaybackRate is used for
    if (ImprovedTube.elements.player && ImprovedTube.elements.player.setPlaybackRate) {

    if (ImprovedTube.elements.player && ImprovedTube.elements.player.setPlaybackRate) {

    but I do know that it causes initPlayer() to be called twice on some links. For example
    https://www.youtube.com/watch?v=PeZxlCFtOWQ&list=PLqPaeYdiNYymLVvaUjj4AzIDJBGQ2kCOl&index=35
    will cause both 63 and 77 lines be executed when loading, but
    https://www.youtube.com/watch?v=PeZxlCFtOWQ&list=PLqPaeYdiNYymLVvaUjj4AzIDJBGQ2kCOl
    will only execute 63. No idea whats going on, please explain if you can.

I did some very limited testing, for example works on those three video correctly detecting them as "music".
https://www.youtube.com/watch?v=b7k0a5hYnSI Natasha Bedingfield - Unwritten (US Version) (Official Video)
https://www.youtube.com/watch?v=cFFBSSntZgs Natasha Bedingfield - Unwritten (Official Video)
https://www.youtube.com/watch?v=WEl4GhdReRk Natasha Bedingfield - Unwritten (Mia Rose Cover)
third one relies on "&& !(/official (music )?video|lyrics|cover[)]]......" condition

unused props
@raszpl raszpl changed the title Raszpl patch 1 playerPlaybackSpeed patch Aug 8, 2023
@ImprovedTube
Copy link
Member

ImprovedTube commented Aug 8, 2023

thanks! (not sure what we could miss here if anything, besides I remember people saying the feature shouldn't undo when manually settting another speed in youtube's player)

let category = document.querySelector('meta[itemprop=genre]')?.content;
let titlekeywords = document.getElementsByTagName('meta')?.title?.content +
document.getElementsByTagName('meta')?.keywords?.content;

👍
in player.js these can come after if (this.storage.player_force_speed_on_music === true ) {.. (besides that all meta info could easily also be used on every page view (for many potential enhancements/features & many people would at some point appreciate a log of it, to analyse later)

(- or we can first improve the childHandler etc. further)


|| (this.storage.player_force_speed_on_music === false // check if NOT music first

( not required / no difference )

The following lines can stay as is (just adding your titlekeywords) :

		(this.storage.player_force_speed_on_music === true ||
			 document.querySelector('h3#title')?.innerText !== 'Music'  // (=buyable/registered music table)
			|| (
			    (ImprovedTube.elements.category !== 'Music' && !/official (music )?video|lyrics|cover[\)\]]|[\(\[]cover|cover version|karaok|(sing|play)[- ]?along|卡拉OK|卡拉OK|الكاريوكيкараоке|カラオケ|노래방/i.test(titlekeywords)       
				) || /do[ck]u|interv[iyj]|back[- ]?stage|インタビュー|entrevista|面试|面試|회견|wawancara|مقابلة|интервью|entretien|기록한 것|记录|記錄|ドキュメンタリ|وثائقي|документальный/i.test(titlekeywords)                 
					  // not category Music (nor title) - unless it is a documentary, interview or backstage. (Tags/keywords shouldnt lie & only a few funny songs might have such words in their title)  
  • (whitelisting will more likely matters to the current users the the few false matches.) (more so since we exclude music byy default). Of course guesses can be improved open-ended (a documentary might usually not come in a song length. etc.), and might stay relevant some more toughts deep. (Besides that the "speeding" still requires optimism in the first place):

#1636

will only execute 63.

document.addEventListener('yt-navigate-finish', function () {

The "yt-navigate-finish" event listener in the YouTube website's scripts will not work in the following situations: When the page is dynamically updated or does not finish loading. (or when the user is not currently on the YouTube website or the listener is not registered ~ # 1585)

@raszpl
Copy link
Contributor Author

raszpl commented Aug 9, 2023

Do you know the answer to my questions 1?


ad question 3

document.addEventListener('yt-navigate-finish', function () {
The "yt-navigate-finish" event listener in the YouTube website's scripts will not work in the following situations: When the page is dynamically updated or does not finish loading. (or when the user is not currently on the YouTube website or the listener is not registered ~ # 1585)

doesnt explain why it triggers when you load
https://www.youtube.com/watch?v=PeZxlCFtOWQ&list=PLqPaeYdiNYymLVvaUjj4AzIDJBGQ2kCOl&index=35
but not when loading exact same playlist and exact same video with slightly different url
https://www.youtube.com/watch?v=PeZxlCFtOWQ&list=PLqPaeYdiNYymLVvaUjj4AzIDJBGQ2kCOl


Answering my own question 2: I see those two arent doing same thing

document.getElementsByTagName('video')[0].playbackRate = 0.5
document.querySelector('.html5-video-player').setPlaybackRate(0.5)
document.getElementsByTagName('video')[0].playbackRate
document.querySelector('.html5-video-player').getPlaybackRate()
  • .playbackRate is lower level, will set 0.1 while .setPlaybackRate() doesnt go below 0.25 and higher than 2.
  • .setPlaybackRate() with parameters below 0.25 or higher than 2 will limit to those boundaries.
  • .getPlaybackRate() doesnt know about .playbackRate changes.
  • YT player GUI Settings/Playback Speed doesnt know about .playbackRate changes.
  • YT player GUI Settings/Playback Speed only updates on .setPlaybackRate() thus is also limited to [0.25-2] range.

feature shouldn't undo when manually settting another speed in youtube's player)

Can you save preferred playback speed in stock YT player? Afaik you have to set it every time manually, and that just works. If you mean something else please explain.


&& document.querySelector('h3#title')?.innerText !== 'Music' // (=buyable/registered music table)
this is problematic, for example https://www.youtube.com/watch?v=-SMWz5WPQmA has nothing to do with music, but will trigger just because YT detected music in the background


btw whats the deal with both listings having same name?
https://chrome.google.com/webstore/detail/youtube/lodjfjlkodalimdjgncejhkadjhacgki
https://chrome.google.com/webstore/detail/youtube/bnomihfieiccainjcjblhegjgglakjdd
one used to be clearly named Beta Testing. Currently if you have both installed you cant tell which one is which :(

@ImprovedTube
Copy link
Member

ImprovedTube commented Aug 13, 2023

hi @raszpl 👍

Q1

yes, waiting for div#description is only for checking ('h3#title')?.innerText !== 'Music #1539

can come after this.storage.player_force_speed_on_music === true || and if the immediate <meta data checks aren't clear already

...('h3#title')?.innerText !== 'Music' // (=buyable/registered music table)
this is problematic, for example youtube.com/watch?v=-SMWz5WPQmA has nothing to do with music, but will trigger just because YT detected music in the background

yes, so this can only be a complement. combined with meta[itemprop=duration] it makes a strong signal.

commonSongDuration: 1:45-7m
(multiple songs in one video: the more songs, the closer their average-duration gets to ~3:30)
rareSongDuration: 7-12 & 0:50-1:45

music is:
likely enough:

  • youtube category music | music keyword

very likely:

  • commonSongDuration | rareSongDuration & music-table `
  • or 2 out of 3 (music-table ?? 0) + (youtube category music ?? 0) + (music keyword) ?? 0) >= 2;

most likely: (only the following cases can overide/outbid the speed-whitelist('documentary, interview,..).

  • commonSongDuration & music-table | youtube category music | music keyword

doesnt explain why it triggers when you load
youtube.com/watch?v=PeZxlCFtOWQ&list=PLqPaeYdiNYymLVvaUjj4AzIDJBGQ2kCOl&index=35
but not when loading exact same playlist and exact same video with slightly different url
youtube.com/watch?v=PeZxlCFtOWQ&list=PLqPaeYdiNYymLVvaUjj4AzIDJBGQ2kCOl

so it seems &index=35 makes a difference to yt-navigate-finish. Maybe our code is redundant/outdated, yet youtube's behavior with Playlist's is interesting #1544

both listings having same name?

Webstore isn't flexible there (can't undo mistakes immediately). Once submitted we got to wait for the review cycle twice (mostly just hours but used to take longer when we submitted twice just to correct a typo.)

@ImprovedTube ImprovedTube marked this pull request as ready for review August 24, 2023 16:44
@ImprovedTube ImprovedTube marked this pull request as draft August 24, 2023 16:45
@ImprovedTube ImprovedTube marked this pull request as ready for review August 29, 2023 19:07
@ImprovedTube ImprovedTube merged commit 7b3020f into code-charity:master Aug 29, 2023
ImprovedTube added a commit that referenced this pull request Aug 29, 2023
@ImprovedTube
Copy link
Member

added most said above(, whishing🤭 not to delay other work you possibly do at the project's core @raszpl )


  • music-detection is again waiting for the description to load & retrying a few times (it actually needs to wait only, not retry)

let category = document.querySelector('meta[itemprop=genre]')?.content;
if (this.storage.player_dont_speed_education === true && category === 'Education') // dont speed education
{ return;}
let titleAndKeywords = document.getElementsByTagName('meta')?.title?.content + " " + document.getElementsByTagName('meta')?.keywords?.content;
let duration = document.querySelector('meta[itemprop=duration]')?.content; // Example: PT1H20M30S
function parseDuration(duration) { const [_, h = 0, m = 0, s = 0] = duration.match(/PT(?:(\d+)?H)?(?:(\d+)?M)?(\d+)?S?/).map(part => parseInt(part) || 0);
return h * 3600 + m * 60 + s; }
let durationInSeconds = parseDuration(duration);
let musicRegexMatch = /official (music )?video|lyrics|cover[\)\]]|[\(\[]cover|cover version|karaok|(sing|play)[- ]?along|卡拉OK|卡拉OK|الكاريوكي|караоке|カラオケ|노래방/i.test(titleAndKeywords);
let notMusicRegexMatch = /do[ck]u|interv[iyj]|back[- ]?stage|インタビュー|entrevista|面试|面試|회견|wawancara|مقابلة|интервью|entretien|기록한 것|记录|記錄|ドキュメンタリ|وثائقي|документальный/i.test(titleAndKeywords);
// (Tags/keywords shouldnt lie & very few songs titles might have these words)
function testSongDuration(s) {
if (135 <= s && s <= 260) {return 'very common';}
if (105 <= s && s <= 420) {return 'common';}
if (420 <= s && s <= 720) {return 'long';}
if (45 <= s && s <= 105) {return 'short';}
let musicSectionLength = document.querySelector('div#items[class*="music-section"]')?.children?.length;
if (musicSectionLength && (120 <= s / musicSectionLength && s / musicSectionLength<= 410)) {return 'multiple';}
}
let songDurationType = testSongDuration(durationInSeconds);
// check if the video is PROBABLY MUSIC:
if ( (category === 'Music' || musicRegexMatch && !notMusicRegexMatch || songDurationType === 'verycommon')
|| (category === 'Music' && musicRegexMatch && typeof songDurationType !== 'undefined'
|| (/album|Álbum|专辑|專輯|एलबम|البوم|アルバム|альбом|앨범/i.test(titleAndKeywords)
&& ([1150, 5000].includes(durationInSeconds))
)
)
)
{ //player.setPlaybackRate(1); video.playbackRate = 1;
}
else { player.setPlaybackRate(Number(option)); video.playbackRate = Number(option); // #1729 question2
// Now this video might rarely be music
// - however we can make extra-sure after waiting for the video descripion to load... (#1539)
tries = 0; intervalMs = 150; if (location.href.indexOf('/watch?') !== -1) {maxTries = 10;} else {maxTries = 1;}
// ...except when it is an embedded player?
var waitForDescription = setInterval(() => {
if (document.querySelector('div#description') || (++tries >= maxTries) ) {clearInterval(waitForDescription);}
if (document.querySelector('h3#title[class*="music-section"]') // indicates buyable/registered music
&& typeof testSongDuration(durationInSeconds) !== 'undefined' ) // resonable duration
{player.setPlaybackRate(1); video.playbackRate = 1;}
intervalMs *= 1.4;
}, intervalMs);
}
} else { player.setPlaybackRate(Number(option)); video.playbackRate = Number(option);} // #1729 question2
}
};

also,

.playbackRate is lower level, will set 0.1 while .setPlaybackRate() doesnt go below 0.25 and higher than 2.
.setPlaybackRate() with parameters below 0.25 or higher than 2 will limit to those boundaries.
.getPlaybackRate() doesnt know about .playbackRate changes.
YT player GUI Settings/Playback Speed doesnt know about .playbackRate changes.
YT player GUI Settings/Playback Speed only updates on .setPlaybackRate() thus is also limited to [0.25-2] range.

this great info can be documentation.

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.

2 participants