From c6e602e5f6d08bbc6635a9a6cbe951f8e904a90e Mon Sep 17 00:00:00 2001 From: Maksym Pavlenko Date: Sat, 7 Mar 2020 14:51:38 -0800 Subject: [PATCH] Refactor ytdl --- cmd/podsync/updater.go | 59 +++++++++++------- cmd/podsync/updater_test.go | 24 +++++++ pkg/ytdl/options.go | 39 ------------ pkg/ytdl/options_audio.go | 33 ---------- pkg/ytdl/options_audio_test.go | 67 -------------------- pkg/ytdl/options_video.go | 50 --------------- pkg/ytdl/options_video_test.go | 110 --------------------------------- pkg/ytdl/ytdl.go | 72 +++++++++++++++++++-- pkg/ytdl/ytdl_test.go | 109 ++++++++++++++++++++++++++++++++ 9 files changed, 236 insertions(+), 327 deletions(-) delete mode 100644 pkg/ytdl/options.go delete mode 100644 pkg/ytdl/options_audio.go delete mode 100644 pkg/ytdl/options_audio_test.go delete mode 100644 pkg/ytdl/options_video.go delete mode 100644 pkg/ytdl/options_video_test.go create mode 100644 pkg/ytdl/ytdl_test.go diff --git a/cmd/podsync/updater.go b/cmd/podsync/updater.go index 9138e392..108bed5f 100644 --- a/cmd/podsync/updater.go +++ b/cmd/podsync/updater.go @@ -3,6 +3,7 @@ package main import ( "context" "fmt" + "io" "io/ioutil" "os" "path/filepath" @@ -20,10 +21,11 @@ import ( "github.com/mxpv/podsync/pkg/feed" "github.com/mxpv/podsync/pkg/link" "github.com/mxpv/podsync/pkg/model" + "github.com/mxpv/podsync/pkg/ytdl" ) type Downloader interface { - Download(ctx context.Context, feedConfig *config.Feed, episode *model.Episode, feedPath string) (string, error) + Download(ctx context.Context, feedConfig *config.Feed, episode *model.Episode) (io.ReadCloser, error) } type Updater struct { @@ -175,16 +177,16 @@ func (u *Updater) downloadEpisodes(ctx context.Context, feedConfig *config.Feed, } // Download episode to disk + // We download the episode to a temp directory first to avoid downloading this file by clients + // while still being processed by youtube-dl (e.g. a file is being downloaded from YT or encoding in progress) logger.Infof("! downloading episode %s", episode.VideoURL) - output, err := u.downloader.Download(ctx, feedConfig, episode, episodePath) + tempFile, err := u.downloader.Download(ctx, feedConfig, episode) if err != nil { - logger.WithError(err).Errorf("youtube-dl error: %s", output) - // YouTube might block host with HTTP Error 429: Too Many Requests // We still need to generate XML, so just stop sending download requests and // retry next time - if strings.Contains(output, "HTTP Error 429") { + if err == ytdl.ErrTooManyRequests { break } @@ -198,20 +200,22 @@ func (u *Updater) downloadEpisodes(ctx context.Context, feedConfig *config.Feed, continue } + logger.Debugf("copying file to: %s", episodePath) + fileSize, err := copyFile(tempFile, episodePath) + + tempFile.Close() + + if err != nil { + logger.WithError(err).Error("failed to copy file") + return err + } + logger.Debugf("copied %d bytes", episode.Size) + // Update file status in database logger.Infof("successfully downloaded file %q", episode.ID) - if err := u.db.UpdateEpisode(feedID, episode.ID, func(episode *model.Episode) error { - // Record file size of newly downloaded file - size, err := u.fileSize(episodePath) - if err != nil { - logger.WithError(err).Error("failed to get episode file size") - } else { - logger.Debugf("file size: %d bytes", episode.Size) - episode.Size = size - } - + episode.Size = fileSize episode.Status = model.EpisodeDownloaded return nil }); err != nil { @@ -225,6 +229,22 @@ func (u *Updater) downloadEpisodes(ctx context.Context, feedConfig *config.Feed, return nil } +func copyFile(source io.Reader, destinationPath string) (int64, error) { + dest, err := os.Create(destinationPath) + if err != nil { + return 0, errors.Wrap(err, "failed to create destination file") + } + + defer dest.Close() + + written, err := io.Copy(dest, source) + if err != nil { + return 0, errors.Wrap(err, "failed to copy data") + } + + return written, nil +} + func (u *Updater) buildXML(ctx context.Context, feedConfig *config.Feed) error { feed, err := u.db.GetFeed(ctx, feedConfig.ID) if err != nil { @@ -363,15 +383,6 @@ func (u *Updater) episodeName(feedConfig *config.Feed, episode *model.Episode) s return fmt.Sprintf("%s.%s", episode.ID, ext) } -func (u *Updater) fileSize(path string) (int64, error) { - info, err := os.Stat(path) - if err != nil { - return 0, err - } - - return info.Size(), nil -} - func (u *Updater) makeBuilder(ctx context.Context, cfg *config.Feed) (feed.Builder, error) { var ( provider feed.Builder diff --git a/cmd/podsync/updater_test.go b/cmd/podsync/updater_test.go index d0f8076e..556af65e 100644 --- a/cmd/podsync/updater_test.go +++ b/cmd/podsync/updater_test.go @@ -1,9 +1,14 @@ package main import ( + "bytes" + "io/ioutil" + "os" + "path/filepath" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/mxpv/podsync/pkg/config" ) @@ -24,3 +29,22 @@ func TestUpdater_hostname(t *testing.T) { u.config.Server.Hostname = "https://localhost:8080/" assert.Equal(t, "https://localhost:8080", u.hostname()) } + +func TestCopyFile(t *testing.T) { + reader := bytes.NewReader([]byte{1, 2, 4}) + + tmpDir, err := ioutil.TempDir("", "podsync-test-") + require.NoError(t, err) + + defer os.RemoveAll(tmpDir) + + file := filepath.Join(tmpDir, "1") + + size, err := copyFile(reader, file) + assert.NoError(t, err) + assert.EqualValues(t, 3, size) + + stat, err := os.Stat(file) + assert.NoError(t, err) + assert.EqualValues(t, 3, stat.Size()) +} diff --git a/pkg/ytdl/options.go b/pkg/ytdl/options.go deleted file mode 100644 index 9f78c601..00000000 --- a/pkg/ytdl/options.go +++ /dev/null @@ -1,39 +0,0 @@ -package ytdl - -import ( - "fmt" - "path/filepath" - - "github.com/mxpv/podsync/pkg/config" - "github.com/mxpv/podsync/pkg/model" -) - -type Options interface { - GetConfig() []string -} - -type OptionsDl struct{} - -func (o OptionsDl) New(feedConfig *config.Feed, episode *model.Episode, feedPath string) []string { - - var ( - arguments []string - options Options - ) - - if feedConfig.Format == model.FormatVideo { - options = NewOptionsVideo(feedConfig) - } else { - options = NewOptionsAudio(feedConfig) - } - - arguments = options.GetConfig() - arguments = append(arguments, "--output", o.makeOutputTemplate(feedPath, episode), episode.VideoURL) - - return arguments -} - -func (o OptionsDl) makeOutputTemplate(feedPath string, episode *model.Episode) string { - filename := fmt.Sprintf("%s.%s", episode.ID, "%(ext)s") - return filepath.Join(feedPath, filename) -} diff --git a/pkg/ytdl/options_audio.go b/pkg/ytdl/options_audio.go deleted file mode 100644 index cf9144aa..00000000 --- a/pkg/ytdl/options_audio.go +++ /dev/null @@ -1,33 +0,0 @@ -package ytdl - -import ( - "github.com/mxpv/podsync/pkg/config" - "github.com/mxpv/podsync/pkg/model" -) - -type OptionsAudio struct { - quality model.Quality -} - -func NewOptionsAudio(feedConfig *config.Feed) *OptionsAudio { - options := &OptionsAudio{} - options.quality = feedConfig.Quality - - return options -} - -func (options OptionsAudio) GetConfig() []string { - var arguments []string - - arguments = append(arguments, "--extract-audio", "--audio-format", "mp3") - - switch options.quality { - case model.QualityLow: - // really? somebody use it? - arguments = append(arguments, "--format", "worstaudio") - default: - arguments = append(arguments, "--format", "bestaudio") - } - - return arguments -} diff --git a/pkg/ytdl/options_audio_test.go b/pkg/ytdl/options_audio_test.go deleted file mode 100644 index 281cbe09..00000000 --- a/pkg/ytdl/options_audio_test.go +++ /dev/null @@ -1,67 +0,0 @@ -package ytdl - -import ( - "reflect" - "testing" - - "github.com/mxpv/podsync/pkg/config" - "github.com/mxpv/podsync/pkg/model" -) - -func TestNewOptionsAudio(t *testing.T) { - type args struct { - feedConfig *config.Feed - } - tests := []struct { - name string - args args - want *OptionsAudio - }{ - { - "Get OptionsAudio in low quality", - args{ - feedConfig: &config.Feed{Quality: model.QualityLow}, - }, - &OptionsAudio{quality: model.QualityLow}, - }, - { - "Get OptionsAudio in high quality", - args{ - feedConfig: &config.Feed{Quality: model.QualityHigh}, - }, - &OptionsAudio{quality: model.QualityHigh}, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if got := NewOptionsAudio(tt.args.feedConfig); !reflect.DeepEqual(got, tt.want) { - t.Errorf("NewOptionsAudio() = %v, want %v", got, tt.want) - } - }) - } -} - -func TestOptionsAudio_GetConfig(t *testing.T) { - type fields struct { - quality model.Quality - } - tests := []struct { - name string - fields fields - want []string - }{ - {"OptionsAudio in unknown quality", fields{quality: model.Quality("unknown")}, []string{"--extract-audio", "--audio-format", "mp3", "--format", "bestaudio"}}, - {"OptionsAudio in low quality", fields{quality: model.Quality("low")}, []string{"--extract-audio", "--audio-format", "mp3", "--format", "worstaudio"}}, - {"OptionsAudio in high quality", fields{quality: model.Quality("high")}, []string{"--extract-audio", "--audio-format", "mp3", "--format", "bestaudio"}}, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - options := OptionsAudio{ - quality: tt.fields.quality, - } - if got := options.GetConfig(); !reflect.DeepEqual(got, tt.want) { - t.Errorf("GetConfig() = %v, want %v", got, tt.want) - } - }) - } -} diff --git a/pkg/ytdl/options_video.go b/pkg/ytdl/options_video.go deleted file mode 100644 index eee01cc7..00000000 --- a/pkg/ytdl/options_video.go +++ /dev/null @@ -1,50 +0,0 @@ -package ytdl - -import ( - "fmt" - - "github.com/mxpv/podsync/pkg/config" - "github.com/mxpv/podsync/pkg/model" -) - -type OptionsVideo struct { - quality model.Quality - maxHeight int -} - -func NewOptionsVideo(feedConfig *config.Feed) *OptionsVideo { - options := &OptionsVideo{} - - options.quality = feedConfig.Quality - options.maxHeight = feedConfig.MaxHeight - - return options -} - -func (options OptionsVideo) GetConfig() []string { - var ( - arguments []string - format string - ) - - switch options.quality { - // I think after enabling MaxHeight param QualityLow option don't need. - // If somebody want download video in low quality then can set MaxHeight to 360p - // ¯\_(ツ)_/¯ - case model.QualityLow: - format = "worstvideo[ext=mp4]+worstaudio[ext=m4a]/worst[ext=mp4]/worst" - default: - format = "bestvideo%s[ext=mp4]+bestaudio[ext=m4a]/best[ext=mp4]/best" - - if options.maxHeight > 0 { - format = fmt.Sprintf(format, fmt.Sprintf("[height<=%d]", options.maxHeight)) - } else { - // unset replace pattern - format = fmt.Sprintf(format, "") - } - } - - arguments = append(arguments, "--format", format) - - return arguments -} diff --git a/pkg/ytdl/options_video_test.go b/pkg/ytdl/options_video_test.go deleted file mode 100644 index e0cc7f75..00000000 --- a/pkg/ytdl/options_video_test.go +++ /dev/null @@ -1,110 +0,0 @@ -package ytdl - -import ( - "reflect" - "testing" - - "github.com/mxpv/podsync/pkg/config" - "github.com/mxpv/podsync/pkg/model" -) - -func TestNewOptionsVideo(t *testing.T) { - type args struct { - feedConfig *config.Feed - } - tests := []struct { - name string - args args - want *OptionsVideo - }{ - { - "Get OptionsVideo in low quality", - args{ - feedConfig: &config.Feed{Quality: model.QualityLow}, - }, - &OptionsVideo{quality: model.QualityLow}, - }, - { - "Get OptionsVideo in low quality with maxheight", - args{ - feedConfig: &config.Feed{Quality: model.QualityLow, MaxHeight: 720}, - }, - &OptionsVideo{quality: model.QualityLow, maxHeight: 720}, - }, - { - "Get OptionsVideo in high quality", - args{ - feedConfig: &config.Feed{Quality: model.QualityHigh}, - }, - &OptionsVideo{quality: model.QualityHigh}, - }, - { - "Get OptionsVideo in high quality with maxheight", - args{ - feedConfig: &config.Feed{Quality: model.QualityHigh, MaxHeight: 720}, - }, - &OptionsVideo{quality: model.QualityHigh, maxHeight: 720}, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if got := NewOptionsVideo(tt.args.feedConfig); !reflect.DeepEqual(got, tt.want) { - t.Errorf("NewOptionsVideo() = %v, want %v", got, tt.want) - } - }) - } -} - -func TestOptionsVideo_GetConfig(t *testing.T) { - type fields struct { - quality model.Quality - maxHeight int - } - tests := []struct { - name string - fields fields - want []string - }{ - { - "OptionsVideo in unknown quality", - fields{quality: model.Quality("unknown")}, - []string{"--format", "bestvideo[ext=mp4]+bestaudio[ext=m4a]/best[ext=mp4]/best"}, - }, - { - "OptionsVideo in unknown quality with maxheight", - fields{quality: model.Quality("unknown"), maxHeight: 720}, - []string{"--format", "bestvideo[height<=720][ext=mp4]+bestaudio[ext=m4a]/best[ext=mp4]/best"}, - }, - { - "OptionsVideo in low quality", - fields{quality: model.QualityLow}, - []string{"--format", "worstvideo[ext=mp4]+worstaudio[ext=m4a]/worst[ext=mp4]/worst"}, - }, - { - "OptionsVideo in low quality with maxheight", - fields{quality: model.QualityLow, maxHeight: 720}, - []string{"--format", "worstvideo[ext=mp4]+worstaudio[ext=m4a]/worst[ext=mp4]/worst"}, - }, - { - "OptionsVideo in high quality", - fields{quality: model.QualityHigh}, - []string{"--format", "bestvideo[ext=mp4]+bestaudio[ext=m4a]/best[ext=mp4]/best"}, - }, - { - "OptionsVideo in high quality with maxheight", - fields{quality: model.QualityHigh, maxHeight: 720}, - []string{"--format", "bestvideo[height<=720][ext=mp4]+bestaudio[ext=m4a]/best[ext=mp4]/best"}, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - options := OptionsVideo{ - quality: tt.fields.quality, - maxHeight: tt.fields.maxHeight, - } - if got := options.GetConfig(); !reflect.DeepEqual(got, tt.want) { - t.Errorf("GetConfig() = %v, want %v", got, tt.want) - } - }) - } -} diff --git a/pkg/ytdl/ytdl.go b/pkg/ytdl/ytdl.go index e2334ce2..409ade0d 100644 --- a/pkg/ytdl/ytdl.go +++ b/pkg/ytdl/ytdl.go @@ -2,7 +2,14 @@ package ytdl import ( "context" + "fmt" + "io" + "io/ioutil" + "net/http" + "os" "os/exec" + "path/filepath" + "strings" "time" "github.com/pkg/errors" @@ -14,6 +21,10 @@ import ( const DownloadTimeout = 10 * time.Minute +var ( + ErrTooManyRequests = errors.New(http.StatusText(http.StatusTooManyRequests)) +) + type YoutubeDl struct{} func New(ctx context.Context) (*YoutubeDl, error) { @@ -38,13 +49,37 @@ func New(ctx context.Context) (*YoutubeDl, error) { return ytdl, nil } -func (dl YoutubeDl) Download(ctx context.Context, feedConfig *config.Feed, episode *model.Episode, feedPath string) (string, error) { - options := &OptionsDl{} +func (dl YoutubeDl) Download(ctx context.Context, feedConfig *config.Feed, episode *model.Episode) (io.ReadCloser, error) { + tmpDir, err := ioutil.TempDir("", "podsync-") + if err != nil { + return nil, errors.Wrap(err, "failed to get temp dir for download") + } - params := options.New(feedConfig, episode, feedPath) + ext := "mp4" + if feedConfig.Format == model.FormatAudio { + ext = "mp3" + } + filePath := filepath.Join(tmpDir, fmt.Sprintf("%s.%s", episode.ID, ext)) - return dl.exec(ctx, params...) + args := buildArgs(feedConfig, episode, filePath) + output, err := dl.exec(ctx, args...) + if err != nil { + log.WithError(err).Errorf("youtube-dl error: %s", filePath) + // YouTube might block host with HTTP Error 429: Too Many Requests + if strings.Contains(output, "HTTP Error 429") { + return nil, ErrTooManyRequests + } + + return nil, errors.New(output) + } + + f, err := os.Open(filePath) + if err != nil { + return nil, errors.Wrap(err, "failed to open downloaded file") + } + + return f, nil } func (YoutubeDl) exec(ctx context.Context, args ...string) (string, error) { @@ -60,3 +95,32 @@ func (YoutubeDl) exec(ctx context.Context, args ...string) (string, error) { return string(output), nil } + +func buildArgs(feedConfig *config.Feed, episode *model.Episode, outputFilePath string) []string { + var args []string + + if feedConfig.Format == model.FormatVideo { + // Video, mp4, high by default + + format := "bestvideo[ext=mp4]+bestaudio[ext=m4a]/best[ext=mp4]/best" + + if feedConfig.Quality == model.QualityLow { + format = "worstvideo[ext=mp4]+worstaudio[ext=m4a]/worst[ext=mp4]/worst" + } else if feedConfig.Quality == model.QualityHigh && feedConfig.MaxHeight > 0 { + format = fmt.Sprintf("bestvideo[height<=%d][ext=mp4]+bestaudio[ext=m4a]/best[ext=mp4]/best", feedConfig.MaxHeight) + } + + args = append(args, "--format", format) + } else { + // Audio, mp3, high by default + format := "bestaudio" + if feedConfig.Quality == model.QualityLow { + format = "worstaudio" + } + + args = append(args, "--extract-audio", "--audio-format", "mp3", "--format", format) + } + + args = append(args, "--output", outputFilePath, episode.VideoURL) + return args +} diff --git a/pkg/ytdl/ytdl_test.go b/pkg/ytdl/ytdl_test.go new file mode 100644 index 00000000..8b2581c4 --- /dev/null +++ b/pkg/ytdl/ytdl_test.go @@ -0,0 +1,109 @@ +package ytdl + +import ( + "testing" + + "github.com/mxpv/podsync/pkg/config" + "github.com/mxpv/podsync/pkg/model" + + "github.com/stretchr/testify/assert" +) + +func TestBuildArgs(t *testing.T) { + tests := []struct { + name string + format model.Format + quality model.Quality + maxHeight int + output string + videoURL string + expect []string + }{ + { + name: "Audio unknown quality", + format: model.FormatAudio, + output: "/tmp/1", + videoURL: "http://url", + expect: []string{"--extract-audio", "--audio-format", "mp3", "--format", "bestaudio", "--output", "/tmp/1", "http://url"}, + }, + { + name: "Audio low quality", + format: model.FormatAudio, + quality: model.QualityLow, + output: "/tmp/1", + videoURL: "http://url", + expect: []string{"--extract-audio", "--audio-format", "mp3", "--format", "worstaudio", "--output", "/tmp/1", "http://url"}, + }, + { + name: "Audio best quality", + format: model.FormatAudio, + quality: model.QualityHigh, + output: "/tmp/1", + videoURL: "http://url", + expect: []string{"--extract-audio", "--audio-format", "mp3", "--format", "bestaudio", "--output", "/tmp/1", "http://url"}, + }, + { + name: "Video unknown quality", + format: model.FormatVideo, + output: "/tmp/1", + videoURL: "http://url", + expect: []string{"--format", "bestvideo[ext=mp4]+bestaudio[ext=m4a]/best[ext=mp4]/best", "--output", "/tmp/1", "http://url"}, + }, + { + name: "Video unknown quality with maxheight", + format: model.FormatVideo, + maxHeight: 720, + output: "/tmp/1", + videoURL: "http://url", + expect: []string{"--format", "bestvideo[ext=mp4]+bestaudio[ext=m4a]/best[ext=mp4]/best", "--output", "/tmp/1", "http://url"}, + }, + { + name: "Video low quality", + format: model.FormatVideo, + quality: model.QualityLow, + output: "/tmp/2", + videoURL: "http://url", + expect: []string{"--format", "worstvideo[ext=mp4]+worstaudio[ext=m4a]/worst[ext=mp4]/worst", "--output", "/tmp/2", "http://url"}, + }, + { + name: "Video low quality with maxheight", + format: model.FormatVideo, + quality: model.QualityLow, + maxHeight: 720, + output: "/tmp/2", + videoURL: "http://url", + expect: []string{"--format", "worstvideo[ext=mp4]+worstaudio[ext=m4a]/worst[ext=mp4]/worst", "--output", "/tmp/2", "http://url"}, + }, + { + name: "Video high quality", + format: model.FormatVideo, + quality: model.QualityHigh, + output: "/tmp/2", + videoURL: "http://url1", + expect: []string{"--format", "bestvideo[ext=mp4]+bestaudio[ext=m4a]/best[ext=mp4]/best", "--output", "/tmp/2", "http://url1"}, + }, + { + name: "Video high quality with maxheight", + format: model.FormatVideo, + quality: model.QualityHigh, + maxHeight: 1024, + output: "/tmp/2", + videoURL: "http://url1", + expect: []string{"--format", "bestvideo[height<=1024][ext=mp4]+bestaudio[ext=m4a]/best[ext=mp4]/best", "--output", "/tmp/2", "http://url1"}, + }, + } + + for _, tst := range tests { + t.Run(tst.name, func(t *testing.T) { + result := buildArgs(&config.Feed{ + Format: tst.format, + Quality: tst.quality, + MaxHeight: tst.maxHeight, + }, &model.Episode{ + VideoURL: tst.videoURL, + }, tst.output) + + assert.EqualValues(t, tst.expect, result) + }) + } +}