Captions: Add ability to use Innertube's transcripts API#4001
Conversation
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
syeopite
requested review from
a team,
SamantazFox,
TheFrenchGhosty and
unixfox
as code owners
|
Welcome back! |
| "6:varint" => 1_i64, | ||
| "7:varint" => 1_i64, | ||
| "8:varint" => 1_i64, |
There was a problem hiding this comment.
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.
syeopite
added
the
need-code-review
SamantazFox
left a comment
SamantazFox
left a comment
There was a problem hiding this comment.
Code overall is looking good!
| # 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') |
There was a problem hiding this comment.
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?
SamantazFox
removed
the
need-code-review
Rename CaptionsMetadata to Metadata Nest Metadata under Captions Unnest LANGUAGES constant from Metadata to main Captions module
Co-authored-by: Samantaz Fox <[email protected]>
Co-authored-by: Samantaz Fox <[email protected]>
Co-authored-by: Samantaz Fox <[email protected]>
|
@unixfox no problems to report on your side? |
|
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
added
ready
and removed
in-testing
SamantazFox
changed the title
|
@TheFrenchGhosty is this PR applicable for the bounty? |
|
@syeopite Yes |
bc1q55ey96q0kgu220y6eaz0qqzfsq770ryqdcv8fd |
|
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) |
4 participants
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_captionsto true within the config.