phoboslab / jsmpeg

MPEG1 Video Decoder in JavaScript
MIT License
6.3k stars 1.43k forks source link

TypeError: Failed to execute 'shaderSource' on 'WebGLRenderingContext': parameter 1 is not of type 'WebGLShader'. #392

Closed humphd closed 2 years ago

humphd commented 2 years ago

I'm using jsmpeg in React, and sometimes I get this Failed to execute 'shaderSource' error (usually on tear-down or some re-render), I suspect due to React destroying the canvas context when jsmpeg doesn't expect it.

I notice that mapbox-gl-js had a similar issue, and added checks on the context being lost. Not a deal breaker, but probably something that someone with more knowledge of WebGL could fix.

Regardless, this thing is absolutely amazing. Thanks for sharing, it works so well!

phoboslab commented 2 years ago

Can you please check if the above commit fixes your problem?

I'm a bit baffled that you saw this particular error message, since shaderSource() is only executed when the player and renderer are instantiated. So it seems like there's an immediate re-layout from React, right after the JSMpeg Player is created!? Maybe you have to postpone the player creation until after React is definitely done, to avoid an unnecessary contextLost and contextRestored cycle?

humphd commented 2 years ago

I'll test this over the coming days, but my initial tests have been positive--it looks to be OK. Thanks a lot.

humphd commented 2 years ago

I'm still hitting this. It's triggered by a re-render in React, so it's probably my bug vs. yours, but in case you're interested, this is what I see:

Screen Shot 2022-03-12 at 10 51 45 AM Screen Shot 2022-03-12 at 10 56 21 AM

gl.createShader(type) with type=35633 and source=attribute vec2 vertex;\nvarying vec2 texCoord;\nvoid main() { texCoord = vertex;\ngl_Position = vec4((vertex * 2.0 - 1.0) * vec2(1, -1), 0.0, 1.0);\n} returns null.

humphd commented 2 years ago

I removed my effect's cleanup logic, and this goes away:

useEffect(() => {
   let player;

    function destroy() {
      if (player) {
        player.destroy();
        player = null;
      }
    }

    const canvasElem = canvasElemRef.current;
    if (!(canvasElem && canvasElem instanceof HTMLCanvasElement)) {
      return destroy();
    }

    if (!isVisible) {
      return destroy();
    }

    try {
      player = new window.JSMpeg.Player(src, {
        canvas: canvasElem,
        audio: false,
      });
    } catch (err) {
      console.warn(err);
      destroy();
    }

  // This is causing the crash for me, removing makes things work
  // return () => {
  //   destroy();
  // };
}, [src, canvasElemRef, isVisible])
phoboslab commented 2 years ago

player.destroy() tries to free up all resources associated with the WebGL context by explicitly calling .loseContext(). So the WebGL context is not usable after this anymore.

Considering that this happens on re-render, only when you previously call player.destroy() (and I don't know much about React): Is the <canvas> element re-created by React on a re-render, or does it try to "re-use" the previous <canvas> element?

If the element is indeed re-used, then canvas.getContext('webgl') would return the old (destroyed) context and trigger this error. I guess checking for gl.isContextLost() directly after canvas.getContext() (and then restoring it) would fix this issue.

Can you try to explicitly re-create the <canvas> element on a re-render to confirm that this is indeed what's causing the problem?