-
Notifications
You must be signed in to change notification settings - Fork 101
Description
The logic for choosing a start time when opening the Multiview Archive Sync feature is somewhat unexpected.
Relevant code is here:
Holodex/src/components/multiview/MultiviewSyncBar.vue
Lines 363 to 389 in 20f86f5
| findStartTime() { | |
| if (this.routeCurrentTs) return +this.routeCurrentTs; | |
| const times = []; | |
| let firstOverlap = this.minTs; | |
| if (!this.$parent?.$refs?.videoCell) return this.minTs; | |
| this.$parent.$refs.videoCell | |
| .forEach((cell, index) => { | |
| const { video, currentTime } = cell; | |
| // Find corresponding overlapping video | |
| const olVideo = video && this.overlapVideos.find((v) => v.id === video.id); | |
| if (!olVideo) return; | |
| const t = currentTime + olVideo.startTs; | |
| // Find times that is within 2 seconds of other videos | |
| if (index === 0 || Math.abs(times[index - 1] - t) < 2000) { | |
| times.push(t); | |
| } | |
| if (olVideo.startTs > firstOverlap && olVideo.endTs > firstOverlap) { | |
| firstOverlap = olVideo.startTs; | |
| } | |
| }); | |
| // Return average if all the times are close | |
| if (times.length === this.overlapVideos.length) { | |
| return times.reduce((a, c) => a + c, 0) / times.length; | |
| } | |
| // Use first overlapping time as start time | |
| return firstOverlap; | |
| }, |
In particular this section:
Holodex/src/components/multiview/MultiviewSyncBar.vue
Lines 384 to 386 in 20f86f5
| if (times.length === this.overlapVideos.length) { | |
| return times.reduce((a, c) => a + c, 0) / times.length; | |
| } |
This causes the average of the streams to be used in almost all cases, when it doesn't seem correct.
For example: https://holodex.net/multiview/AAMYQIQnwYp3tjY%2CMAMYYj6CH1ZdGpU
These two streams start around 27 minutes apart, and yet the average time is chosen and so we start 13 minutes in to the first stream and 13 minutes before the second.
There is a comment above:
| // Return average if all the times are close |
This to me implies that the intention was to have a condition more like:
if (times.length === this.overlapVideos.length && Math.max(times) - Math.min(times) < MAX_VIDEO_OVERLAP_AVERAGE_DIFFERENCE) {Which would only apply the average if the streams start within a small amount of time. However, if MAX_VIDEO_OVERLAP_AVERAGE_DIFFERENCE is set too large, the behaviour would appear odd to the user (like the above example), and if small it would be identical to just jumping to the start of one of the specific streams.
We could:
- Always start at the earliest stream start time
- Always start at the 'first overlap' (the first time two streams overlap, although I think the current way this is calculated has some issues)
- Always start at the latest stream start time