[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Public WebGL] texImage2D on unfinished images and video



My concern is if you have a video player library that plays a playlist of videos and then you decide later to add some WebGL to it, you have to go through all kinds of contortions just to avoid an INVALID_OPERATION. The framework you're using for the video playlists seems like it would be unlikely to have the needed functionality built in to let you know when top stop calling texImage2D and when to start again.

If the error was removed and 1x1 was substituted that problem would go away

I don't see what error adds except to make it more complicated.



On Fri, Oct 19, 2012 at 3:09 PM, Kenneth Russell <kbr@google.com> wrote:
On Tue, Oct 16, 2012 at 4:46 PM, Gregg Tavares (社用) <gman@google.com> wrote:
>
>
>
> On Tue, Oct 16, 2012 at 4:28 PM, Kenneth Russell <kbr@google.com> wrote:
>>
>> On Tue, Oct 16, 2012 at 3:54 PM, Gregg Tavares (社用) <gman@google.com>
>> wrote:
>> > I don't think the spec makes this clear what happens when you try to
>> > call
>> > texImage2D or texSubImage2D on an image or video that is not yet loaded.
>>
>> Right, the behavior in this case is not defined. I'm pretty sure this
>> was discussed a long time ago in the working group, but it seemed
>> difficult to write a test for any defined behavior, so by consensus
>> implementations generate INVALID_OPERATION when attempting to upload
>> incomplete images or video to WebGL textures.
>
>
> I don't see a problem writing a test. Basically
>
> var playing = false
> video.src = ""> > video.play();
> video.addEventListener('playing', function() { playing = true;});
>
> waitForVideo() {
>   gl.bindTexture(...)
>   gl.texImage2D(..., video);
>   if (playing) {
>      doTestsOnVideoContent();
>      if (gl.getError() != gl.NO_ERROR) { testFailed("there should be no
> errors"); }
>   } else {
>     requestAnimationFrame(waitForVideo);
> }
>
> This basically says an implementation is never allowed to generate an error.
> it might not be a perfect test but neither is the current one. In fact this
> should test it just fine
>
>   video = document.createElement("video");
>   gl.texImage2D(..., video);
>   glErrorShouldBe(gl.NO_ERROR);
>   video.src = ""> >   gl.texImage2D(..., video);
>   glErrorShouldBe(gl.NO_ERROR);
>   video.src = ""> >   gl.texImage2D(..., video);
>   glErrorShouldBe(gl.NO_ERROR);
>
> Then do the other 'playing' event tests.
>
>>
>>
>> > Example
>> >
>> >    video = document.createElement("video");
>> >    video.src = "" href="http://mysite.com/myvideo" target="_blank">http://mysite.com/myvideo";
>> >    video.play();
>> >
>> >    function render() {
>> >      gl.bindTexture(...)
>> >      gl.texImage2D(..., video);
>> >      gl.drawArrays(...);
>> >      window.requestAnimationFrame(render);
>> >   }
>> >
>> > Chrome right now will synthesize a GL error if the system hasn't
>> > actually
>> > gotten the video to start (as in if it's still buffering).
>> >
>> > Off the top of my head, it seems like it would just be friendlier to
>> > make a
>> > black texture (0,0,0,1) or (0,0,0,0) 1x1 pixels for videos that aren't
>> > loaded yet.
>> >
>> > Otherwise, having to check that the video is ready before calling
>> > texImage2D
>> > seems kind of burdensome on the developer. If they want to check they
>> > should
>> > use video.addEventListener('playing') or similar. If they were making a
>> > video player they'd have to add a bunch of logic when queuing the next
>> > video.
>> >
>> > Same with images.
>> >
>> >    img = document.createElement("img");
>> >    img.src = "" href="http://mysite.com/myimage" target="_blank">http://mysite.com/myimage";
>> >
>> >    function render() {
>> >      gl.bindTexture(...)
>> >      gl.texImage2D(..., img);
>> >      gl.drawArrays(...);
>> >      window.requestAnimationFrame(render);
>> >   }
>> >
>> > If you want to know if the image has loaded use img.onload but otherwise
>> > don't fail the call?
>> >
>> > What do you think? Good idea? Bad idea?
>>
>> My initial impression is that this change is not a good idea. It would
>> expose the specification, implementations and applications to a lot of
>> corner case behaviors. For example, if a video's width and height
>> hasn't been received yet, then texImage2D(..., video) would have to
>> allocate a 1x1 texture; but if the width and height are known, then it
>> would have to allocate the right amount of storage. A naive app might
>> call texImage2D only the first time and texSubImage2D subsequently, so
>> if the first texImage2D call was made before the metadata was
>> downloaded, it would never render correctly. I think the current
>> fail-fast behavior is best, and already has the result that it renders
>> a black texture if the upload fails; the call will (implicitly)
>> generate INVALID_OPERATION, and the texture will be incomplete.
>
>
> I don't see how that's worse than the current situation which is you call
> texImage2D and pray it works. You have no idea if it's going to work or not.
> If you do this does it work?
>
>   okToUseVideo = false;
>   video = document.createElement("video");
>   video.src = ""> >   video.addEventListener('playing', function() { okToUseVideo = true; }
>
>   frameCount = 0;
>
>   function render() {
>      if (okToUseVideo) {
>         gl.texImage2D(...  , video);
>
>         ++frameCount;
>         if (frameCount > 1000) {
>           video.src = ""> >         }
>      }
>   }
>
>
> Basically after some amount of time I switch video.src to a new movie. Is
> the video off limits now?

The assumption should be "yes". When the source of the media element
is set, it is immediately invalid to use until the onload / playing
handler is called.

> Will it use the old frame from the old movie until
> the new movie is buffered or will it give me INVALID_OPERATION? I have no
> idea and it's not specified. Same with img.
>
> img.src = ""> > img. {
>   img.src = ""> >   gl.texImage2D(...., img);  // what's this? old image, no image,
> INVALID_OPERATION?
> }

Yes, this should be assumed to produce INVALID_OPERATION.


> The texSubImage2D issue is not helped by the current spec. If video.src =""> > useChoosenURL then you have no idea what the width and height are until the
> 'playing' event (or whatever event) which is no different than if we changed
> it.
>
> Changing it IMO means far less broken websites and I can't see any
> disadvantages. Sure you can get a 1x1 pixel texture to start and call
> texSubImage2D now but you can do that already.

With the current fail-fast behavior, where INVALID_OPERATION will be
generated, the texture will be in one of two states: (1) its previous
state, because the texImage2D call failed; or (2) having the width and
height of the incoming media element.

With the proposed behavior to never generate an error, the texture
will be either 1x1 or width x height. Another problem with this
proposal is that there is no way with the ES 2.0 or WebGL API to query
the size of a level of a texture, because GetTexLevelParameter was
removed from the OpenGL ES API (and, unfortunately, not reintroduced
in ES 3.0). Therefore the behavior is completely silent -- there is no
way for the application developer to find out what happened (no error
reported, and still renders like an incomplete texture would).

I agree that the failing behavior should be specified and, more
importantly, tests written verifying it. If the INVALID_OPERATION
error were spec'ed, would that address your primary concern? I'm not
convinced that silently making the texture 1x1 is a good path to take.

-Ken


>>
>>
>> If we want to spec this more tightly then we'll need to do more work
>> in the conformance suite to forcibly stall HTTP downloads of the video
>> resources in the test suite at well known points. I'm vaguely aware
>> that WebKit's HTTP tests do this with a custom server. Requiring a
>> custom server in order to run the WebGL conformance suite at all would
>> have pretty significant disadvantages.
>>
>> -Ken
>
>