dlna: simplify search method for associating subtitles with media nodes

Seems to be some corner cases that are not being handled, so taking a different
approach that should be a little more robust.

Also, changing resources to be served under a subpath:  We've been serving
media at /res?path=%2Fdir%2Ffilename.mp4; change that to be just /r/dir/filename.mp4.
It's cleaner, easier to reason about, and a necessary first step towards just
serving the resources via httplib anyway.

cmd/serve/dlna/cds.go

@@ -12,7 +12,6 @@ import (
 	"path"
 	"path/filepath"
 	"regexp"
-	"sort"
 	"strings"
 
 	"github.com/anacrolix/dms/dlna"
@@ -121,10 +120,7 @@ func (cds *contentDirectoryService) cdsObjectToUpnpavObject(cdsObject object, fi
 		URL: (&url.URL{
 			Scheme: "http",
 			Host:   host,
-			Path:   resPath,
+			Path:   path.Join(resPath, cdsObject.Path),
-			RawQuery: url.Values{
-				"path": {cdsObject.Path},
-			}.Encode(),
 		}).String(),
 		ProtocolInfo: fmt.Sprintf("http-get:*:%s:%s", mimeType, dlna.ContentFeatures{
 			SupportRange: true,
@@ -132,15 +128,11 @@ func (cds *contentDirectoryService) cdsObjectToUpnpavObject(cdsObject object, fi
 		Size: uint64(fileInfo.Size()),
 	})
 
-	basePath, _ := path.Split(cdsObject.Path)
 	for _, resource := range resources {
 		subtitleURL := (&url.URL{
 			Scheme: "http",
 			Host:   host,
-			Path:   resPath,
+			Path:   path.Join(resPath, resource.Path()),
-			RawQuery: url.Values{
-				"path": {basePath + resource.Path()},
-			}.Encode(),
 		}).String()
 		item.Res = append(item.Res, upnpav.Resource{
 			URL:          subtitleURL,
@@ -171,13 +163,12 @@ func (cds *contentDirectoryService) readContainer(o object, host string) (ret []
 		return
 	}
 
-	dirEntries, extraResources := partitionExtraResources(dirEntries)
+	dirEntries, mediaResources := mediaWithResources(dirEntries)
-
 	for _, de := range dirEntries {
 		child := object{
 			path.Join(o.Path, de.Name()),
 		}
-		obj, err := cds.cdsObjectToUpnpavObject(child, de, extraResources[de], host)
+		obj, err := cds.cdsObjectToUpnpavObject(child, de, mediaResources[de], host)
 		if err != nil {
 			fs.Errorf(cds, "error with %s: %s", child.FilePath(), err)
 			continue
@@ -193,50 +184,47 @@ func (cds *contentDirectoryService) readContainer(o object, host string) (ret []
 }
 
 // Given a list of nodes, separate them into potential media items and any associated resources (external subtitles,
-// thumbnails, metadata, etc.)
+// for example.)
-func partitionExtraResources(nodes vfs.Nodes) (vfs.Nodes, map[vfs.Node]vfs.Nodes) {
+//
-	// First, separate out the subtitles into a separate list from the media
+// The result is a a slice of potential media nodes (in their original order) and a map containing associated
-	media, subtitles := make(vfs.Nodes, 0), make(vfs.Nodes, 0)
+// resources nodes of each media node, if any.
+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.Node), make(map[string]vfs.Node)
 	for _, node := range nodes {
-		name := strings.ToLower(node.Name()) // case insensitive
+		baseName, ext := splitExt(strings.ToLower(node.Name()))
-		switch path.Ext(name) {
+		switch ext {
 		case ".srt":
-			subtitles = append(subtitles, node)
+			subtitlesByName[baseName] = node
 		default:
+			mediaByName[baseName] = node
 			media = append(media, node)
 		}
 	}
 
 	// Find the associated media file for each subtitle
-	extraResources := make(map[vfs.Node]vfs.Nodes)
+	for baseName, node := range subtitlesByName {
-	for _, node := range subtitles {
+		// Find a media file with the same basename (video.mp4 for video.srt)
-		subtitleName := strings.ToLower(node.Name())
+		mediaNode, found := mediaByName[baseName]
+		if !found {
+			// Or basename of the basename (video.mp4 for video.en.srt)
+			baseName, _ = splitExt(baseName)
+			mediaNode, found = mediaByName[baseName]
+		}
 
-		// For a media file named "My Video.mp4", we want to associated any subtitles named like
+		// Just advise if no match found
-		// "My Video.srt", "My Video.en.srt", "My Video.es.srt", "My Video.forced.srt"
+		if !found {
-		// note: nodes must be sorted!  vfs.dir.ReadDirAll() results are already sorted ..
-		mediaIdx := sort.Search(len(media), func(idx int) bool {
-			mediaName := strings.ToLower(media[idx].Name())
-			basename := strings.SplitN(mediaName, ".", 2)[0]
-			if strings.Compare(subtitleName, basename) <= 0 {
-				return true
-			}
-			if strings.HasPrefix(subtitleName, basename) {
-				return subtitleName[len(basename)] == '.'
-			}
-			return false
-		})
-		if mediaIdx == -1 {
 			fs.Infof(node, "could not find associated media for subtitle: %s", node.Name())
 			continue
 		}
 
-		mediaNode := media[mediaIdx]
 		fs.Debugf(mediaNode, "associating subtitle: %s", node.Name())
-		extraResources[mediaNode] = append(extraResources[mediaNode], node)
+		mediaResources[mediaNode] = append(mediaResources[mediaNode], node)
 	}
 
-	return media, extraResources
+	return media, mediaResources
 }
 
 type browse struct {

cmd/serve/dlna/dlna.go

@@ -62,7 +62,7 @@ players might show files that they are not able to play back correctly.
 const (
 	serverField       = "Linux/3.4 DLNADOC/1.50 UPnP/1.0 DMS/1.0"
 	rootDescPath      = "/rootDesc.xml"
-	resPath           = "/res"
+	resPath           = "/r/"
 	serviceControlURL = "/ctl"
 )
 
@@ -122,7 +122,8 @@ func newServer(f fs.Fs, opt *dlnaflags.Options) *server {
 
 	// Setup the various http routes.
 	r := http.NewServeMux()
-	r.HandleFunc(resPath, s.resourceHandler)
+	r.Handle(resPath, http.StripPrefix(resPath,
+		http.HandlerFunc(s.resourceHandler)))
 	if opt.LogTrace {
 		r.Handle(rootDescPath, traceLogging(http.HandlerFunc(s.rootDescHandler)))
 		r.Handle(serviceControlURL, traceLogging(http.HandlerFunc(s.serviceControlHandler)))
@@ -224,8 +225,8 @@ func (s *server) soapActionResponse(sa upnp.SoapAction, actionRequestXML []byte,
 
 // Serves actual resources (media files).
 func (s *server) resourceHandler(w http.ResponseWriter, r *http.Request) {
-	remotePath := r.URL.Query().Get("path")
+	remotePath := r.URL.Path
-	node, err := s.vfs.Stat(remotePath)
+	node, err := s.vfs.Stat(r.URL.Path)
 	if err != nil {
 		http.NotFound(w, r)
 		return

cmd/serve/dlna/dlna_test.go

@@ -7,7 +7,6 @@ import (
 	"html"
 	"io/ioutil"
 	"net/http"
-	"net/url"
 	"os"
 	"strings"
 	"testing"
@@ -26,7 +25,7 @@ import (
 
 var (
 	dlnaServer *server
-	testURL    string
+	baseURL    string
 )
 
 const (
@@ -38,7 +37,7 @@ func startServer(t *testing.T, f fs.Fs) {
 	opt.ListenAddr = testBindAddress
 	dlnaServer = newServer(f, &opt)
 	assert.NoError(t, dlnaServer.Serve())
-	testURL = "http://" + dlnaServer.HTTPConn.Addr().String() + "/"
+	baseURL = "http://" + dlnaServer.HTTPConn.Addr().String()
 }
 
 func TestInit(t *testing.T) {
@@ -54,7 +53,7 @@ func TestInit(t *testing.T) {
 
 // Make sure that it serves rootDesc.xml (SCPD in uPnP parlance).
 func TestRootSCPD(t *testing.T) {
-	req, err := http.NewRequest("GET", testURL+"rootDesc.xml", nil)
+	req, err := http.NewRequest("GET", baseURL+rootDescPath, nil)
 	require.NoError(t, err)
 	resp, err := http.DefaultClient.Do(req)
 	require.NoError(t, err)
@@ -73,9 +72,7 @@ func TestRootSCPD(t *testing.T) {
 
 // Make sure that it serves content from the remote.
 func TestServeContent(t *testing.T) {
-	itemPath := "/small_jpeg.jpg"
+	req, err := http.NewRequest("GET", baseURL+resPath+"video.mp4", nil)
-	pathQuery := url.QueryEscape(itemPath)
-	req, err := http.NewRequest("GET", testURL+"res?path="+pathQuery, nil)
 	require.NoError(t, err)
 	resp, err := http.DefaultClient.Do(req)
 	require.NoError(t, err)
@@ -85,7 +82,7 @@ func TestServeContent(t *testing.T) {
 	assert.NoError(t, err)
 
 	// Now compare the contents with the golden file.
-	node, err := dlnaServer.vfs.Stat(itemPath)
+	node, err := dlnaServer.vfs.Stat("/video.mp4")
 	assert.NoError(t, err)
 	goldenFile := node.(*vfs.File)
 	goldenReader, err := goldenFile.Open(os.O_RDONLY)
@@ -100,7 +97,7 @@ func TestServeContent(t *testing.T) {
 // Check that ContentDirectory#Browse returns appropriate metadata on the root container.
 func TestContentDirectoryBrowseMetadata(t *testing.T) {
 	// Sample from: https://github.com/rclone/rclone/issues/3253#issuecomment-524317469
-	req, err := http.NewRequest("POST", testURL+"ctl", strings.NewReader(`
+	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/">
@@ -126,7 +123,7 @@ func TestContentDirectoryBrowseMetadata(t *testing.T) {
 	require.Contains(t, string(body), html.EscapeString("<container "))
 	require.NotContains(t, string(body), html.EscapeString("<item "))
 	// with a non-zero childCount
-	require.Contains(t, string(body), html.EscapeString(`childCount="1"`))
+	require.Regexp(t, " childCount=&#34;[1-9]", string(body))
 }
 
 // Check that the X_MS_MediaReceiverRegistrar is faked out properly.
@@ -136,7 +133,7 @@ func TestMediaReceiverRegistrarService(t *testing.T) {
 			Action: []byte("RegisterDevice"),
 		},
 	}
-	req, err := http.NewRequest("POST", testURL+"ctl", bytes.NewReader(mustMarshalXML(env)))
+	req, err := http.NewRequest("POST", baseURL+serviceControlURL, bytes.NewReader(mustMarshalXML(env)))
 	require.NoError(t, err)
 	req.Header.Set("SOAPACTION", `"urn:microsoft.com:service:X_MS_MediaReceiverRegistrar:1#RegisterDevice"`)
 	resp, err := http.DefaultClient.Do(req)
@@ -146,3 +143,61 @@ func TestMediaReceiverRegistrarService(t *testing.T) {
 	require.NoError(t, err)
 	require.Contains(t, string(body), "<RegistrationRespMsg>")
 }
+
+// Check that ContentDirectory#Browse returns the expected items.
+func TestContentDirectoryBrowseDirectChildren(t *testing.T) {
+	// First the root...
+	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>0</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 := ioutil.ReadAll(resp.Body)
+	require.NoError(t, err)
+	// expect video.mp4, video.srt, video.en.srt URLs to be in the DIDL
+	require.Contains(t, string(body), "/r/video.mp4")
+	require.Contains(t, string(body), "/r/video.srt")
+	require.Contains(t, string(body), "/r/video.en.srt")
+
+	// Then a subdirectory
+	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>%2Fsubdir</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 = ioutil.ReadAll(resp.Body)
+	require.NoError(t, err)
+	// expect video.mp4, video.srt, URLs to be in the DIDL
+	require.Contains(t, string(body), "/r/subdir/video.mp4")
+	require.Contains(t, string(body), "/r/subdir/video.srt")
+}

cmd/serve/dlna/dlna_util.go

@@ -218,3 +218,14 @@ func serveError(what interface{}, w http.ResponseWriter, text string, err error)
 	fs.Errorf(what, "%s: %v", text, err)
 	http.Error(w, text+".", http.StatusInternalServerError)
 }
+
+// Splits a path into (root, ext) such that root + ext == path, and ext is empty
+// or begins with a period.  Extended version of path.Ext().
+func splitExt(path string) (string, string) {
+	for i := len(path) - 1; i >= 0 && path[i] != '/'; i-- {
+		if path[i] == '.' {
+			return path[:i], path[i:]
+		}
+	}
+	return path, ""
+}

cmd/serve/dlna/testdata/files/subdir/video.srt

@@ -0,0 +1,3 @@
+1
+00:00:00,000 --> 00:02:00,000
+Test

cmd/serve/dlna/testdata/files/video.en.srt

@@ -0,0 +1,3 @@
+1
+00:00:00,000 --> 00:02:00,000
+Test

cmd/serve/dlna/testdata/files/video.srt

@@ -0,0 +1,3 @@
+1
+00:00:00,000 --> 00:02:00,000
+Test