Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix StreamBuilder #7537

Merged
merged 3 commits into from Apr 15, 2022
Merged

Conversation

dmitrylyzo
Copy link
Contributor

@dmitrylyzo dmitrylyzo commented Apr 1, 2022

IMO, ApplyConditions and Conditions were mixed up in #7325

Changes

  • Use ApplyConditions to determine the applicability of the current codec (Conditions).
  • Add AndroidTV tests
  • Add Tizen 3/4 tests
  • Use VideoAudio for the audio tracks in the video.

Issues
Fixes jellyfin/jellyfin-androidtv#1576

⚠️ Since codec conditions are tested even if the codec is unsupported, there is an extra VideoLevelNotSupported (for HEVC). Ideally, we shouldn't test codec conditions if the codec isn't supported.
Don't add codec failure reasons if the codec isn't supported.

⚠️ AndroidTV profile may not be perfect.

@crobibero crobibero added the stable backport Backport into the next stable release label Apr 1, 2022
@dmitrylyzo dmitrylyzo force-pushed the fix-streambuilder branch 2 times, most recently from 516cb1c to 2b97fdf Compare April 3, 2022 20:38
@dmitrylyzo dmitrylyzo marked this pull request as ready for review April 3, 2022 21:09
@dmitrylyzo
Copy link
Contributor Author

@eyezak Could you also take a look?

@jellyfin-bot jellyfin-bot added the merge conflict Merge conflicts should be resolved before a merge label Apr 4, 2022
@jellyfin-bot jellyfin-bot removed the merge conflict Merge conflicts should be resolved before a merge label Apr 4, 2022
`ApplyConditions` is used to determine the applicability of
the current codec (`Conditions`).
`Conditions` is the actual conditions for the stream.

`CodecType.VideoAudio` (not `CodecType.Video`) must be used
for the audio tracks in the video.
Don't add codec failure reasons if the codec isn't supported.
@@ -750,10 +750,13 @@ private TranscodingProfile GetVideoTranscodeProfile(MediaSourceInfo item, VideoO
var container = transcodingProfile.Container;
var appliedVideoConditions = options.Profile.CodecProfiles
.Where(i => i.Type == CodecType.Video &&
i.ContainsAnyCodec(videoCodec, container))
i.ContainsAnyCodec(videoCodec, container) &&
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style-wise, we put the && before the lines, not after

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then this is what it would look like (since I don't want to touch more lines than enough):

.Where(i => i.Type == CodecType.Video &&
    i.ContainsAnyCodec(videoCodec, container)
    && i.ApplyConditions.All(...))

And below:

.Where(codecProfile => codecProfile.Type == CodecType.Video && codecProfile.ContainsAnyCodec(videoStream?.Codec, container)
    && !checkVideoConditions(codecProfile.ApplyConditions).Any())

@crobibero crobibero merged commit 5833c70 into jellyfin:release-10.8.z Apr 15, 2022
jellyfin-bot pushed a commit that referenced this pull request Apr 17, 2022
(cherry picked from commit 5833c70)
Signed-off-by: Joshua Boniface <joshua@boniface.me>
@jellyfin-bot jellyfin-bot removed the stable backport Backport into the next stable release label Apr 17, 2022
@dmitrylyzo dmitrylyzo deleted the fix-streambuilder branch April 18, 2022 07:53
nyanmisaka added a commit to nyanmisaka/jellyfin that referenced this pull request Jun 1, 2022
…ilder"

This reverts commit 5833c70, reversing
changes made to cd93f49.
nyanmisaka added a commit to nyanmisaka/jellyfin that referenced this pull request Jun 16, 2022
…ilder"

This reverts commit 5833c70, reversing
changes made to cd93f49.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants