fix: prevent MEDIA_ERR_DECODE in Chromium by splitting DASH AdaptationSets by codec#253
fix: prevent MEDIA_ERR_DECODE in Chromium by splitting DASH AdaptationSets by codec#253populace-gizmo wants to merge 10 commits intoiv-org:masterfrom
Conversation
Sort DASH adaptive formats by preferred codec priority and filter duplicate heights to prevent codec switching issues that cause MEDIA_ERR_DECODE errors. Also remove SupplementalProperty elements from the manifest that trigger color characteristic transfer errors during quality switches. Additionally fix videoPlaybackProxy streaming: - Properly close stream on successful chunk completion - Add onAbort handler for client disconnect logging - Improve error logging for failed chunk fetches Fixes iv-org#172 Co-authored-by: MMaster <5315755+MMaster@users.noreply.github.com>
|
Thanks but the issue is that I really wanted this to be fixed inside youtube.js directly. And not reprocessing again a new dash after the one generated by youtube.js |
Should I continue to make changes to this PR (e.g. remove dependency on JSDOM) or would you like me to pivot to a youtube.js PR instead? |
|
Thank you for this effort! I will just add that based on my testing browser will unfortunately still throw "video frame append error" when it switches to different adaptation set with different codec mid stream automatically. That is why I had to completely filter out all different codecs except for 1 in my temporary fix. Most if not all dash manifests from YT that I've tested already had the sets separated by codec. |
|
@MMaster I have recently filtered only MP4 containers: 7600e9e. With this PR, which seems to split different codecs, this wouldn't work like you expected?
Now that I think about it again, Youtube.js may not accept this patch because this seems inherent to the usage of video.js. Youtube.js is designed to be used in multiple video player libraries. I'm fine with adding this function inside Invidious companion if this can help with the issues reported at #172 |
@unixfox I've seen the change for the mp4 containers but that only applies to audio streams since video streams were already filtered to only mp4. Audio streams were not really an issue in this case but as I mentioned in #172 it was probably good idea to filter out audio streams to mp4 only too. AFAIK The manifests presented by youtube.js already have AdaptationSets separated by codec - iirc I've not seen a single video that would present manifest with AdaptationSet containing mixed codecs so I don't think this changes anything. I think the primary thing in this PR that partially helps is removing the SupplementalProperties from the set which solves the issue as long as the player stays within single one codec set. It would be nice if youtube.js offered an option to not add those. But as I mentioned in my last comment in #172 I've tested removing SupplementalProperties without filtering out sets with different codecs and it doesn't solve the issue. As soon as the player picks different set with different codec mid stream the browser will throw the "video frame append error", the player will become unusable and when user refreshes the page the video will start from the beginning. The only solution that worked for me (that I'm using for last 2 months) and completely solved the error was to only present single video adaptation set with single codec family and no other. This is also what original invidious implementation (without companion) of dash manifest did even tho it did it unintentionally as I mentioned in #172. Original implementation would basically just present single adaptation set with the first stream found in manifest for each height no matter the codec / no matter the fps, but because of how the manifests are presented by YT it would always be the same codec for all streams. |
Two-part fix based on investigation by MMaster in issue iv-org#172: 1. Filter adaptive formats before DASH generation to keep only one codec per height+fps combination (prevents codec switching mid-stream) 2. Remove SupplementalProperty elements with urn:mpeg:mpegB:* scheme (prevents color transfer issues when switching qualities) - Move fix logic to dedicated helper file (src/lib/helpers/fixDashManifest.ts) - Preserve audio formats regardless of container type - Codec priority: av01 > vp09 > hev > avc - Keep both 30fps and 60fps options available Co-authored-by: MMaster <5315755+MMaster@users.noreply.github.com>
populace-gizmo
force-pushed
the
master
branch
from
b04f14a to
8ccdffd
Compare
Both changes are needed.
As I wrote in the original comment - this is my personal preference and you may want to change this. Main reason to do this was to get as close as possible to the original behavior of invidious by keeping only single stream per video height - I wanted all streams to be the same FPS so I had to do some logic of which FPS is preferred. The video player didn't show options to pick quality based on FPS and I wanted to prevent possible issues of mixing streams with different fps mid stream - I don't know if there are any issues with it if both are using the same codec. Since there is no way to pick in client I picked 30 fps server side primarily to use less bandwidth. I know it would be much better if there was an option for client to pick their preferred FPS + server owner forcing their preferred FPS as well as the codec preferences - but that was something that I didn't need / didn't have time to do :) |
…havior Change from height+fps filtering to height-only filtering (unique_res). Now keeps first format per height after sorting by codec/height/fps desc, matching the original Invidious approach. This simplifies the logic while still preventing codec switching issues. - Remove complex fps preference scoring in favor of simple fps desc sort - Filter by height only instead of height+fps combination - Update comments to reflect legacy
…idious behavior
Change sorting from height to width (descending) before filtering by height.
This matches the original Invidious unique_res implementation which sorted
by {width, fps} desc then filtered duplicate heights. Preserves the same
end result while being more faithful to the legacy code.
|
Just to add my 2 cents: But of course I will keep that to the Invidious maintainers to decide - my primary goal was to help with the bug which I hope I did. Thank you again for your work on this! |
|
I think that's a valid concern, would love more input on this from others -- the current code is just motivated by the legacy behavior at https://github.com/iv-org/invidious/blob/f7a31aa3dee1f37cb90a22303b6d45bec0033a3f/src/invidious/routes/api/manifest.cr#L54 |
YouTube.js wouldn't accept the patch because the current DASH manifest generation follows the DASH specification, so works correctly in most players and the changes in this pull request don't. If you need to generate broken manifests (in the same way that the legacy Invidious ones did), that don't follow the specification, please do that on your side. Also a reminder that Invidious is using a very old version of video.js. |
4 participants
Fixes #172
Problem
Chromium browsers (and video.js) throw MEDIA_ERR_DECODE errors when switching between video qualities that use different codecs within the same DASH AdaptationSet. This happens because MSE SourceBuffer cannot handle mid-stream codec changes.
Solution
What's preserved
Disclosure: This code was generated with AI assistance, based on the debugging and diagnosis work by MMaster in #172. I did verify that these changes greatly reduce the occurrence of the MEDIA_ERR_DECODE issue for my instance on Chromium.
Co-authored-by: MMaster 5315755+MMaster@users.noreply.github.com