Skip to content

Panorama video viewer (copied from viewer)#57

Open
kyle-fitzp wants to merge 3 commits into
masterfrom
360-video
Open

Panorama video viewer (copied from viewer)#57
kyle-fitzp wants to merge 3 commits into
masterfrom
360-video

Conversation

@kyle-fitzp

Copy link
Copy Markdown

Same changes from OSH-Viewer: Botts-Innovative-Research/osh-viewer#284

@salsajeries salsajeries left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding @Cardy2 as reviewer

@salsajeries
salsajeries requested a review from Cardy2 July 1, 2026 17:17
Comment thread source/core/ui/view/video/VideoView.js Outdated
_initCameraControls() {
const canvas = this.canvasElement;
const DRAG_SENS = 0.003;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this method wires itself into the permanent root window, we need to clean this up properly. Nothing calls back into the YUVCanvas/YUV360Canvas to detach its listeners

Store the bound handler refs to an array so they can be easily destroyed later:

	this._domListeners = [];
	const add = (target, type, handler, options) => {
		target.addEventListener(type, handler, options);
		this._domListeners.push({ target, type, handler, options });
	};

then below we can use add() instead of addEventListener() for each instance:
e.g.

		add(canvas, 'mousedown', (e) => onDragStart(e.clientX, e.clientY));
		add(window, 'mousemove', (e) => onDragMove(e.clientX, e.clientY));
		add(window, 'mouseup',   onDragEnd);

canvas.addEventListener('touchend', () => { lastPinchDist = null; });
canvas.addEventListener('touchcancel', () => { lastPinchDist = null; });
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a dispose() method to be called in FFMPEG360View:

	dispose() {
		if (this._domListeners) {
			for (const { target, type, handler, options } of this._domListeners) {
				target.removeEventListener(type, handler, options);
			}
			this._domListeners = [];
		}

		const gl = this.contextGL;
		if (gl) {
			gl.getExtension('WEBGL_lose_context')?.loseContext();
			this.contextGL = null;
		}
	}

if (this.yuvCanvas) this.canvas360.setProjection(projection);
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add destroy() override so the existing VideoView.destroy() -> FFMPEGView.destroy() teardown chain reaches the 360canvas:

	destroy() {
		this.canvas360?.dispose();
		super.destroy();
	}


createVideoView(compression) {
if(compression === 'jpeg') {
if (this.props360 != null) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since we dont have a view that handles 360 mjpeg maybe the handling here should read:

-->      if (this.props360 != null && compression !== 'jpeg') {
            this.videoView = new FFMPEG360View({
                ...this.properties,
                codec: compression,
                layers: []
            });
        } else if(compression === 'jpeg') {
-->           if (this.props360 != null) {
                console.warn('VideoView: 360 display is not supported for MJPEG streams; falling back to standard MjpegView.');
            }
            // create MJPEG View
            this.videoView = new MjpegView({
                ...this.properties,
                layers: []
            });

@Cardy2 Cardy2 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Mostly small fixes and need to add some cleanup detaching the event listeners in 360Canvas

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants