Add support for SSA Bold Italic style #8654
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,9 +18,11 @@ | |
| import static com.google.android.exoplayer2.text.Cue.LINE_TYPE_FRACTION; | ||
| import static com.google.android.exoplayer2.util.Util.castNonNull; | ||
|
|
||
| import android.graphics.Typeface; | ||
| import android.text.Layout; | ||
| import android.text.SpannableString; | ||
| import android.text.style.ForegroundColorSpan; | ||
| import android.text.style.StyleSpan; | ||
| import androidx.annotation.Nullable; | ||
| import com.google.android.exoplayer2.C; | ||
| import com.google.android.exoplayer2.text.Cue; | ||
|
|
@@ -319,6 +321,27 @@ private static Cue createCue( | |
| style.fontSize / screenHeight, | ||
| Cue.TEXT_SIZE_TYPE_FRACTIONAL_IGNORE_PADDING); | ||
| } | ||
| if (style.bold && style.italic) { | ||
| spannableText.setSpan( | ||
| new StyleSpan(Typeface.BOLD_ITALIC), | ||
| 0, | ||
| spannableText.length(), | ||
| SpannableString.SPAN_EXCLUSIVE_EXCLUSIVE); | ||
| } | ||
| else if(style.bold){ | ||
|
There was a problem hiding this comment. I wouldn't put the And lastly, you have a couple of missing whitespaces, e.g. There was a problem hiding this comment. Thank you. I will fix this. There was a problem hiding this comment. I agree with @szaboa - but don't worry too much about the styling/whitespace. When we merge PRs we run them through https://github.com/google/google-java-format (same for all the code we author ourselves), which sorts all this stuff out without needing to think too hard about it. |
||
| spannableText.setSpan( | ||
| new StyleSpan(Typeface.BOLD), | ||
| 0, | ||
| spannableText.length(), | ||
| SpannableString.SPAN_EXCLUSIVE_EXCLUSIVE); | ||
| } | ||
| else if(style.italic){ | ||
| spannableText.setSpan( | ||
| new StyleSpan(Typeface.ITALIC), | ||
| 0, | ||
| spannableText.length(), | ||
| SpannableString.SPAN_EXCLUSIVE_EXCLUSIVE); | ||
| } | ||
| } | ||
|
|
||
| @SsaStyle.SsaAlignment int alignment; | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -92,16 +92,22 @@ | |
| @SsaAlignment public final int alignment; | ||
| @Nullable @ColorInt public final Integer primaryColor; | ||
| public final float fontSize; | ||
| public final boolean bold; | ||
| public final boolean italic; | ||
|
|
||
| private SsaStyle( | ||
| String name, | ||
| @SsaAlignment int alignment, | ||
| @Nullable @ColorInt Integer primaryColor, | ||
| float fontSize, | ||
| boolean bold, | ||
| boolean italic) { | ||
| this.name = name; | ||
| this.alignment = alignment; | ||
| this.primaryColor = primaryColor; | ||
| this.fontSize = fontSize; | ||
| this.bold = bold; | ||
| this.italic = italic; | ||
| } | ||
|
|
||
| @Nullable | ||
|
|
@@ -121,7 +127,9 @@ public static SsaStyle fromStyleLine(String styleLine, Format format) { | |
| styleValues[format.nameIndex].trim(), | ||
| parseAlignment(styleValues[format.alignmentIndex].trim()), | ||
| parseColor(styleValues[format.primaryColorIndex].trim()), | ||
| parseFontSize(styleValues[format.fontSizeIndex].trim()), | ||
| parseBold(styleValues[format.boldIndex].trim()), | ||
| parseItalic(styleValues[format.italicIndex].trim())); | ||
| } catch (RuntimeException e) { | ||
| Log.w(TAG, "Skipping malformed 'Style:' line: '" + styleLine + "'", e); | ||
| return null; | ||
|
|
@@ -207,6 +215,36 @@ private static float parseFontSize(String fontSize) { | |
| } | ||
| } | ||
|
|
||
| private static boolean parseBold(String bold) { | ||
| try { | ||
| int boldFlag = Integer.parseInt(bold); | ||
| if(boldFlag == 1 || boldFlag == -1){ | ||
|
There was a problem hiding this comment. Based on the specs, only -1 means true, or am I missing something? Also you could simplify it by inlining the return like There was a problem hiding this comment. Yeah based on the specs it should only be -1 but when I was testing with vlc they took 1 as well. Do you think we should go for a clean implementation or a more forgiving one? There was a problem hiding this comment. I've also checked the libass implementation and it seems they are doing the same (-1, 1): https://github.com/libass/libass/blob/82b225b3d6653091d028b39d561d185ed76a7be5/libass/ass_parse.c#L118 So IMO we can go with the more forgiving implementation, but lets wait for @icbaker 's confirmation. There was a problem hiding this comment. +1 to more forgiving though weirdly for italic it seems libass only consider 1 to mean Maybe it would be easier just to do |
||
| return true; | ||
| } | ||
| else{ | ||
| return false; | ||
| } | ||
| } catch (NumberFormatException e) { | ||
| Log.w(TAG, "Failed to parse bold: '" + bold + "'", e); | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| private static boolean parseItalic(String italic) { | ||
| try { | ||
| int italicFlag = Integer.parseInt(italic); | ||
| if(italicFlag == 1 || italicFlag == -1){ | ||
| return true; | ||
| } | ||
| else{ | ||
| return false; | ||
| } | ||
| } catch (NumberFormatException e) { | ||
| Log.w(TAG, "Failed to parse italic: '" + italic + "'", e); | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Represents a {@code Format:} line from the {@code [V4+ Styles]} section | ||
| * | ||
|
|
@@ -219,18 +257,24 @@ private static float parseFontSize(String fontSize) { | |
| public final int alignmentIndex; | ||
| public final int primaryColorIndex; | ||
| public final int fontSizeIndex; | ||
| public final int boldIndex; | ||
| public final int italicIndex; | ||
| public final int length; | ||
|
|
||
| private Format( | ||
| int nameIndex, | ||
| int alignmentIndex, | ||
| int primaryColorIndex, | ||
| int fontSizeIndex, | ||
| int boldIndex, | ||
| int italicIndex, | ||
| int length) { | ||
| this.nameIndex = nameIndex; | ||
| this.alignmentIndex = alignmentIndex; | ||
| this.primaryColorIndex = primaryColorIndex; | ||
| this.fontSizeIndex = fontSizeIndex; | ||
| this.boldIndex = boldIndex; | ||
| this.italicIndex = italicIndex; | ||
| this.length = length; | ||
| } | ||
|
|
||
|
|
@@ -245,6 +289,8 @@ public static Format fromFormatLine(String styleFormatLine) { | |
| int alignmentIndex = C.INDEX_UNSET; | ||
| int primaryColorIndex = C.INDEX_UNSET; | ||
| int fontSizeIndex = C.INDEX_UNSET; | ||
| int boldIndex = C.INDEX_UNSET; | ||
| int italicIndex = C.INDEX_UNSET; | ||
| String[] keys = | ||
| TextUtils.split(styleFormatLine.substring(SsaDecoder.FORMAT_LINE_PREFIX.length()), ","); | ||
| for (int i = 0; i < keys.length; i++) { | ||
|
|
@@ -261,10 +307,16 @@ public static Format fromFormatLine(String styleFormatLine) { | |
| case "fontsize": | ||
| fontSizeIndex = i; | ||
| break; | ||
| case "bold": | ||
| boldIndex = i; | ||
| break; | ||
| case "italic": | ||
| italicIndex = i; | ||
| break; | ||
| } | ||
| } | ||
| return nameIndex != C.INDEX_UNSET | ||
| ? new Format(nameIndex, alignmentIndex, primaryColorIndex, fontSizeIndex, boldIndex, italicIndex, keys.length) | ||
| : null; | ||
| } | ||
| } | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| [Script Info] | ||
| Title: SSA/ASS Test | ||
| Original Script: Abel | ||
| Script Type: V4.00+ | ||
| PlayResX: 1280 | ||
| PlayResY: 720 | ||
|
|
||
| [V4+ Styles] | ||
| Format: Name, Fontname, Fontsize, PrimaryColour, SecondaryColour, OutlineColour, BackColour, Bold, Italic, Underline, StrikeOut, ScaleX, ScaleY, Spacing, Angle, BorderStyle, Outline, Shadow, Alignment, MarginL, MarginR, MarginV, Encoding | ||
| Style: FontBold ,Roboto,50,&H000000FF,&H000000FF,&H00000000,&H00000000,-1,0,0,0,100,100,0,0,1,3,0,2,50,50,70,1 | ||
| Style: FontItalic ,Roboto,50,&H000000FF,&H000000FF,&H00000000,&H00000000,0,-1,0,0,100,100,0,0,1,3,0,2,50,50,70,1 | ||
| Style: FontBoldItalic ,Roboto,50,&H000000FF,&H000000FF,&H00000000,&H00000000,-1,-1,0,0,100,100,0,0,1,3,0,2,50,50,70,1 | ||
|
|
||
|
|
||
|
|
||
| [Events] | ||
| Format: Layer, Start, End, Style, Name, MarginL, MarginR, MarginV, Effect, Text | ||
| Dialogue: 0,0:00:01.00,0:00:03.00,FontBold ,Abel,0,0,0,,First line with Bold. | ||
| Dialogue: 0,0:00:05.00,0:00:07.00,FontItalic ,Abel,0,0,0,,Second line with Italic. | ||
| Dialogue: 0,0:00:09.00,0:00:11.00,FontBoldItalic ,Abel,0,0,0,,Third line with Bold Italic. |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for including this to make testing easier, it's really helpful - FYI I'll drop it before actually merging the PR though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I didn't realise this exercises more styles than just italic/bold - this does actually look like a useful file to add to the demo app (but probably not as part of this PR).
If you're happy to contribute this file, can you open a separate PR containing just the SSA file and I'll take it from there an upload it to our cloud bucket and add an entry to the demo app. Thanks!