diff --git a/docs/technical/building.md b/docs/technical/building.md index 2543cf6..009675d 100644 --- a/docs/technical/building.md +++ b/docs/technical/building.md @@ -85,6 +85,7 @@ bash tests/run_dvd_tests.sh # a DVD-Video disc, as a folder and as an bash tests/run_bd_audio_tests.sh # the sound a disc carries, written out 39 bash tests/run_vc1_tests.sh # the VC-1 encoder, put through a decoder 4 bash tests/run_vp9_av1_tests.sh # VP9 and AV1: cut, and still the source's bytes 5 +bash tests/run_sar_tests.sh # SAR: 44 exports + 16 outlines 60 bash tests/run_vfr_tests.sh # variable rate, both ways round 8 bash tests/run_demux_tests.sh # what a reader loses by reading what it does not 9 bash tests/run_transrate_tests.sh # fitting a disc: the identity, the size, the cost 15 diff --git a/rust/crates/core/src/cut.rs b/rust/crates/core/src/cut.rs index cf8f6e7..99e9f4d 100644 --- a/rust/crates/core/src/cut.rs +++ b/rust/crates/core/src/cut.rs @@ -9397,7 +9397,19 @@ fn cut_into( .collect(); let (ictx, ist_index) = open_input(&src.input.url)?; - let params = ictx.stream(ist_index).unwrap().parameters(); + let ist = ictx.stream(ist_index).unwrap(); + let params = ist.parameters(); + // A container may override the display aspect without rewriting the + // pictures (for example, mkvmerge --aspect-ratio). Keep that display SAR + // on the output stream, but leave codecpar alone: boundary encoders must + // say the same SAR as the copied pictures, not alternate at every seam. + // AV1's container-only SAR needs no corresponding encoder setting. + let display_sar = unsafe { + let sar = ff::ffi::av_guess_sample_aspect_ratio( + ictx.as_ptr() as *mut _, ist.as_ptr() as *mut _, std::ptr::null_mut(), + ); + if sar.num > 0 && sar.den > 0 { sar } else { (*params.as_ptr()).sample_aspect_ratio } + }; let extradata = unsafe { let p = params.as_ptr(); if (*p).extradata.is_null() || (*p).extradata_size <= 0 { @@ -10055,6 +10067,12 @@ fn cut_into( let mut ost = octx.add_stream(ff::encoder::find(ff::codec::Id::None))?; ost.set_parameters(params.clone()); ost.set_time_base(grid.time_base()); + unsafe { + (*ost.as_mut_ptr()).sample_aspect_ratio = display_sar; + // libavformat requires these two output fields to agree. This is + // the muxer's copy, not the input parameters used by encoders. + (*ost.parameters().as_mut_ptr()).sample_aspect_ratio = display_sar; + } if let Some((num, den)) = stated_rate { ost.set_avg_frame_rate(ff::Rational::new(num, den)); } diff --git a/rust/crates/core/src/lib.rs b/rust/crates/core/src/lib.rs index 79a6dad..44f23d5 100644 --- a/rust/crates/core/src/lib.rs +++ b/rust/crates/core/src/lib.rs @@ -1777,7 +1777,12 @@ fn outline_of(path: &str) -> Result<(Outline, input::Demux)> { } else { std::slice::from_raw_parts((*p).extradata, (*p).extradata_size as usize).to_vec() }; - let sar = (*p).sample_aspect_ratio; + // Container-only display aspect (notably AV1) can live on AVStream + // without a corresponding SAR in codecpar. + let sar = ff::ffi::av_guess_sample_aspect_ratio( + ictx.as_ptr() as *mut _, stream.as_ptr() as *mut _, std::ptr::null_mut(), + ); + let sar = if sar.num > 0 && sar.den > 0 { sar } else { (*p).sample_aspect_ratio }; ( (*p).width as u32, (*p).height as u32, diff --git a/tests/ci.sh b/tests/ci.sh index 55166ad..efc930c 100755 --- a/tests/ci.sh +++ b/tests/ci.sh @@ -28,7 +28,7 @@ SUITES=( audio_codec audio_smart audio_format audio_head bilingual index proxy disc bdav udf dvd bd_audio - vp9_av1 vfr demux transrate join + vp9_av1 sar vfr demux transrate join ) [ $# -gt 0 ] && SUITES=("$@") diff --git a/tests/run_sar_tests.sh b/tests/run_sar_tests.sh new file mode 100644 index 0000000..a216af2 --- /dev/null +++ b/tests/run_sar_tests.sh @@ -0,0 +1,5 @@ +#!/usr/bin/env bash +set -eu +cd "$(dirname "$0")/.." +[ -x rust/target/release/examples/conformdiag ] || (cd rust && cargo build --release --locked --example conformdiag) +python3 tests/sar_tests.py "$@" diff --git a/tests/sar_tests.py b/tests/sar_tests.py new file mode 100644 index 0000000..8ae1e84 --- /dev/null +++ b/tests/sar_tests.py @@ -0,0 +1,208 @@ +"""Container SAR regression, independent of AV1 guards and clock changes. + +Requires Python's standard library and FFmpeg with libx264, libx265 and +libaom-av1. Retains generated media, commands and hashes in a new directory. +""" +import argparse +from fractions import Fraction +import hashlib +import json +import os +from pathlib import Path +import re +import subprocess +import tempfile + + +def main(): + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument('--cut', type=Path, default=Path('rust/target/release/smartcut' + ('.exe' if os.name == 'nt' else ''))) + parser.add_argument('--ffmpeg', default=os.environ.get('FFMPEG', 'ffmpeg')) + parser.add_argument('--ffprobe', default=os.environ.get('FFPROBE', 'ffprobe')) + parser.add_argument('--work-dir', type=Path) + parser.add_argument('--fixtures', type=Path, help='reuse/generate source clips here for exact paired comparisons') + parser.add_argument('--outline-probe', type=Path, help='existing conformdiag example; defaults beside the CLI under examples/') + args = parser.parse_args() + work = Path(tempfile.mkdtemp(prefix='smartcut-sar-', dir=args.work_dir)) + fixtures = args.fixtures.resolve() if args.fixtures else work + fixtures.mkdir(parents=True, exist_ok=True) + cut = args.cut.resolve() + outline_probe = (args.outline_probe or cut.parent/'examples'/('conformdiag.exe' if os.name == 'nt' else 'conformdiag')).resolve() + commands, cases, outlines, overrides = [], [], [], [] + + def sha(path): + with path.open('rb') as stream: + return hashlib.file_digest(stream, 'sha256').hexdigest() + + def run(command): + command = list(map(str, command)) + p = subprocess.run(command, capture_output=True, timeout=180) + commands.append({'command': command, 'exit_code': p.returncode}) + assert p.returncode == 0, f'exit {p.returncode}: {command}\n{p.stderr.decode(errors="replace")[-2000:]}' + return p + + def probe(path): + data = run([args.ffprobe, '-v', 'error', '-select_streams', 'v:0', '-count_frames', + '-show_entries', 'stream=codec_name,width,height,sample_aspect_ratio,nb_read_frames', + '-of', 'json', path]).stdout + return json.loads(data)['streams'][0] + + def sar(stream): + value = stream.get('sample_aspect_ratio', 'N/A') + return Fraction(value.replace(':', '/')) if value not in ('N/A', '0:1') else Fraction(1) + + def header_sars(path): + # Inspect every SPS, including the ones written by boundary encoders; + # ffprobe's stream SAR alone can hide a change at a seam. + log = run([args.ffmpeg, '-v', 'info', '-i', path, '-map', '0:v:0', + '-c', 'copy', '-bsf:v', 'trace_headers', '-f', 'null', '-']).stderr.decode(errors='replace') + (work/(path.name+'.headers.log')).write_text(log, encoding='utf-8') + fields = re.findall(r'\b(aspect_ratio_idc|sar_width|sar_height)\s+[01]+\s*=\s*(\d+)', log) + ratios = [] + index = 0 + while index < len(fields): + field, value = fields[index] + assert field == 'aspect_ratio_idc', (path, fields[index:]) + value = int(value) + if value == 255: + assert [v[0] for v in fields[index+1:index+3]] == ['sar_width', 'sar_height'], path + ratios.append(Fraction(int(fields[index+1][1]), int(fields[index+2][1]))) + index += 3 + else: + # These fixtures deliberately cover square and non-square SPS SAR. + assert value in (1, 14), (path, value) + ratios.append({1: Fraction(1), 14: Fraction(4, 3)}[value]) + index += 1 + assert ratios, ('no SPS SAR found', path) + assert len(ratios) == len(re.findall(r'\] Sequence Parameter Set\s*$', log, re.M)), ('SPS without SAR', path) + return ratios + + encoders = { + 'h264': ['libx264', '-preset', 'medium', '-crf', '14'], + 'hevc': ['libx265', '-preset', 'medium', '-crf', '18', '-x265-params', 'log-level=error:pools=none'], + 'av1': ['libaom-av1', '-cpu-used', '8', '-crf', '22', '-b:v', '0'], + } + cuts = {'copy': ('0-4.8', 120), 'mixed': ('0.52-4.28', 94), 'encoded': ('0.52-0.76', 6)} + for codec, encoder in encoders.items(): + for ratio in ('4/3', '1/1'): + mp4 = fixtures/f'{codec}-sar{ratio.replace("/", "_")}.mp4' + if not mp4.exists(): + run([args.ffmpeg, '-v', 'error', '-y', '-f', 'lavfi', '-i', 'testsrc2=size=192x112:rate=25', + '-frames:v', '120', '-an', '-vf', f'setsar={ratio}', '-c:v', *encoder, + '-g', '24', '-pix_fmt', 'yuv420p', mp4]) + for container in ('.mp4', '.mkv'): + source = mp4.with_suffix(container) + if container == '.mkv' and not source.exists(): + run([args.ffmpeg, '-v', 'error', '-y', '-i', mp4, '-map', '0:v:0', '-c', 'copy', source]) + original = probe(source) + assert sar(original) == Fraction(ratio), ('fixture SAR', source, original) + shape = run([outline_probe, '--shape', source]).stdout.decode(errors='replace') + found = re.search(r'\bsar=([\d.]+)', shape) + correct = found is not None and abs(float(found[1])-float(Fraction(ratio))) < 0.0001 + outlines.append({'input': source.name, 'input_sha256': sha(source), + 'expected_sar': ratio, 'passed': correct, 'shape': shape}) + print('ok' if correct else 'FAIL', 'outline', codec, ratio, container, flush=True) + for mode, (bounds, frames) in cuts.items(): + out = work/f'{source.stem}-{mode}{container}' + row = {'codec': codec, 'container': container, 'sar': ratio, 'mode': mode, + 'input': source.name, 'input_sha256': sha(source), 'output': out.name} + try: + log = run([cut, source, '--no-chapters', '--keep', bounds, '-o', out]) + out.with_suffix(container+'.log').write_bytes(log.stdout+log.stderr) + made = probe(out) + assert sar(made) == Fraction(ratio), ('output SAR', made) + assert (made['width'], made['height']) == (192, 112), made + assert made['codec_name'] == codec, made + assert int(made['nb_read_frames']) == frames, made + run([args.ffmpeg, '-v', 'error', '-xerror', '-err_detect', 'explode', '-i', out, + '-map', '0:v:0', '-f', 'null', '-']) + row.update(passed=True, output_sha256=sha(out)) + except (AssertionError, subprocess.TimeoutExpired) as error: + row.update(passed=False, error=str(error)) + if out.exists(): + row['output_sha256'] = sha(out) + cases.append(row) + print('ok' if row['passed'] else 'FAIL', codec, ratio, container, mode, + row.get('error', ''), flush=True) + + # A display-aspect override must not leak into the boundary SPS. The first + # fixture uses the reviewer's exact 30 s / 640x360 H.264 generation command. + # Also cover HEVC, a non-square coded SAR, both output containers, and a + # later TS remux where the container's display override no longer applies. + for codec in ('h264', 'hevc'): + for coded_sar in ('1/1', '4/3'): + stem = f'override-{codec}-{coded_sar.replace("/", "_")}' + original = fixtures/(stem+'.mp4') + source = fixtures/(stem+'.mkv') + if not original.exists(): + options = [] if coded_sar == '1/1' else ['-vf', f'setsar={coded_sar}'] + encoder = ['libx264'] if codec == 'h264' else ['libx265', '-x265-params', 'log-level=error:pools=none'] + run([args.ffmpeg, '-v', 'error', '-y', '-f', 'lavfi', '-i', + 'testsrc2=rate=25:size=640x360', '-t', '30', *options, + '-c:v', *encoder, '-g', '50', original]) + if not source.exists(): + run([args.ffmpeg, '-v', 'error', '-y', '-i', original, + '-c', 'copy', '-aspect', '4:3', source]) + expected = Fraction(coded_sar) + source_sars = header_sars(source) + assert all(s == expected for s in source_sars), ('fixture SPS SAR', source, source_sars) + assert sar(probe(source)) == Fraction(3, 4), ('fixture container SAR', source) + shape = run([outline_probe, '--shape', source]).stdout.decode(errors='replace') + found = re.search(r'\bsar=([\d.]+)', shape) + correct = found is not None and abs(float(found[1])-0.75) < 0.0001 + outlines.append({'input': source.name, 'input_sha256': sha(source), + 'expected_sar': '3/4', 'passed': correct, 'shape': shape}) + print('ok' if correct else 'FAIL', 'outline override', codec, coded_sar, flush=True) + for container in ('.mp4', '.mkv'): + out = work/(stem+'-cut'+container) + remux = work/(out.name+'.ts') + row = {'codec': codec, 'container': container, 'coded_sar': coded_sar, + 'display_sar': '3/4', 'input': source.name, 'input_sha256': sha(source), + 'source_sps_sars': [str(s) for s in source_sars], 'output': out.name} + try: + log = run([cut, source, '--keep', '3.3-12.1', '--keep', '15-22.7', '-o', out]) + text = (log.stdout+log.stderr).decode(errors='replace') + (work/(out.name+'.log')).write_text(text, encoding='utf-8') + assert re.search(r'^\s*copy\s+', text, re.M), 'fixture must exercise copied pictures' + assert re.search(r'^\s*reencode\s+', text, re.M), 'fixture must exercise boundary encoders' + made = probe(out) + assert sar(made) == Fraction(3, 4), ('output container SAR', made) + assert (made['width'], made['height'], made['codec_name']) == (640, 360, codec), made + run([args.ffmpeg, '-v', 'error', '-xerror', '-err_detect', 'explode', '-i', out, + '-map', '0:v:0', '-f', 'null', '-']) + values = header_sars(out) + row['output_sps_sars'] = [str(s) for s in values] + # Inspect the TS even when the original output has a bad SPS, + # so the retained evidence shows the downstream effect too. + run([args.ffmpeg, '-v', 'error', '-y', '-i', out, '-map', '0:v:0', '-c', 'copy', remux]) + remux_sars = header_sars(remux) + row['ts_sps_sars'] = [str(s) for s in remux_sars] + row['ts_sha256'] = sha(remux) + # The FFmpeg 7.1 HEVC TS remux also has a strict-decode + # failure with unmodified v0.8.39. Check its SAR headers + # here without making this suite a TS decoding fix. + if codec == 'h264': + run([args.ffmpeg, '-v', 'error', '-xerror', '-err_detect', 'explode', '-i', remux, + '-map', '0:v:0', '-f', 'null', '-']) + assert all(s == expected for s in values), ('output SPS SAR changes', row['output_sps_sars']) + assert all(s == expected for s in remux_sars), ('remux SPS SAR changes', row['ts_sps_sars']) + row.update(passed=True, frames=int(made['nb_read_frames'])) + except (AssertionError, subprocess.TimeoutExpired) as error: + row.update(passed=False, error=str(error)) + if out.exists(): + row['output_sha256'] = sha(out) + overrides.append(row) + print('ok' if row['passed'] else 'FAIL', 'override', codec, coded_sar, container, + row.get('error', ''), flush=True) + record = {'cut': str(cut), 'cut_sha256': sha(cut), 'generator_sha256': sha(Path(__file__)), + 'outline_probe_sha256': sha(outline_probe), 'fixture_directory': str(fixtures), 'commands': commands, + 'cases': cases, 'outline_checks': outlines, 'override_cases': overrides} + (work/'results.json').write_text(json.dumps(record, indent=2)+'\n', encoding='utf-8') + checks = cases + outlines + overrides + failures = sum(not c['passed'] for c in checks) + print(f'{len(checks)-failures} passed, {failures} failed ({len(cases)+len(overrides)} exports, {len(outlines)} outlines); evidence: {work}', flush=True) + raise SystemExit(1 if failures else 0) + + +if __name__ == '__main__': + main()