[Webkit-unassigned] [Bug 28327] Media layout tests should have a way to provide test files in different formats

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=28327





--- Comment #5 from Hin-Chung Lam <hclam at google.com>  2009-09-08 15:44:10 PDT ---
(In reply to comment #4)
> (From update of attachment 38997 [details])
> > +++ b/LayoutTests/media/controls-strict.html
> > +++ b/LayoutTests/media/controls-styling.html
> > +++ b/LayoutTests/media/video-aspect-ratio.html
> > +++ b/LayoutTests/media/video-controls-rendering.html
> > +++ b/LayoutTests/media/video-layer-crash.html
> > +++ b/LayoutTests/media/video-transformed.html
> > +++ b/LayoutTests/media/video-zoom-controls.html
> 
>   Passing the media file path to init() will only work as long as every pixel
> tests use a single media file. I think it would make more sense to add a
> function to each test file to set the src attribute for each video element
> before calling the existing init().

As far as I can tell, all tests that use init() are using the same test file:
test.mp4. If it makes more sense to you I can add two helper methods to
media-file.js:

// Sets src for all tags of name "tagName"
function setSrcByTagName(tagName, src)

// Sets src of tag of id "id"
function setSrcById(id, src)

> 
> > +++ b/LayoutTests/media/media-file.js
> 
> > +function findMediaFile(type, name) {
> > +    var codecs;
> > +    if (type == "audio")
> > +        codecs = audioCodecs;
> > +    else
> > +        codecs = videoCodecs;
> > +
> > +    var element = document.createElement(type);
> > +    for (var i = 0; i < codecs.length; ++i) {
> > +        if (element.canPlayType(codecs[i][0]))
> > +            return name + "." + codecs[i][1];
> > +    }
> > +
> > +    return "";
> > +}
> 
> 
>   It should be more efficient to call canPlayType() on an existing element if
> there is one. I would check to see if one exists before creating a new one.
> 
> 
> > --- a/LayoutTests/media/video-append-source.html
> 
> >  <script>
> >      testExpected("video.currentSrc", "");
> >      var source = document.createElement("source");
> > -    source.setAttribute("src", "content/test.mp4");
> > +    source.setAttribute("src", findMediaFile("video", "content/test"));
> >      video.appendChild(source);
> >  
> >      testExpected("video.currentSrc", "");
> >  
> >      waitForEvent("load", function () {
> > -        testExpected("relativeURL(video.currentSrc) ", "content/test.mp4");
> > +        testExpected("relativeURL(video.currentSrc) ", findMediaFile("video", "content/test"));
> >          endTest();
> >      });
> 
>   'findMediaFile("video", "content/test")' will always return the same result,
> you should only call it once.
> 
> +++ b/LayoutTests/media/video-paint-test.js
> @@ -1,6 +1,11 @@
> -function init()
> +function init(videoFile)
>  {
> -    var totalCount = document.getElementsByTagName('video').length;
> +    var videos = document.getElementsByTagName('video');
> +    var totalCount = videos.length;
> +    for (var i = 0; i < totalCount; ++i) {
> +        videos[i].src = videoFile;
> +    }
> 
>   These changes aren't necessary with the first suggestion above.
> 
> r+ if you make these changes (or convince me they aren't necessary).

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.