From 2e007f89c705d4f8cb176989929d70e5415afef8 Mon Sep 17 00:00:00 2001 From: Florian Klink Date: Fri, 24 Nov 2023 20:44:29 +0200 Subject: [PATCH] dlna: don't swallow video.{idx,sub} .idx and .sub subtitle files only work if both are present, but the code was overwriting the first-inserted element to subtitlesByName, as it was keyed by the basename without extension. Make subtitlesByName point to a slice of nodes instead. --- cmd/serve/dlna/cds.go | 15 +++--- cmd/serve/dlna/cds_test.go | 46 ++++++++++++++++++ cmd/serve/dlna/dlna_test.go | 31 ++++++++++++ .../testdata/files/subdir3/Subs/video.idx | 0 .../testdata/files/subdir3/Subs/video.sub | 0 .../dlna/testdata/files/subdir3/video.mp4 | Bin 0 -> 262 bytes 6 files changed, 85 insertions(+), 7 deletions(-) create mode 100644 cmd/serve/dlna/testdata/files/subdir3/Subs/video.idx create mode 100644 cmd/serve/dlna/testdata/files/subdir3/Subs/video.sub create mode 100644 cmd/serve/dlna/testdata/files/subdir3/video.mp4 diff --git a/cmd/serve/dlna/cds.go b/cmd/serve/dlna/cds.go index 8678ad5cb..46bb7c192 100644 --- a/cmd/serve/dlna/cds.go +++ b/cmd/serve/dlna/cds.go @@ -173,14 +173,14 @@ func mediaWithResources(nodes vfs.Nodes) (vfs.Nodes, map[vfs.Node]vfs.Nodes) { media, mediaResources := vfs.Nodes{}, make(map[vfs.Node]vfs.Nodes) // First, separate out the subtitles and media into maps, keyed by their lowercase base names. - mediaByName, subtitlesByName := make(map[string]vfs.Nodes), make(map[string]vfs.Node) + mediaByName, subtitlesByName := make(map[string]vfs.Nodes), make(map[string]vfs.Nodes) for _, node := range nodes { baseName, ext := splitExt(strings.ToLower(node.Name())) switch ext { case ".srt", ".ass", ".ssa", ".sub", ".idx", ".sup", ".jss", ".txt", ".usf", ".cue", ".vtt", ".css": // .idx should be with .sub, .css should be with vtt otherwise they should be culled, // and their mimeTypes are not consistent, but anyway these negatives don't throw errors. - subtitlesByName[baseName] = node + subtitlesByName[baseName] = append(subtitlesByName[baseName], node) default: mediaByName[baseName] = append(mediaByName[baseName], node) media = append(media, node) @@ -188,25 +188,26 @@ func mediaWithResources(nodes vfs.Nodes) (vfs.Nodes, map[vfs.Node]vfs.Nodes) { } // Find the associated media file for each subtitle - for baseName, node := range subtitlesByName { + for baseName, nodes := range subtitlesByName { // Find a media file with the same basename (video.mp4 for video.srt) mediaNodes, found := mediaByName[baseName] if !found { // Or basename of the basename (video.mp4 for video.en.srt) - baseName, _ = splitExt(baseName) + baseName, _ := splitExt(baseName) mediaNodes, found = mediaByName[baseName] } // Just advise if no match found if !found { - fs.Infof(node, "could not find associated media for subtitle: %s", node.Name()) + fs.Infof(nodes, "could not find associated media for subtitle: %s", baseName) + fs.Infof(mediaByName, "mediaByName is this, baseName is %s", baseName) continue } // Associate with all potential media nodes - fs.Debugf(mediaNodes, "associating subtitle: %s", node.Name()) + fs.Debugf(mediaNodes, "associating subtitle: %s", baseName) for _, mediaNode := range mediaNodes { - mediaResources[mediaNode] = append(mediaResources[mediaNode], node) + mediaResources[mediaNode] = append(mediaResources[mediaNode], nodes...) } } diff --git a/cmd/serve/dlna/cds_test.go b/cmd/serve/dlna/cds_test.go index 6ab793f0c..cc9b9fca2 100644 --- a/cmd/serve/dlna/cds_test.go +++ b/cmd/serve/dlna/cds_test.go @@ -99,4 +99,50 @@ func TestMediaWithResources(t *testing.T) { sort.Strings(assocVideoResourceNames) assert.Equal(t, []string{"video.en.srt", "video.srt"}, assocVideoResourceNames) } + + // Now test subdir3. It contains a video.mpv, as well as Sub/video.{idx,sub}. + { + rootNode, err := myvfs.Stat("subdir3") + require.NoError(t, err) + + subtitleNode, err := myvfs.Stat("subdir3/Subs") + require.NoError(t, err) + + rootDir := rootNode.(*vfs.Dir) + subtitleDir := subtitleNode.(*vfs.Dir) + + dirEntries, err := rootDir.ReadDirAll() + require.NoError(t, err) + + subtitleEntries, err := subtitleDir.ReadDirAll() + require.NoError(t, err) + + dirEntries = append(dirEntries, subtitleEntries...) + + mediaItems, assocResources := mediaWithResources(dirEntries) + + // ensure mediaItems contains some items we care about. + // We specifically check that the .mp4 file is kept. + var videoMp4 *vfs.Node + for _, mediaItem := range mediaItems { + if mediaItem.Name() == "video.mp4" { + videoMp4 = &mediaItem + } + } + + assert.True(t, videoMp4 != nil, "expected mp4 to be found") + + // test assocResources to point from the video file to the subtitles + assocVideoResource, ok := assocResources[*videoMp4] + require.True(t, ok, "expected video.mp4 to have assoc video resource") + + // ensure both video.idx and video.sub are in assocVideoResource. + assocVideoResourceNames := make([]string, 0) + for _, e := range assocVideoResource { + assocVideoResourceNames = append(assocVideoResourceNames, e.Name()) + } + sort.Strings(assocVideoResourceNames) + assert.Equal(t, []string{"video.idx", "video.sub"}, assocVideoResourceNames) + } + } diff --git a/cmd/serve/dlna/dlna_test.go b/cmd/serve/dlna/dlna_test.go index 5b090acd3..9b7354620 100644 --- a/cmd/serve/dlna/dlna_test.go +++ b/cmd/serve/dlna/dlna_test.go @@ -239,4 +239,35 @@ func TestContentDirectoryBrowseDirectChildren(t *testing.T) { require.Contains(t, string(body), "/r/subdir2/Subs/video.srt") } + + // Then a subdirectory with subtitles in Subs/*.{idx,sub} (subdir3) + { + req, err = http.NewRequest("POST", baseURL+serviceControlURL, strings.NewReader(` + + + + + %2Fsubdir3 + BrowseDirectChildren + * + 0 + 0 + + + +`)) + require.NoError(t, err) + req.Header.Set("SOAPACTION", `"urn:schemas-upnp-org:service:ContentDirectory:1#Browse"`) + resp, err = http.DefaultClient.Do(req) + require.NoError(t, err) + assert.Equal(t, http.StatusOK, resp.StatusCode) + body, err = io.ReadAll(resp.Body) + require.NoError(t, err) + // expect video.mp4, Subs/video.srt, URLs to be in the DIDL + require.Contains(t, string(body), "/r/subdir3/video.mp4") + require.Contains(t, string(body), "/r/subdir3/Subs/video.idx") + require.Contains(t, string(body), "/r/subdir3/Subs/video.sub") + + } } diff --git a/cmd/serve/dlna/testdata/files/subdir3/Subs/video.idx b/cmd/serve/dlna/testdata/files/subdir3/Subs/video.idx new file mode 100644 index 000000000..e69de29bb diff --git a/cmd/serve/dlna/testdata/files/subdir3/Subs/video.sub b/cmd/serve/dlna/testdata/files/subdir3/Subs/video.sub new file mode 100644 index 000000000..e69de29bb diff --git a/cmd/serve/dlna/testdata/files/subdir3/video.mp4 b/cmd/serve/dlna/testdata/files/subdir3/video.mp4 new file mode 100644 index 0000000000000000000000000000000000000000..7f6eeace9b093f873d56b3b39625d1c9c2a167c3 GIT binary patch literal 262 zcmZQzU{FXasVvAW&d+6FU}6B#Kx~v)mTZ_?U}DI?z`&7Kl$r{nb5jyafb_N8{QNQ? zos(OZkpiTV0P_nlhmnB+h!6mU0~AK%J0MhIV=(~*lS)%c5`lD7ZYr1tsZ-2I$teOc zKp;0Ivna8kAP2&Okh+;U#UKZ(t}MyV2hy@Y_k#=pTkn%tmS$?MXJV*lXkY*U4p$`j literal 0 HcmV?d00001