Fix WebVTT subtitle overlap issue

[WeiChungChang]

Fix VTT subtitle overlap issue.

[WeiChungChang]

Create a new pull request here to distinguish the request from #2697
It is the same as #2693.

move the original review comment here.

from Sir @ojw28

I don't understand this change. I'm dubious it's correct given the method description includes "Returns whether or not this cue should be placed in the default position" and you're changing it to return true for something that specifically requests a non-default position... Please provide a proper description.

The detailed issue description & report could be provided later.

[ojw28]

I don't understand this. We're not going to merge something we don't understand and wait for the description of what it does to be provided later. That's not the correct order to be doing things in.

[WeiChungChang]

Dear @ojw28
Please, read the spec of WebVtt (https://w3c.github.io/webvtt/)
Here I just summarize related tips & the issued snapshot will be provided later (or you could reproduce yourself)

WebVtt said the cues could overlap with others, as below:

The WebVTT cue timings give the start and end offsets of the WebVTT cue block. Different cues can overlap. Cues are always listed ordered by their start time.

Here is an example:

WEBVTT

00:00.000 --> 01:00.000 align:left position:19%
The First Minute

00:30.000 --> 01:30.000 align:left position:19%
The Final Minute`

In this ninety-second example, the two cues partly overlap, with the first ending before the second ends and the second starting before the first ends. This therefore is not a WebVTT file using only nested cues.

From spec,
A line is "By default, the line is set to auto."

The line defines positioning of the cue box.

The line offsets the cue box from the top, the right or left of the video viewport as defined by the writing direction, the snap-to-lines flag, or the lines occupied by any other showing tracks.

The line is set either as a number of lines, a percentage of the video viewport height or width, or as the special value auto, which means the offset is to depend on the other showing tracks.

By default, the line is set to auto.

If the writing direction is horizontal, then the line percentages are relative to the height of the video, otherwise to the width of the video.

A WebVTT cue has a computed line whose value is that returned by the following algorithm, which is defined in terms of the other aspects of the cue: ........

As the result, you could generate an issued pattern such as

00:00.000 --> 01:00.000 align:left position:19%
1 2 3 4 5 6 7 8 9 10

00:30.000 --> 01:30.000 align:left position:19%
a b c d e

You could see the subtitle becomes something like:

---

a b c d e 6 7 8 9 10

---

But the subtitle should be in the form below by the descriptions of the spec.

---

1 2 3 4 5 6 7 8 9 10
a b c d e

---

Google applies this form in many cases.

The problem is position only decides indent so it should NOT prevent the auto roll-up from happening, as the comments said:

      // we want to merge all of the normal cues into a single cue to ensure they are drawn
      // correctly (i.e. don't overlap) and to emulate roll-up, but only if there are multiple
      // normal cues, otherwise we can just append the single normal cue

Thanks.

PS: I can only reproduce it later since my test environment is NOT available for a while,

[WeiChungChang]

Issued case:
https://youtu.be/xmSJYE0zSlY

screenshot

1. Only 1st cue; it is O.K
   

2. 1st cue & 2nd cue overlap
   

[ojw28]

Thanks for the explanation. I see the problem, but isn't this change only a partial solution? In particular, the way WebvttSubtitle.getCues works looks like it would cause the position (19% in your example) to be discarded. I think a more complete fix would be to:

1. Change isNormalCue to isAutoLine and implement it as in this change, only updating the Javadoc.
2. Fix WebvttSubtitle.getCues to do something sensible that correctly retains the position of each cue. Note that the positions may be different (e.g. 19%, then 25% for the second line) and I think the line still needs to be incremented in this case.

Thoughts?

[ojw28]

Closing due to lack of follow up.