mirror of
https://github.com/navidrome/navidrome.git
synced 2026-06-19 07:37:15 +00:00
fix(transcoding): honor player forced format on the WebUI transcode flow (#5613)
* feat(stream): add ClientInfo.ForceFormat for browser-aware forced format Restricts the client to a forced transcoding format and suppresses direct play, but only when the client declares it supports that format. Part of #5583. * fix(transcoding): honor player forced format on getTranscodeDecision When the WebUI player has a forced transcoding format configured and the browser declares it can play that format, transcode to it (suppressing direct play). Fall back to normal negotiation with a warning when the format is unsupported. The MaxBitRate cap still applies on top. Fixes #5583. * test(e2e): cover player forced format on getTranscodeDecision Forced format honored when the client supports it, falls back to negotiation otherwise, and the MaxBitRate cap still applies on top. Part of #5583. * feat(ui): remove obsolete 'format ignored' helper text on player form The web player now honors the forced transcoding format, so the caveat added in #5611 no longer applies. Reverts the Transcoding field to a plain selector. Part of #5583.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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))
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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": {
|
||||
|
||||
@@ -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() {
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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() {
|
||||
|
||||
@@ -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": {
|
||||
|
||||
@@ -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 <Title subTitle={`${resourceName} ${record ? record.name : ''}`} />
|
||||
}
|
||||
|
||||
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) && (
|
||||
|
||||
@@ -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',
|
||||
)
|
||||
})
|
||||
})
|
||||
Reference in New Issue
Block a user