Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improve Transcode Quality #832

Merged
merged 13 commits into from
Sep 18, 2023
Merged

Improve Transcode Quality #832

Changes from 4 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
df3eb84
Add CRF param to transcode requests
leszko Aug 29, 2023
c67adf0
Merge branch 'main' of https://github.com/livepeer/dms-api into rafal…
leszko Sep 4, 2023
f661dab
Adjust transcode quality params
leszko Sep 6, 2023
5c24368
Merge branch 'main' into rafal/transcode-quality-params
leszko Sep 6, 2023
d72e764
Merge branch 'main' into rafal/transcode-quality-params
leszko Sep 7, 2023
9dcabff
Rename `crf` to `quality`
leszko Sep 8, 2023
921c2bb
Merge branch 'main' into rafal/transcode-quality-params
leszko Sep 8, 2023
5e3d658
Merge remote-tracking branch 'origin/rafal/transcode-quality-params' …
leszko Sep 8, 2023
7a62999
update fixtures and include a regen facility
mjh1 Sep 8, 2023
4a96b00
Fix unit tests
leszko Sep 8, 2023
13a6bf2
Add 1.2 multiplier also for the source video bitrate
leszko Sep 12, 2023
b3be1d0
Update profile fixtures
leszko Sep 12, 2023
e90142a
Merge branch 'main' into rafal/transcode-quality-params
leszko Sep 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 18 additions & 4 deletions video/profiles.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ const (
MaxVideoBitrate = 288_000_000
TrackTypeVideo = "video"
TrackTypeAudio = "audio"
defaultCRF = 27
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why 27? I think ffmpeg default is 23 actually.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just from some tests. Please check this part of design doc for details. 23 will give way bigger segment size.

)

type InputVideo struct {
Expand Down Expand Up @@ -73,13 +74,15 @@ var DefaultProfile360p = EncodedProfile{
Bitrate: 1_000_000,
Width: 640,
Height: 360,
CRF: defaultCRF,
}
var DefaultProfile720p = EncodedProfile{
Name: "720p0",
FPS: 0,
Bitrate: 4_000_000,
Width: 1280,
Height: 720,
CRF: defaultCRF,
}

// DefaultTranscodeProfiles defines the default set of encoding profiles to use when none are specified
Expand All @@ -103,22 +106,27 @@ func SetTranscodeProfiles(inputVideoStats InputVideo, reqTranscodeProfiles []Enc
// a single profile that matches the input video's specs with the target bitrate
} else if len(reqTranscodeProfiles) == 1 {
if reqTranscodeProfiles[0].Width == 0 && reqTranscodeProfiles[0].Height == 0 && reqTranscodeProfiles[0].Bitrate != 0 {
transcodeProfiles = GenerateSingleProfileWithTargetBitrate(videoTrack, reqTranscodeProfiles[0].Bitrate)
transcodeProfiles = GenerateSingleProfileWithTargetParams(videoTrack, reqTranscodeProfiles[0])
return transcodeProfiles, nil
}
}
return reqTranscodeProfiles, nil
}

func GenerateSingleProfileWithTargetBitrate(videoTrack InputTrack, videoBitrate int64) []EncodedProfile {
func GenerateSingleProfileWithTargetParams(videoTrack InputTrack, videoProfile EncodedProfile) []EncodedProfile {
profiles := make([]EncodedProfile, 0)
var CRF uint = defaultCRF
if videoProfile.CRF != 0 {
CRF = videoProfile.CRF
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CRF = 0 is still a valid value (meaning lossless). Are we explicitly saying crf = 0 should never be allowed or should this check for values between 0 and 51?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good point. The problem with 0 is that everywhere in go and protobuf, it's the default => so, the assumption is that 0 is a not set value.

Anyway, since as you know, there is no direct correspondence of the the quality value and the crf / cq, let's maybe not think about this corner case too much.

}

profiles = append(profiles, EncodedProfile{
Name: strconv.FormatInt(videoTrack.Height, 10) + "p0",
Bitrate: videoBitrate,
Bitrate: videoProfile.Bitrate,
FPS: 0,
Width: videoTrack.Width,
Height: videoTrack.Height,
CRF: CRF,
})
return profiles
}
Expand All @@ -134,7 +142,10 @@ func GetDefaultPlaybackProfiles(video InputTrack) ([]EncodedProfile, error) {
// check it here.
lowerQualityThanSrc := profile.Height < video.Height && profile.Bitrate < video.Bitrate
if lowerQualityThanSrc {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we also want this for the rendition that's the same resolution as the source?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding is that no, because for the same resolution as the source we create a profile in line 157. So, this logic here is (and always was) for the profile resolution lower than source.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ohh wait, maybe you're right that we should also increase the bitrate in line 157. Will update it there.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated in 13a6bf2

relativeBitrate := float64(profile.Width*profile.Height) * (float64(videoBitrate) / float64(video.Width*video.Height))
// relativeBitrate needs to be slightly higher than the proportional average bitrate of the source video.
// Livepeer network uses bitrate to set max bitrate for encoding, so for the video to look good, we multiply
// it by a factor of 1.2.
relativeBitrate := 1.2 * float64(profile.Width*profile.Height) * (float64(videoBitrate) / float64(video.Width*video.Height))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how did we determine the 1.2 multiplier?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't really. I just know that what we currently have is too low, because we're setting: max bitrate = source avg bitrate which gives poor results.

As described in the doc, I plan to not set it at all, when the Os start to support the quality param (Phase 2).

br := math.Min(relativeBitrate, float64(profile.Bitrate))
profile.Bitrate = int64(br)
profiles = append(profiles, profile)
Expand All @@ -149,6 +160,7 @@ func GetDefaultPlaybackProfiles(video InputTrack) ([]EncodedProfile, error) {
FPS: 0,
Width: nearestEven(video.Width),
Height: nearestEven(video.Height),
CRF: defaultCRF,
})
return profiles, nil
}
Expand All @@ -170,6 +182,7 @@ func lowBitrateProfile(video InputTrack) EncodedProfile {
Bitrate: bitrate,
Width: nearestEven(video.Width),
Height: nearestEven(video.Height),
CRF: defaultCRF,
}
}

Expand All @@ -189,6 +202,7 @@ type EncodedProfile struct {
Encoder string `json:"encoder,omitempty"`
ColorDepth int64 `json:"colorDepth,omitempty"`
ChromaFormat int64 `json:"chromaFormat,omitempty"`
CRF uint `json:"crf"`
}

type OutputVideo struct {
Expand Down
Loading