[bugfix] Parse video metadata more accurately; allow Range in fileserver (#1342)

* don't serve unused fields for video attachments

* parse video bitrate + duration more accurately

* use ServeContent where appropriate to respect Range

* abstract temp file seeker into its own function

5 files changed, 113 insertions, 27 deletions

diff --git a/internal/media/manager_test.go b/internal/media/manager_test.go
index 8febaddae..d912c9d87 100644
--- a/internal/media/manager_test.go
+++ b/internal/media/manager_test.go
@@ -414,9 +414,9 @@ func (suite *ManagerTestSuite) TestSlothVineProcessBlocking() {
 	suite.Equal(240, attachment.FileMeta.Original.Height)
 	suite.Equal(81120, attachment.FileMeta.Original.Size)
 	suite.EqualValues(1.4083333, attachment.FileMeta.Original.Aspect)
-	suite.EqualValues(6.5862, *attachment.FileMeta.Original.Duration)
+	suite.EqualValues(6.640907, *attachment.FileMeta.Original.Duration)
 	suite.EqualValues(29.000029, *attachment.FileMeta.Original.Framerate)
-	suite.EqualValues(0x3b3e1, *attachment.FileMeta.Original.Bitrate)
+	suite.EqualValues(0x59e74, *attachment.FileMeta.Original.Bitrate)
 	suite.EqualValues(gtsmodel.Small{
 		Width: 338, Height: 240, Size: 81120, Aspect: 1.4083333333333334,
 	}, attachment.FileMeta.Small)
@@ -531,6 +531,82 @@ func (suite *ManagerTestSuite) TestLongerMp4ProcessBlocking() {
 	suite.Equal(processedThumbnailBytesExpected, processedThumbnailBytes)
 }
 
+func (suite *ManagerTestSuite) TestBirdnestMp4ProcessBlocking() {
+	ctx := context.Background()
+
+	data := func(_ context.Context) (io.ReadCloser, int64, error) {
+		// load bytes from a test video
+		b, err := os.ReadFile("./test/birdnest-original.mp4")
+		if err != nil {
+			panic(err)
+		}
+		return io.NopCloser(bytes.NewBuffer(b)), int64(len(b)), nil
+	}
+
+	accountID := "01FS1X72SK9ZPW0J1QQ68BD264"
+
+	// process the media with no additional info provided
+	processingMedia, err := suite.manager.ProcessMedia(ctx, data, nil, accountID, nil)
+	suite.NoError(err)
+	// fetch the attachment id from the processing media
+	attachmentID := processingMedia.AttachmentID()
+
+	// do a blocking call to fetch the attachment
+	attachment, err := processingMedia.LoadAttachment(ctx)
+	suite.NoError(err)
+	suite.NotNil(attachment)
+
+	// make sure it's got the stuff set on it that we expect
+	// the attachment ID and accountID we expect
+	suite.Equal(attachmentID, attachment.ID)
+	suite.Equal(accountID, attachment.AccountID)
+
+	// file meta should be correctly derived from the video
+	suite.Equal(404, attachment.FileMeta.Original.Width)
+	suite.Equal(720, attachment.FileMeta.Original.Height)
+	suite.Equal(290880, attachment.FileMeta.Original.Size)
+	suite.EqualValues(0.5611111, attachment.FileMeta.Original.Aspect)
+	suite.EqualValues(9.822041, *attachment.FileMeta.Original.Duration)
+	suite.EqualValues(30, *attachment.FileMeta.Original.Framerate)
+	suite.EqualValues(0x117c79, *attachment.FileMeta.Original.Bitrate)
+	suite.EqualValues(gtsmodel.Small{
+		Width: 287, Height: 512, Size: 146944, Aspect: 0.5605469,
+	}, attachment.FileMeta.Small)
+	suite.Equal("video/mp4", attachment.File.ContentType)
+	suite.Equal("image/jpeg", attachment.Thumbnail.ContentType)
+	suite.Equal(1409577, attachment.File.FileSize)
+	suite.Equal("L00000fQfQfQfQfQfQfQfQfQfQfQ", attachment.Blurhash)
+
+	// now make sure the attachment is in the database
+	dbAttachment, err := suite.db.GetAttachmentByID(ctx, attachmentID)
+	suite.NoError(err)
+	suite.NotNil(dbAttachment)
+
+	// make sure the processed file is in storage
+	processedFullBytes, err := suite.storage.Get(ctx, attachment.File.Path)
+	suite.NoError(err)
+	suite.NotEmpty(processedFullBytes)
+
+	// load the processed bytes from our test folder, to compare
+	processedFullBytesExpected, err := os.ReadFile("./test/birdnest-processed.mp4")
+	suite.NoError(err)
+	suite.NotEmpty(processedFullBytesExpected)
+
+	// the bytes in storage should be what we expected
+	suite.Equal(processedFullBytesExpected, processedFullBytes)
+
+	// now do the same for the thumbnail and make sure it's what we expected
+	processedThumbnailBytes, err := suite.storage.Get(ctx, attachment.Thumbnail.Path)
+	suite.NoError(err)
+	suite.NotEmpty(processedThumbnailBytes)
+
+	processedThumbnailBytesExpected, err := os.ReadFile("./test/birdnest-thumbnail.jpg")
+	suite.NoError(err)
+	suite.NotEmpty(processedThumbnailBytesExpected)
+
+	suite.Equal(processedThumbnailBytesExpected, processedThumbnailBytes)
+}
+
 func (suite *ManagerTestSuite) TestNotAnMp4ProcessBlocking() {
 	// try to load an 'mp4' that's actually an mkv in disguise
 
@@ -553,7 +629,7 @@ func (suite *ManagerTestSuite) TestNotAnMp4ProcessBlocking() {
 
 	// we should get an error while loading
 	attachment, err := processingMedia.LoadAttachment(ctx)
-	suite.EqualError(err, "error decoding video: error determining video metadata: [width height duration framerate bitrate]")
+	suite.EqualError(err, "error decoding video: error determining video metadata: [width height framerate]")
 	suite.Nil(attachment)
 }
 
diff --git a/internal/media/test/birdnest-original.mp4 b/internal/media/test/birdnest-original.mp4
new file mode 100644
index 000000000..2ecc075cd
--- /dev/null
+++ b/internal/media/test/birdnest-original.mp4
diff --git a/internal/media/test/birdnest-processed.mp4 b/internal/media/test/birdnest-processed.mp4
new file mode 100644
index 000000000..2ecc075cd
--- /dev/null
+++ b/internal/media/test/birdnest-processed.mp4
diff --git a/internal/media/test/birdnest-thumbnail.jpg b/internal/media/test/birdnest-thumbnail.jpg
new file mode 100644
index 000000000..b20de32a3
--- /dev/null
+++ b/internal/media/test/birdnest-thumbnail.jpg
diff --git a/internal/media/video.go b/internal/media/video.go
index bffdfbbba..38b2dbdce 100644
--- a/internal/media/video.go
+++ b/internal/media/video.go
@@ -21,9 +21,10 @@ package media
 import (
 	"fmt"
 	"io"
-	"os"
 
 	"github.com/abema/go-mp4"
+	"github.com/superseriousbusiness/gotosocial/internal/iotools"
+	"github.com/superseriousbusiness/gotosocial/internal/log"
 )
 
 type gtsVideo struct {
@@ -36,43 +37,48 @@ type gtsVideo struct {
 // decodeVideoFrame decodes and returns an image from a single frame in the given video stream.
 // (note: currently this only returns a blank image resized to fit video dimensions).
 func decodeVideoFrame(r io.Reader) (*gtsVideo, error) {
-	// We'll need a readseeker to decode the video. We can get a readseeker
-	// without burning too much mem by first copying the reader into a temp file.
-	// First create the file in the temporary directory...
-	tmp, err := os.CreateTemp(os.TempDir(), "gotosocial-")
+	// we need a readseeker to decode the video...
+	tfs, err := iotools.TempFileSeeker(r)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("error creating temp file seeker: %w", err)
 	}
-
 	defer func() {
-		tmp.Close()
-		os.Remove(tmp.Name())
+		if err := tfs.Close(); err != nil {
+			log.Errorf("error closing temp file seeker: %s", err)
+		}
 	}()
 
-	// Now copy the entire reader we've been provided into the
-	// temporary file; we won't use the reader again after this.
-	if _, err := io.Copy(tmp, r); err != nil {
-		return nil, err
-	}
-
 	// probe the video file to extract useful metadata from it; for methodology, see:
 	// https://github.com/abema/go-mp4/blob/7d8e5a7c5e644e0394261b0cf72fef79ce246d31/mp4tool/probe/probe.go#L85-L154
-	info, err := mp4.Probe(tmp)
+	info, err := mp4.Probe(tfs)
 	if err != nil {
-		return nil, fmt.Errorf("error probing tmp file %s: %w", tmp.Name(), err)
+		return nil, fmt.Errorf("error during mp4 probe: %w", err)
 	}
 
 	var (
-		width  int
-		height int
-		video  gtsVideo
+		width        int
+		height       int
+		videoBitrate uint64
+		audioBitrate uint64
+		video        gtsVideo
 	)
 
 	for _, tr := range info.Tracks {
 		if tr.AVC == nil {
+			// audio track
+			if br := tr.Samples.GetBitrate(tr.Timescale); br > audioBitrate {
+				audioBitrate = br
+			} else if br := info.Segments.GetBitrate(tr.TrackID, tr.Timescale); br > audioBitrate {
+				audioBitrate = br
+			}
+
+			if d := float64(tr.Duration) / float64(tr.Timescale); d > float64(video.duration) {
+				video.duration = float32(d)
+			}
 			continue
 		}
 
+		// video track
 		if w := int(tr.AVC.Width); w > width {
 			width = w
 		}
@@ -81,10 +87,10 @@ func decodeVideoFrame(r io.Reader) (*gtsVideo, error) {
 			height = h
 		}
 
-		if br := tr.Samples.GetBitrate(tr.Timescale); br > video.bitrate {
-			video.bitrate = br
-		} else if br := info.Segments.GetBitrate(tr.TrackID, tr.Timescale); br > video.bitrate {
-			video.bitrate = br
+		if br := tr.Samples.GetBitrate(tr.Timescale); br > videoBitrate {
+			videoBitrate = br
+		} else if br := info.Segments.GetBitrate(tr.TrackID, tr.Timescale); br > videoBitrate {
+			videoBitrate = br
 		}
 
 		if d := float64(tr.Duration) / float64(tr.Timescale); d > float64(video.duration) {
@@ -93,6 +99,10 @@ func decodeVideoFrame(r io.Reader) (*gtsVideo, error) {
 		}
 	}
 
+	// overall bitrate should be audio + video combined
+	// (since they're both playing at the same time)
+	video.bitrate = audioBitrate + videoBitrate
+
 	// Check for empty video metadata.
 	var empty []string
 	if width == 0 {