Skip to content

Commit

Permalink
Dont send videos with rotation to mist
Browse files Browse the repository at this point in the history
  • Loading branch information
mjh1 committed Apr 2, 2023
1 parent da00102 commit 86397ce
Show file tree
Hide file tree
Showing 9 changed files with 109 additions and 49 deletions.
33 changes: 0 additions & 33 deletions clients/mediaconvert_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -291,39 +291,6 @@ func Test_MP4OutDurationCheck(t *testing.T) {
}
}

func TestProbe(t *testing.T) {
require := require.New(t)
probe := video.Probe{}
iv, err := probe.ProbeFile("./fixtures/mediaconvert_payloads/sample.mp4")
require.NoError(err)

expectedInput := video.InputVideo{
Duration: 16.254,
Tracks: []video.InputTrack{
{
Type: video.TrackTypeVideo,
Codec: "h264",
Bitrate: 1234521,
VideoTrack: video.VideoTrack{
Width: 576,
Height: 1024,
FPS: 30,
},
},
{
Type: video.TrackTypeAudio,
Codec: "aac",
Bitrate: 128248,
AudioTrack: video.AudioTrack{
Channels: 2,
},
},
},
SizeBytes: 2779520,
}
require.Equal(expectedInput, iv)
}

func loadFixture(t *testing.T, expectedPath, actual string) string {
if os.Getenv("REGEN_FIXTURES") != "" {
err := os.WriteFile(expectedPath, []byte(actual), 0644)
Expand Down
2 changes: 2 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -174,3 +174,5 @@ require (
gopkg.in/yaml.v2 v2.4.0 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
)

replace gopkg.in/vansante/go-ffprobe.v2 => github.com/mjh1/go-ffprobe v0.0.0-20230402193815-b6c2609ab5da
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,8 @@ github.com/mitchellh/gox v0.4.0/go.mod h1:Sd9lOJ0+aimLBi73mGofS1ycjY8lL3uZM3JPS4
github.com/mitchellh/iochan v1.0.0/go.mod h1:JwYml1nuB7xOzsp52dPpHFffvOCDupsG0QubkSMEySY=
github.com/mitchellh/mapstructure v0.0.0-20160808181253-ca63d7c062ee/go.mod h1:FVVH3fgwuzCH5S8UJGiWEs2h04kUh9fWfEaFds41c1Y=
github.com/mitchellh/mapstructure v1.1.2/go.mod h1:FVVH3fgwuzCH5S8UJGiWEs2h04kUh9fWfEaFds41c1Y=
github.com/mjh1/go-ffprobe v0.0.0-20230402193815-b6c2609ab5da h1:5iB+hAHnQfJV5I25duS9MKeI72n6Rkc6uVS80EU5VYQ=
github.com/mjh1/go-ffprobe v0.0.0-20230402193815-b6c2609ab5da/go.mod h1:qF0AlAjk7Nqzqf3y333Ly+KxN3cKF2JqA3JT5ZheUGE=
github.com/modern-go/concurrent v0.0.0-20180228061459-e0a39a4cb421/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q=
github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd h1:TRLaZ9cD/w8PVh93nsPXa1VrQ6jlwL5oN8l14QlcNfg=
github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q=
Expand Down Expand Up @@ -1093,8 +1095,6 @@ gopkg.in/ini.v1 v1.67.0 h1:Dgnx+6+nfE+IfzjUEISNeydPJh9AXNNsWbGP9KzCsOA=
gopkg.in/ini.v1 v1.67.0/go.mod h1:pNLf8WUiyNEtQjuu5G5vTm06TEv9tsIgeAvK8hOrP4k=
gopkg.in/resty.v1 v1.12.0/go.mod h1:mDo4pnntr5jdWRML875a/NmxYqAlA73dVijT2AXvQQo=
gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7/go.mod h1:dt/ZhP58zS4L8KSrWDmTeBkI65Dw0HsyUHuEVlX15mw=
gopkg.in/vansante/go-ffprobe.v2 v2.1.1 h1:DIh5fMn+tlBvG7pXyUZdemVmLdERnf2xX6XOFF+0BBU=
gopkg.in/vansante/go-ffprobe.v2 v2.1.1/go.mod h1:qF0AlAjk7Nqzqf3y333Ly+KxN3cKF2JqA3JT5ZheUGE=
gopkg.in/yaml.v2 v2.0.0-20170812160011-eb3733d160e7/go.mod h1:JAlM8MvJe8wmxCU4Bli9HhUf9+ttbYbLASfIpnQbh74=
gopkg.in/yaml.v2 v2.2.1/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
Expand Down
24 changes: 16 additions & 8 deletions pipeline/coordinator.go
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ func (c *Coordinator) startUploadJob(p UploadJobPayload) {
if p.PipelineStrategy.IsValid() {
strategy = p.PipelineStrategy
}
p.MistSupported, strategy = checkMistCompatibleCodecs(p.RequestID, strategy, p.InputFileInfo)
p.MistSupported, strategy = checkMistCompatible(p.RequestID, strategy, p.InputFileInfo)
log.AddContext(p.RequestID, "strategy", strategy)

switch strategy {
Expand Down Expand Up @@ -291,24 +291,32 @@ func (c *Coordinator) startUploadJob(p UploadJobPayload) {
}
}

// checkMistCompatibleCodecs checks if the input codecs are compatible with mist and overrides the pipeline strategy
// checkMistCompatible checks if the input codecs are compatible with mist and overrides the pipeline strategy
// to external if they are incompatible
func checkMistCompatibleCodecs(requestID string, strategy Strategy, iv video.InputVideo) (bool, Strategy) {
func checkMistCompatible(requestID string, strategy Strategy, iv video.InputVideo) (bool, Strategy) {
for _, track := range iv.Tracks {
// if the codecs are not compatible then override to external pipeline to avoid sending to mist
if (track.Type == video.TrackTypeVideo && strings.ToLower(track.Codec) != "h264") ||
(track.Type == video.TrackTypeAudio && strings.ToLower(track.Codec) != "aac") {
log.Log(requestID, "codec not supported by mist", "trackType", track.Type, "codec", track.Codec)
// allow StrategyCatalystDominance to pass through as this is used in tests and we might want to manually force it for debugging
if strategy == StrategyCatalystDominance {
return false, strategy
}
return false, StrategyExternalDominance
return mistNotSupported(strategy)
}
if track.Type == video.TrackTypeVideo && track.Rotation != 0 {
log.Log(requestID, "video rotation not supported by mist", "rotation", track.Rotation)
return mistNotSupported(strategy)
}
}
return true, strategy
}

func mistNotSupported(strategy Strategy) (bool, Strategy) {
// allow StrategyCatalystDominance to pass through as this is used in tests and we might want to manually force it for debugging
if strategy == StrategyCatalystDominance {
return false, strategy
}
return false, StrategyExternalDominance
}

// Starts a single upload job with specified pipeline Handler. If the job is
// running in background (foreground=false) then:
// - the job will have a different requestID
Expand Down
24 changes: 22 additions & 2 deletions pipeline/coordinator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -662,7 +662,7 @@ func (f stubFFprobe) ProbeFile(_ string, _ ...string) (video.InputVideo, error)
}, nil
}

func Test_checkMistCompatibleCodecs(t *testing.T) {
func Test_checkMistCompatible(t *testing.T) {
type args struct {
strategy Strategy
iv video.InputVideo
Expand Down Expand Up @@ -715,6 +715,17 @@ func Test_checkMistCompatibleCodecs(t *testing.T) {
},
},
}
videoRotation := video.InputVideo{
Tracks: []video.InputTrack{
{
Codec: "h264",
Type: video.TrackTypeVideo,
VideoTrack: video.VideoTrack{
Rotation: 1,
},
},
},
}
tests := []struct {
name string
args args
Expand Down Expand Up @@ -793,10 +804,19 @@ func Test_checkMistCompatibleCodecs(t *testing.T) {
want: StrategyFallbackExternal,
wantSupported: true,
},
{
name: "incompatible with mist - video rotation",
args: args{
strategy: StrategyFallbackExternal,
iv: videoRotation,
},
want: StrategyExternalDominance,
wantSupported: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
supported, got := checkMistCompatibleCodecs("requestID", tt.args.strategy, tt.args.iv)
supported, got := checkMistCompatible("requestID", tt.args.strategy, tt.args.iv)
require.Equal(t, tt.want, got)
require.Equal(t, tt.wantSupported, supported)
})
Expand Down
Binary file added video/fixtures/bbb-180rotated.mov
Binary file not shown.
28 changes: 24 additions & 4 deletions video/probe.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@ import (
"context"
"errors"
"fmt"
"gopkg.in/vansante/go-ffprobe.v2"
"strconv"
"strings"
"time"

"github.com/cenkalti/backoff/v4"
"gopkg.in/vansante/go-ffprobe.v2"
)

type Prober interface {
Expand Down Expand Up @@ -81,6 +81,15 @@ func parseProbeOutput(probeData *ffprobe.ProbeData) (InputVideo, error) {
if err != nil {
return InputVideo{}, fmt.Errorf("error parsing fps numerator from probed data: %w", err)
}

var rotation float64
for _, sideData := range videoStream.SideDataList {
r := getSideData[float64](sideData, "rotation")
if r != nil {
rotation = *r
}
}

// format file stats into InputVideo
iv := InputVideo{
Tracks: []InputTrack{
Expand All @@ -89,9 +98,10 @@ func parseProbeOutput(probeData *ffprobe.ProbeData) (InputVideo, error) {
Codec: videoStream.CodecName,
Bitrate: bitrate,
VideoTrack: VideoTrack{
Width: int64(videoStream.Width),
Height: int64(videoStream.Height),
FPS: fps,
Width: int64(videoStream.Width),
Height: int64(videoStream.Height),
FPS: fps,
Rotation: rotation,
},
},
},
Expand Down Expand Up @@ -157,3 +167,13 @@ func parseFps(framerate string) (float64, error) {

return float64(num) / float64(den), nil
}

func getSideData[T any](s ffprobe.SideData, key string) *T {
v := s[key]
switch v.(type) {
case T:
res := v.(T)
return &res
}
return nil
}
42 changes: 42 additions & 0 deletions video/probe_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,3 +68,45 @@ func TestDefaultBitrate(t *testing.T) {
require.NoError(t, err)
require.Equal(t, DefaultProfile720p.Bitrate, track.Bitrate)
}

func TestProbe(t *testing.T) {
require := require.New(t)
probe := Probe{}
iv, err := probe.ProbeFile("../clients/fixtures/mediaconvert_payloads/sample.mp4")
require.NoError(err)

expectedInput := InputVideo{
Duration: 16.254,
Tracks: []InputTrack{
{
Type: TrackTypeVideo,
Codec: "h264",
Bitrate: 1234521,
VideoTrack: VideoTrack{
Width: 576,
Height: 1024,
FPS: 30,
},
},
{
Type: TrackTypeAudio,
Codec: "aac",
Bitrate: 128248,
AudioTrack: AudioTrack{
Channels: 2,
},
},
},
SizeBytes: 2779520,
}
require.Equal(expectedInput, iv)
}

func TestProbe_VideoRotation(t *testing.T) {
probe := Probe{}
iv, err := probe.ProbeFile("./fixtures/bbb-180rotated.mov")
require.NoError(t, err)
track, err := iv.GetTrack("video")
require.NoError(t, err)
require.Equal(t, float64(-180), track.Rotation)
}
1 change: 1 addition & 0 deletions video/profiles.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ type VideoTrack struct {
Height int64 `json:"height,omitempty"`
PixelFormat string `json:"pixel_format,omitempty"`
FPS float64 `json:"fps,omitempty"`
Rotation float64 `json:"rotation,omitempty"`
}

type AudioTrack struct {
Expand Down

0 comments on commit 86397ce

Please sign in to comment.