UniversalViewer
Present captions on audio media
Description of what you did:
Caption support was already there, but was only wired up for video. This adds the existing text track support for audio media as well.
Addresses part of #747
If approved, this is ready to be merged
![codesandbox[bot] avatar](https://avatars.githubusercontent.com/in/206332?v=4 "Published by a pub.dev verified publisher"){kind=small}
The latest updates on your projects. Learn more about Vercel for Git ↗︎
| Name | Status | Preview | Comments | Updated (UTC) |
|---|---|---|---|---|
| universalviewer | ✅ Ready (Inspect) | Visit Preview | 💬 Add feedback | Jun 5, 2024 1:27am |
![vercel[bot] avatar](https://avatars.githubusercontent.com/in/8329?v=4 "Published by a pub.dev verified publisher"){kind=small}
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.
![codesandbox-ci[bot] avatar](https://avatars.githubusercontent.com/in/38409?v=4 "Published by a pub.dev verified publisher"){kind=small}
@DanOlson, do you have any example manifests we can use to test/demonstrate this new functionality?
@demiankatz yes, here's one for an audio media item:
https://gist.githubusercontent.com/DanOlson/945c901a49ade5524bd1fb7fa920ceef/raw/e770069a5a77db5b20b0e7cc3326b1722e3b6f1f/interview-with-h-wyman-malmsten.json
and here's one for a video media item, for regression testing
https://gist.githubusercontent.com/DanOlson/e22318c3e6ea843784d5bc23d2f9b1ae/raw/c27df9dc0839ce0b071f80715928e1ed19da9064/assignment-for-tomorrow.json
The changes here assume the same structure for both audio and video manifests, where the canvas item's rendering property contains a Text object of format "text/vtt". No changes were made with regard to the expected structure of the manifest within MediaElementCenterPanel; I was just expanding on what existed previously for videos.
Please let me know if you need anything else. Thanks!
@DanOlson @demiankatz Test the caption functionality in both video and audio media using the provided manifests in PR #1047 across the universalviewer.dev and the Vercel preview environments.
Tested Environments:
- universalviewer.dev
- Vercel Preview (linked in the PR)
Results:
Video Manifest Testing:
Outcome: The video manifest was tested successfully in both environments.
Observation: Captions were present as expected. The CC (Caption/Subtitles) icon functioned correctly, allowing the user to toggle subtitles on and off.
Evidence: [Screenshot showing captions in the video media].
Audio Manifest Testing:
Outcome: The audio manifest was also tested in both environments.
Observation: The CC icon was not present while playing the audio media, meaning there was no option to toggle captions.
Evidence: [Screenshot while playing the audio media].
Conclusion: Caption functionality appears to be correctly implemented for video media, but the CC icon is absent in audio media when using the provided manifest. @DanOlson If you have already tested the audio manifest on your end and the captions appear as expected, please let me know. This could help determine whether the issue is environment-specific or related to the manifest provided
@LanieOkorodudu, did you test using the Vercel preview in this PR? When I load the manifest into that version, I do see captions on the audio-only example.
@LanieOkorodudu, did you test using the Vercel preview in this PR? When I load the manifest into that version, I do see captions on the audio-only example.
Yes I did. I will have to try it again. I tested in chrome and Firefox. could it be a caching problem.
@demiankatz I tried it again, and yes, it does have captions in the Vercel PR but not in universalviewer.dev. I assume this is because the PR has not yet been merged into dev? Once merged I will check and test it again.
@LanieOkorodudu, that is correct: this PR is designed to correct the missing captions in dev, so your finding that captions work here and do not work in dev is what we would expect. I think this can likely be approved and merged. However, I see that the PR currently targets the main branch; I will adjust it to point to dev and make sure that doesn't break anything.