1536762 - [webvtt] use actual cue box height as a step to adjust cue's position

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, enhancement, P2)

Description

[Alastor Wu [:alwu]]

Now we're using font size as a step to adjust cue's position. However, it would cause accumulated error because there is a difference (< 1px) between the font size and the cue box's height.

In this bug, we should do

(1) use actual cue box's height as a positioning step
(2) get the correct line height when the cue text are wrapped into multiple line
(3) get the correct line height when applying CSS transformation

Comment 1

[Alastor Wu [:alwu]]

Now I'm trying to find the correct fuzzy threshold for those tests we will enable in this bug, because there are some weird white pixel color difference (difference <2 per channel) in the background.

Comment 2

[Alastor Wu [:alwu]]

Attachment: Bug 1536762 - part1 : use unscaled bounding box'size as a cue box's size.

There is a tiny difference between font size and actual line box height, we should use actual line box's height or width to adjust cue's position in order to eliminate the positioning error.

As we use line's bounding box height, we should also consider the effect which CSS scale brings, because the bounding box which we get have already been applied transformation (eg. scaled).

Therefore, we would use the unscaled bounding box length to do the calculation.

In addition, when the text is wrapped into multiple lines due to the line box is not large enough to show all text in the single line, we would calculate what the single line box height is.

Comment 3

[Alastor Wu [:alwu]]

Attachment: Bug 1536762 - part3 : enable wpt tests and add 'fuzzy' comparision.

Comment 4

[Alastor Wu [:alwu]]

Attachment: Bug 1536762 - part3 : allow 'fuzzy' comparision for rendering wpt.

Sometime there would be some color rendering differences between the test and the reference, which is small enough to be ignored because human can't aware that difference.

Comment 5

[Alastor Wu [:alwu]]

Attachment: Bug 1536762 - part4 : change wpt failed state.

Now these two tests would permanently fail, not being timeout.

Comment 6

[Alastor Wu [:alwu]]

Attachment: Bug 1536762 - part5 : re-enable fail wpt.

There two tests should be permanently fail now.

Comment 7

[Alastor Wu [:alwu]]

Attachment: Bug 1536762 - part4 : disable tests on Android and Windows.

There is a weird fail that the white video frame would be rendered to black, it's Android only fail. I've filed bug1546128 for investigation.

Comment 8

[Alastor Wu [:alwu]]

There are two problems we have to do in this bug.

1. Find correct height of the cue box
   As we are using font size as the cue box's height, but it has a difference from the real box height, which causes having a small positioning difference and resulted in the wpt ref test failures.

Therefore, we should use the actual box's size for positioning. There are three attributes we might use, offsetHeight, clientHeight and bounding box's height. It seems that we should use the bounding box's height, however, it's value would be affected by CSS transformation, we can only get the adjusted value.

Using adjusted value would cause incorrect result, so we would like to have a value which is not affected by any CSS tranformation. So we should consider offsetHeight or clientHeight. According to [1], clientHeight seems what we eager for.

However, during testing, I found that sometime the clientHeight is not equal to the actual box's height, there would have a tiny difference. After some investigate, I finally found that the difference is just because clientHeight we return is integer [2], not double!

After discussed with :heycam, I am going to add new chrome-only APIs to get the double version clientHeight and clientWidth.

2. Get the single line height
   As cue box might contain multiple line, using the cue box's height directly as a basic positioning unit is wrong. We should find the single line and get its height.

The spec said [3]

Let step be the heigh or width of the first line box in boxes

We can use getClientRects() to get the independent line boxes, and then use the first box's height as a basic unit.

[1] https://developer.mozilla.org/en-US/docs/Web/API/CSS_Object_Model/Determining_the_dimensions_of_elements
[2] https://searchfox.org/mozilla-central/rev/f0ef51bfafc8ad0c3a2f523bf076edc57dc4891a/dom/base/Element.h#1277
[3] See step2, https://www.w3.org/TR/webvtt1/#ref-for-webvtt-cue-snap-to-lines-flag-12

Comment 9

[Alastor Wu [:alwu]]

Attachment: Bug 1536762 - part2 : use first line box's size as positioning basic unit.

According to the spec [1] 7.2.10.2, we should use the first line box's height or width as positioning unit to adjust box's position.

We will also use this value to adjust box when snap-to-line is false.

There, we implement a new chrome-only API to acquire this information, which would return the first line box's size in the block frame.

[1] https://www.w3.org/TR/webvtt1/#ref-for-webvtt-cue-snap-to-lines-flag-12

Comment 10

[Alastor Wu [:alwu]]

Attachment: Bug 1536762 - part5 : adjustment should only happen when box doesn't go over the boundary along the moving direction.

When adjusting box's position, we would choose a axis first and move the box along this axis to see whether we can place this box without overlapping with other boxes and fully inside the rendering area.

If the box has been over the boundary along the moving direction, we should move box back to the original position and change the moving direction to see whether we can find another best place for the box.

Although the adjustment can run without timeout now, it still doesn't match the result of the reference, so change the state to fail.

Comment 11

[Alastor Wu [:alwu]]

In order to prevent causing any fail, push patches to try server after rebasing, and the result [1] looks good.

[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=f16d7397781ca2f1448da2e9564525d94fca4a10&selectedJob=245605726

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2d810269b72e
part1 : use unscaled bounding box'size as a cue box's size. r=baku,heycam
https://hg.mozilla.org/integration/autoland/rev/1fa720d18b38
part2 : use first line box's size as positioning basic unit. r=heycam,baku
https://hg.mozilla.org/integration/autoland/rev/9250cfd8c4e4
part3 : enable wpt tests and add 'fuzzy' comparision. r=heycam
https://hg.mozilla.org/integration/autoland/rev/f009e75ac871
part4 : disable tests on Android and Windows. r=heycam
https://hg.mozilla.org/integration/autoland/rev/f40e952d1552
part5 : adjustment should only happen when box doesn't go over the boundary along the moving direction. r=heycam

Comment 13

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/2d810269b72e
https://hg.mozilla.org/mozilla-central/rev/1fa720d18b38
https://hg.mozilla.org/mozilla-central/rev/9250cfd8c4e4
https://hg.mozilla.org/mozilla-central/rev/f009e75ac871
https://hg.mozilla.org/mozilla-central/rev/f40e952d1552