From a8a1fd8927449c91720e001237fc9d7f4268ec0f Mon Sep 17 00:00:00 2001 From: Albin Date: Tue, 31 Jan 2023 20:44:25 +0100 Subject: [PATCH 1/5] perf(text): return earlier if payload is not a real payload No need to perform the extra operations that only applies for real payloads --- lib/text/text_utils.js | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/text/text_utils.js b/lib/text/text_utils.js index feb5a1e79b..4e202bf732 100644 --- a/lib/text/text_utils.js +++ b/lib/text/text_utils.js @@ -19,6 +19,14 @@ shaka.text.Utils = class { * @private */ static flattenPayload_(cue) { + if (cue.lineBreak) { + // This is a vertical lineBreak, so insert a newline. + return '\n'; + } + if (cue.nestedCues.length) { + return cue.nestedCues.map(shaka.text.Utils.flattenPayload_).join(''); + } + // Handle styles (currently bold/italics/underline). // TODO: add support for color rendering. const openStyleTags = []; @@ -44,15 +52,7 @@ shaka.text.Utils = class { return `${acc}`; }, ''); - if (cue.lineBreak) { - // This is a vertical lineBreak, so insert a newline. - return '\n'; - } else if (cue.nestedCues.length) { - return cue.nestedCues.map(shaka.text.Utils.flattenPayload_).join(''); - } else { - // This is a real cue. - return prefixStyleTags + cue.payload + suffixStyleTags; - } + return prefixStyleTags + cue.payload + suffixStyleTags; } /** From 81ec2d51c7da2f30a90addf8a6c41e731d2e89dd Mon Sep 17 00:00:00 2001 From: Albin Date: Tue, 31 Jan 2023 21:04:18 +0100 Subject: [PATCH 2/5] refactor(text): simplify reducer 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. --- lib/text/text_utils.js | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/lib/text/text_utils.js b/lib/text/text_utils.js index 4e202bf732..f02162a1db 100644 --- a/lib/text/text_utils.js +++ b/lib/text/text_utils.js @@ -44,15 +44,9 @@ shaka.text.Utils = class { openStyleTags.push('u'); } - // Prefix opens tags, suffix closes tags in reverse order of opening. - const prefixStyleTags = openStyleTags.reduce((acc, tag) => { - return `${acc}<${tag}>`; - }, ''); - const suffixStyleTags = openStyleTags.reduceRight((acc, tag) => { - return `${acc}`; - }, ''); - - return prefixStyleTags + cue.payload + suffixStyleTags; + return openStyleTags.reduceRight((acc, tag) => { + return `<${tag}>${acc}`; + }, cue.payload); } /** From 9e4110c3a4e0e50427d8399b2c305f92679be50d Mon Sep 17 00:00:00 2001 From: Albin Date: Tue, 31 Jan 2023 23:08:59 +0100 Subject: [PATCH 3/5] feat(WebVTT): Add support for color classes for vtt output 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. --- lib/text/cue.js | 32 +++++++++++++++---------------- lib/text/text_utils.js | 24 ++++++++++++++++------- test/text/vtt_text_parser_unit.js | 30 ++++++++++++++--------------- 3 files changed, 48 insertions(+), 38 deletions(-) diff --git a/lib/text/cue.js b/lib/text/cue.js index 570fa6cbda..c6b1d05eec 100644 --- a/lib/text/cue.js +++ b/lib/text/cue.js @@ -415,14 +415,14 @@ shaka.text.Cue.lineAlign = { * @export */ shaka.text.Cue.defaultTextColor = { - 'white': '#FFF', - 'lime': '#0F0', - 'cyan': '#0FF', - 'red': '#F00', - 'yellow': '#FF0', - 'magenta': '#F0F', - 'blue': '#00F', - 'black': '#000', + 'white': 'white', + 'lime': 'lime', + 'cyan': 'cyan', + 'red': 'red', + 'yellow': 'yellow', + 'magenta': 'magenta', + 'blue': 'blue', + 'black': 'black', }; @@ -433,14 +433,14 @@ shaka.text.Cue.defaultTextColor = { * @export */ shaka.text.Cue.defaultTextBackgroundColor = { - 'bg_white': '#FFF', - 'bg_lime': '#0F0', - 'bg_cyan': '#0FF', - 'bg_red': '#F00', - 'bg_yellow': '#FF0', - 'bg_magenta': '#F0F', - 'bg_blue': '#00F', - 'bg_black': '#000', + 'bg_white': 'white', + 'bg_lime': 'lime', + 'bg_cyan': 'cyan', + 'bg_red': 'red', + 'bg_yellow': 'yellow', + 'bg_magenta': 'magenta', + 'bg_blue': 'blue', + 'bg_black': 'black', }; diff --git a/lib/text/text_utils.js b/lib/text/text_utils.js index f02162a1db..80946cbd32 100644 --- a/lib/text/text_utils.js +++ b/lib/text/text_utils.js @@ -27,25 +27,35 @@ shaka.text.Utils = class { return cue.nestedCues.map(shaka.text.Utils.flattenPayload_).join(''); } - // Handle styles (currently bold/italics/underline). - // TODO: add support for color rendering. + // Handle bold, italics and underline const openStyleTags = []; const bold = cue.fontWeight >= shaka.text.Cue.fontWeight.BOLD; const italics = cue.fontStyle == shaka.text.Cue.fontStyle.ITALIC; const underline = cue.textDecoration.includes( shaka.text.Cue.textDecoration.UNDERLINE); if (bold) { - openStyleTags.push('b'); + openStyleTags.push(['b']); } if (italics) { - openStyleTags.push('i'); + openStyleTags.push(['i']); } if (underline) { - openStyleTags.push('u'); + openStyleTags.push(['u']); + } + // Handle color classes, if the value consists of letters + let classes = ''; + if (cue.color && /^[a-zA-Z]+$/.test(cue.color)) { + classes += `.${cue.color}`; + } + if (cue.backgroundColor && /^[a-zA-Z]+$/.test(cue.backgroundColor)) { + classes += `.bg_${cue.backgroundColor}`; + } + if (classes) { + openStyleTags.push(['c', classes]); } - return openStyleTags.reduceRight((acc, tag) => { - return `<${tag}>${acc}`; + return openStyleTags.reduceRight((acc, [tag, classes = '']) => { + return `<${tag}${classes}>${acc}`; }, cue.payload); } diff --git a/test/text/vtt_text_parser_unit.js b/test/text/vtt_text_parser_unit.js index 836590be17..aadc45f950 100644 --- a/test/text/vtt_text_parser_unit.js +++ b/test/text/vtt_text_parser_unit.js @@ -910,7 +910,7 @@ describe('VttTextParser', () => { startTime: 20, endTime: 40, payload: 'Test', - color: '#FF0', + color: 'yellow', }, ], }, @@ -922,8 +922,8 @@ describe('VttTextParser', () => { startTime: 40, endTime: 50, payload: 'Test2', - color: '#0FF', - backgroundColor: '#00F', + color: 'cyan', + backgroundColor: 'blue', }, ], }, @@ -935,8 +935,8 @@ describe('VttTextParser', () => { startTime: 50, endTime: 60, payload: 'Test 3', - color: '#F0F', - backgroundColor: '#000', + color: 'magenta', + backgroundColor: 'black', }, ], }, @@ -954,7 +954,7 @@ describe('VttTextParser', () => { startTime: 60, endTime: 70, payload: 'Test4.1', - color: '#FF0', + color: 'yellow', }, { startTime: 60, @@ -971,7 +971,7 @@ describe('VttTextParser', () => { startTime: 60, endTime: 70, payload: 'Test4.2', - color: '#00F', + color: 'blue', }, ], }, @@ -984,13 +984,13 @@ describe('VttTextParser', () => { startTime: 70, endTime: 80, payload: 'Test5.1', - color: '#F00', + color: 'red', }, { startTime: 70, endTime: 80, payload: 'Test5.2', - color: '#0F0', + color: 'lime', }, ], }, @@ -1015,7 +1015,7 @@ describe('VttTextParser', () => { startTime: 100, endTime: 110, payload: 'forward slash 1/2 in text', - color: '#0F0', + color: 'lime', }, ], }, @@ -1132,8 +1132,8 @@ describe('VttTextParser', () => { startTime: 10, endTime: 20, payload: 'Example 1', - color: '#F00', - backgroundColor: '#FF0', + color: 'red', + backgroundColor: 'yellow', fontSize: '10px', }, ], @@ -1141,7 +1141,7 @@ describe('VttTextParser', () => { ], 'WEBVTT\n\n' + 'STYLE\n' + - '::cue(.bg_blue) { font-size: 10px; background-color: #FF0 }\n\n' + + '::cue(.bg_blue) { font-size: 10px; background-color: yellow }\n\n' + '00:00:10.000 --> 00:00:20.000\n' + 'Example 1\n\n', {periodStart: 0, segmentStart: 0, segmentEnd: 0, vttOffset: 0}); @@ -1159,7 +1159,7 @@ describe('VttTextParser', () => { startTime: 10, endTime: 20, payload: '1', - color: '#F0F', + color: 'magenta', }, { startTime: 10, @@ -1171,7 +1171,7 @@ describe('VttTextParser', () => { startTime: 10, endTime: 20, payload: '2', - color: '#F0F', + color: 'magenta', fontStyle: Cue.fontStyle.ITALIC, }, ], From 5f967f67aea3f20595510e50d5d9daba45e75fbb Mon Sep 17 00:00:00 2001 From: Alvaro Velad Galvan Date: Wed, 1 Feb 2023 17:37:05 +0100 Subject: [PATCH 4/5] feat(WebVTT): Add support for more colors --- lib/text/text_utils.js | 55 +++++++++++++++++++++++++++++++++++++++--- 1 file changed, 51 insertions(+), 4 deletions(-) diff --git a/lib/text/text_utils.js b/lib/text/text_utils.js index 80946cbd32..95c9601090 100644 --- a/lib/text/text_utils.js +++ b/lib/text/text_utils.js @@ -44,11 +44,13 @@ shaka.text.Utils = class { } // Handle color classes, if the value consists of letters let classes = ''; - if (cue.color && /^[a-zA-Z]+$/.test(cue.color)) { - classes += `.${cue.color}`; + const color = shaka.text.Utils.getColorName_(cue.color); + if (color) { + classes += `.${color}`; } - if (cue.backgroundColor && /^[a-zA-Z]+$/.test(cue.backgroundColor)) { - classes += `.bg_${cue.backgroundColor}`; + const bgColor = shaka.text.Utils.getColorName_(cue.backgroundColor); + if (bgColor) { + classes += `.bg_${bgColor}`; } if (classes) { openStyleTags.push(['c', classes]); @@ -59,6 +61,51 @@ shaka.text.Utils = class { }, cue.payload); } + /** + * Gets the color name from a color string. + * + * @param {string} string + * @return {?string} + * @private + */ + static getColorName_(string) { + switch (string.toLowerCase()) { + case 'white': + case '#fff': + case '#ffffff': + return 'white'; + case 'lime': + case '#0f0': + case '#00ff00': + return 'lime'; + case 'cyan': + case '#0ff': + case '#00ffff': + return 'cyan'; + case 'red': + case '#f00': + case '#ff0000': + return 'red'; + case 'yellow': + case '#ff0': + case '#ffff00': + return 'yellow'; + case 'magenta': + case '#f0f': + return 'magenta'; + case 'blue': + case '#00f': + case '#0000ff': + return 'blue'; + case 'black': + case '#000': + case '#000000': + return 'black'; + } + // No color name + return null; + } + /** * We don't want to modify the array or objects passed in, since we don't * technically own them. So we build a new array and replace certain items From a546b4b32905760fbd9b0100df079f030f9ef734 Mon Sep 17 00:00:00 2001 From: Alvaro Velad Galvan Date: Wed, 1 Feb 2023 17:37:28 +0100 Subject: [PATCH 5/5] feat(WebVTT): Add tests --- test/text/web_vtt_generator_unit.js | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/test/text/web_vtt_generator_unit.js b/test/text/web_vtt_generator_unit.js index 25b9d06c41..8ef71b4cef 100644 --- a/test/text/web_vtt_generator_unit.js +++ b/test/text/web_vtt_generator_unit.js @@ -13,11 +13,17 @@ describe('WebVttGenerator', () => { const shakaCue1 = new shaka.text.Cue(20, 40, 'Test'); shakaCue1.textAlign = shaka.text.Cue.textAlign.LEFT; shakaCue1.writingMode = shaka.text.Cue.writingMode.VERTICAL_LEFT_TO_RIGHT; + shakaCue1.color = 'red'; + shakaCue1.backgroundColor = '#f0f'; const shakaCue2 = new shaka.text.Cue(40, 50, 'Test2'); shakaCue2.textAlign = shaka.text.Cue.textAlign.RIGHT; shakaCue2.writingMode = shaka.text.Cue.writingMode.VERTICAL_RIGHT_TO_LEFT; + shakaCue2.color = '#0f0'; + shakaCue2.backgroundColor = 'lime'; const shakaCue3 = new shaka.text.Cue(50, 51, 'Test3'); shakaCue3.textAlign = shaka.text.Cue.textAlign.CENTER; + shakaCue3.color = '#ffff00'; + shakaCue3.backgroundColor = '#00ffff'; const shakaCue4 = new shaka.text.Cue(52, 53, 'Test4'); shakaCue4.textAlign = shaka.text.Cue.textAlign.START; const shakaCue5 = new shaka.text.Cue(53, 54, 'Test5'); @@ -36,11 +42,11 @@ describe('WebVttGenerator', () => { adCuePoints, 'WEBVTT\n\n' + '00:00:20.000 --> 00:00:40.000 align:left vertical:lr\n' + - 'Test\n\n' + + 'Test\n\n' + '00:00:40.000 --> 00:00:50.000 align:right vertical:rl\n' + - 'Test2\n\n' + + 'Test2\n\n' + '00:00:50.000 --> 00:00:51.000 align:middle\n' + - 'Test3\n\n' + + 'Test3\n\n' + '00:00:52.000 --> 00:00:53.000 align:start\n' + 'Test4\n\n' + '00:00:53.000 --> 00:00:54.000 align:end\n' +