mirror of
https://github.com/navidrome/navidrome.git
synced 2026-08-31 07:30:32 +00:00
refactor(transcode): simplify code after review
Extract getIntClaim helper to eliminate repeated int/int64/float64 JWT claim extraction pattern in paramsFromToken and ClaimsFromToken. Rewrite checkIntLimitation as a one-liner delegating to applyIntLimitation. Return probe result from ensureProbed to avoid redundant JSON round-trip. Extract toResponseStreamDetails helper and mediaTypeSong constant in the API layer, and use transcode.ProtocolHTTP constant instead of hardcoded string. Signed-off-by: Deluan <deluan@navidrome.org>
This commit is contained in:
parent
54d7c3a284
commit
4c0dd76bd7
@ -136,39 +136,7 @@ func applyIntLimitation(comparison string, values []string, current int, setter
|
||||
}
|
||||
|
||||
func checkIntLimitation(value int, comparison string, values []string) bool {
|
||||
if len(values) == 0 {
|
||||
return true
|
||||
}
|
||||
|
||||
switch comparison {
|
||||
case ComparisonLessThanEqual:
|
||||
limit, ok := parseInt(values[0])
|
||||
if !ok {
|
||||
return true
|
||||
}
|
||||
return value <= limit
|
||||
case ComparisonGreaterThanEqual:
|
||||
limit, ok := parseInt(values[0])
|
||||
if !ok {
|
||||
return true
|
||||
}
|
||||
return value >= limit
|
||||
case ComparisonEquals:
|
||||
for _, v := range values {
|
||||
if limit, ok := parseInt(v); ok && value == limit {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
case ComparisonNotEquals:
|
||||
for _, v := range values {
|
||||
if limit, ok := parseInt(v); ok && value == limit {
|
||||
return false
|
||||
}
|
||||
}
|
||||
return true
|
||||
}
|
||||
return true
|
||||
return applyIntLimitation(comparison, values, value, func(int) {}) == adjustNone
|
||||
}
|
||||
|
||||
// checkStringLimitation checks a string value against a limitation.
|
||||
|
||||
@ -39,12 +39,13 @@ func (s *deciderService) MakeDecision(ctx context.Context, mf *model.MediaFile,
|
||||
SourceUpdatedAt: mf.UpdatedAt,
|
||||
}
|
||||
|
||||
if err := s.ensureProbed(ctx, mf); err != nil {
|
||||
probe, err := s.ensureProbed(ctx, mf)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
// Build source stream details (uses probe data if available)
|
||||
decision.SourceStream = buildSourceStream(mf)
|
||||
decision.SourceStream = buildSourceStream(mf, probe)
|
||||
src := &decision.SourceStream
|
||||
|
||||
log.Trace(ctx, "Making transcode decision", "mediaID", mf.ID, "container", src.Container,
|
||||
@ -106,15 +107,20 @@ func (s *deciderService) MakeDecision(ctx context.Context, mf *model.MediaFile,
|
||||
return decision, nil
|
||||
}
|
||||
|
||||
func buildSourceStream(mf *model.MediaFile) StreamDetails {
|
||||
func buildSourceStream(mf *model.MediaFile, probe *ffmpeg.AudioProbeResult) StreamDetails {
|
||||
sd := StreamDetails{
|
||||
Container: mf.Suffix,
|
||||
Duration: mf.Duration,
|
||||
Size: mf.Size,
|
||||
}
|
||||
|
||||
// Use pre-parsed probe result, or fall back to parsing stored probe data
|
||||
if probe == nil {
|
||||
probe, _ = parseProbeData(mf.ProbeData)
|
||||
}
|
||||
|
||||
// Use probe data if available for authoritative values
|
||||
if probe, err := parseProbeData(mf.ProbeData); err == nil && probe != nil {
|
||||
if probe != nil {
|
||||
sd.Codec = normalizeProbeCodec(probe.Codec)
|
||||
sd.Profile = probe.Profile
|
||||
sd.Bitrate = probe.BitRate
|
||||
@ -327,22 +333,25 @@ func (s *deciderService) applyCodecLimitations(ctx context.Context, sourceBitrat
|
||||
return true
|
||||
}
|
||||
|
||||
func (s *deciderService) ensureProbed(ctx context.Context, mf *model.MediaFile) error {
|
||||
// ensureProbed runs ffprobe if probe data is missing, persists it, and returns
|
||||
// the parsed result. Returns (nil, nil) when probing is skipped or data already exists
|
||||
// (in which case the caller should parse mf.ProbeData).
|
||||
func (s *deciderService) ensureProbed(ctx context.Context, mf *model.MediaFile) (*ffmpeg.AudioProbeResult, error) {
|
||||
if mf.ProbeData != "" {
|
||||
return nil
|
||||
return nil, nil
|
||||
}
|
||||
if !conf.Server.DevEnableMediaFileProbe {
|
||||
return nil
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
result, err := s.ff.ProbeAudioStream(ctx, mf.AbsolutePath())
|
||||
if err != nil {
|
||||
return fmt.Errorf("probing media file %s: %w", mf.ID, err)
|
||||
return nil, fmt.Errorf("probing media file %s: %w", mf.ID, err)
|
||||
}
|
||||
|
||||
data, err := json.Marshal(result)
|
||||
if err != nil {
|
||||
return fmt.Errorf("marshaling probe result for %s: %w", mf.ID, err)
|
||||
return nil, fmt.Errorf("marshaling probe result for %s: %w", mf.ID, err)
|
||||
}
|
||||
mf.ProbeData = string(data)
|
||||
|
||||
@ -354,7 +363,7 @@ func (s *deciderService) ensureProbed(ctx context.Context, mf *model.MediaFile)
|
||||
log.Debug(ctx, "Probed media file", "mediaID", mf.ID, "codec", result.Codec,
|
||||
"profile", result.Profile, "bitRate", result.BitRate,
|
||||
"sampleRate", result.SampleRate, "bitDepth", result.BitDepth, "channels", result.Channels)
|
||||
return nil
|
||||
return result, nil
|
||||
}
|
||||
|
||||
func (s *deciderService) CreateTranscodeParams(decision *Decision) (string, error) {
|
||||
|
||||
@ -896,21 +896,18 @@ var _ = Describe("Decider", func() {
|
||||
}
|
||||
|
||||
svc := NewDecider(ds, ff).(*deciderService)
|
||||
err := svc.ensureProbed(ctx, mf)
|
||||
probe, err := svc.ensureProbed(ctx, mf)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(mf.ProbeData).ToNot(BeEmpty())
|
||||
Expect(probe).ToNot(BeNil())
|
||||
Expect(probe.Codec).To(Equal("mp3"))
|
||||
Expect(probe.BitRate).To(Equal(320))
|
||||
Expect(probe.SampleRate).To(Equal(44100))
|
||||
Expect(probe.Channels).To(Equal(2))
|
||||
|
||||
// Verify persisted to DB
|
||||
stored := mockMFRepo.Data["probe-1"]
|
||||
Expect(stored.ProbeData).To(Equal(mf.ProbeData))
|
||||
|
||||
// Verify correct JSON content
|
||||
var result ffmpeg.AudioProbeResult
|
||||
Expect(json.Unmarshal([]byte(mf.ProbeData), &result)).To(Succeed())
|
||||
Expect(result.Codec).To(Equal("mp3"))
|
||||
Expect(result.BitRate).To(Equal(320))
|
||||
Expect(result.SampleRate).To(Equal(44100))
|
||||
Expect(result.Channels).To(Equal(2))
|
||||
})
|
||||
|
||||
It("skips ffprobe when ProbeData is already set", func() {
|
||||
@ -920,8 +917,9 @@ var _ = Describe("Decider", func() {
|
||||
ff.Error = fmt.Errorf("should not be called")
|
||||
|
||||
svc := NewDecider(ds, ff).(*deciderService)
|
||||
err := svc.ensureProbed(ctx, mf)
|
||||
probe, err := svc.ensureProbed(ctx, mf)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(probe).To(BeNil())
|
||||
})
|
||||
|
||||
It("returns error when ffprobe fails", func() {
|
||||
@ -929,7 +927,7 @@ var _ = Describe("Decider", func() {
|
||||
ff.Error = fmt.Errorf("ffprobe not found")
|
||||
|
||||
svc := NewDecider(ds, ff).(*deciderService)
|
||||
err := svc.ensureProbed(ctx, mf)
|
||||
_, err := svc.ensureProbed(ctx, mf)
|
||||
Expect(err).To(HaveOccurred())
|
||||
Expect(err.Error()).To(ContainSubstring("probing media file"))
|
||||
Expect(mf.ProbeData).To(BeEmpty())
|
||||
@ -944,8 +942,9 @@ var _ = Describe("Decider", func() {
|
||||
ff.ProbeAudioResult = &ffmpeg.AudioProbeResult{Codec: "mp3"}
|
||||
|
||||
svc := NewDecider(ds, ff).(*deciderService)
|
||||
err := svc.ensureProbed(ctx, mf)
|
||||
probe, err := svc.ensureProbed(ctx, mf)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(probe).To(BeNil())
|
||||
Expect(mf.ProbeData).To(BeEmpty())
|
||||
})
|
||||
})
|
||||
|
||||
@ -187,14 +187,9 @@ func paramsFromToken(token jwt.Token) (*Params, error) {
|
||||
p.DirectPlay = dp
|
||||
}
|
||||
|
||||
var ua int64
|
||||
if err := token.Get("ua", &ua); err == nil {
|
||||
p.SourceUpdatedAt = time.Unix(ua, 0)
|
||||
} else {
|
||||
var uaf float64
|
||||
if err := token.Get("ua", &uaf); err == nil {
|
||||
p.SourceUpdatedAt = time.Unix(int64(uaf), 0)
|
||||
}
|
||||
ua := getIntClaim(token, "ua")
|
||||
if ua != 0 {
|
||||
p.SourceUpdatedAt = time.Unix(int64(ua), 0)
|
||||
}
|
||||
if p.SourceUpdatedAt.IsZero() {
|
||||
return nil, fmt.Errorf("%w: missing source timestamp", ErrTokenInvalid)
|
||||
@ -204,29 +199,27 @@ func paramsFromToken(token jwt.Token) (*Params, error) {
|
||||
if err := token.Get("f", &f); err == nil {
|
||||
p.TargetFormat = f
|
||||
}
|
||||
if err := token.Get("b", &p.TargetBitrate); err != nil {
|
||||
var bf float64
|
||||
if err := token.Get("b", &bf); err == nil {
|
||||
p.TargetBitrate = int(bf)
|
||||
}
|
||||
}
|
||||
if err := token.Get("ch", &p.TargetChannels); err != nil {
|
||||
var chf float64
|
||||
if err := token.Get("ch", &chf); err == nil {
|
||||
p.TargetChannels = int(chf)
|
||||
}
|
||||
}
|
||||
if err := token.Get("sr", &p.TargetSampleRate); err != nil {
|
||||
var srf float64
|
||||
if err := token.Get("sr", &srf); err == nil {
|
||||
p.TargetSampleRate = int(srf)
|
||||
}
|
||||
}
|
||||
if err := token.Get("bd", &p.TargetBitDepth); err != nil {
|
||||
var bdf float64
|
||||
if err := token.Get("bd", &bdf); err == nil {
|
||||
p.TargetBitDepth = int(bdf)
|
||||
}
|
||||
}
|
||||
p.TargetBitrate = getIntClaim(token, "b")
|
||||
p.TargetChannels = getIntClaim(token, "ch")
|
||||
p.TargetSampleRate = getIntClaim(token, "sr")
|
||||
p.TargetBitDepth = getIntClaim(token, "bd")
|
||||
return &p, nil
|
||||
}
|
||||
|
||||
// getIntClaim extracts an int claim from a JWT token, handling the case where
|
||||
// the value may be stored as int64 or float64 (common in JSON-based JWT libraries).
|
||||
func getIntClaim(token jwt.Token, key string) int {
|
||||
var v int
|
||||
if err := token.Get(key, &v); err == nil {
|
||||
return v
|
||||
}
|
||||
var v64 int64
|
||||
if err := token.Get(key, &v64); err == nil {
|
||||
return int(v64)
|
||||
}
|
||||
var f float64
|
||||
if err := token.Get(key, &f); err == nil {
|
||||
return int(f)
|
||||
}
|
||||
return 0
|
||||
}
|
||||
|
||||
@ -166,6 +166,15 @@ func (r *clientInfoRequest) validate() error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// Only support songs for now
|
||||
var validMediaTypes = []string{
|
||||
"song",
|
||||
}
|
||||
|
||||
func isValidMediaType(mediaType string) bool {
|
||||
return slices.Contains(validMediaTypes, mediaType)
|
||||
}
|
||||
|
||||
var validProtocols = []string{
|
||||
transcode.ProtocolHTTP,
|
||||
transcode.ProtocolHLS,
|
||||
@ -206,6 +215,20 @@ func isValidComparison(c string) bool {
|
||||
return slices.Contains(validComparisons, c)
|
||||
}
|
||||
|
||||
// toResponseStreamDetails converts a core StreamDetails to the API response type.
|
||||
func toResponseStreamDetails(sd *transcode.StreamDetails) *responses.StreamDetails {
|
||||
return &responses.StreamDetails{
|
||||
Protocol: transcode.ProtocolHTTP,
|
||||
Container: sd.Container,
|
||||
Codec: sd.Codec,
|
||||
AudioBitrate: int32(kbpsToBps(sd.Bitrate)),
|
||||
AudioProfile: sd.Profile,
|
||||
AudioSamplerate: int32(sd.SampleRate),
|
||||
AudioBitdepth: int32(sd.BitDepth),
|
||||
AudioChannels: int32(sd.Channels),
|
||||
}
|
||||
}
|
||||
|
||||
// GetTranscodeDecision handles the OpenSubsonic getTranscodeDecision endpoint.
|
||||
// It receives client capabilities and returns a decision on whether to direct play or transcode.
|
||||
func (api *Router) GetTranscodeDecision(w http.ResponseWriter, r *http.Request) (*responses.Subsonic, error) {
|
||||
@ -228,8 +251,7 @@ func (api *Router) GetTranscodeDecision(w http.ResponseWriter, r *http.Request)
|
||||
return nil, newError(responses.ErrorMissingParameter, "missing required parameter: mediaType")
|
||||
}
|
||||
|
||||
// Only support songs for now
|
||||
if mediaType != "song" {
|
||||
if !isValidMediaType(mediaType) {
|
||||
return nil, newError(responses.ErrorGeneric, "mediaType '%s' is not yet supported", mediaType)
|
||||
}
|
||||
|
||||
@ -276,29 +298,11 @@ func (api *Router) GetTranscodeDecision(w http.ResponseWriter, r *http.Request)
|
||||
TranscodeReasons: decision.TranscodeReasons,
|
||||
ErrorReason: decision.ErrorReason,
|
||||
TranscodeParams: transcodeParams,
|
||||
SourceStream: &responses.StreamDetails{
|
||||
Protocol: "http",
|
||||
Container: decision.SourceStream.Container,
|
||||
Codec: decision.SourceStream.Codec,
|
||||
AudioBitrate: int32(kbpsToBps(decision.SourceStream.Bitrate)),
|
||||
AudioProfile: decision.SourceStream.Profile,
|
||||
AudioSamplerate: int32(decision.SourceStream.SampleRate),
|
||||
AudioBitdepth: int32(decision.SourceStream.BitDepth),
|
||||
AudioChannels: int32(decision.SourceStream.Channels),
|
||||
},
|
||||
SourceStream: toResponseStreamDetails(&decision.SourceStream),
|
||||
}
|
||||
|
||||
if decision.TranscodeStream != nil {
|
||||
response.TranscodeDecision.TranscodeStream = &responses.StreamDetails{
|
||||
Protocol: "http",
|
||||
Container: decision.TranscodeStream.Container,
|
||||
Codec: decision.TranscodeStream.Codec,
|
||||
AudioBitrate: int32(kbpsToBps(decision.TranscodeStream.Bitrate)),
|
||||
AudioProfile: decision.TranscodeStream.Profile,
|
||||
AudioSamplerate: int32(decision.TranscodeStream.SampleRate),
|
||||
AudioBitdepth: int32(decision.TranscodeStream.BitDepth),
|
||||
AudioChannels: int32(decision.TranscodeStream.Channels),
|
||||
}
|
||||
response.TranscodeDecision.TranscodeStream = toResponseStreamDetails(decision.TranscodeStream)
|
||||
}
|
||||
|
||||
return response, nil
|
||||
@ -329,8 +333,7 @@ func (api *Router) GetTranscodeStream(w http.ResponseWriter, r *http.Request) (*
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
// Only support songs for now
|
||||
if mediaType != "song" {
|
||||
if !isValidMediaType(mediaType) {
|
||||
http.Error(w, "Bad Request", http.StatusBadRequest)
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user