[Player-59] camera widget #159

jparez · 2025-12-23T16:01:43Z

Description

This branch introduces camera injection management and improves the local development workflow.

Front and back camera are implemented but back camera is hidden, waiting for system handle more than one stream

Camera Functionality (feat(Camera))

Camera Plugin: Added Camera.js to handle video stream injection.
Media Manager: Updated MediaManager.js to support new sources (Dummy, local video) and front / back camera
User Interface:
- Added the GmDropdown component for source selection.
- Integrated a fileUploader for manage streaming custom video files.
- Added / updated associated SCSS styles (_camera.scss, _fileUploader.scss).
Testing: Added unit tests (camera.test.js).

Configuration & Dev (chore(vite))

HMR Improvements: Adjusted vite.config.js with an html-transform plugin to optimize Hot Module Replacement. This prevents conflicts during script/style injection, ensuring better stability while developing the camera features.

caarmen · 2026-01-12T15:57:02Z

vite.config.js

+                transformIndexHtml(html) {
+                    if (mode === 'development') {
+                        let newHtml = html.replace(
+                            /<script src="\.\.\/dist\/js\/device-renderer\.min\.js" data-player-url><\/script>/,


Would it be possible to move this code into another file, to keep the vite.config.js as lean and as declarative as possible?

caarmen · 2026-01-12T15:59:43Z

src/plugins/util/fileUploader.js

+                } else {
+                    const acceptedTypes = accept.split(',').map((t) => t.trim().toLowerCase());
+                    isValid = acceptedTypes.some((type) => {
+                        if (type.endsWith('/*')) {


Could we have one example of a type value, in a comment, in each of these if branches?

caarmen · 2026-01-12T16:00:46Z

src/plugins/util/fileUploader.js

        accept = null,
        classes = '',
        i18n = {},
+        mode = 'upload',


I don't see mode in the jsdoc. Did I miss something?

I see that in Camera.js, we specify mode: "select". Would be nice to document the two possible modes in the jsdoc.

caarmen · 2026-01-12T16:02:29Z

vite.config.js

    return {
-        // plugins: [viteSingleFile()],
+        plugins: [
+            basicSsl(),


Is this basicSsl related to the hot reload?

I see that an ssl dependency is added, but not in this commit which introduces the plugin.

No dependency to the hot reload, but browser grant access to camera only on https scheme

caarmen · 2026-01-12T16:11:57Z

src/plugins/Camera.js

+            if (this.instance.options.microphone && !isImage) {
+                try {
+                    if (!this.audioContext) {
+                        this.audioContext = new (window.AudioContext || window.webkitAudioContext)();


Is this AudioContext ever closed?

i had push a small commit with a small refacto of destroy flow.

Here is a summary of the changes:

Lifecycle Management: DeviceRenderer.destroy() now dynamically iterates over all registered widgets and calls their destroy() method.

AudioContext: The Camera plugin now implements a destroy() method that explicitly closes the AudioContext and removes event listeners.

Store Cleanup: Added a destroy() method to the Store to centrally clear all subscriptions, preventing memory leaks from lingering listeners.

General Cleanup: Also addressed minor leaks like unrevoked Blob URLs and dead code in the Factory.

So to answer the question: Yes, the AudioContext is now properly closed when the player is destroyed :)

caarmen · 2026-01-12T16:12:55Z

src/plugins/Camera.js

+                    }
+
+                    this.audioNodes = this.audioNodes || [];
+                    this.audioNodes.push({source, dest, type});


Copilot mentions that this audioNodes array might have a potential memory leak. Not completely sure to understand: maybe because we put source nodes and destination nodes in the array, and when we're done with the camera, we don't clean the array? We'll have dangling references to these source/destination nodes?

see comment above

caarmen

I know it's in draft, but I had a couple of hours to look at this.

If any of my comments are irrelevant because you've been working on changes, just ignore them :)

I basically just looked at two files today:

GmDropdown.js
Camera.js

I'll take a look at other files when I get a chance.

caarmen · 2026-01-13T15:45:04Z

src/components/GmDropdown.js

            return item;
        }
-        return item.valueToDisplay ?? item.element?.textContent ?? item.value;
+        return item.label ?? item.valueToDisplay ?? item.element?.textContent ?? item.value;


Why do we need to support a new attribute label?

Why can't Camera.js supply valueToDisplay? It seems to have the same results.

Could this component have some documentation in set items(newItems) explaining what are the accepted types of objects that can be passed for newItems?

caarmen · 2026-01-13T15:45:44Z

src/plugins/Camera.js

+            if (d.label) {
+                videoOptions.push({
+                    value: d.deviceId,
+                    label: d.label,


Why not supply valueToDisplay: d.label instead? (Same a few lines above).

caarmen · 2026-01-13T16:00:17Z

src/plugins/Camera.js

+        }
+    }
+
+    onFileSelected(file, type) {


Not sure where to put this:
If I select the same file multiple times, it only seems to work the first time.

media-injection-bug.mp4

works on firefox, fixed on chrome

src/plugins/Camera.js

caarmen · 2026-01-13T16:25:53Z

src/plugins/Camera.js

+        } catch (error) {
+            this.permissionRequestView.classList.add('hidden');
+
+            if (error.name === 'NotFoundError' || error.message.includes('The object can not be found')) {


Will these value NotFoundError and The object can not be found be consistent across browsers?

NotFoundError is standard, we will keep only this check this modern browser send this code

caarmen · 2026-01-13T17:07:31Z

src/plugins/Camera.js

+            this.audioNodes = this.audioNodes.filter((node) => {
+                if (node.type === type) {
+                    try {
+                        node.source.disconnect();


Should we be having side effects inside a filter? I think this isn't idiomatic for functional programming (methods like map, filter, etc typically should just return data, not have side effects).

Doing a filter first to get the relevant audioNodes, and then doing a for/forEach to disconnect() them would indeed end up processing the list twice.

But we're talking about just a few elements at most, right? Not sure this is a critical performance path. Clarity might be a better focus here in the clarity/perf tradeoff, no?

caarmen · 2026-01-13T17:11:41Z

src/plugins/Camera.js

+            if (typeof mediaElement.pause === 'function') {
+                mediaElement.pause();
+            }
+            if (typeof mediaElement.load === 'function') {


Why not:

mediaElement.src = ''; if (typeof mediaElement.load === 'function') { mediaElement.load(); }

caarmen · 2026-01-13T17:13:28Z

src/plugins/Camera.js

+        }
+
+        const mediaElement = type === 'front' ? this.frontMediaElement : this.backMediaElement;
+        if (mediaElement) {


Could we have a comment at the top of this block?

Basically, in functions which are kind of large (this one is around 70 lines), with several sections, it might help to browse the code if each section has a quick comment.

Another option would be to split the function into multiple smaller functions, each one doing only one very small thing. Then, comments might not be needed. Another tradeoff :)

caarmen · 2026-01-13T17:19:54Z

src/plugins/Camera.js

+            this.backFileInfo.classList.add('hidden');
+            this.backUploader.classList.remove('hidden');
+
+            this.instance.mediaManager.stopVideoStreaming('back');


Quite a bit of copy/paste for the actions on the front/back streams. Not sure it would be simple to reduce this though. It might require more refactoring. Like extracting this into a function that takes a stream, video, fileinfo, and uploader arguments.

Then we could do something like:

if (type == "front") { myFunctionWithAGoodName( this.frontStream, this.frontVideo, this.frontFileInfo, this.frontUploader, "front" }; } else { myFunctionWithAGoodName( this.backStream, this.backVideo, this.backFileInfo, this.backUploader, "back" }; } myFunctionWithAGoodName( stream, video, fileInfo, uploader, someBetterName ){ if (stream) { stream.getTracks().forEach((t) => { t.stop(); }); stream = null; } video.classList.add('hidden'); video.srcObject = null; fileInfo.classList.add('hidden'); uploader.classList.remove('hidden'); this.instance.mediaManager.stopVideoStreaming(someBetterName); }

A way to avoid passing in so many arguments would be to define an object containing properties for all these objects.

caarmen · 2026-01-13T17:21:28Z

src/plugins/Camera.js

+                this.toolbarBtn.setIndicator('');
+            }
+            return;
+        }


Looks like the exact same logic for deviceId === 'none' and deviceId === 'file'. Did I miss something?

the first branch has placeholder, the second one has uploader

caarmen · 2026-01-15T16:49:33Z

src/plugins/MediaManager.js

            }
-            // if the if the flag for bundled audio&video stream is set, we'll remove the audio track too
+            /*
+             * if the if the flag for bundled audio&video stream is set, we'll remove the audio track too


if the if the flag => If the flag

caarmen · 2026-01-15T16:55:52Z

tests/unit/camera.test.js

+        test('widget is rendered with correct elements', () => {
+            // Check for dropdowns
+            expect(document.getElementsByClassName('gm-camera-dropdown-front')).toHaveLength(1);
+            /*


Could you check this comment?

caarmen · 2026-01-15T16:57:54Z

tests/unit/gappsInstall.test.js

            DRAG_DROP_TEXT: 'TEST DRAG DROP TEXT',
            BROWSE_BUTTON_TEXT: 'TEST BROWSE',
-            FILE_TYPE_NOT_APK: 'TEST FILE TYPE NOT APK',
+            FILE_TYPE_INVALID: 'TEST FILE TYPE NOT APK',


Should the value be changed too?

Since it's for test i18 it could, but in fact this is the right message

caarmen · 2026-01-15T17:00:51Z

package.json

    "types": "dist/index.d.ts",
    "engines": {
-        "node": ">=16"
+        "node": ">=20"


This could be a breaking change for some applications using the player.

I had changed the logic to avoid force the engine for the users of the player and check node version for the developper

…leaks - Lifecycle Management: DeviceRenderer.destroy() now dynamically iterates over all registered widgets and calls their destroy() method. - AudioContext: The Camera plugin now implements a destroy() method that explicitly closes the AudioContext and removes event listeners. - Store Cleanup: Added a destroy() method to the Store to centrally clear all subscriptions, preventing memory leaks from lingering listeners. - General Cleanup: Also addressed minor leaks like unrevoked Blob URLs and dead code in the Factory.

…der to increase connexion stability

… logic refacto

jparez force-pushed the dev/player-50-camera-widget branch 2 times, most recently from 18a0722 to 908fec7 Compare December 23, 2025 16:11

jparez force-pushed the dev/player-50-camera-widget branch 2 times, most recently from 908fec7 to 68558f8 Compare January 5, 2026 10:21

jparez force-pushed the dev/PLAYER-92-refacto-components branch from 59e77c6 to f8e069f Compare January 5, 2026 12:30

Base automatically changed from dev/PLAYER-92-refacto-components to epic/player-90-91-92-50 January 5, 2026 12:31

jparez force-pushed the dev/player-50-camera-widget branch 9 times, most recently from 59db0d4 to 2bd6787 Compare January 7, 2026 14:13

jparez changed the base branch from epic/player-90-91-92-50 to dev/PLAYER-92-refacto-components-2 January 7, 2026 14:36

jparez force-pushed the dev/player-50-camera-widget branch 4 times, most recently from a76ab4c to c2ebcf0 Compare January 9, 2026 12:23

jparez requested a review from caarmen January 9, 2026 12:25

caarmen reviewed Jan 12, 2026

View reviewed changes

jparez marked this pull request as draft January 12, 2026 17:23

jparez changed the title ~~[Player-50] camera widget~~ [Player-59] camera widget Jan 13, 2026

jparez force-pushed the dev/PLAYER-92-refacto-components-2 branch 3 times, most recently from 7386fee to c40ae3f Compare January 13, 2026 08:30

jparez force-pushed the dev/player-50-camera-widget branch from c2ebcf0 to 4eae368 Compare January 13, 2026 08:42

jparez force-pushed the dev/PLAYER-92-refacto-components-2 branch 2 times, most recently from b4ea2dc to 353b8f9 Compare January 13, 2026 10:44

jparez force-pushed the dev/PLAYER-92-refacto-components-2 branch from 353b8f9 to 68c7a82 Compare January 13, 2026 11:09

caarmen reviewed Jan 13, 2026

View reviewed changes

caarmen mentioned this pull request Jan 15, 2026

[PLAYER-92] refacto components #163

Merged

caarmen reviewed Jan 15, 2026

View reviewed changes

jparez force-pushed the dev/PLAYER-92-refacto-components-2 branch 2 times, most recently from 9ee7468 to b3fea28 Compare January 19, 2026 12:40

jparez force-pushed the dev/player-50-camera-widget branch 4 times, most recently from 9208f3f to fdfa7a9 Compare January 21, 2026 07:30

jparez force-pushed the dev/PLAYER-92-refacto-components-2 branch from b3fea28 to b7d73c5 Compare January 21, 2026 16:44

jparez force-pushed the dev/player-50-camera-widget branch from fdfa7a9 to 48e2ec2 Compare January 21, 2026 16:46

jparez force-pushed the dev/PLAYER-92-refacto-components-2 branch from b7d73c5 to 7371ded Compare January 22, 2026 09:32

Base automatically changed from dev/PLAYER-92-refacto-components-2 to epic/player-90-91-92-50 January 22, 2026 09:39

fatoldsun00 and others added 8 commits January 22, 2026 10:46

feat(Camera): adding plugin to manage camera injection

a520d64

chore(vite): better handle of hot reload with auto reconnect

b928c1f

chore: delete generated package-lock

f8973f1

build: relax node engine requirement for consumers, enforce for dev

7041fe5

docs: adding 'mode' param to jsdoc. Adding sone comments

18b04fb

refacto(camera): loop which draw ctx for file and image stream

c3de986

refacto(camera): refacto size of video upload as custom stream, in or…

c3304a5

…der to increase connexion stability

jparez force-pushed the dev/player-50-camera-widget branch from 48e2ec2 to c3304a5 Compare January 22, 2026 09:46

refacto(camera): fix microphone support, fix / adding coments, slight…

6982567

… logic refacto

jparez force-pushed the dev/player-50-camera-widget branch from d88f07b to 6982567 Compare January 27, 2026 16:03

[Player-59] camera widget #159

Are you sure you want to change the base?

[Player-59] camera widget #159

Uh oh!

Conversation

jparez commented Dec 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Camera Functionality (feat(Camera))

Configuration & Dev (chore(vite))

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

caarmen Jan 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

caarmen left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

jparez commented Dec 23, 2025 •

edited

Loading

caarmen Jan 12, 2026 •

edited

Loading