While we have unit tests we lack an end-to-end test for handling videos that change dimensions during playback.
Created attachment 134717 [details] Patch
Hey Eric mind taking a look over this test? The VP8 specification doesn't disallow against resolution changes mid-stream. I tried to get a clip generated for other codecs but failed so for now I'm checking it under LayoutTests/platform/chromium -- let me know if you want me to take another approach. For now I'm mostly interested in getting a test in so we don't regress in this area again. As a side... from a spec perspective any thoughts on whether the element size should change to reflect the new resolution or should it remain the same? I can see arguments both ways :)
(In reply to comment #2) > Hey Eric mind taking a look over this test? > > The VP8 specification doesn't disallow against resolution changes mid-stream. I tried to get a clip generated for other codecs but failed so for now I'm checking it under LayoutTests/platform/chromium -- let me know if you want me to take another approach. For now I'm mostly interested in getting a test in so we don't regress in this area again. > The dimensions of some streamed formats (eg. MPEG2 transport stream) can certainly change when tracks appear or disappear, but we aren't set up to stream files for layout tests yet so I think it is OK for this to be chromium-specific for now. > As a side... from a spec perspective any thoughts on whether the element size should change to reflect the new resolution or should it remain the same? I can see arguments both ways :) I think it is clear that videoWidth and videoHeight must change, and and in the absence of explicit sizing instructions an element's size is set from the content size, so I think it should reflect the new resolution.
Comment on attachment 134717 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=134717&action=review > LayoutTests/platform/chromium/media/video-frame-size-change.html:19 > + waitForEvent("canplay", function() { > + testExpected(video.clientWidth, 1920); > + testExpected(video.clientHeight, 1080); > + testExpected(video.videoWidth, 1920); > + testExpected(video.videoHeight, 1080); > + video.play(); > + }); It is probably worth having another <video> element with "width" and "height" attributes to make sure that .clientWidth and .clientHeight do NOT change when the intrinsic size changes. > LayoutTests/platform/chromium/media/video-frame-size-change.html:34 > + // Note that it's not specified whether we should scale the new frame size > + // or adjust the size of the element. See the following for details: > + // http://code.google.com/p/chromium/issues/detail?id=117629 > + // I don't think this is necessary.
Comment on attachment 134717 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=134717&action=review >> LayoutTests/platform/chromium/media/video-frame-size-change.html:19 >> + }); > > It is probably worth having another <video> element with "width" and "height" attributes to make sure that .clientWidth and .clientHeight do NOT change when the intrinsic size changes. Good idea! >> LayoutTests/platform/chromium/media/video-frame-size-change.html:34 >> + // > > I don't think this is necessary. Removed.
Committed as http://trac.webkit.org/changeset/112920