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

Captions: Add ability to use Innertube's transcripts API #4001

Merged
merged 11 commits into from
Sep 18, 2023

Conversation

syeopite
Copy link
Member

Closes #2567

If applicable, I'll like to get the bounty.

Theoretically, this should reenable closed captions support on larger instances that would otherwise be rate-limited by YouTube.

There are some minor differences between the text received via transcripts and the regular timed text but nothing too major according to my testing. In addition, this will likely block us off implementing subtitle styling, animation, etc seen on YouTube until YT's timedtext API becomes an InnerTube endpoint.

To enable, simply set the use_innertube_for_captions to true within the config.

syeopite added 6 commits July 23, 2023 14:40
The Caption object does not actually store any text lines for the
subtitles. Instead it stores the metadata needed to display and fetch
the actual captions from the YT timedtext API.

Therefore it may be wiser to rename the struct to be more reflective of
its current usage as well as the future usage once the current caption
retrival system is replaced via InnerTube's transcript API
@TheFrenchGhosty
Copy link
Member

Welcome back!

Comment on lines +23 to +25
"6:varint" => 1_i64,
"7:varint" => 1_i64,
"8:varint" => 1_i64,
Copy link
Member Author

Choose a reason for hiding this comment

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

I've noticed that varint 6 and 8 can fluctuate between 1 and 0 on the actual YouTube client's requests. It doesn't seem to do anything though and I can't deduce any patterns about it.

@unixfox unixfox added the in-testing This feature has been deployed and is being tested label Jul 26, 2023
@unixfox
Copy link
Member

unixfox commented Jul 26, 2023

in testing on https://yewtu.be

Copy link
Member

@SamantazFox SamantazFox left a comment

Choose a reason for hiding this comment

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

Code overall is looking good!

Comment on lines +56 to +60
# start_time
vtt << start_time.hours.to_s.rjust(2, '0')
vtt << ':' << start_time.minutes.to_s.rjust(2, '0')
vtt << ':' << start_time.seconds.to_s.rjust(2, '0')
vtt << '.' << start_time.milliseconds.to_s.rjust(3, '0')
Copy link
Member

Choose a reason for hiding this comment

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

Could you make a function for that code, as it is now duplicated 4 times? (or 6 if you also count the two not using a string builder). Preferably, one that takes both the String::Builder and a Time::Span.

Or (probably even better) try to merge the various VTT-building functions into a single one?

Copy link
Member Author

Choose a reason for hiding this comment

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

@SamantazFox SamantazFox removed the need-code-review A crystal developper need to check if the code is correct. label Aug 24, 2023
syeopite and others added 3 commits August 24, 2023 16:00
Rename CaptionsMetadata to Metadata
Nest Metadata under Captions
Unnest LANGUAGES constant from Metadata to main Captions module
@SamantazFox
Copy link
Member

@unixfox no problems to report on your side?
On my side, I didn't have any issues (on the test instance).

@unixfox
Copy link
Member

unixfox commented Sep 10, 2023

I removed the patch because it wasn't compatible with my other patches anymore I guess.

But that was working well, I vote on merging

@SamantazFox SamantazFox added ready and removed in-testing This feature has been deployed and is being tested labels Sep 16, 2023
@SamantazFox SamantazFox changed the title Add ability to use Innertube's transcripts API for closed captions Captions: Add ability to use Innertube's transcripts API Sep 18, 2023
@SamantazFox SamantazFox merged commit 842e9fa into iv-org:master Sep 18, 2023
@syeopite syeopite deleted the captions-workaround branch September 18, 2023 21:40
@syeopite
Copy link
Member Author

@TheFrenchGhosty is this PR applicable for the bounty?

@TheFrenchGhosty
Copy link
Member

@syeopite Yes

@syeopite
Copy link
Member Author

@syeopite Yes

bc1q55ey96q0kgu220y6eaz0qqzfsq770ryqdcv8fd

@TheFrenchGhosty
Copy link
Member

A 20$ bounty has been rewarded to @syeopite for this PR.

Transaction (ea4fc9272a4951c54edaf5bead0491c8aa80f1a9e3e16bfe207f5d096c29ee54):

Amount sent: 0.000736 BTC (~18.67€ or ~20$)

Mining fees (30 sat/byte): 0.0000426 BTC BTC (~1.08€) (expensive today)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bounty:paid The bounty has been paid bounty ready
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Enhancement] Switch to innertube for the captions - captions do not work on public instances
4 participants