Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit e3f4365:
|
5fa25c9 to
2d39bdd
Compare
2d39bdd to
eef118c
Compare
| FilesetResolver.forVisionTasks(basePath) | ||
| .then((vision) => FaceLandmarkerImpl.createFromOptions(vision, options)) | ||
| .then((faceLandmarker) => setFaceLandmarker(faceLandmarker)) | ||
| .catch((err) => console.error('error while creating faceLandmarker', err)) |
There was a problem hiding this comment.
I'd consider using suspense here.
There was a problem hiding this comment.
but how to deal with ret.close() cleanup with suspend?
There was a problem hiding this comment.
Ok, following @drcmda additional advices on that, I've now converted it to suspend, but I let you check my impl is correct
| return () => void ret?.close() | ||
| }, [basePath, options]) |
There was a problem hiding this comment.
If options is an object, this will break reference every rerender. Either destructure or use an expression that is immune like JSON.stringify -- not ideal but it works.
There was a problem hiding this comment.
Ok, I've set opts = JSON.stringify(options) and set my dep with
| const [stream, setStream] = useState<MediaStream>() | ||
|
|
||
| const videoTextureApiRef = useRef<VideoTextureApi>(null) | ||
|
|
||
| const faceControls = useFaceControls() | ||
| useEffect(() => { | ||
| let strm: MediaStream | ||
|
|
||
| if (!videoTextureSrc) { | ||
| navigator.mediaDevices | ||
| .getUserMedia({ | ||
| audio: false, | ||
| video: { facingMode: 'user' }, | ||
| }) | ||
| .then((s) => { | ||
| strm = s | ||
| faceControls.dispatchEvent({ type: 'stream', stream: strm }) | ||
| setStream(strm) | ||
| }) | ||
| .catch(console.error) | ||
| } | ||
|
|
||
| return () => strm?.getTracks().forEach((track) => track.stop()) | ||
| }, [faceControls, videoTextureSrc]) |
There was a problem hiding this comment.
Suspense would also be helpful here. I'm pedantic about this now as it's breaking later.
There was a problem hiding this comment.
ok now done ;) I let you check my impl
|
thanks for the review @CodyJasonBennett, will integrate them tomorrow ;) |
| <color attach="background" args={['#303030']} /> | ||
| <axesHelper /> | ||
|
|
||
| <React.Suspense fallback={null}> |
There was a problem hiding this comment.
I guess React.Suspense is now required here, since <FaceLandmarker> provider is now suspended?
I'm not sure
| <FaceControlsContext.Provider value={api}> | ||
| {webcam && <Webcam ref={webcamApiRef} autostart={autostart} videoTextureSrc={webcamVideoTextureSrc} />} | ||
| {webcam && ( | ||
| <Suspense fallback={null}> |
There was a problem hiding this comment.
same here: is Suspense really required? I'm not sure
There was a problem hiding this comment.
Your call whether suspense should go here or in user-land around the component.
|
@CodyJasonBennett I think I've now addressed your precious feedbacks. If you have no other ones, I think I would be ready to merge |
|
🎉 This PR is included in version 9.74.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
FYI this commit breaks react native builds. Use <= v9.73.4 for react native. |
|
@markhughes, can you create an issue? I think we'll have to remove it from the native target since iOS doesn't support JIT/WASM. |
Note
Merged, see
<FaceControls>docWhy / What
DEMO: https://abernier.github.io/fld/ as well as the story
facecontrols-videotexture_720p.mov
Control the camera with your head/eyes:
or a custom camera, with your eyes:
This PR basically consists in:
@mediapipe/tasks-visionChecklist