Repository navigation
Captions can render two or three times over: a race in the lazy text-track load #1557
Replies: 4 comments
|
@sounderdata there's a fix on #1560 , I'd appreciate if you can check it out and confirm it fixes this problem! |
|
I've tested this and have merged the PR, version 8.4.0 should be published in the next few minutes with a fix |
|
Markos,
That was fast — thank you. I saw #1560 merged and 8.4.0 out this morning.
On your request to confirm the fix: I want to be honest with you about what I
can and can't verify. We're running a fork that's still on 7.2.2, so I
can't drop
8.4.0 onto a live install and tell you it's fixed in production. What I did
do is I read it and also had my AI (Claude Code) read the change closely
against the behaviour we originally saw, and it addresses the actual cause.
It resolving the target track before touching any modes means nothing gets
taken back through disabled while its .vtt is still in flight, which was
the whole problem. guardAgainstDuplicateCues is a sensible belt-and-braces
on top. I'd expect it to hold.
One thing I did find while reading it is *The fix keys on kind ===
'subtitles' throughout, including the dedupe **watcher.* On a stock install
that's fine, because subtitles_info doesn't emit a kind and VideoJSPlayer
hard-codes subtitles when it adds tracks, so every track is a subtitles
track and the guard catches everything.
However, a same-language transcript of a video's own audio is properly
kind="captions", not subtitles. That difference is important because assistive
technology and browser-level caption preferences act on it, and WCAG 1.2.2
(Captions) and 1.2.3 (Audio Description or Media Alternative) are separate
criteria. A deaf viewer asking for captions is asking for the sound to be
described; a subtitles track doesn't answer that request.
The moment anything declares a track as captions (and your own migration
0014_alter_subtitle_options_and_more renames the Subtitle model's display name
to "Caption", so I realize you may be heading that way)
guardAgainstDuplicateCues silently stops watching it, and
applySubtitlePreference stops finding it, so the remembered-language
preference quietly does nothing. Both failures are silent, which is not good
.
It's already inconsistent in one spot: VideoJSPlayer.jsx has a track.kind
=== 'subtitles' || track.kind === 'captions' test around line 1715, while
everything in #1560 checks only subtitles.
On our fork we ended up adding a Subtitle.kind field and routing every one of
those ~25 checks through a single isCaptionTrack() helper that accepts both.
Happy to open a PR for just the predicate change if that's useful — it's small
and it doesn't commit you to the model field. Let me know!
To your other questions, since I never properly answered them:
*Q: In-house or outsourced?*
A: I own a small digital agency (pilotdigital.com) and we've built a embedding
video product on MediaCMS, which we call Sounder Video. HIPAA is a US
privacy law that healthcare organizations have to follow closely. One
violation of HIPAA is embedding videos from YouTube because they
automatically send private health info to Google. Sounder Video is a
HIPAA-safe video host/embedding tool for those healthcare organisations. Pilot
has a team of three programmers (Seth, Nick & Tyler). I used to be a
programmer (studied in the 80s and did a lot of programming in the 90s),
but I've fallen very far behind. For the past 18 years, much of my job is
managing our various companies. So, for programming, I rely fairly heavily
on AI to produce code and I have my "real" programmers review what I
produce. I mention all of that because I read #1519 and I agree with it;
Pilot will never send you anything our human programmers and/or I haven't
checked.
*Q: Is updating easy?*
A: Unfortunately the answer is no, *but that's our fault!* We drifted to
7.2.2 while you got to 8.4.0. I spent this week measuring the gap and it's
far less frightening than the big gap in version numbers makes it feel: the
dependency stack is already identical apart from gunicorn and PyLTI1p3, and
the real conflict is fourteen hunks across seven files. A trial merge
builds and
has passed our test suite.
I have three questions about that, if you have a minute. You're the one
person who actually knows the answers:
1.
Is a single merge to the latest release the intended path from 7.2,
or is there
an order you'd recommend?
2.
Was the uwsgi → gunicorn switch driven by something specific we should
know about, and are the --workers=2 --threads=2 defaults tuned or
placeholders?
3.
Is USE_X_ACCEL_REDIRECT meant to be permanent, or a migration aid that will
go away? We'd like to upgrade with it off and switch it on as a separate
step, and I'd rather not build on something you plan to remove.
*Q: Contributing?*
A: Yes, we would like that very much, however only where it's useful to
you. Two issues coming shortly: the caption font-size control being
overridden by !important rules in VideoJS.css, and Whisper captions getting
whisper as a language code, which isn't valid BCP-47 so assistive tech
can't identify the caption language.
There's also a bigger thing I can offer: we have built an accessibility
test harness for the player. It uses real keypresses through Playwright against
the built bundle, with each assertion tied to a WCAG criterion. It's
how we found
the caption duplication. I'll write it up as an issue so you can say no e
asily!
Thanks again for turning that around so quickly.
Rod
Rod Holmes
Founding Partner/Director of Sales
Direct: 773.840.0578
*Want to Talk?* My Calendar <http://rodholmes.youcanbook.me>
*Need Help?* Submit a Support Ticket <https://pilotdigital.com/support/>
*We support nonprofits with pro bono work! Apply here
<https://docs.google.com/forms/d/e/1FAIpQLSckBAwFwIvhATpoEe_dE6XuJ7_XG8SjO4stQvXox0H4or-Csg/alreadyresponded>!*
…On Tue, Aug 25, 2026 at 2:07 AM Markos Gogoulos ***@***.***> wrote:
I've tested this and have merged the PR, version 8.4.0 should be published
in the next few minutes with a fix
—
Reply to this email directly, view it on GitHub
<#1557?email_source=notifications&email_token=B5M7OFDFSUQRDUAROACNYXD5LU3K3A5CNFSNUABIM5UWIORPF5TWS5BNNB2WEL2ENFZWG5LTONUW63SDN5WW2ZLOOQXTCOBRGQ2DINJWUZZGKYLTN5XKO3LFNZ2GS33OUVSXMZLOOSWGM33PORSXEX3DNRUWG2Y#discussioncomment-18144456>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/B5M7OFDLX3NWOZJMWQRFIHD5LU3K3AVCNFSNUABIKJSXA33TNF2G64TZHMZTEMJXHA2TCMRXHNCGS43DOVZXG2LPNY5TCMBWGYZDONJVUF3AE>
.
Triage notifications, keep track of coding agent tasks and review pull
requests on the go with GitHub Mobile for iOS
<https://github.com/notifications/mobile/ios/B5M7OFEPHT5ZJU27FCB6LST5LU3K3A5CNFSNUABIM5UWIORPF5TWS5BNNB2WEL2ENFZWG5LTONUW63SDN5WW2ZLOOQXTCOBRGQ2DINJWUZZGKYLTN5XKO3LFNZ2GS33OUVSXMZLOOSVGM33PORSXEX3JN5ZQ>
and Android
<https://github.com/notifications/mobile/android/B5M7OFHCBNRFGQ6ZGRGN7DT5LU3K3A5CNFSNUABIM5UWIORPF5TWS5BNNB2WEL2ENFZWG5LTONUW63SDN5WW2ZLOOQXTCOBRGQ2DINJWUZZGKYLTN5XKO3LFNZ2GS33OUVSXMZLOOSXGM33PORSXEX3BNZSHE33JMQ>.
Download it today!
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
Hi @sounderdata , lots of interesting topics on the same thread :)
Any big difference is usually noted on the releases page. One example of backward incompatible change that was addressed in the releases page has been the upgrade of Postgresql from 15 to 17, that took place in v4.3.0.
Good to see your use case and the value a system as MediaCMS can offer. We're open to contributions of course, specially if PRs are specific to one thing (vs one PR addressing different things) and there are instructions on what/how to test |
Uh oh!
There was an error while loading. Please reload this page.
Hello — and thanks for MediaCMS.
We (PilotDigital.com) run a fork in production for US healthcare clients who use it to embed videos into their websites. They can't embed YouTube videos because it violates privacy laws by sending personal data to YT/Google. In the US, healthcare clients in particular are getting a lot of pressure about accessibility and have a May 2027 deadline to be highly compliant.
Accessibility compliance meant we had to dig into 1) the captioning system and 2) keyboard navigation of embedded videos. A caption bug we found sent us deep into the video.js 8 player. I think what we found affects all installs, so it seemed like it was worth writing up for you. We hope you find it useful.
Symptom
Every caption line renders two or three times, stacked on top of each other. On one of our installs I measured 111 cues on a track whose
.vttfile contains 37.The issue is intermittent in a way that sent us looking in the wrong place. A cached
.vttlooks fine, but a freshly uploaded or freshly edited one doesn't. So it looked like the caption editor was corrupting files, which is what we spent time investigating.Cause
Video.js loads a text track's
.vttlazily. The fetch is triggered by the transition out ofmode = 'disabled', and it's only controlled bycues.length === 0(video.cjs.js:7710in 8.23.3). There's no in-flight check, so a second mode change while the first fetch is still outstanding starts a second fetch, and both append their cues to the same track.UserPreferences.applySubtitlePreferencedisables every subtitles track and then re-enables the chosen one, i.e.showing => disabled => showing. It's called three times during start-up, immediately, onloadeddata, and oncanplay. Three overlapping fetches, all appending to one track, whenever the file is slow enough to still be in flight. That explains the intermittency, and the 3×37.Fix
Compute the target track first, then disable only the tracks that aren't it, and never cycle the one you're about to show:
We also added a small belt-and-braces guard that de-duplicates a track's cues on its
loadeddataevent, because any future code path that toggles mode quickly can reintroduce the race, and the failure is silent until a viewer sees it.Happy to open a PR for the root-cause change on its own if that's useful. It's small and self-contained.
Reproducing it
Easiest with the network throttled, since it needs the fetch to still be in flight:
applySubtitlePreferencehas something to restore.player.textTracks()[n].cues.lengthagainst the line count in the.vtt.One last thing, just so you know it exists: the same review turned up three more findings. 1) The caption Font Size control is being overridden by an
!importantrule, 2) an invalid BCP-47 language code on Whisper-generated captions, and 3) an accessible-name bug in video.js's own menu items that MediaCMS triggers viaexperimentalSvgIcons.We also had Claude make a keyboard-driven accessibility test harness that runs against the built bundle, including the embed build.
I don't want to dump all of that on you out of the blue. If you'd like, I'll write them up separately, or open PRs, whichever you perfer.
For transparency: this investigation was AI-assisted (Claude) but all the findings above were verified by hand on a running install.
— Rod Holmes, Pilot Digital
All reactions