Closed Bug 1536762 Opened 6 years ago Closed 6 years ago

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

Categories

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

enhancement

Tracking

()

RESOLVED FIXED
mozilla68
Tracking Status
firefox68 --- fixed

People

(Reporter: alwu, Assigned: alwu)

References

(Blocks 2 open bugs)

Details

Attachments

(5 files, 3 obsolete files)

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

Blocks: 1534862

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.

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.

Depends on: 1544661

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.

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

There two tests should be permanently fail now.

Attachment #9059368 - Attachment is obsolete: true
Attachment #9059369 - Attachment is obsolete: true
Attachment #9059370 - Attachment is obsolete: true
Attachment #9057701 - Attachment description: Bug 1536762 - part2 : enable wpt tests. → Bug 1536762 - part2 : enable wpt tests and add 'fuzzy' comparision.

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.

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.

  1. 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

Attachment #9059852 - Attachment description: Bug 1536762 - part3 : expected fail on Android. → Bug 1536762 - part3 : disable tests on Android and Windows.
Attachment #9057700 - Attachment description: Bug 1536762 - part1 : use acutal line box height as a basic positioning unit. → Bug 1536762 - part1 : use unscaled bounding box'size to adjust cue's posiition.
Attachment #9057700 - Attachment description: Bug 1536762 - part1 : use unscaled bounding box'size to adjust cue's posiition. → Bug 1536762 - part1 : use unscaled bounding box'size as a cue box's size.

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

Attachment #9057701 - Attachment description: Bug 1536762 - part2 : enable wpt tests and add 'fuzzy' comparision. → Bug 1536762 - part3 : enable wpt tests and add 'fuzzy' comparision.
Attachment #9059852 - Attachment description: Bug 1536762 - part3 : disable tests on Android and Windows. → Bug 1536762 - part4 : disable tests on Android and Windows.

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.

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

Pushed by alwu@mozilla.com: 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
Blocks: webvtt-wpt
No longer blocks: 1534862
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: