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.
This commit is contained in:
Florian Klink 2023-11-24 20:44:29 +02:00 committed by Nick Craig-Wood
parent edd9347694
commit 2e007f89c7
6 changed files with 85 additions and 7 deletions

View file

@ -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...)
}
}

View file

@ -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)
}
}

View file

@ -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(`
<?xml version="1.0" encoding="utf-8"?>
<s:Envelope xmlns:s="http://schemas.xmlsoap.org/soap/envelope/"
s:encodingStyle="http://schemas.xmlsoap.org/soap/encoding/">
<s:Body>
<u:Browse xmlns:u="urn:schemas-upnp-org:service:ContentDirectory:1">
<ObjectID>%2Fsubdir3</ObjectID>
<BrowseFlag>BrowseDirectChildren</BrowseFlag>
<Filter>*</Filter>
<StartingIndex>0</StartingIndex>
<RequestedCount>0</RequestedCount>
<SortCriteria></SortCriteria>
</u:Browse>
</s:Body>
</s:Envelope>`))
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")
}
}

View file

View file

Binary file not shown.