feat(webvtt): webvtt colors output #4954
Conversation
No need to perform the extra operations that only applies for real payloads
reduceRight doesn't add any significance over just reduce, but need it for the output order to stay the same as before to pass current tests.
Fixes #4545 Will cover all valid color classes for webvtt cues, as well as classes that are not valid webvtt class names, but are valid class names Will exclude hex colors, rgb, rgba etc that could come from a ttml input.
|
Incremental code coverage: 98.77% |
avelad
added
type: enhancement
|
@friday I'll add some minor changes to support more TTML colors (eg), and I'll also try to add some test with your permission. |
Appreciated 👍 |
|
@friday I have already added the changes, in case you want to review. I think it would also be good if you updated the description of the PR. |
|
@joeyparrish , I think the Eg: |
There was a problem hiding this comment.
Approved @avelad 👍 (and updated description)
This does mean that other colors like orange, pink etc would be removed, which may or may not be a good thing. I like the clarity of drawing the line at these 8 colors (and their hex variants) though, and nice to lose the regex 👍
About TTML color formats, I never used them myself, so I don't know what is more common, but from what I read I believe they can be named colors, 6 or 8 digit hex (for alpha channel), rgb or rgba. I wouldn't be surprised if 3 digit hex was also commonly supported and used though.
Supporting rgb(a) is harder to parse efficiently since they're not white-space sensitive, and it would add more complexity and regex.
github-actions
bot
added
the
status: archived
Adds color support for SimpleTextDisplayer and WebVttGenerator (only one place to fix both now thanks to #4941).
It's limited to the 8 colors classes supported by the WebVTT specification, and also works with their 3 or 6-digit hex variants (if the stream has TTML subtitles).
It does not support rgb, rgba or any colors other than these 8.
Fixes #4545