Friday, 19 April 2013

WebVTT Reftest Bug

For the bug 855633 in Mozilla's bug tracker.

https://bugzilla.mozilla.org/show_bug.cgi?id=855633

I've written many reftests for WebVTT. Outstanding tests include positioning cues with right-to-left test. There appears to be a bug in the specification. Overlapping cue tests need to be written. Tests for the tags ruby, class, and voice are not done. In addition, there are no tests for new CSS properties such as ::cue.

I've had to make a number of basic test changes. The first is that the height of the first line in a cue should be found using the css property height instead of line-height. Line-height returns "normal" on some browsers but height is always a number. Also, the specification wants the height of the first line not the line-height of the first line.

Another issue was that the height may not always be an integer. Decimal pixel values are valid (eg. 27.6px) so parseFloat should be used instead of parseInt.

The following tests are written.


Test the default behaviour. No particular rule. (April 17, 2013)
basic

Make sure cues do not render for the audio element. (April 17, 2013)
audio

Make sure control interface and cue do not overlap. (April 17, 2013)
controls

Ensure cues display in the correct order for multiple cues and tracks. (April 17, 2013)
2tracks-start-time-a
2tracks-start-time-b
1track-start-time-a
1track-start-time-b
2tracks-track-order-a
2tracks-track-order-b
2tracks-end-time-a
2tracks-end-time-b
1track-end-time-a
1track-end-time-b
1track-cue-order-a
1track-cue-order-b

Right to left writing direction test (April 17, 2013)
writing-direction-rtl

Writing mode vertical test (April 17, 2013)
writing-mode-vertical-lr
writing-mode-vertical-rl

Cue writing direction, alignment, and writting mode tests. (April 17, 2013)
The rules for direction, alignment, and mode depend on each other
so tests for each individually would be the identical.
There is a specification bug for positioning cues with right-to-left text. Those tests need to be done.
setting-horizontal-end-ltr
setting-horizontal-left-ltr
setting-horizontal-middle49-ltr
setting-horizontal-middle50-ltr
setting-horizontal-middle51-ltr
setting-horizontal-right-ltr
setting-horizontal-start-ltr
setting-vertical-lr-end
setting-vertical-lr-left
setting-vertical-lr-middle49
setting-vertical-lr-middle50
setting-vertical-lr-middle51
setting-vertical-lr-right
setting-vertical-lr-start
setting-vertical-rl-end
setting-vertical-rl-left
setting-vertical-rl-middle49
setting-vertical-rl-middle50
setting-vertical-rl-middle51
setting-vertical-rl-right
setting-vertical-rl-start

Snap-to-line tests
snap-to-lines-false-horizontal
snap-to-lines-false-vertical-lr
snap-to-lines-false-vertical-rl
snap-to-lines-true-horizontal
snap-to-lines-true-vertical-lr
snap-to-lines-true-vertical-rl

Text wrapping tests
wrap-balance
wrap-break-word

Cue test payload tag tests
tag-bold
tag-italic
tag-underline.vtt

Thursday, 18 April 2013

WebVTT Cue Display Order


I was working on the order that cues are displayed for a WebVTT file and I came across some interesting things. I was looking to test the following render rule which is a little hard to figure out but the HTML5 specification was very specific on the order.

"8. For each track track in tracks, append to cues all the cues from track's list of cues that have their text track cue active flag set."


http://dev.w3.org/html5/webvtt/#cues-with-video (April, 17, 2013)


This means that cues from different tracks should display the same as cues for the same track.

"13. Sort the tasks in events in ascending time order (tasks with earlier times first).
Further sort tasks in events that have the same time by the relative text track cue order of the text track cues associated with these tasks."


http://www.w3.org/html/wg/drafts/html/master/embedded-content-0.html#list-of-newly-introduced-cues (April, 17, 2013)


The event time is the start time for entering cues, and later of the start and end time for exiting cues.

"text track cue order, which is determined as follows: first group the cues by their text track, with the groups being sorted in the same order as their text tracks appear in the media element's list of text tracks; then, within each group, cues must be sorted by their start time, earliest first; then, any cues with the same start time must be sorted by their end time, latest first; and finally, any cues with identical end times must be sorted in the order they were last added to their respective text track list of cues, oldest first"


http://www.w3.org/html/wg/drafts/html/master/embedded-content-0.html#text-track-cue-order (April, 17, 2013)


Therefore the correct cue order is:
  1. start time (ascending)
  2. track order (top to bottom)
  3. end time (descending)
  4. cue order (top to bottom)
There are a few important things to take away from this. First, the start time is more important than anything else and the cues from multiple tracks will be mixed together. Also, end times are descending. If the cue ending soonest were under the longer lasting cue, the lasting cue would drop when the other exits. This may make it look like another cue, so it's better to have the lasting one stay in the same place on the bottom.

With the starting time being most important, one would think the ending time would be second. Instead the track order is second. This at first seems odd, but because the tracks are likely for different purposes, separating them is useful. Start time trumps track order because cues could appear in between other cues instead of at the top.

There are 12 reftests to test all possible cases with 1 track and 2 tracks.

Wednesday, 17 April 2013

WebVTT Cue Layout Reference

The purpose of this post is to describe how a WebVTT file should render. I will reference rules from the rendering cues for video section.


3. Let output be an empty list of absolutely positioned CSS block boxes.


This is a div element with the the CSS property position set to absolute.

4. If the user agent is exposing a user interface for video, add to output one or more completely transparent positioned CSS block boxes that cover the same region as the user interface.


If the video has controls visible, add a empty div over the control area.

9. For each track track in tracks, append to cues all the cues from track's list of cues that have their text track cue active flag set.
10. If reset is false, then, for each text track cue cue in cues: if cue's text track cue display state has a set of CSS boxes, then add those boxes to output, and remove cue from cues.
10. 17. Add the CSS boxes in boxes to output.


For every cue to be displayed in all tracks, add a div. If the cue has already been rendered, use the existing div.

10. 12. - The children of the nodes must be wrapped in an anonymous box whose 'display' property has the value 'inline'. This is the WebVTT cue background box.


The cue contents should be rendered into a signle <div style="display:inline"> inside the cue div.

10. 12. - Text runs must be wrapped according to the CSS line-wrapping rules, with the following additional constraints:
10. 12. - * Regardless of the value of the 'white-space' property, lines must be wrapped at the edge of their containing blocks, even if doing so requires splitting a word where there is no line breaking opportunity. (Thus, normally text wraps as needed, but if there is a particularly long word, it does not overflow as it normally would in CSS, it is instead forcibly wrapped at the box's edge.)
10. 12. - * Regardless of the value of the 'white-space' property, any line breaks inserted by the user agent for the purposes of line wrapping must be placed so as to minimize Δ across each run of consecutive lines between preserved newlines in the source. Δ for a set of lines is defined as the sum over each line of the absolute of the difference between the line's length and the mean line length of the set.


If a word is longer than the allowed width break the work with a hyphen. Don't use the CSS property word-wrap to break-word, this needs to be determined dynamically. After the number of lines is know and words broken, minimize the cue width without creating new lines.

For example

This is a really long sentence that needs to be displayed on
two lines.


should be

This is a really long sentence that
needs to be displayed on two lines.


Viewport

The specification uses the CSS viewport units vw and vh. I cannot get these to work properly with video, so I've calculated the values based on the video.


Layout

This is what the layout should be for a basic webvtt cue.

Test Video CSS



#testVideo {
 position: absolute;
 left: 0px;
 top: 0px;
 width: 640px;
 height: 480px;
}


Basic Cue Reference CSS



/* cue constants, same for every cue */
.cueBox {
 position: absolute;
 unicode-bidi: plaintext;
 font: 24px sans-serif;    /* 5vh = 24px */
 color: rgba(255,255,255,1);
 white-space: pre-line;
 
}
.cueBackgroundBox {
 display:inline;
 background: rgba(0,0,0,0.8);
}

/* cue variables, depends on cue */
#testCue {
 direction: ltr;
 writing-mode: horizontal-tb;
 left: 0px;    /* 0vw = 0px */
 top: 0px;    /* 0vh = 0px */
 width: 640px;    /* 100vw = 640px */
 text-align: center;
}

Thursday, 28 March 2013

WebVTT reftests

A reftest is a pass or fail test which checks that two web pages look identical. It does this by producing two bitmap images and verifying if they are identical or not. HTML content can be rendered the same way using different methods, such as an image and a specific frame of video. Reftests are useful to ensure that something is rendered correctly by comparing to what is ought to look like. Specifically, this is done by creating two web pages and comparing them.

Further information

Importance

This is tremendously important for WebVTT because it is the only way to ensure consistency to the specification, and therefore consistency between implementations. This means that a WebVTT file will look the same anywhere it is used, and thus enables content developers to use advanced features. WebVTT is a web standard after all.

The goal of web standards is to make implementation interoperable. (W3C) This means "the situation in which all implementations process particular content in a reliable and identical or equivalent way." (W3C) Historically when format implementations were not consistent with each other, developers only used a limited subset of the format features, had multiple versions for each implementation, or used a third-party solution. Recall the browser wars and the push for browsers to comply with web standards. (Web Standards Project)

Reftest Format

A reftest consists of three files:
  • Test page with feature to be tested
  • Reference page that show have the feature should look
  • A reftest.list file that lists the assertions

Example

The following test should pass and serves to demonstrate the basic format.

spaces1.html
<html><body>
X X
</body></html>

spaces2.html
<html><body>
X&nbsp;X
</body></html>

reftests.list
== spaces1.html spaces2.html

Details

There are two main types of assertions, expect pass (==) or expect fail (!=).

The two tests will be processed as soon as the page finished loading. Sometimes the test needs to be delayed for asynchronous content. This can be accomplished by adding the class "reftest-wait" to the HTML element and removing it at the appropriate time. This is required for WebVTT test because the video and text tracks are loaded  asynchronously.

Basic WebVTT Test

basic.html
<html class="reftest-wait">
<head>
 <meta charset="UTF-8">
 
 <style>
  #testVideo {
   position: absolute;
   left: 0px;
   top: 0px;
   width: 640px;
   height: 480px;
   margin: 0px;
   padding: 0px;
  }
 </style>
 
 <title>WebVTT</title>
</head>
<body>
 <video id="testVideo">
  <source src="grey320x240.ogv" type="video/ogg">
  <track src="basic.vtt">
 </video>
 
 <script src="testScript.js"></script>
</body>
</html>

basic-ref.html
<html class="reftest-wait">
<head>
 <meta charset="UTF-8">
 
 <style>
  #testVideo {
   position: absolute;
   left: 0px;
   top: 0px;
   width: 640px;
   height: 480px;
   margin: 0px;
   padding: 0px;
  }
  #testDiv {
   position: absolute;
   left: 0px;
   top: 250px;
   width: 640px;
   margin: 0px;
   padding: 0px;
  }
 </style>
 
 <title>WebVTT</title>
</head>
<body>
 <video id="testVideo">
  <source src="grey320x240.ogv" type="video/ogg">
 </video>
 <div id="testDiv">WebVTT Test</div>

 <script src="testScript.js"></script>
</body>
</html>

testScript.js
/*
   Make sure video is loaded,
   and that it is always at the same frame.
*/
document.getElementById('testVideo');

// Need to play to load video
testVideo.onplay = function() {
 
 // Stop video and seek to 5 seconds
 testVideo.onpause = function() {
 
  // When video is loaded, preform test
  testVideo.oncanplaythrough = function() {
   document.documentElement.className = "";
  };
  testVideo.currentTime = 5;
 };
 testVideo.pause();
};
testVideo.play();

Breakdown

CSS
The style is to make sure margins, padding, and positions are not a factor. We are not testing that, and by knowing the exact properties of the video we can create precisely what the caption should look for. It is important to only test one thing in each test.

Javascript
In order to make sure the test is correct, the bitmap must be created on exactly the same frame in the video. The following steps are performed to ensure that.

  • The video must be loaded and is not loaded until the video has been instructed to play.
  • The test must be performed on the same frame so the video must be paused on that frame.
  • To set the video to the correct frame for the test the video must be set to a predetermined frame.
    • currentTime is a decimal number in seconds
    • 1/24 is a second represents one frame in a 24 frame per second (fps) video
  • It may take time for the video to load the frame so it must wait until the video is loaded
    • A loading overlay may be shown while the video is loading. It must be gone before the test is performed.
  • Remove the "reftest-wait" class to perform the test.

Video

I created the video for the WebVTT tests. To do this I created a completely grey image. I chose grey so that white and black will be easily seen and not blend into the background. I used VirtualDub to create a raw video AVI using an AviSynth file with the ImageSource function and a silent audio file I created. I then converted the AVI to the Theora video format (OGV). Theora was chose because it is a free and open format, is specified by the HTML5 standard, and works in most browsers including Mozilla Firefox and Google Chrome.

File size is 30 kilobytes.

video
Created by Kyle Barnhart (me)
Released to public domain under Unlicense

Running Tests

Test are easily run in a Mozilla build. Just used "./mach reftest" to run all tests. You can also specify a particular set of tests. For WebVTT I used the following.

./mach reftest layout/reftests/webvtt/reftest.list

This also shows the standard location for reftests in mozilla-central.

After you have built the code, running the tests only takes a few seconds or more depending on the number of tests and the time it takes to clear the "reftest-wait" class in each test that must be delayed.

Saturday, 9 March 2013

WebVTT Parser Rules

In some recent discussion the nature of the specification has come up again. Since this has never been dealt with, I was told to ignore and leave the issue, an update seems in order.

There is a new bug that has been posted where the following is stated.
"Also add a notice at syntax specification that implementers need to read the parsing section."
- Bug 21230 - [WebVTT] specify extension points in syntax spec

This comes from a discussion on the mailing list. I'll add the following relevant sections.

[Discussion of conflict between syntax and parser rules in WebVTT specification]

No, there is no conflict. The first one is the current spec, the second is the requirement on how to parse it so that the current spec can be extended in the future.

- Silvia Pfeiffer


Ralph Giles wrote:
"If you do want to do something application-specific here, at least try to follow the syntax rules implied by Silvia's draft. Then when you have some implementation experience, we should try to spec what those rules actually are."

Please don't do that, either. Don't put anything in there at all until we're sure of what the format should be.

- Glenn Maynard


Just to clarify (at least to my understanding): The first is the file format, and the second is the parser.  Both are part of the spec.  The file format tells you what's valid--what authors should be writing.  The parser tells you how to deal with every possible input, which includes error handling (inputs that don't follow the file format) and--as you said--forwards-compatibility.

I've seen a couple people confused by this, leading to people trying to write implementations by looking at the file format, which is bad.  Browser vendors understand it, since that's how the HTML spec works, but since non-browser people without experience with that spec may be implementing this (more than most other parts of the web), maybe there should be a brief note at the top of the file format section explaining this.  ("If you're implementing WebVTT, you're in the wrong place!")

- Glenn Maynard


Agreed, we need to clarify this. I want to make sure to specify extension points in the syntax specification to make sure that implementers are made aware of this.

- Silvia Pfeiffer

Tuesday, 5 March 2013

WebVTT Mozilla Reftest

I'm working on what needs to be done for testing the WebVTT rendering rules. The best way to go looks like using reftest. Mozilla MDN has a page on it. Full documentation for Mozilla is here.

It is very straightforward. Make two HTML pages that are identical except for the thing you want to test. For example, webvtt-basic.html will have a <track> in a <video>. webvtt-basic-ref.html will have <div> in <video>. The results must look exactly the same to pass the test.

Reftest works be taking a screenshot of the windows immediately after the document is loaded and comparing. However, since this is a video that might take a second to load, adding the class reftest-wait and removing it when the time is right with document.documentElement.className = "".

Here is a basic test.
<!DOCTYPE html>
<html class="reftest-wait">
<title>WebVTT</title>
<body>
<video id="testVideo" autoplay>
    <source src="test.ogv" type="video/ogg">
    <track src="test.vtt">
</video>
</body>
<script>
    document.getElementById('testVideo').onplay = function(e) {
  document.documentElement.className = "";
    };
</script>
</html>


I found a nice public domain video to use.

video

WebVTT Rendering Issues

I've been working on the rendering stage of WebVTT. I've been trying to work out how WebVTT Cues are converted into DOM elements. I've found a problem with our interpreter.

It should work like this. This is based on the API in the HTML5 specification.
  1. A media element (i.e. video element) populates a list of text tracks.

  2. For each valid track element create a new track.

  3. For new tracks: "The text track list of cues is initially empty. It is dynamically modified when the referenced file is parsed."

  4. When a track is created, changed, or its parent is changed to a new media element, then start the track processing model for that track element.

  5. If the track element is valid, then the fetching algorithm load's the resource indicated by the URL. If the resource's type is a valid text track format, then the data is passed to the parser where output is a list of cues.

    • [ISSUE 1] How the cues are added to the list of cues is not perfectly clear, but it appears the parser is passed a list of cues as the parameter "output" and adds cues incrementally. I say this because the output is not a stream, just a list, so it cannot return a stream of cues which the browser would add incrementally. After double checking I don't believe our interpreter does this, but I may be wrong.

      • From WebVTT Parsing Rules:

        "A WebVTT parser, given an input byte stream and a text track list of cues output"

      • From HTML5 Track Processing Model:

        "... the resource's data must be passed to the appropriate parser (e.g. the WebVTT parser) as it is received, with the text track list of cues being used for that parser's output."

    • [ISSUE 2] According to rule 47, "Cue text processing", when the parser creates a cue, it sets the text track cue's text to the text loaded without processing the text. The text track cue text should just be a string, initially empty. Currently it is a list of nodes.

      • From WebVTT Parsing Algorithm:

        "47. Cue text processing: Let the text track cue text of cue be cue text, and let the rules for its interpretation be the WebVTT cue text parsing rules, the WebVTT cue text rendering rules, and the WebVTT cue text DOM construction rules."

  6. When a cue is displayed then the render rules are run.

    • [ISSUE 3] The unprocessed text track cue text is processed only when it is to be displayed during the playback loop for cues. What should happen is that the cue is associated with the parser algorithm for rendering and calls that when it needs to render.

      • From HTML5 Playback Loop for Cues:

        "18. Run the rules for updating the text track rendering of each of the text tracks in affected tracks that are showing. For example, for text tracks based on WebVTT, the rules for updating the display of WebVTT text tracks."

      • From TeckTrackCue getCueAsHTML Method:

        "The getCueAsHTML() method must convert the text track cue text to a DocumentFragment for the script's document of the entry script, using the appropriate rules for doing so. For example, for WebVTT, those rules are the WebVTT cue text parsing rules and the WebVTT cue text DOM construction rules."




This is useful for dealing with order. I haven't checked if the parser sorts. It shouldn't.

The default render rules for dynamically created TextTrackCues are the WebVTT render rules.

4. Let cue's text track cue text be the value of the text argument, and let the rules for its interpretation be the WebVTT cue text parsing rules, the WebVTT cue text rendering rules, and the WebVTT cue text DOM construction rules.


This is how cues are ordered. I wouldn't worry about it. The ordering is done during the playback loop for cues via steps 1 and 13.

The text track cues of a media element's text tracks are ordered relative to each other in the text track cue order, which is determined as follows: first group the cues by their text track, with the groups being sorted in the same order as their text tracks appear in the media element's list of text tracks; then, within each group, cues must be sorted by their start time, earliest first; then, any cues with the same start time must be sorted by their end time, latest first; and finally, any cues with identical end times must be sorted in the order they were last added to their respective text track list of cues, oldest first (so e.g. for cues from a WebVTT file, that would initially be the order in which the cues were listed in the file).

HTML Track Status

I've been looking at how the track gets implemented and how WebVTT cues are loaded. This is where things are concerning the HTML element track.


Bugzilla:
https://bugzilla.mozilla.org/show_bug.cgi?id=833386


Track

interface HTMLMediaElement
readonly attribute TextTrackList textTracks
TextTrack addTextTrack(DOMString kind, optional DOMString label, optional DOMString language)

Timed Text Track Model
this describes how HTMLMediaElement deals with text tracks. it's quite extensive.



interface HTMLTrackElement
readonly attribute TextTrack track



interface TextTrackList
getter TextTrack (unsigned long index)

REPO: unknown


interface TextTrack
readonly attribute TextTrackCueList? cues
readonly attribute TextTrackCueList? activeCues
void addCue(TextTrackCue cue)
void removeCue(TextTrackCue cue)



interface TextTrackCueList
getter TextTrackCue (unsigned long index)
TextTrackCue? getCueById(DOMString id)



interface TextTrackCue
readonly attribute TextTrack? track
DocumentFragment getCueAsHTML()


The getCueAsHTML() method must convert the text track cue text to a DocumentFragment for the script's document of the entry script, using the appropriate rules for doing so. For example, for WebVTT, those rules are the WebVTT cue text parsing rules and the WebVTT cue text DOM construction rules.


------------------------------------


Coming Changes to Specification

There are big changes coming to the track element and WebVTT. Either the TextTrackCue interface is moving to the WebVTT spec, or the parsing, DOM construction, and rendering rules will move to the HTML spec.

See this and replies: http://lists.w3.org/Archives/Public/public-texttracks/2013Feb/0005.html

-------------------------------------

Also, I'm working on rendering. Just to note, there are 5 sections to the WebVTT specification, these three are in addition to syntax and parsing.


WebVTT

WebVTT DOM Construction Rules:

WebVTT Rendering Rules:

WebVTT CSS Rules:

Wednesday, 13 February 2013

Mozilla Developer Network HTMLTrackElement

I added HTMLTrackElement to the Mozilla Developer Network (MDN) wiki. I made sure it was technically correct and conformed to formatting standards already set by similar pages on MDN.

The TextTrack object is not defined in MDN. I did not do so because similar attributes are not present in other pages. For example the read only attributes audioTracks, videoTracks, and textTracks, which are not defined in MDN's HTMLMediaElement but are in the specification.

https://developer.mozilla.org/en-US/docs/DOM/HTMLTrackElement

Thursday, 31 January 2013

WebVTT Parser Specification IRC Chat

I when onto #whatwg on irc.freenode.org. It is the main chat room for WHATWG. Here is their IRC page. Please note the regulars.

Log: http://krijnhoetmer.nl/irc-logs/whatwg/20130131

  1. # [18:22] <KyleBarnhart> Hi. Is there someone of authority how can definitively answer a question regarding the implementation of the WebVTT text track specification?
  2. # [18:22] <KyleBarnhart> Specifically...
  3. # [18:24] <KyleBarnhart> I am part of a team who is trying to implement the standard. Whereas I take the view that the parser section of the specification is to be adhered to when writing an implementation, they take the view that the implementation is should be written to the syntax rules and the parser section can be ignored.
  4. # [18:25] <Ms2ger> The parser section is the only relevant one for an implementation
  5. # [18:26] <KyleBarnhart> It is one. Not a validator. It is parsing the format for the eventual use in Mozilla.
  6. # [18:26] <Ms2ger> Right
  7. # [18:26] <Ms2ger> Then you want to follow the parsing section
  8. # [18:27] <Ms2ger> I understand your first patches are due today?
  9. # [18:28] <KyleBarnhart> I believe.
  10. # [18:28] <KyleBarnhart> I'm not responsible for that. I'm handling testing at the moment.
  11. # [18:29] <Ms2ger> Great, we'll need quite a few of those :)
  12. # [18:34] <KyleBarnhart> Is it okay if the parser (and I mean in the final future version) differs in some significant ways from the specification? Such as when to and not to discard a cue, and accepting input that the specification would not allow.
  13. # [18:43] <Ms2ger> If the code is to end up in Gecko, it had better follow the specification to the letter
  14. # [18:44] <Hixie> KyleBarnhart: what do they think the spec is for, if not following?
  15. # [18:45] <MikeSmith> inspiration
  16. # [18:46] <Ms2ger> Suggestions

...

  1. # [18:56] <KyleBarnhart> Thank you very much for your answer. I'm going to go now and discuss this with our team.
  2. # [18:56] <Hixie> KyleBarnhart: please don't hesitate to ask any other questions
  3. # [18:57] <KyleBarnhart> Thank you :)
  4. # [18:57] <Velmont> KyleBarnhart: What's also good to know is that specs are never really set in stone.
  5. # [18:58] <Hixie> MikeSmith: when you have a moment and happen to be at bugzilla's admin page again, if you could add a "Needs Research" between "2022 Q4" and "Needs Impl Interest", that'd be great
  6. # [18:58] <Velmont> KyleBarnhart: It should follow what implementations do as well. So if there's something strange there it might be a bug in the spec.
  7. # [18:58] <Hixie> KyleBarnhart: yeah, what Velmont said. If there's a reason to implement something other than the spec, we should change the spec.
  8. # [18:58] <Hixie> KyleBarnhart: the reason to have a spec is to make sure everyone does the same thing, but so long as they all do the same thing, it doesn't really matter what that thing is exactly
  9. # [18:58] <Velmont> KyleBarnhart: Although there is already implementions of that one, so hopefully there shouldn't be too much :-)

WebVTT File Structure Unit Tests Review

As usual I've changed the comments to reflect the specification for the parser. Fortunately a few of the comments were correct and I didn't have to change them.

File Structure Unit Tests Review (filestructure_unittest.cpp)

  • BlankFile
    • Added false to the load function. The parser fails if the first line has less that 6 characters.
    • Since it should fail there should be an error. I've added "WEBVTT_MALFORMED_TAG" error check.
  • BlankFileWithBOM
    • Added false to the load function. The parser fails if the first line has less that 6 characters.
    • Since it should fail there should be an error. I've added "WEBVTT_MALFORMED_TAG" error check.
  • HeaderNoNewLine
    • Nothing wrong. First comment ever that I didn't have to change.
  • MultiCueNoNewlineBetweenCues
    • There should be two cues. Changed load function cue count check to 2.
  • BOMGarbageData
    • This throws an error. Added "WEBVTT_MALFORMED_TAG" error check.
  • BOMTabWebvtt
    • Tab before WEBVTT string throws an error. Added "WEBVTT_MALFORMED_TAG" error check.
  • MissingNewLineBetweenCues
    • Duplicate of "MultiCueNoNewlineBetweenCues" test.
    • Removed test.

WebVTT Cue Timings Unit Test Review

I should note this file had almost 3000 lines.

I've changed all the comments to reflect the parser rules since that is what the tests need to match. I've added comments for any syntax error. The comments should be separate since they are from different parts of the specifications and regarding different things.

Cue Timings Unit Test Review (cuetimes_unittest.cpp)

  • FromOneDigitMinute
    • The cue will be bad and discarded.
      • Changed cues returned from 1 to 0.
      • Removed timestamp assert.
  • FromThreeDigitMinute
    • The cue will be bad and discarded.
      • Changed cues returned from 1 to 0.
      • Removed timestamp assert.
  • FromOneDigitSecond
    • The cue will be bad and discarded.
      • Changed cues returned from 1 to 0.
      • Removed timestamp assert.
  • FromThreeDigitSecond
    • The cue will be bad and discarded.
      • Changed cues returned from 1 to 0.
      • Removed timestamp assert
  • FromTwoDigitMillisecond
    • The cue will be bad and discarded.
      • Changed cues returned from 1 to 0.
      • Removed timestamp assert
  • FromFourDigitMillisecond
    • The cue will be bad and discarded.
      • Changed cues returned from 1 to 0.
      • Removed timestamp assert
  • FromMinuteGreaterThan59
    • The cue will be bad and discarded.
      • Changed cues returned from 1 to 0.
      • Removed timestamp assert
  • FromSecondGreaterThan59
    • The cue will be bad and discarded.
      • Changed cues returned from 1 to 0.
      • Removed timestamp assert
  • FromMillisecondGreaterThan999
    • Technically this is the same test as "FromFourDigitMillisecond", but I left it in.
    • The cue will be bad and discarded.
      • Changed cues returned from 1 to 0.
      • Removed timestamp assert
  • FromBadDelimiterMinuteSecond
    • To for the deliminator between minute and second, the hour must not be 2 digits, otherwise it checked for the deliminator between second and millisecond.
      • Changed hour from 00 to 000 in "bad-delimiter-minute-second.vtt".
  • FromBadDelimiterHourMinute
    • Keep consistant with "FromBadDelimiterMinuteSecond" test.
      • Changed hour from 00 to 000 in "bad-delimiter-hour-minute.vtt".
  • FromBadDelimiterSecondMillisecond
    • Keep consistant with "FromBadDelimiterMinuteSecond" test.
      • Changed hour from 00 to 000 in "bad-delimiter-second-millisecond.vtt".
  • UntilMinuteWithinLowBoundary
    • Minutes in timestamp not consistent with similar tests. It was the same as in "UntilMinuteWithinHighBoundary".
      • Changed "minute-within-low-boundary.vtt"
      • Changed test to match vtt file.
  • UntilMinuteWithinHighBoundary
    • Minutes in timestamp not consistent with similar tests. It was the same as in "UntilMinuteWithinLowBoundary".
      • Changed "minute-within-high-boundary.vtt".
      • Changed test to match vtt file.
  • UntilSecondWithinLowBoundary
    • Seconds in timestamp not consistent with similar tests. It was the same as in "UntilSecondWithinHighBoundary".
      • Changed "second-within-low-boundary.vtt"
      • Changed test to match vtt file.
  • UntilSecondWithinHighBoundary
    • Seconds in timestamp not consistent with similar tests. It was the same as in "UntilSecondWithinLowBoundary".
      • Changed "second-within-high-boundary.vtt".
      • Changed test to match vtt file.
  • UntilMillisecondWithinLowBoundary
    • Milliseconds in timestamp not consistent with similar tests. It was the same as in "UntilMillisecondWithinHighBoundary".
      • Changed "millisecond-within-low-boundary.vtt"
      • Changed test to match vtt file.
  • UntilMillisecondWithinHighBoundary
    • Milliseconds in timestamp not consistent with similar tests. It was the same as in "UntilMillisecondWithinLowBoundary".
      • Changed "millisecond-within-high-boundary.vtt".
      • Changed test to match vtt file.
  • UntilOneDigitMinute
    • The cue will be bad and discarded.
      • Changed cues returned from 1 to 0.
      • Removed timestamp assert.
  • UntilThreeDigitMinute
    • The cue will be bad and discarded.
      • Changed cues returned from 1 to 0.
      • Removed timestamp assert.
  • UntilOneDigitSecond
    • The cue will be bad and discarded.
      • Changed cues returned from 1 to 0.
      • Removed timestamp assert.
  • UntilThreeDigitSecond
    • The cue will be bad and discarded.
      • Changed cues returned from 1 to 0.
      • Removed timestamp assert
  • UntilTwoDigitMillisecond
    • The cue will be bad and discarded.
      • Changed cues returned from 1 to 0.
      • Removed timestamp assert
  • UntilFourDigitMillisecond
    • The cue will be bad and discarded.
      • Changed cues returned from 1 to 0.
      • Removed timestamp assert
  • UntilMinuteGreaterThan59
    • The cue will be bad and discarded.
      • Changed cues returned from 1 to 0.
      • Removed timestamp assert
  • UntilSecondGreaterThan59
    • The cue will be bad and discarded.
      • Changed cues returned from 1 to 0.
      • Removed timestamp assert
  • UntilMillisecondGreaterThan999
    • Technically this is the same test as "FromFourDigitMillisecond", but I left it in.
    • The cue will be bad and discarded.
      • Changed cues returned from 1 to 0.
      • Removed timestamp assert
  • UntilBadDelimiterMinuteSecond
    • To for the deliminator between minute and second, the hour must not be 2 digits, otherwise it checked for the deliminator between second and millisecond.
      • Changed hour from 00 to 000 in "bad-delimiter-minute-second.vtt".
  • UntilBadDelimiterHourMinute
    • Keep consistant with "UntilBadDelimiterMinuteSecond" test.
      • Changed hour from 00 to 000 in "bad-delimiter-hour-minute.vtt".
  • UntilBadDelimiterSecondMillisecond
    • Keep consistant with "UntilBadDelimiterMinuteSecond" test.
      • Changed hour from 00 to 000 in "bad-delimiter-second-millisecond.vtt".
  • BareMinimum
    • Added timestamps asserts since the goal of these tests is to test timestamps.
  • BareMinimumWithContent
    • Added timestamps asserts since the goal of these tests is to test timestamps.
  • HighestValues
    • Added timestamps asserts since the goal of these tests is to test timestamps.
  • SpaceTabs
    • Duplicate test. Removed. See separator tests.
  • Tabs
    • Duplicate test. Removed. See separator tests.
  • Hours
    • Added timestamps asserts since the goal of these tests is to test timestamps.
  • MultiCues
    • Added timestamps asserts since the goal of these tests is to test timestamps.
  • NestedCues
    • Added timestamps asserts since the goal of these tests is to test timestamps.
  • MissingSpaces
    • Duplicate test. Removed. See separator tests.
  • MissingSpaceLeft
    • Duplicate test. Removed. See separator tests.
  • MissingSpaceRight
    • Duplicate test. Removed. See separator tests.
  • NoNumsInTimeStamp
    • Duplicate test. Removed. See from and until in timings tests (this file).
  • ArrowsWrongDirection
    • Duplicate test. Removed. See separator tests.
  • BadTimeStamps1
    • Duplicate test. Removed. See deliminator in from and until in timings tests (this file).
  • BadTimeStamps2
    • Duplicate test. Removed. See deliminator in from and until in timings tests (this file).
  • TimeStampHoursOmitted renamed to StartTimeStampHoursOmitted.
  • EndTimeStampHoursOmitted  added.

WebVTT Cue Identifier and Timings Unit Test Reivew

I went over the cue identifier tests and general timings tests again to updated the file for the new repo.

I noticed some important mistakes about treating the second cue differently than the first and I've removed those tests. The first and second cue are treated the same, except if there is no blank line between the first cue's identifier and the WEBVTT string, then the identifier is treated as the file header and the cue is processed as if it had no header.

I also fixed some comments to reflect that the first and second cues are treated the same.

I ran the tests. Many of the tests crash (beyond the 4 I've changed)  since all the DISABLEDs have been removed. So I did not do anything to point out which one crashes.

The following are changed other than removing the unnecessary tests I added.

Cue Identifier Unit Tests

Identifier Line Ending Unit Tests (cilineendings_unittest.cpp)

  • LongStringLF
    • Shortend strings. Long identifier test is seperate. We shouldn't tests two things.
    • Renamed to "TwoLinesLF"
  • LongStringCRLF
    • Shortend strings. Long identifier test is seperate. We shouldn't tests two things.
    • Rename to "TwoLinesCRLF"

Wednesday, 23 January 2013

WebVTT Cue Timings General Unit Test Review

I am splitting these reviews up for one per C++ file. This should make them easier to deal with.

Changed all comments to reflect rules for parsers.

Added checks to make sure the start and end times are correct.

Fixed spelling "Seperator" to "Separator"

Unused vtt files deleted:
  • cue-times/separator/missing_both_spaces_bad.vtt
  • cue-times/separator/missing_separator_bad.vtt

Cue Timings General (ctgenstructure_unittest.cpp)

Changes

  • CueTimingsSeperator1LeftMix
    • Changes test file to actual filename "cuetimings_separator_1_space_left_mix.vtt"
  • CueTimingsSeperator1RightMix
    • Changes test file to actual filename "cuetimings_separator_1_space_right_mix.vtt"
  • CueTimingsSeperatorNTab
    • Changes test file to actual filename "cuetimings_separator_n_tab.vtt"
  • CueTimingsSeperatorNLMix
    • Changes test file to actual filename "cuetimings_separator_n_spaces_left_mix.vtt"
  • CueTimingsSeperatorNRMix
    • Changes test file to actual filename "cuetimings_separator_n_spaces_right_mix.vtt"
  • CueTimingsSeperatorXMix
    • Changes test file to actual filename "cuetimings_separator_n_spaces_tabs_on_both_sides.vtt"

Added

  • CueTimingsSeparatorMissingLeft
    •  Parser does not require whitespace around separator.
    • Added file "cuetimings_separator_0_spaces_left.vtt"
  • CueTimingsSeperatorMissingRight
    •  Parser does not require whitespace around separator.
    • Added file "cuetimings_separator_0_spaces_right.vtt"
  • CueTimingsSeperatorMissingBoth
    •  Parser does not require whitespace around separator.
    • Added file "cuetimings_separator_0_spaces.vtt"
  • CueTimingsArrowMalformed
    • If the first or second line does not contain --> the cue is bad. The parser will discard bad cues.
    • Added file "cuetimings_arrow_bad.vtt"
  • CueTimingsArrowMissing
    • If the first or second line does not contain --> the cue is bad. The parser will discard bad cues.
    • Added file "cuetimings_arrow_missing.vtt"

Failing Test

[  FAILED  ] CueTimingsGeneralStructure.CueTimingsSeperator1LeftMix
[  FAILED  ] CueTimingsGeneralStructure.CueTimingsSeperator1RightMix
[  FAILED  ] CueTimingsGeneralStructure.CueTimingsSeperatorNSpace
[  FAILED  ] CueTimingsGeneralStructure.CueTimingsSeperatorNTab
[  FAILED  ] CueTimingsGeneralStructure.CueTimingsSeperatorNLMix
[  FAILED  ] CueTimingsGeneralStructure.CueTimingsSeperatorNRMix
[  FAILED  ] CueTimingsGeneralStructure.CueTimingsSeperatorXMix
[  FAILED  ] CueTimingsGeneralStructure.CueTimingsSeparatorMissingLeft
[  FAILED  ] CueTimingsGeneralStructure.CueTimingsSeparatorMissingRight
[  FAILED  ] CueTimingsGeneralStructure.CueTimingsSeperatorMissingBoth
[  FAILED  ] CueTimingsGeneralStructure.CueTimingsArrowMalformed
[  FAILED  ] CueTimingsGeneralStructure.CueTimingsArrowMissing