CLI: Improve "photoprism dl" post-processing and default settings #5219

Signed-off-by: Michael Mayer <michael@photoprism.app>
2025-09-26 21:01:58 +08:00 · 2025-09-20 14:36:41 +02:00
parent 5e84da55e5
commit f7fe6b569a
10 changed files with 250 additions and 29 deletions
--- a/internal/commands/download.go
+++ b/internal/commands/download.go
@@ -69,7 +69,7 @@ var DownloadCommand = &cli.Command{
 		&cli.StringFlag{
 			Name:    "file-remux",
 			Aliases: []string{"r"},
-			Value:   "always",
+			Value:   "auto",
 			Usage:   "remux `POLICY` for videos when using --dl-method file: auto (skip if MP4), always, or skip",
 		},
 	},
@@ -155,7 +155,7 @@ func downloadAction(ctx *cli.Context) error {
 	}
 	fileRemux := strings.ToLower(strings.TrimSpace(ctx.String("file-remux")))
 	if fileRemux == "" {
-		fileRemux = "always"
+		fileRemux = "auto"
 	}
 	switch fileRemux {
 	case "always", "auto", "skip":
@@ -214,6 +214,12 @@ func downloadAction(ctx *cli.Context) error {
 				continue
 			}

+			// Best-effort creation time for file method when not remuxing locally.
+			if created := dl.CreatedFromInfo(result.Info); !created.IsZero() {
+				// Apply via yt-dlp ffmpeg post-processor so creation_time exists even without our remux.
+				result.Options.FFmpegPostArgs = "-metadata creation_time=" + created.UTC().Format(time.RFC3339)
+			}
+
 			// Base filename for pipe method
 			if dlName := clean.DlName(result.Info.Title); dlName != "" {
 				downloadFile = dlName + fs.ExtMp4
--- a/internal/commands/download_e2e_test.go
+++ b/internal/commands/download_e2e_test.go
@@ -35,7 +35,7 @@ func createFakeYtDlp(t *testing.T) string {
 	b.WriteString("set -euo pipefail\n")
 	b.WriteString("OUT_TPL=\"\"\n")
 	b.WriteString("i=0; while [[ $i -lt $# ]]; do i=$((i+1)); arg=\"${!i}\"; if [[ \"$arg\" == \"--dump-single-json\" ]]; then echo '{\"id\":\"abc\",\"title\":\"Test\",\"url\":\"http://example.com\",\"_type\":\"video\"}'; exit 0; fi; if [[ \"$arg\" == \"--output\" ]]; then i=$((i+1)); OUT_TPL=\"${!i}\"; fi; done\n")
-	b.WriteString("if [[ $* == *'--print '* ]]; then OUT=\"$OUT_TPL\"; OUT=${OUT//%(id)s/abc}; OUT=${OUT//%(ext)s/mp4}; mkdir -p \"$(dirname \"$OUT\")\"; echo 'dummy' > \"$OUT\"; echo \"$OUT\"; exit 0; fi\n")
+    b.WriteString("if [[ $* == *'--print '* ]]; then OUT=\"$OUT_TPL\"; OUT=${OUT//%(id)s/abc}; OUT=${OUT//%(ext)s/mp4}; mkdir -p \"$(dirname \"$OUT\")\"; CONTENT=\"${YTDLP_DUMMY_CONTENT:-dummy}\"; echo \"$CONTENT\" > \"$OUT\"; echo \"$OUT\"; exit 0; fi\n")
 	if err := os.WriteFile(path, []byte(b.String()), 0o755); err != nil {
 		t.Fatalf("failed to write fake yt-dlp: %v", err)
 	}
@@ -72,14 +72,47 @@ func TestDownloadImpl_FileMethod_AutoSkipsRemux(t *testing.T) {
 		FileRemux: "auto",
 	}, []string{"https://example.com/video"})
 	if err != nil {
-		t.Fatalf("runDownload failed: %v", err)
+		t.Fatalf("runDownload failed (auto should skip remux): %v", err)
 	}

-	// Verify a file exists under Originals/dest with .mp4 extension
-	c := get.Config()
-	if c == nil {
+	// Cleanup destination folder (best effort)
+	if c := get.Config(); c != nil {
+		outDir := filepath.Join(c.OriginalsPath(), dest)
+		_ = os.RemoveAll(outDir)
+	}
+}
+
+func TestDownloadImpl_FileMethod_Skip_NoRemux(t *testing.T) {
+    fake := createFakeYtDlp(t)
+    orig := dl.YtDlpBin
+    defer func() { dl.YtDlpBin = orig }()
+
+    dest := "dl-e2e-skip"
+    // Ensure different file content so duplicate detection won't collapse into prior test's file
+    t.Setenv("YTDLP_DUMMY_CONTENT", "dummy2")
+    if c := get.Config(); c != nil {
+        c.Options().FFmpegBin = "/bin/false" // would fail if remux attempted
+        s := c.Settings()
+        s.Index.Convert = false
+    }
+	conf := get.Config()
+	if conf == nil {
 		t.Fatalf("missing test config")
 	}
+	_ = conf.Init()
+	conf.RegisterDb()
+	dl.YtDlpBin = fake
+
+	if err := runDownload(conf, DownloadOpts{
+		Dest:      dest,
+		Method:    "file",
+		FileRemux: "skip",
+	}, []string{"https://example.com/video"}); err != nil {
+		t.Fatalf("runDownload failed with skip remux: %v", err)
+	}
+
+	// Verify an mp4 exists under Originals/dest
+	c := get.Config()
 	outDir := filepath.Join(c.OriginalsPath(), dest)
 	found := false
 	_ = filepath.WalkDir(outDir, func(path string, d os.DirEntry, err error) error {
@@ -95,7 +128,39 @@ func TestDownloadImpl_FileMethod_AutoSkipsRemux(t *testing.T) {
 	if !found {
 		t.Fatalf("expected at least one mp4 in %s", outDir)
 	}
-
-	// Cleanup destination folder
+	_ = os.RemoveAll(outDir)
+}
+
+func TestDownloadImpl_FileMethod_Always_RemuxFails(t *testing.T) {
+	fake := createFakeYtDlp(t)
+	orig := dl.YtDlpBin
+	defer func() { dl.YtDlpBin = orig }()
+
+	dest := "dl-e2e-always"
+	if c := get.Config(); c != nil {
+		c.Options().FFmpegBin = "/bin/false" // force remux failure when called
+		s := c.Settings()
+		s.Index.Convert = false
+	}
+	conf := get.Config()
+	if conf == nil {
+		t.Fatalf("missing test config")
+	}
+	_ = conf.Init()
+	conf.RegisterDb()
+	dl.YtDlpBin = fake
+
+	err := runDownload(conf, DownloadOpts{
+		Dest:      dest,
+		Method:    "file",
+		FileRemux: "always",
+	}, []string{"https://example.com/video"})
+	if err == nil {
+		t.Fatalf("expected failure when remux is required but ffmpeg is unavailable")
+	}
+
+	// Cleanup destination folder if anything was created
+	c := get.Config()
+	outDir := filepath.Join(c.OriginalsPath(), dest)
 	_ = os.RemoveAll(outDir)
 }
--- a/internal/commands/download_help_test.go
+++ b/internal/commands/download_help_test.go
@@ -13,6 +13,7 @@ func TestDownloadCommand_HelpFlagsAndArgs(t *testing.T) {
 		"cookies":    false,
 		"add-header": false,
 		"dl-method":  false,
+		"file-remux": false,
 	}
 	for _, f := range DownloadCommand.Flags {
 		name := f.Names()[0]
--- a/internal/commands/download_impl.go
+++ b/internal/commands/download_impl.go
@@ -71,7 +71,7 @@ func runDownload(conf *config.Config, opts DownloadOpts, inputURLs []string) err
 	}
 	fileRemux := strings.ToLower(strings.TrimSpace(opts.FileRemux))
 	if fileRemux == "" {
-		fileRemux = "always"
+		fileRemux = "auto"
 	}
 	switch fileRemux {
 	case "always", "auto", "skip":
@@ -129,6 +129,12 @@ func runDownload(conf *config.Config, opts DownloadOpts, inputURLs []string) err
 				failures++
 				continue
 			}
+
+			// Best-effort creation time for file method when not remuxing locally.
+			if created := dl.CreatedFromInfo(result.Info); !created.IsZero() {
+				// Apply via yt-dlp ffmpeg post-processor so creation_time exists even without our remux.
+				result.Options.FFmpegPostArgs = "-metadata creation_time=" + created.UTC().Format(time.RFC3339)
+			}
 			if dlName := clean.DlName(result.Info.Title); dlName != "" {
 				downloadFile = dlName + fs.ExtMp4
 			} else {
--- a/internal/photoprism/dl/README.md
+++ b/internal/photoprism/dl/README.md
@@ -8,7 +8,8 @@ It currently supports two invocation methods:

 ## Auth & Headers

- Supports `--cookies` and repeatable `--add-header` for both metadata and download flows.
+- Supports `--cookies`, `--cookies-from-browser BROWSER[:PROFILE]`, and repeatable `--add-header` for both metadata and download flows.  
+  - Container note: The `photoprism dl` CLI runs in a container by default and therefore does not expose a `--cookies-from-browser` flag (no access to local browser profiles). Use `--cookies <path>` with a Netscape cookies.txt file.
 - Secrets are never logged; header values are redacted in trace logs.

 ## Key APIs
@@ -27,4 +28,3 @@ It currently supports two invocation methods:

 - Prefer the file method for sources with separate audio/video streams; the pipe method cannot always merge in that case.
 - When the CLI’s `--file-remux=auto` is used, the final ffmpeg remux is skipped for MP4 outputs that already include metadata.
-
--- a/internal/photoprism/dl/file.go
+++ b/internal/photoprism/dl/file.go
@@ -166,6 +166,10 @@ func (result Metadata) DownloadToFileWithOptions(

 	cmd.Dir = tempPath

+	if strings.TrimSpace(result.Options.FFmpegPostArgs) != "" {
+		cmd.Args = append(cmd.Args, "--postprocessor-args", "ffmpeg:"+result.Options.FFmpegPostArgs)
+	}
+
 	// Capture stdout/stderr for parsing results and errors
 	stdoutBuf := &bytes.Buffer{}
 	stderrBuf := &bytes.Buffer{}
--- a/internal/photoprism/dl/meta.go
+++ b/internal/photoprism/dl/meta.go
@@ -10,6 +10,37 @@ import (
 	"github.com/photoprism/photoprism/pkg/fs"
 )

+// CreatedFromInfo returns the best-effort creation time from Info.
+// Priority:
+// 1) Timestamp (UNIX seconds),
+// 2) UploadDate (YYYYMMDD),
+// 3) ReleaseDate (YYYYMMDD).
+// Returns zero time if none can be parsed.
+func CreatedFromInfo(info Info) time.Time {
+	if info.Timestamp > 1 {
+		sec, dec := math.Modf(info.Timestamp)
+		return time.Unix(int64(sec), int64(dec*(1e9))).UTC()
+	}
+	parseYYYYMMDD := func(s string) (time.Time, bool) {
+		s = strings.TrimSpace(s)
+		if len(s) != 8 {
+			return time.Time{}, false
+		}
+		t, err := time.Parse("20060102", s)
+		if err != nil {
+			return time.Time{}, false
+		}
+		return t.UTC(), true
+	}
+	if t, ok := parseYYYYMMDD(info.UploadDate); ok {
+		return t
+	}
+	if t, ok := parseYYYYMMDD(info.ReleaseDate); ok {
+		return t
+	}
+	return time.Time{}
+}
+
 // RemuxOptionsFromInfo builds ffmpeg remux options (container + metadata)
 // based on yt-dlp Info and the source URL. The returned options enforce the
 // target container and set title, description, author, comment, and created
@@ -40,9 +71,8 @@ func RemuxOptionsFromInfo(ffmpegBin string, container fs.Type, info Info, source
 		opt.Author = author
 	}

-	if info.Timestamp > 1 {
-		sec, dec := math.Modf(info.Timestamp)
-		opt.Created = time.Unix(int64(sec), int64(dec*(1e9)))
+	if created := CreatedFromInfo(info); !created.IsZero() {
+		opt.Created = created
 	}

 	return opt
--- a/internal/photoprism/dl/meta_test.go
+++ b/internal/photoprism/dl/meta_test.go
@@ -32,3 +32,31 @@ func TestRemuxOptionsFromInfo(t *testing.T) {
 		t.Fatalf("Created timestamp should be set")
 	}
 }
+
+func TestCreatedFromInfo_UploadDateFallback(t *testing.T) {
+	info := Info{
+		Title:      "X",
+		UploadDate: "20211120",
+	}
+	created := CreatedFromInfo(info)
+	if created.IsZero() {
+		t.Fatalf("expected created time from UploadDate fallback")
+	}
+	if got, want := created.UTC().Format(time.RFC3339), "2021-11-20T00:00:00Z"; got != want {
+		t.Fatalf("created mismatch: got %s want %s", got, want)
+	}
+}
+
+func TestCreatedFromInfo_ReleaseDateFallback(t *testing.T) {
+	info := Info{
+		Title:       "Y",
+		ReleaseDate: "20190501",
+	}
+	created := CreatedFromInfo(info)
+	if created.IsZero() {
+		t.Fatalf("expected created time from ReleaseDate fallback")
+	}
+	if got, want := created.UTC().Format(time.RFC3339), "2019-05-01T00:00:00Z"; got != want {
+		t.Fatalf("created mismatch: got %s want %s", got, want)
+	}
+}
--- a/internal/photoprism/dl/options.go
+++ b/internal/photoprism/dl/options.go
@@ -24,20 +24,27 @@ type Options struct {
 	Downloader         string // --downloader
 	DownloadThumbnail  bool
 	DownloadSubtitles  bool
-	DownloadSections   string                        // --download-sections
-	Impersonate        string                        // --impersonate
-	ProxyUrl           string                        // --proxy URL  http://host:port or socks5://host:port
-	UseIPV4            bool                          // -4 Make all connections via IPv4
-	Cookies            string                        // --cookies FILE
-	CookiesFromBrowser string                        // --cookies-from-browser BROWSER[:FOLDER]
-	AddHeaders         []string                      // --add-header "Name: Value" (repeatable)
-	StderrFn           func(cmd *exec.Cmd) io.Writer // if not nil, function to get Writer for stderr
-	HttpClient         *http.Client                  // Client for download thumbnail and subtitles (nil use http.DefaultClient)
-	MergeOutputFormat  string                        // --merge-output-format
-	RemuxVideo         string                        // --remux-video
-	RecodeVideo        string                        // --recode-video
-	Fixup              string                        // --fixup
-	SortingFormat      string                        // --format-sort
+	DownloadSections   string // --download-sections
+	Impersonate        string // --impersonate
+	ProxyUrl           string // --proxy URL  http://host:port or socks5://host:port
+	UseIPV4            bool   // -4 Make all connections via IPv4
+	Cookies            string // --cookies FILE
+	CookiesFromBrowser string // --cookies-from-browser BROWSER[:FOLDER]
+	// Note: The PhotoPrism CLI intentionally does NOT expose a --cookies-from-browser flag
+	// because the default runtime is a container without access to local browser profiles.
+	// This field remains for tests and non-container scenarios.
+	AddHeaders        []string                      // --add-header "Name: Value" (repeatable)
+	StderrFn          func(cmd *exec.Cmd) io.Writer // if not nil, function to get Writer for stderr
+	HttpClient        *http.Client                  // Client for download thumbnail and subtitles (nil use http.DefaultClient)
+	MergeOutputFormat string                        // --merge-output-format
+	RemuxVideo        string                        // --remux-video
+	RecodeVideo       string                        // --recode-video
+	Fixup             string                        // --fixup
+	SortingFormat     string                        // --format-sort
+	// FFmpegPostArgs are additional ffmpeg post-processor args, e.g.,
+	// "-metadata creation_time=2025-09-20T00:00:00Z". Applied via
+	// yt-dlp's --postprocessor-args for the ffmpeg post-processor.
+	FFmpegPostArgs string

 	// Set to true if you don't want to use the result.Info structure after the goutubedl.NewMetadata() call,
 	// so the given URL will be downloaded in a single pass in the DownloadResult.Download() call.
@@ -240,6 +247,10 @@ func (result Metadata) DownloadWithOptions(
 		)
 	}

+	if strings.TrimSpace(result.Options.FFmpegPostArgs) != "" {
+		cmd.Args = append(cmd.Args, "--postprocessor-args", "ffmpeg:"+result.Options.FFmpegPostArgs)
+	}
+
 	cmd.Dir = tempPath
 	var stdoutW io.WriteCloser
 	var stderrW io.WriteCloser
--- a/internal/photoprism/dl/options_headers_test.go
+++ b/internal/photoprism/dl/options_headers_test.go
@@ -170,3 +170,73 @@ func TestDownloadToFileWithOptions_PrintsAndCreatesFiles(t *testing.T) {
 		t.Fatalf("expected file to exist: %v", statErr)
 	}
 }
+
+func TestDownloadToFileWithOptions_IncludesPostprocessorArgs(t *testing.T) {
+	fake := createFakeYtDlp(t)
+	orig := YtDlpBin
+	YtDlpBin = fake
+	defer func() { YtDlpBin = orig }()
+
+	argsLog := filepath.Join(t.TempDir(), "args.log")
+	t.Setenv("YTDLP_ARGS_LOG", argsLog)
+
+	outDir := t.TempDir()
+	outFile := filepath.Join(outDir, "ppdl_test.mp4")
+	t.Setenv("YTDLP_OUTPUT_FILE", outFile)
+
+	r := Metadata{
+		RawURL: "https://example.com/v",
+		Options: Options{
+			noInfoDownload: true,
+			FFmpegPostArgs: "-metadata creation_time=2021-11-20T00:00:00Z",
+		},
+	}
+	_, err := r.DownloadToFileWithOptions(context.Background(), DownloadOptions{Output: filepath.Join(outDir, "ppdl_%(id)s.%(ext)s")})
+	if err != nil {
+		t.Fatalf("DownloadToFileWithOptions error: %v", err)
+	}
+
+	data, err := os.ReadFile(argsLog)
+	if err != nil {
+		t.Fatalf("reading args log failed: %v", err)
+	}
+	s := string(data)
+	if !strings.Contains(s, "--postprocessor-args") || !strings.Contains(s, "ffmpeg:-metadata creation_time=2021-11-20T00:00:00Z") {
+		t.Fatalf("missing postprocessor args in yt-dlp invocation: %s", s)
+	}
+}
+
+func TestDownloadWithOptions_IncludesPostprocessorArgs_Pipe(t *testing.T) {
+	fake := createFakeYtDlp(t)
+	orig := YtDlpBin
+	YtDlpBin = fake
+	defer func() { YtDlpBin = orig }()
+
+	argsLog := filepath.Join(t.TempDir(), "args.log")
+	t.Setenv("YTDLP_ARGS_LOG", argsLog)
+
+	r := Metadata{
+		RawURL: "https://example.com/v",
+		Options: Options{
+			noInfoDownload: true,
+			FFmpegPostArgs: "-metadata creation_time=2021-11-20T00:00:00Z",
+		},
+	}
+	dr, err := r.DownloadWithOptions(context.Background(), DownloadOptions{})
+	if err != nil {
+		t.Fatalf("DownloadWithOptions error: %v", err)
+	}
+	// Read a bit and close to finish the process
+	buf := make([]byte, 4)
+	_, _ = dr.Read(buf)
+	_ = dr.Close()
+
+	data, err := os.ReadFile(argsLog)
+	if err != nil {
+		t.Fatalf("reading args log failed: %v", err)
+	}
+	s := string(data)
+	if !strings.Contains(s, "--postprocessor-args") || !strings.Contains(s, "ffmpeg:-metadata creation_time=2021-11-20T00:00:00Z") {
+		t.Fatalf("missing postprocessor args in yt-dlp invocation: %s", s)
+	}
+}