livekit / track-processors-js

Pre-built track processors for background images, blur, etc for use with the LiveKit JS Client SDK
https://livekit.io
Apache License 2.0
31 stars 14 forks source link

fix WebGL context limit warnings by forcibly terminating the context #30

Closed kand193 closed 6 months ago

changeset-bot[bot] commented 8 months ago

⚠️ No Changeset found

Latest commit: bba942432cbac9470b54d01b65ff9a22975fde7a

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

wjaykim commented 8 months ago

In my opinion, releasing resources must be handled by mediapipe side. As an alternative, maybe passing shared canvas to ImageSegmenterOptions.canvas would prevent creating new canvas and webgl contexts every time we create ImageSegmenter?

lukasIO commented 8 months ago

totally agree, that this is just a workaround for a bug in mediapipe!

for the canvas option, I don't think it will work, the JS docs say

The task will initialize a WebGL context and throw an error if

  • this fails (e.g. if you have already initialized a different type of
  • context).

which sounds a lot like they would still create a new context and if that isn't cleared, then throw an error...

wjaykim commented 8 months ago

I think the doc is saying like that because in MDN docs:

Later calls to this method on the same canvas element, with the same contextType argument, will always return the same drawing context instance as was returned the first time the method was invoked. It is not possible to get a different drawing context object on a given canvas element.

Since we're not going to get context ourside but mediapipe will, they will get same type of context on every creation and won't produce error.

lukasIO commented 8 months ago

That makes sense! the segmenter options are exposed anyways, so the question would be if clearing (and passing) that canvas should be part of this library or if users of the library are in charge of handling it themselves. I think as long as the canvas option is exposed to users, then users would have to handle it. What do you think?

wjaykim commented 8 months ago

I think as long as the canvas option is exposed to users, then users would have to handle it.

Same for me. But for now, only ImageSegmenterOptions.baseOption can be customized by user. We will have to make users be able to customize entire ImageSegmenterOptions.

lukasIO commented 7 months ago

sorry for the delay! https://github.com/livekit/track-processors-js/pull/32 exposes all SegmenterOptions now!