Fix FFmpeg VMAF fails to validate even when Target Quality is not used #1103

BoatsMcGee · 2025-07-30T04:21:01Z

Resolves #1102

Target Metric VMAF is now only validated if the user supplies --target-quality or specifies at least 1 Zone where Target Quality is enabled. Zones are now validated after parsing so any issues with a specified Target Metric can be addressed in the new function.

I initially wanted to cache the validation as I planned to validate each Zone as it's parsed but instead went with parsing all Zones, then doing a filter check for the exact condition(s) that require validation. At worst, XPSNR could be validated up to 3 times, 2 for everything else. With the function only executing up to a maximum of 3 times, I decided to forgo complicating things with lazy loading and caching.

Thanks,
- Boats M.

Add Zones validation

codecov · 2025-07-30T04:37:13Z

Codecov Report

❌ Patch coverage is 7.47664% with 99 lines in your changes missing coverage. Please review.
✅ Project coverage is 61.98%. Comparing base (1ad7e86) to head (b189144).
⚠️ Report is 6 commits behind head on master.

Files with missing lines	Patch %	Lines
av1an-core/src/settings.rs	4.54%	63 Missing ⚠️
av1an-core/src/zones.rs	10.00%	36 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1103      +/-   ##
==========================================
- Coverage   62.31%   61.98%   -0.34%     
==========================================
  Files          23       23              
  Lines        6303     6410     +107     
==========================================
+ Hits         3928     3973      +45     
- Misses       2375     2437      +62

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Khaoklong51 · 2025-07-30T07:43:08Z

fix confirmed

Copilot

Pull Request Overview

This PR fixes an issue where FFmpeg VMAF validation was being performed even when Target Quality was not used. The changes ensure that Target Metric validation only occurs when --target-quality is supplied or when at least one Zone has Target Quality enabled.

Extracted metric validation logic into reusable methods in EncodeArgs
Added a new validate_zones function to validate Target Metrics used in zones
Moved zone validation to occur after parsing all zones

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File	Description
av1an-core/src/zones.rs	Adds new `validate_zones` function that conditionally validates Target Metrics based on zone configurations
av1an-core/src/settings.rs	Refactors metric validation logic into separate methods and adds conditional validation for global target quality
av1an-core/src/context.rs	Integrates zone validation after zone parsing

Copilot · 2025-08-10T01:04:45Z

av1an-core/src/zones.rs

+            .and_then(|tq| tq.target.as_ref().filter(|_| tq.metric == TargetMetric::SSIMULACRA2))
+            .is_some()
+    }) {
+        args.validate_ssimululacra2()?;


Method name 'validate_ssimululacra2' contains a typo - should be 'validate_ssimulacra2' (missing 'u' in 'ssimulacra')

Suggested change

args.validate_ssimululacra2()?;

args.validate_ssimulacra2()?;

Copilot · 2025-08-10T01:04:46Z

av1an-core/src/settings.rs

    }
+
+    #[inline]
+    pub fn validate_ssimululacra2(&self) -> anyhow::Result<()> {


Function name 'validate_ssimululacra2' contains a typo - should be 'validate_ssimulacra2' (missing 'u' in 'ssimulacra')

Suggested change

pub fn validate_ssimululacra2(&self) -> anyhow::Result<()> {

pub fn validate_ssimulacra2(&self) -> anyhow::Result<()> {

Copilot · 2025-08-10T01:04:46Z

av1an-core/src/settings.rs

+        if self.target_quality.target.is_some() {
+            match self.target_quality.metric {
+                TargetMetric::VMAF => validate_libvmaf()?,
+                TargetMetric::SSIMULACRA2 => self.validate_ssimululacra2()?,


Method call 'validate_ssimululacra2' contains a typo - should be 'validate_ssimulacra2' (missing 'u' in 'ssimulacra')

Suggested change

TargetMetric::SSIMULACRA2 => self.validate_ssimululacra2()?,

TargetMetric::SSIMULACRA2 => self.validate_ssimulacra2()?,

Copilot · 2025-08-10T01:04:46Z

av1an-core/src/zones.rs

+pub(crate) fn validate_zones(args: &EncodeArgs, zones: &[Scene]) -> anyhow::Result<()> {
+    if zones.is_empty() {
+        // No zones to validate
+        return Ok(());
+    }
+
+    // Using VMAF, validate libvmaf
+    if zones.iter().any(|zone| {
+        zone.zone_overrides
+            .as_ref()
+            .and_then(|ovr| ovr.target_quality.as_ref())
+            .and_then(|tq| tq.target.as_ref().filter(|_| tq.metric == TargetMetric::VMAF))
+            .is_some()
+    }) {
+        validate_libvmaf()?;
+    }
+
+    // Using SSIMULACRA2, validate SSIMULACRA2
+    if zones.iter().any(|zone| {
+        zone.zone_overrides
+            .as_ref()
+            .and_then(|ovr| ovr.target_quality.as_ref())
+            .and_then(|tq| tq.target.as_ref().filter(|_| tq.metric == TargetMetric::SSIMULACRA2))
+            .is_some()
+    }) {
+        args.validate_ssimululacra2()?;
+    }
+
+    // Using butteraugli-Inf, validate butteraugli-Inf
+    if zones.iter().any(|zone| {
+        zone.zone_overrides
+            .as_ref()
+            .and_then(|ovr| ovr.target_quality.as_ref())
+            .and_then(|tq| tq.target.as_ref().filter(|_| tq.metric == TargetMetric::ButteraugliINF))
+            .is_some()
+    }) {
+        args.validate_butteraugli_inf()?;
+    }


The validation logic has significant code duplication across all metric checks. Consider extracting a helper function that takes a predicate to reduce repetition and improve maintainability.

Suggested change

pub(crate) fn validate_zones(args: &EncodeArgs, zones: &[Scene]) -> anyhow::Result<()> {

if zones.is_empty() {

// No zones to validate

return Ok(());

}

// Using VMAF, validate libvmaf

if zones.iter().any(|zone| {

zone.zone_overrides

.as_ref()

.and_then(|ovr| ovr.target_quality.as_ref())

.and_then(|tq| tq.target.as_ref().filter(|_| tq.metric == TargetMetric::VMAF))

.is_some()

}) {

validate_libvmaf()?;

}

// Using SSIMULACRA2, validate SSIMULACRA2

if zones.iter().any(|zone| {

zone.zone_overrides

.as_ref()

.and_then(|ovr| ovr.target_quality.as_ref())

.and_then(|tq| tq.target.as_ref().filter(|_| tq.metric == TargetMetric::SSIMULACRA2))

.is_some()

}) {

args.validate_ssimululacra2()?;

}

// Using butteraugli-Inf, validate butteraugli-Inf

if zones.iter().any(|zone| {

zone.zone_overrides

.as_ref()

.and_then(|ovr| ovr.target_quality.as_ref())

.and_then(|tq| tq.target.as_ref().filter(|_| tq.metric == TargetMetric::ButteraugliINF))

.is_some()

}) {

args.validate_butteraugli_inf()?;

}

fn validate_metric_in_zones<F>(

zones: &[Scene],

metric: TargetMetric,

validate_fn: F,

) -> anyhow::Result<()>

where

F: Fn() -> anyhow::Result<()>,

{

if zones.iter().any(|zone| {

zone.zone_overrides

.as_ref()

.and_then(|ovr| ovr.target_quality.as_ref())

.and_then(|tq| tq.target.as_ref().filter(|_| tq.metric == metric))

.is_some()

}) {

validate_fn()?;

}

Ok(())

}

pub(crate) fn validate_zones(args: &EncodeArgs, zones: &[Scene]) -> anyhow::Result<()> {

if zones.is_empty() {

// No zones to validate

return Ok(());

}

// Using VMAF, validate libvmaf

validate_metric_in_zones(zones, TargetMetric::VMAF, || validate_libvmaf())?;

// Using SSIMULACRA2, validate SSIMULACRA2

validate_metric_in_zones(zones, TargetMetric::SSIMULACRA2, || args.validate_ssimululacra2())?;

// Using butteraugli-Inf, validate butteraugli-Inf

validate_metric_in_zones(zones, TargetMetric::ButteraugliINF, || args.validate_butteraugli_inf())?;

Should have done this the Rust Idiomatic way in the first place.

shssoichiro · 2025-08-10T01:09:51Z

The comments from copilot seem valid, if you wouldn't mind fixing those, then we can merge.

Add ensure_chunk_method to consolidate ChunkMethod checks Add generic tq_used_and functions to consolidate condition checks for Zones Target Quality validation

Fix FFmpeg VMAF fails to validate even when Target Quality is not used

9b46535

Add Zones validation

shssoichiro approved these changes Aug 10, 2025

View reviewed changes

shssoichiro requested a review from Copilot August 10, 2025 01:04

Copilot AI reviewed Aug 10, 2025

View reviewed changes

Fix SSIMULACRA2 typo

b189144

Add ensure_chunk_method to consolidate ChunkMethod checks Add generic tq_used_and functions to consolidate condition checks for Zones Target Quality validation

shssoichiro merged commit 8786a18 into rust-av:master Aug 11, 2025
8 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix FFmpeg VMAF fails to validate even when Target Quality is not used #1103

Fix FFmpeg VMAF fails to validate even when Target Quality is not used #1103

Uh oh!

BoatsMcGee commented Jul 30, 2025

Uh oh!

codecov bot commented Jul 30, 2025 •

edited

Loading

Uh oh!

Khaoklong51 commented Jul 30, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Aug 10, 2025

Uh oh!

Copilot AI Aug 10, 2025

Uh oh!

Copilot AI Aug 10, 2025

Uh oh!

Copilot AI Aug 10, 2025

Uh oh!

BoatsMcGee Aug 10, 2025

Uh oh!

shssoichiro commented Aug 10, 2025

Uh oh!

Uh oh!

Uh oh!

	args.validate_ssimululacra2()?;
	args.validate_ssimulacra2()?;

	pub fn validate_ssimululacra2(&self) -> anyhow::Result<()> {
	pub fn validate_ssimulacra2(&self) -> anyhow::Result<()> {

	TargetMetric::SSIMULACRA2 => self.validate_ssimululacra2()?,
	TargetMetric::SSIMULACRA2 => self.validate_ssimulacra2()?,

Fix FFmpeg VMAF fails to validate even when Target Quality is not used #1103

Fix FFmpeg VMAF fails to validate even when Target Quality is not used #1103

Uh oh!

Conversation

BoatsMcGee commented Jul 30, 2025

Uh oh!

codecov bot commented Jul 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Khaoklong51 commented Jul 30, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Aug 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 10, 2025

Choose a reason for hiding this comment

Uh oh!

BoatsMcGee Aug 10, 2025

Choose a reason for hiding this comment

Uh oh!

shssoichiro commented Aug 10, 2025

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Jul 30, 2025 •

edited

Loading