[PR #12639] [MERGED] Test StreamBuilder more precisely & improve the transcode reason #13299

Closed
opened 2026-02-07 07:10:31 +03:00 by OVERLORD · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/jellyfin/jellyfin/pull/12639
Author: @gnattu
Created: 9/13/2024
Status: Merged
Merged: 9/16/2024
Merged by: @crobibero

Base: masterHead: more-streambuilder-test


📝 Commits (10+)

  • 6395f48 Update unit test for StreamBuilder to reflect current server and clients
  • de8bb15 Return more precise transcoding reasons
  • edc15c8 Add broken fps mkv test
  • 5913db9 Improve readability
  • cefcbcb Add mkv h264 ac3 tests
  • af92b43 Fix safari test
  • b0e6c35 Restore progressive transcoding tests
  • 118c583 Add Dolby Vision testing
  • 6a5f22f Revert "Return more precise transcoding reasons"
  • 77c6fd5 Improve direct profile ranking

📊 Changes

13 files changed (+1606 additions, -757 deletions)

View changed files

📝 MediaBrowser.Model/Dlna/StreamBuilder.cs (+6 -4)
📝 tests/Jellyfin.Model.Tests/Dlna/StreamBuilderTests.cs (+124 -96)
📝 tests/Jellyfin.Model.Tests/Test Data/DeviceProfile-Chrome.json (+151 -238)
📝 tests/Jellyfin.Model.Tests/Test Data/DeviceProfile-Firefox.json (+188 -257)
📝 tests/Jellyfin.Model.Tests/Test Data/DeviceProfile-SafariNext.json (+160 -162)
tests/Jellyfin.Model.Tests/Test Data/DeviceProfile-WebOS-23.json (+355 -0)
tests/Jellyfin.Model.Tests/Test Data/MediaSourceInfo-mkv-dvhe.05-eac3-28000k.json (+95 -0)
tests/Jellyfin.Model.Tests/Test Data/MediaSourceInfo-mkv-dvhe.08-eac3-15200k.json (+97 -0)
tests/Jellyfin.Model.Tests/Test Data/MediaSourceInfo-mkv-h264-ac3-srt-2600k.json (+71 -0)
tests/Jellyfin.Model.Tests/Test Data/MediaSourceInfo-mkv-h264-hi10p-aac-5000k-brokenfps.json (+82 -0)
tests/Jellyfin.Model.Tests/Test Data/MediaSourceInfo-mp4-dvh1.05-eac3-15200k.json (+94 -0)
tests/Jellyfin.Model.Tests/Test Data/MediaSourceInfo-mp4-dvhe.08-eac3-15200k.json (+97 -0)
tests/Jellyfin.Model.Tests/Test Data/MediaSourceInfo-mp4-h264-hi10p-aac-5000k.json (+86 -0)

📄 Description

The unit tests for StreamBuilder is a bit outdated and does not reflect the actual results of all clients. This PR updates both device profiles and the test cases to make the unit tests reflects what the actual browsers may expecting. During the test case updating, I also noticed that our current transcode reason is not accurate enough to write a reasonable test case, and I updated how we derive transcode reasons from the direct play profiles, the details are explained below. Additional test cases using Hi10P video that utilizes the newly added SubContainer field introduced in #12420 for remux and an mkv file with broken fps detection described in #12603 is also added.

For all major browsers (Chrome, Firefox, Safari), I updated their device profile to be the same as what a 10.10 client would look like. For other clients' device profiles, I modified the test cases so that their current profile and the server behavior is respected.

We previously was using the first direct play profile's transcoding reason as the conclusive transcoding reason, but that is somewhat arbitrary, as many clients' first profile may not be the most compatible one. For instance, browsers often set WebM as the first profile, which doesn’t support common codecs like H.264 and AAC by design. This causes VideoCodecNotSupported and AudioCodecNotSupported to be returned, even if the browser supports those codecs. Only use those reasons when all profiles indicate that the codec is not supported.

Changes

  • Update StreamBuilder to return more precise transcode reasons
  • Add more StreamBuilder test cases
  • Update StreamBuilder test cases

Issues


🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/jellyfin/jellyfin/pull/12639 **Author:** [@gnattu](https://github.com/gnattu) **Created:** 9/13/2024 **Status:** ✅ Merged **Merged:** 9/16/2024 **Merged by:** [@crobibero](https://github.com/crobibero) **Base:** `master` ← **Head:** `more-streambuilder-test` --- ### 📝 Commits (10+) - [`6395f48`](https://github.com/jellyfin/jellyfin/commit/6395f4889d18bf4b12567ca7c28e9d5a22506e73) Update unit test for StreamBuilder to reflect current server and clients - [`de8bb15`](https://github.com/jellyfin/jellyfin/commit/de8bb15c7854858e4391982e72fe6eb686a6fd87) Return more precise transcoding reasons - [`edc15c8`](https://github.com/jellyfin/jellyfin/commit/edc15c8e923610c7074328f1950d18f01926d552) Add broken fps mkv test - [`5913db9`](https://github.com/jellyfin/jellyfin/commit/5913db991bca9628d7147df688874adae428ad74) Improve readability - [`cefcbcb`](https://github.com/jellyfin/jellyfin/commit/cefcbcb2ac2f631e841c26b912113623d344a422) Add mkv h264 ac3 tests - [`af92b43`](https://github.com/jellyfin/jellyfin/commit/af92b4370f82be19622d41c4f464805b33b480c5) Fix safari test - [`b0e6c35`](https://github.com/jellyfin/jellyfin/commit/b0e6c357f706e1181be74321ea1af075bb375be7) Restore progressive transcoding tests - [`118c583`](https://github.com/jellyfin/jellyfin/commit/118c583bff65453fe3999cbe2e6bddf3483ed703) Add Dolby Vision testing - [`6a5f22f`](https://github.com/jellyfin/jellyfin/commit/6a5f22fc2f965d8dcfbcf8ab361365f8d7be253d) Revert "Return more precise transcoding reasons" - [`77c6fd5`](https://github.com/jellyfin/jellyfin/commit/77c6fd5ab2a76a6c442debe20735cff610d460a5) Improve direct profile ranking ### 📊 Changes **13 files changed** (+1606 additions, -757 deletions) <details> <summary>View changed files</summary> 📝 `MediaBrowser.Model/Dlna/StreamBuilder.cs` (+6 -4) 📝 `tests/Jellyfin.Model.Tests/Dlna/StreamBuilderTests.cs` (+124 -96) 📝 `tests/Jellyfin.Model.Tests/Test Data/DeviceProfile-Chrome.json` (+151 -238) 📝 `tests/Jellyfin.Model.Tests/Test Data/DeviceProfile-Firefox.json` (+188 -257) 📝 `tests/Jellyfin.Model.Tests/Test Data/DeviceProfile-SafariNext.json` (+160 -162) ➕ `tests/Jellyfin.Model.Tests/Test Data/DeviceProfile-WebOS-23.json` (+355 -0) ➕ `tests/Jellyfin.Model.Tests/Test Data/MediaSourceInfo-mkv-dvhe.05-eac3-28000k.json` (+95 -0) ➕ `tests/Jellyfin.Model.Tests/Test Data/MediaSourceInfo-mkv-dvhe.08-eac3-15200k.json` (+97 -0) ➕ `tests/Jellyfin.Model.Tests/Test Data/MediaSourceInfo-mkv-h264-ac3-srt-2600k.json` (+71 -0) ➕ `tests/Jellyfin.Model.Tests/Test Data/MediaSourceInfo-mkv-h264-hi10p-aac-5000k-brokenfps.json` (+82 -0) ➕ `tests/Jellyfin.Model.Tests/Test Data/MediaSourceInfo-mp4-dvh1.05-eac3-15200k.json` (+94 -0) ➕ `tests/Jellyfin.Model.Tests/Test Data/MediaSourceInfo-mp4-dvhe.08-eac3-15200k.json` (+97 -0) ➕ `tests/Jellyfin.Model.Tests/Test Data/MediaSourceInfo-mp4-h264-hi10p-aac-5000k.json` (+86 -0) </details> ### 📄 Description <!-- Ensure your title is short, descriptive, and in the imperative mood (Fix X, Change Y, instead of Fixed X, Changed Y). For a good inspiration of what to write in commit messages and PRs please review https://chris.beams.io/posts/git-commit/ and our documentation. --> The unit tests for `StreamBuilder` is a bit outdated and does not reflect the actual results of all clients. This PR updates both device profiles and the test cases to make the unit tests reflects what the actual browsers may expecting. During the test case updating, I also noticed that our current transcode reason is not accurate enough to write a reasonable test case, and I updated how we derive transcode reasons from the direct play profiles, the details are explained below. Additional test cases using Hi10P video that utilizes the newly added `SubContainer` field introduced in #12420 for remux and an mkv file with broken fps detection described in #12603 is also added. For all major browsers (Chrome, Firefox, Safari), I updated their device profile to be the same as what a 10.10 client would look like. For other clients' device profiles, I modified the test cases so that their current profile and the server behavior is respected. We previously was using the first direct play profile's transcoding reason as the conclusive transcoding reason, but that is somewhat arbitrary, as many clients' first profile may not be the most compatible one. For instance, browsers often set WebM as the first profile, which doesn’t support common codecs like H.264 and AAC by design. This causes `VideoCodecNotSupported` and `AudioCodecNotSupported` to be returned, even if the browser supports those codecs. Only use those reasons when all profiles indicate that the codec is not supported. **Changes** <!-- Describe your changes here in 1-5 sentences. --> - Update StreamBuilder to return more precise transcode reasons - Add more StreamBuilder test cases - Update StreamBuilder test cases **Issues** <!-- Tag any issues that this PR solves here. ex. Fixes # --> --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
OVERLORD added the pull-request label 2026-02-07 07:10:31 +03:00
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/jellyfin#13299