diff --git a/core/stream/types.go b/core/stream/types.go index 11642c11c..19474dd91 100644 --- a/core/stream/types.go +++ b/core/stream/types.go @@ -59,6 +59,31 @@ func (ci *ClientInfo) CapBitrate(maxKbps int) bool { return changed } +// ForceFormat narrows the client to transcoding to targetFormat and suppresses +// direct play, but only if the client already declares a profile for that +// format. All matching profiles are kept so negotiation can still pick among +// them (e.g. by protocol). Returns false (no-op) when targetFormat is empty or +// unsupported. +func (ci *ClientInfo) ForceFormat(targetFormat string) bool { + if targetFormat == "" { + return false + } + var matched []Profile + for i := range ci.TranscodingProfiles { + // matchesContainer is alias-aware, so a forced "oga" (legacy Opus + // target_format) still matches a resolved "opus" profile. + if _, format := resolveTargetFormat(&ci.TranscodingProfiles[i]); matchesContainer(format, []string{targetFormat}) { + matched = append(matched, ci.TranscodingProfiles[i]) + } + } + if len(matched) == 0 { + return false + } + ci.TranscodingProfiles = matched + ci.DirectPlayProfiles = nil + return true +} + // DirectPlayProfile describes a format the client can play directly type DirectPlayProfile struct { Containers []string diff --git a/core/stream/types_test.go b/core/stream/types_test.go index 2d2a83d06..eff408362 100644 --- a/core/stream/types_test.go +++ b/core/stream/types_test.go @@ -56,4 +56,78 @@ var _ = Describe("ClientInfo", func() { Expect(ci.MaxTranscodingAudioBitrate).To(Equal(192)) }) }) + + Describe("ForceFormat", func() { + It("restricts to the forced format and clears direct play when supported", func() { + ci := &ClientInfo{ + DirectPlayProfiles: []DirectPlayProfile{{Containers: []string{"flac"}, AudioCodecs: []string{"flac"}}}, + TranscodingProfiles: []Profile{ + {Container: "flac", AudioCodec: "flac", Protocol: ProtocolHTTP}, + {Container: "ogg", AudioCodec: "opus", Protocol: ProtocolHTTP}, + {Container: "mp3", AudioCodec: "mp3", Protocol: ProtocolHTTP}, + }, + } + ok := ci.ForceFormat("opus") + Expect(ok).To(BeTrue()) + Expect(ci.TranscodingProfiles).To(HaveLen(1)) + Expect(ci.TranscodingProfiles[0].AudioCodec).To(Equal("opus")) + Expect(ci.DirectPlayProfiles).To(BeEmpty()) + }) + + It("matches a container-only forced format (mp3)", func() { + ci := &ClientInfo{ + TranscodingProfiles: []Profile{ + {Container: "ogg", AudioCodec: "opus", Protocol: ProtocolHTTP}, + {Container: "mp3", AudioCodec: "mp3", Protocol: ProtocolHTTP}, + }, + } + ok := ci.ForceFormat("mp3") + Expect(ok).To(BeTrue()) + Expect(ci.TranscodingProfiles).To(HaveLen(1)) + Expect(ci.TranscodingProfiles[0].Container).To(Equal("mp3")) + }) + + It("matches the forced format against codec aliases (oga/opus)", func() { + ci := &ClientInfo{ + TranscodingProfiles: []Profile{ + {Container: "ogg", AudioCodec: "opus", Protocol: ProtocolHTTP}, + {Container: "mp3", AudioCodec: "mp3", Protocol: ProtocolHTTP}, + }, + } + // Legacy DBs may store the Opus transcoding as target_format "oga". + ok := ci.ForceFormat("oga") + Expect(ok).To(BeTrue()) + Expect(ci.TranscodingProfiles).To(HaveLen(1)) + Expect(ci.TranscodingProfiles[0].AudioCodec).To(Equal("opus")) + }) + + It("is a no-op when the forced format is not supported by the client", func() { + original := []Profile{{Container: "mp3", AudioCodec: "mp3", Protocol: ProtocolHTTP}} + ci := &ClientInfo{ + DirectPlayProfiles: []DirectPlayProfile{{Containers: []string{"flac"}}}, + TranscodingProfiles: original, + } + ok := ci.ForceFormat("opus") + Expect(ok).To(BeFalse()) + Expect(ci.TranscodingProfiles).To(Equal(original)) + Expect(ci.DirectPlayProfiles).To(HaveLen(1)) + }) + + It("is a no-op for an empty target format", func() { + ci := &ClientInfo{TranscodingProfiles: []Profile{{Container: "mp3", AudioCodec: "mp3"}}} + Expect(ci.ForceFormat("")).To(BeFalse()) + }) + + It("keeps all matching profiles when multiple resolve to the forced format", func() { + first := Profile{Container: "ogg", AudioCodec: "opus", Protocol: ProtocolHTTP} + second := Profile{Container: "ogg", AudioCodec: "opus", Protocol: ProtocolHTTP, MaxAudioChannels: 2} + other := Profile{Container: "mp3", AudioCodec: "mp3", Protocol: ProtocolHTTP} + ci := &ClientInfo{TranscodingProfiles: []Profile{first, other, second}} + + ok := ci.ForceFormat("opus") + + Expect(ok).To(BeTrue()) + Expect(ci.TranscodingProfiles).To(ConsistOf(first, second)) + }) + }) }) diff --git a/resources/i18n/pt-br.json b/resources/i18n/pt-br.json index bc4d149a1..b3b3bab2f 100644 --- a/resources/i18n/pt-br.json +++ b/resources/i18n/pt-br.json @@ -187,9 +187,6 @@ "lastSeen": "Últ. acesso", "reportRealPath": "Use paths reais", "scrobbleEnabled": "Enviar scrobbles para serviços externos" - }, - "helperTexts": { - "transcodingId": "O player web ignora o formato de conversão e aplica apenas o limite de Bitrate máx." } }, "transcoding": { diff --git a/server/e2e/subsonic_transcode_test.go b/server/e2e/subsonic_transcode_test.go index c769fd2d4..afe7d52ca 100644 --- a/server/e2e/subsonic_transcode_test.go +++ b/server/e2e/subsonic_transcode_test.go @@ -159,11 +159,22 @@ var _ = Describe("Transcode Endpoints", Ordered, func() { Expect(ds.Player(ctx).Put(player)).To(Succeed()) } + setPlayerForcedFormat := func(format string) { + doReq("ping") + player, err := ds.Player(ctx).FindMatch(adminUser.ID, "test-client", "") + Expect(err).ToNot(HaveOccurred()) + trc, err := ds.Transcoding(ctx).FindByFormat(format) + Expect(err).ToNot(HaveOccurred()) + player.TranscodingId = trc.ID + Expect(ds.Player(ctx).Put(player)).To(Succeed()) + } + AfterEach(func() { // Reset player MaxBitRate to 0 after each test player, err := ds.Player(ctx).FindMatch(adminUser.ID, "test-client", "") if err == nil { player.MaxBitRate = 0 + player.TranscodingId = "" _ = ds.Player(ctx).Put(player) } }) @@ -509,6 +520,43 @@ var _ = Describe("Transcode Endpoints", Ordered, func() { Expect(resp.TranscodeDecision.TranscodeStream.AudioBitrate).To(Equal(int32(192000))) }) }) + + Describe("player forced format", func() { + It("transcodes a FLAC to the forced opus format when the client supports it", func() { + setPlayerForcedFormat("opus") + + resp := doPostReq("getTranscodeDecision", opusTranscodeClient, "mediaId", flacTrackID, "mediaType", "song") + Expect(resp.Status).To(Equal(responses.StatusOK)) + Expect(resp.TranscodeDecision).ToNot(BeNil()) + Expect(resp.TranscodeDecision.CanTranscode).To(BeTrue()) + Expect(resp.TranscodeDecision.TranscodeStream).ToNot(BeNil()) + Expect(resp.TranscodeDecision.TranscodeStream.Codec).To(Equal("opus")) + }) + + It("falls back to negotiation when the client does not support the forced format", func() { + setPlayerForcedFormat("opus") + + resp := doPostReq("getTranscodeDecision", mp3OnlyClient, "mediaId", flacTrackID, "mediaType", "song") + Expect(resp.Status).To(Equal(responses.StatusOK)) + Expect(resp.TranscodeDecision).ToNot(BeNil()) + Expect(resp.TranscodeDecision.CanTranscode).To(BeTrue()) + Expect(resp.TranscodeDecision.TranscodeStream).ToNot(BeNil()) + Expect(resp.TranscodeDecision.TranscodeStream.Container).To(Equal("mp3")) + }) + + It("applies maxBitRate on top of the forced format", func() { + setPlayerForcedFormat("opus") + setPlayerMaxBitRate(96) + + resp := doPostReq("getTranscodeDecision", opusTranscodeClient, "mediaId", flacTrackID, "mediaType", "song") + Expect(resp.Status).To(Equal(responses.StatusOK)) + Expect(resp.TranscodeDecision).ToNot(BeNil()) + Expect(resp.TranscodeDecision.CanTranscode).To(BeTrue()) + Expect(resp.TranscodeDecision.TranscodeStream).ToNot(BeNil()) + Expect(resp.TranscodeDecision.TranscodeStream.Codec).To(Equal("opus")) + Expect(resp.TranscodeDecision.TranscodeStream.AudioBitrate).To(Equal(int32(96000))) + }) + }) }) Describe("getTranscodeStream", func() { diff --git a/server/subsonic/transcode.go b/server/subsonic/transcode.go index c95c25cb0..511db2b85 100644 --- a/server/subsonic/transcode.go +++ b/server/subsonic/transcode.go @@ -279,6 +279,19 @@ func (api *Router) GetTranscodeDecision(w http.ResponseWriter, r *http.Request) return stream.IsAACCodec(p.Container) }) + // Honor the player's forced transcoding format, falling back to normal + // negotiation when the client can't play it (issue #5583). + if trc, ok := request.TranscodingFrom(ctx); ok && trc.TargetFormat != "" { + if !clientInfo.ForceFormat(trc.TargetFormat) { + clientName := clientInfo.Name + if player, ok := request.PlayerFrom(ctx); ok && player.Client != "" { + clientName = player.Client + } + log.Debug(ctx, "Player forced format not supported by client; falling back to negotiation", + "forcedFormat", trc.TargetFormat, "client", clientName) + } + } + // Apply the player's MaxBitRate as a ceiling on the client's declared // limits (issue #5583). Both fields are capped because the client sends // them independently here; capping only MaxAudioBitrate would let an diff --git a/server/subsonic/transcode_test.go b/server/subsonic/transcode_test.go index 4a1017752..7e36ab243 100644 --- a/server/subsonic/transcode_test.go +++ b/server/subsonic/transcode_test.go @@ -305,6 +305,73 @@ var _ = Describe("Transcode endpoints", func() { Expect(mockTD.capturedClient.MaxAudioBitrate).To(Equal(320)) }) }) + + Describe("player forced format", func() { + withForcedFormat := func(r *http.Request, format string, maxBitRate int) *http.Request { + ctx := r.Context() + ctx = request.WithTranscoding(ctx, model.Transcoding{TargetFormat: format}) + if maxBitRate > 0 { + ctx = request.WithPlayer(ctx, model.Player{Client: "NavidromeUI", MaxBitRate: maxBitRate}) + } + return r.WithContext(ctx) + } + + BeforeEach(func() { + mockMFRepo.SetData(model.MediaFiles{ + {ID: "song-1", Suffix: "flac", Codec: "FLAC", BitRate: 900, Channels: 2, SampleRate: 44100}, + }) + mockTD.decision = &stream.TranscodeDecision{MediaID: "song-1", CanTranscode: true} + mockTD.token = "token" + }) + + It("forces a supported format and clears direct play", func() { + body := `{"directPlayProfiles":[{"containers":["flac"],"audioCodecs":["flac"],"protocols":["http"]}], + "transcodingProfiles":[{"container":"ogg","audioCodec":"opus","protocol":"http"}, + {"container":"mp3","audioCodec":"mp3","protocol":"http"}]}` + r := withForcedFormat(newJSONPostRequest("mediaId=song-1&mediaType=song", body), "opus", 0) + + _, err := router.GetTranscodeDecision(w, r) + + Expect(err).ToNot(HaveOccurred()) + Expect(mockTD.capturedClient.TranscodingProfiles).To(HaveLen(1)) + Expect(mockTD.capturedClient.TranscodingProfiles[0].AudioCodec).To(Equal("opus")) + Expect(mockTD.capturedClient.DirectPlayProfiles).To(BeEmpty()) + }) + + It("falls back to negotiation when the forced format is unsupported", func() { + // Forced format is opus, but the client only declares mp3 and flac. + // Should fall back to negotiating among the client's own profiles. + body := `{"directPlayProfiles":[{"containers":["flac"],"audioCodecs":["flac"],"protocols":["http"]}], + "transcodingProfiles":[ + {"container":"flac","audioCodec":"flac","protocol":"http"}, + {"container":"mp3","audioCodec":"mp3","protocol":"http"}]}` + r := withForcedFormat(newJSONPostRequest("mediaId=song-1&mediaType=song", body), "opus", 0) + + _, err := router.GetTranscodeDecision(w, r) + + Expect(err).ToNot(HaveOccurred()) + // Profiles left intact for normal negotiation (forced format not applied). + Expect(mockTD.capturedClient.TranscodingProfiles).To(HaveLen(2)) + Expect(mockTD.capturedClient.DirectPlayProfiles).ToNot(BeEmpty()) + }) + + It("applies the maxBitRate cap on top of the forced format", func() { + // Client supports opus + mp3; forced format opus must be selected, + // and the maxBitRate cap applied on top. + body := `{"transcodingProfiles":[ + {"container":"ogg","audioCodec":"opus","protocol":"http"}, + {"container":"mp3","audioCodec":"mp3","protocol":"http"}]}` + r := withForcedFormat(newJSONPostRequest("mediaId=song-1&mediaType=song", body), "opus", 128) + + _, err := router.GetTranscodeDecision(w, r) + + Expect(err).ToNot(HaveOccurred()) + Expect(mockTD.capturedClient.TranscodingProfiles).To(HaveLen(1)) + Expect(mockTD.capturedClient.TranscodingProfiles[0].AudioCodec).To(Equal("opus")) + Expect(mockTD.capturedClient.MaxAudioBitrate).To(Equal(128)) + Expect(mockTD.capturedClient.MaxTranscodingAudioBitrate).To(Equal(128)) + }) + }) }) Describe("GetTranscodeStream", func() { diff --git a/ui/src/i18n/en.json b/ui/src/i18n/en.json index 595b20a0d..74fb23ab9 100644 --- a/ui/src/i18n/en.json +++ b/ui/src/i18n/en.json @@ -187,9 +187,6 @@ "lastSeen": "Last Seen At", "reportRealPath": "Report Real Path", "scrobbleEnabled": "Send Scrobbles to external services" - }, - "helperTexts": { - "transcodingId": "The web player ignores the transcoding format and only enforces the Max. Bit Rate limit." } }, "transcoding": { diff --git a/ui/src/player/PlayerEdit.jsx b/ui/src/player/PlayerEdit.jsx index d09ed855f..1826500bd 100644 --- a/ui/src/player/PlayerEdit.jsx +++ b/ui/src/player/PlayerEdit.jsx @@ -8,7 +8,6 @@ import { SelectInput, ReferenceInput, useTranslate, - useRecordContext, } from 'react-admin' import { Title } from '../common' import config from '../config' @@ -20,35 +19,17 @@ const PlayerTitle = ({ record }) => { return } -export const TranscodingInput = (props) => { - const translate = useTranslate() - const record = useRecordContext(props) - const isWebPlayer = record?.client === 'NavidromeUI' - return ( - <ReferenceInput - {...props} - source="transcodingId" - reference="transcoding" - sort={{ field: 'name', order: 'ASC' }} - > - <SelectInput - source="name" - resettable - helperText={ - isWebPlayer - ? translate('resources.player.helperTexts.transcodingId') - : undefined - } - /> - </ReferenceInput> - ) -} - const PlayerEdit = (props) => ( <Edit title={<PlayerTitle />} {...props}> <SimpleForm variant={'outlined'}> <TextInput source="name" validate={[required()]} /> - <TranscodingInput /> + <ReferenceInput + source="transcodingId" + reference="transcoding" + sort={{ field: 'name', order: 'ASC' }} + > + <SelectInput source="name" resettable /> + </ReferenceInput> <SelectInput source="maxBitRate" resettable choices={BITRATE_CHOICES} /> <BooleanInput source="reportRealPath" fullWidth /> {(config.lastFMEnabled || config.listenBrainzEnabled) && ( diff --git a/ui/src/player/PlayerEdit.test.jsx b/ui/src/player/PlayerEdit.test.jsx deleted file mode 100644 index 2b8c862e2..000000000 --- a/ui/src/player/PlayerEdit.test.jsx +++ /dev/null @@ -1,54 +0,0 @@ -import * as React from 'react' -import { render, screen, cleanup } from '@testing-library/react' -import { describe, it, expect, afterEach, vi, beforeEach } from 'vitest' -import { useRecordContext } from 'react-admin' -import { TranscodingInput } from './PlayerEdit' - -vi.mock('react-admin', async () => { - const actual = await vi.importActual('react-admin') - return { - ...actual, - useRecordContext: vi.fn(), - // Mock useTranslate to return the key verbatim so assertions don't depend - // on ra-core's out-of-provider translation behavior. - useTranslate: () => (key) => key, - // Render the inputs as simple stand-ins so we can read their props. - ReferenceInput: ({ children, variant }) => ( - <div data-testid="reference-input" data-variant={variant || ''}> - {children} - </div> - ), - SelectInput: ({ helperText }) => ( - <div data-testid="select-input" data-helpertext={helperText || ''} /> - ), - } -}) - -describe('<TranscodingInput />', () => { - beforeEach(() => { - useRecordContext.mockReset() - }) - afterEach(cleanup) - - it('shows helper text for the NavidromeUI player', () => { - useRecordContext.mockReturnValue({ client: 'NavidromeUI' }) - render(<TranscodingInput />) - expect(screen.getByTestId('select-input').dataset.helpertext).toBe( - 'resources.player.helperTexts.transcodingId', - ) - }) - - it('shows no helper text for other clients', () => { - useRecordContext.mockReturnValue({ client: 'DSub' }) - render(<TranscodingInput />) - expect(screen.getByTestId('select-input').dataset.helpertext).toBe('') - }) - - it('forwards the form variant injected by SimpleForm to the input', () => { - useRecordContext.mockReturnValue({ client: 'DSub' }) - render(<TranscodingInput variant="outlined" />) - expect(screen.getByTestId('reference-input').dataset.variant).toBe( - 'outlined', - ) - }) -})