[go: up one dir, main page]
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.

Sign up for GitHub

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

Jump to bottom

Use filtered codecs to build appliedConditions #12562

Merged
merged 1 commit into from
Sep 3, 2024
Merged

Use filtered codecs to build appliedConditions #12562

merged 1 commit into from
Sep 3, 2024

Conversation

nyanmisaka
Copy link
Member

Based on the new suggestions here. #12521 (comment)

Changes

  • Use filtered codecs to build appliedConditions

@nyanmisaka nyanmisaka added the stable backport Backport into the next stable release label Sep 1, 2024
@nyanmisaka nyanmisaka requested a review from a team September 1, 2024 11:27
@dmitrylyzo
Copy link
Contributor
dmitrylyzo commented Sep 1, 2024 •
edited
Loading

I'm not subscribed and continue posting on that PR. 😅

Also, here

jellyfin/MediaBrowser.Model/Dlna/StreamBuilder.cs

Line 907 in 2fe13f5

var transcodingVideoCodecs = ContainerProfile.SplitValue(videoCodec);

and here

jellyfin/MediaBrowser.Model/Dlna/StreamBuilder.cs

Line 940 in 2fe13f5

var transcodingAudioCodecs = ContainerProfile.SplitValue(audioCodec);

Or maybe just assign it back?

videoCodec = playlistItem.VideoCodecs;
audioCodec = playlistItem.AudioCodecs;

@nyanmisaka
Copy link
Member Author 
videoCodec = playlistItem.VideoCodecs;
audioCodec = playlistItem.AudioCodecs;

One is a string and the other is a string array. How about using videoCodecs and audioCodecs?

@dmitrylyzo
Copy link
Contributor

One is a string and the other is a string array. How about using videoCodecs and audioCodecs?

Would work.
We need to assign audioCodecs after this line

jellyfin/MediaBrowser.Model/Dlna/StreamBuilder.cs

Line 861 in 073d93c

playlistItem.AudioCodecs = new[] { audioStream.Codec };

@nyanmisaka
Use filtered codecs to build appliedConditions
8095d39 
Signed-off-by: nyanmisaka <nst799610810@gmail.com>
@nyanmisaka
Copy link
Member Author

One is a string and the other is a string array. How about using videoCodecs and audioCodecs?

Would work. We need to assign audioCodecs after this line

jellyfin/MediaBrowser.Model/Dlna/StreamBuilder.cs

Line 861 in 073d93c

playlistItem.AudioCodecs = new[] { audioStream.Codec };

Done.

dmitrylyzo
dmitrylyzo approved these changes Sep 1, 2024
View reviewed changes
Copy link
Contributor
@dmitrylyzo dmitrylyzo left a comment

Choose a reason for hiding this comment

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

LGTM.
Also optimized by omitting SplitValue. 👍

@Bond-009 Bond-009 merged commit b3efae7 into jellyfin:release-10.9.z Sep 3, 2024
12 of 13 checks passed
@nyanmisaka nyanmisaka deleted the fix-profiles branch September 3, 2024 07:23
gnattu added a commit to gnattu/jellyfin that referenced this pull request Sep 7, 2024
@gnattu
Backport jellyfin#12562
d25adda 
Signed-off-by: gnattu <gnattuoc@me.com>
@gnattu gnattu mentioned this pull request Sep 7, 2024
Merged
@gnattu gnattu removed the stable backport Backport into the next stable release label Sep 7, 2024
crobibero pushed a commit that referenced this pull request Sep 7, 2024
@gnattu @dmitrylyzo @nyanmisaka
Backport #12562 and #12521 (#12602)
df00909 
Co-authored-by: Dmitry Lyzo <56478732+dmitrylyzo@users.noreply.github.com>
Co-authored-by: Nyanmisaka <nst799610810@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cvium cvium cvium approved these changes

@Shadowghost Shadowghost Shadowghost approved these changes

@Bond-009 Bond-009 Bond-009 approved these changes

@dmitrylyzo dmitrylyzo dmitrylyzo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@nyanmisaka @dmitrylyzo @cvium @Shadowghost @Bond-009 @gnattu