From abc9ec648a48d5eee4b4b743b5efecdffbd0cd8c Mon Sep 17 00:00:00 2001 From: alexandra-3 <118532397+alexandra-3@users.noreply.github.com> Date: Mon, 21 Nov 2022 16:21:27 +1000 Subject: [PATCH] Fix a few cases where ffmpeg produces no output (#3161) * Treat no output from ffmpeg as an error condition * Distinguish file vs. video duration, and use later where appropriate * Check for empty file in generateFile Co-authored-by: WithoutPants <53250216+WithoutPants@users.noreply.github.com> --- internal/manager/generator.go | 6 ++--- internal/manager/generator_sprite.go | 10 ++++---- internal/manager/task_generate_preview.go | 2 +- pkg/ffmpeg/ffprobe.go | 30 +++++++++++++++-------- pkg/file/video/scan.go | 2 +- pkg/scene/generate/generator.go | 14 +++++++++++ 6 files changed, 44 insertions(+), 20 deletions(-) diff --git a/internal/manager/generator.go b/internal/manager/generator.go index 0b14941e2..d2ca95016 100644 --- a/internal/manager/generator.go +++ b/internal/manager/generator.go @@ -43,8 +43,8 @@ func (g *generatorInfo) calculateFrameRate(videoStream *ffmpeg.FFProbeStream) er numberOfFrames, _ := strconv.Atoi(videoStream.NbFrames) - if numberOfFrames == 0 && isValidFloat64(framerate) && g.VideoFile.Duration > 0 { // TODO: test - numberOfFrames = int(framerate * g.VideoFile.Duration) + if numberOfFrames == 0 && isValidFloat64(framerate) && g.VideoFile.VideoStreamDuration > 0 { // TODO: test + numberOfFrames = int(framerate * g.VideoFile.VideoStreamDuration) } // If we are missing the frame count or frame rate then seek through the file and extract the info with regex @@ -68,7 +68,7 @@ func (g *generatorInfo) calculateFrameRate(videoStream *ffmpeg.FFProbeStream) er "number of frames or framerate is 0. nb_frames <%s> framerate <%f> duration <%f>", videoStream.NbFrames, framerate, - g.VideoFile.Duration, + g.VideoFile.VideoStreamDuration, ) } diff --git a/internal/manager/generator_sprite.go b/internal/manager/generator_sprite.go index 72138b387..47110462d 100644 --- a/internal/manager/generator_sprite.go +++ b/internal/manager/generator_sprite.go @@ -39,12 +39,12 @@ func NewSpriteGenerator(videoFile ffmpeg.VideoFile, videoChecksum string, imageO chunkCount := rows * cols // For files with small duration / low frame count try to seek using frame number intead of seconds - if videoFile.Duration < 5 || (0 < videoFile.FrameCount && videoFile.FrameCount <= int64(chunkCount)) { // some files can have FrameCount == 0, only use SlowSeek if duration < 5 - if videoFile.Duration <= 0 { - s := fmt.Sprintf("video %s: duration(%.3f)/frame count(%d) invalid, skipping sprite creation", videoFile.Path, videoFile.Duration, videoFile.FrameCount) + if videoFile.VideoStreamDuration < 5 || (0 < videoFile.FrameCount && videoFile.FrameCount <= int64(chunkCount)) { // some files can have FrameCount == 0, only use SlowSeek if duration < 5 + if videoFile.VideoStreamDuration <= 0 { + s := fmt.Sprintf("video %s: duration(%.3f)/frame count(%d) invalid, skipping sprite creation", videoFile.Path, videoFile.VideoStreamDuration, videoFile.FrameCount) return nil, errors.New(s) } - logger.Warnf("[generator] video %s too short (%.3fs, %d frames), using frame seeking", videoFile.Path, videoFile.Duration, videoFile.FrameCount) + logger.Warnf("[generator] video %s too short (%.3fs, %d frames), using frame seeking", videoFile.Path, videoFile.VideoStreamDuration, videoFile.FrameCount) slowSeek = true // do an actual frame count of the file ( number of frames = read frames) ffprobe := GetInstance().FFProbe @@ -102,7 +102,7 @@ func (g *SpriteGenerator) generateSpriteImage() error { if !g.SlowSeek { logger.Infof("[generator] generating sprite image for %s", g.Info.VideoFile.Path) // generate `ChunkCount` thumbnails - stepSize := g.Info.VideoFile.Duration / float64(g.Info.ChunkCount) + stepSize := g.Info.VideoFile.VideoStreamDuration / float64(g.Info.ChunkCount) for i := 0; i < g.Info.ChunkCount; i++ { time := float64(i) * stepSize diff --git a/internal/manager/task_generate_preview.go b/internal/manager/task_generate_preview.go index 30ab77a9c..37ec51ec2 100644 --- a/internal/manager/task_generate_preview.go +++ b/internal/manager/task_generate_preview.go @@ -40,7 +40,7 @@ func (t *GeneratePreviewTask) Start(ctx context.Context) { videoChecksum := t.Scene.GetHash(t.fileNamingAlgorithm) - if err := t.generateVideo(videoChecksum, videoFile.Duration); err != nil { + if err := t.generateVideo(videoChecksum, videoFile.VideoStreamDuration); err != nil { logger.Errorf("error generating preview: %v", err) logErrorOutput(err) return diff --git a/pkg/ffmpeg/ffprobe.go b/pkg/ffmpeg/ffprobe.go index fc946b6c1..d7bda62db 100644 --- a/pkg/ffmpeg/ffprobe.go +++ b/pkg/ffmpeg/ffprobe.go @@ -19,15 +19,20 @@ type VideoFile struct { AudioStream *FFProbeStream VideoStream *FFProbeStream - Path string - Title string - Comment string - Container string - Duration float64 - StartTime float64 - Bitrate int64 - Size int64 - CreationTime time.Time + Path string + Title string + Comment string + Container string + // FileDuration is the declared (meta-data) duration of the *file*. + // In most cases (sprites, previews, etc.) we actually care about the duration of the video stream specifically, + // because those two can differ slightly (e.g. audio stream longer than the video stream, making the whole file + // longer). + FileDuration float64 + VideoStreamDuration float64 + StartTime float64 + Bitrate int64 + Size int64 + CreationTime time.Time VideoCodec string VideoBitrate int64 @@ -127,7 +132,7 @@ func parse(filePath string, probeJSON *FFProbeJSON) (*VideoFile, error) { result.Container = probeJSON.Format.FormatName duration, _ := strconv.ParseFloat(probeJSON.Format.Duration, 64) - result.Duration = math.Round(duration*100) / 100 + result.FileDuration = math.Round(duration*100) / 100 fileStat, err := os.Stat(filePath) if err != nil { statErr := fmt.Errorf("error statting file <%s>: %w", filePath, err) @@ -178,6 +183,11 @@ func parse(filePath string, probeJSON *FFProbeJSON) (*VideoFile, error) { result.Width = videoStream.Width result.Height = videoStream.Height } + result.VideoStreamDuration, err = strconv.ParseFloat(videoStream.Duration, 64) + if err != nil { + // Revert to the historical behaviour, which is still correct in the vast majority of cases. + result.VideoStreamDuration = result.FileDuration + } } return result, nil diff --git a/pkg/file/video/scan.go b/pkg/file/video/scan.go index 141d6cc63..1f3d7817f 100644 --- a/pkg/file/video/scan.go +++ b/pkg/file/video/scan.go @@ -49,7 +49,7 @@ func (d *Decorator) Decorate(ctx context.Context, fs file.FS, f file.File) (file AudioCodec: videoFile.AudioCodec, Width: videoFile.Width, Height: videoFile.Height, - Duration: videoFile.Duration, + Duration: videoFile.FileDuration, FrameRate: videoFile.FrameRate, BitRate: videoFile.Bitrate, Interactive: interactive, diff --git a/pkg/scene/generate/generator.go b/pkg/scene/generate/generator.go index 1caaf6799..a76c7ce84 100644 --- a/pkg/scene/generate/generator.go +++ b/pkg/scene/generate/generator.go @@ -83,6 +83,16 @@ func (g Generator) generateFile(lockCtx *fsutil.LockContext, p Paths, pattern st return err } + // check if generated empty file + stat, err := os.Stat(tmpFn) + if err != nil { + return fmt.Errorf("error getting file stat: %w", err) + } + + if stat.Size() == 0 { + return fmt.Errorf("ffmpeg command produced no output") + } + if err := fsutil.SafeMove(tmpFn, output); err != nil { return fmt.Errorf("moving %s to %s", tmpFn, output) } @@ -142,5 +152,9 @@ func (g Generator) generateOutput(lockCtx *fsutil.LockContext, args []string) ([ return nil, fmt.Errorf("error running ffmpeg command <%s>: %w", strings.Join(args, " "), err) } + if stdout.Len() == 0 { + return nil, fmt.Errorf("ffmpeg command produced no output: <%s>", strings.Join(args, " ")) + } + return stdout.Bytes(), nil }