Add support for SSA (v4+) FontSize style

[szaboa]

@icbaker

This is the initial PR for the "FontSize" style (SSA v4+). I'll remove the entry from the media.exolist.json file and add tests on the decoder side after resolving any upcoming comments.

However, I am bit confused of this style, because the specs are not mentioning the unit (in the override section the "point size" mentioned though). Based on my tests with VLC, the font size is relative to the specified PlayResY in the [Script Info], but its value doesn't seem to be represent pixels exactly and couldn't find a constant coefficient either, however visually this implementation seems to be proportional with the VLC one :)

Furthermore, I wasn't sure about these small things:

1. If the "fontSize" can't be parsed, should we return null or Cue.DIMEN_UNSET from parseFontSize? We could avoid an unboxing with Cue.DIMEN_UNSET but in the other hand, returning null would be consistent with the primary color parsing.

2. It is OK to do the size calculation (style.fontSize / screenHeight) on decoder side or should be placed in the SsaStyle? If the latter, then need to pass screenWidth and screenHeight to SsaStyle.

3. In case of missing PlayResY, the "fontSize" still has an effect in VLC, but not sure based on what. Should we try to figure this out or just ignore the "fontSize" if PlayResY is not specified?

[icbaker]

• If the "fontSize" can't be parsed, should we return null or Cue.DIMEN_UNSET from parseFontSize? We could avoid an unboxing with Cue.DIMEN_UNSET but in the other hand, returning null would be consistent with the primary color parsing.

I think DIMEN_UNSET is perfect here - since there is a value we can use that isn't a valid font size (which is the problem we hit with color).

• It is OK to do the size calculation (style.fontSize / screenHeight) on decoder side or should be placed in the SsaStyle? If the latter, then need to pass screenWidth and screenHeight to SsaStyle.

Where you've put the calculation looks fine to me.

• In case of missing PlayResY, the "fontSize" still has an effect in VLC, but not sure based on what. Should we try to figure this out or just ignore the "fontSize" if PlayResY is not specified?

Hypothesis (untested): VLC assumes a PlayResY of 288 if it's missing. This seems to be a default assumed in a couple of places in libass, e.g.:
https://github.com/libass/libass/blob/59eb317aaa495ad5331c9efdf8d7bf3d860c2992/libass/ass.c#L1545-L1549

https://github.com/libass/libass/blob/706f23d84180d36538c467e20b461677781cab16/libass/ass_render.c#L898-L901

If that aligns with your observations then I think we should add this default logic to all usages of screenWidth and screenHeight, probably here, and probably in a separate PR (and so keep the existing logic of ignoring FontSize if PlayResY is missing for now):

ExoPlayer/library/core/src/main/java/com/google/android/exoplayer2/text/ssa/SsaDecoder.java

Lines 88 to 89 in b100094

	screenWidth = Cue.DIMEN_UNSET;
	screenHeight = Cue.DIMEN_UNSET;

[szaboa]

Applied the suggested changes, ready for review. I'll look into the missing "PlayResY" case and create a separate PR for that as you suggested.

[icbaker]

Thanks, looks good! I'll work on getting this merged.

[rgrk11]

Is it better to use Cue.DIMEN_UNSET here if fontSizeIndex == C.INDEX_UNSET? More specifically, all valid styles should not be discarded if some other styles are not found for whatever reason.

[icbaker]

Yeah, that's a good point, seems the intention of this code is to be robust against both different Format line orderings and presence or absence of fields, but it's currently only defensive against the ordering and breaks if fields we expect to see are absent. This is obviously getting worse as we add support for more style properties.

I don't think this PR is the right place to fix this though - i'll send a separate change. Thanks for flagging!