noe/arthur
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Measure the frame rate rather than believing the container

Browse source 
`probe` took `r_frame_rate` whenever it was at or under the cap, on the
grounds that it is the rate that keeps every distinct source frame. It is
not a claim about frames at all: ordinary iPhone footage declares 120 over
a stream whose timestamps are 1/30s apart, and resampling it up turned an
11-second clip into 1293 proxy frames instead of 323 — four times the
encode, four times the tracing stills (91MB against 23MB), four times the
blobs and the rows, for 970 frames that are copies of their neighbours.

So `_measured_rate` reads the timestamps and `_choose_rate` keeps whichever
declared rate they bear out. Two details carry it: the times are sorted
before differencing, because an HEVC stream arrives in decode order and
differencing that measures the reordering delay instead of the rate; and
the statistic is the MEDIAN interval, which is what keeps the property the
nominal rate was being taken for — a take held on one frame still reports
the rate of the parts that move, so no distinct frame is dropped. A genuine
120fps capture still extracts at 120, and there is a test on that
specifically.

`Source.probe` also stopped being the place a reading goes to be preserved.
The facts are a pure function of bytes that are the row's own identity, so
a re-upload re-reads them: otherwise every already-uploaded source would
have gone on resampling to four times the frames with no way to correct it
short of deleting the row.

Already-extracted footage is untouched — `extraction_key` still says
scheme 3, so those jobs stay done and reachable. Bumping it re-extracts
everything at the corrected rate.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Your Name 2026-10-04 23:22:50 -04:00
parent 8d20097e61
commit e26ad723fa
3 changed files with 210 additions and 7 deletions
Download patch file Download diff file Expand all files Collapse all files

98
clips/tests/test_api.py
View file

@ -908,6 +908,37 @@ class UploadTests(TestCase):
self.assertEqual("image/jpeg", still["Content-Type"])
self.assertEqual(200, self.client.get(footage["audio"]).status_code)
def test_re_uploading_a_source_re_reads_its_facts(self):
# A SOURCE ROW HOLDS A READING, NOT A DECISION. The facts are a pure
# function of bytes that are themselves this row's identity, so the row
# cannot be the place a reading goes to be preserved: `probe` got better
# at phone footage — it stopped believing a declared 120 over timestamps
# 1/30s apart — and a stored reading that nothing can replace would have
# left every already-uploaded source resampling to four times the frames
# with no way to correct it short of deleting the row.
with tempfile.TemporaryDirectory() as directory:
path = Path(directory) / "four-frames.mp4"
subprocess.run([
"ffmpeg", "-hide_banner", "-loglevel", "error", "-y",
"-f", "lavfi", "-i", "color=c=red:s=64x48:r=4:d=1",
"-c:v", "mpeg4", str(path),
], check=True, capture_output=True)
payload = path.read_bytes()
first = self.client.post("/api/sources", {
"file": SimpleUploadedFile("four-frames.mp4", payload, content_type="video/mp4")})
self.assertEqual(201, first.status_code, first.content)
self.assertEqual(4.0, first.json()["probe"]["fps"])
better = dict(first.json()["probe"], fps=12.0, rate="12/1", measured_fps=12.0)
with patch("clips.extraction.probe", return_value=better):
again = self.client.post("/api/sources", {
"file": SimpleUploadedFile("same.mp4", payload, content_type="video/mp4")})
self.assertEqual(200, again.status_code, again.content)
self.assertFalse(again.json()["created"], "the same bytes are the same source")
self.assertEqual("12/1", again.json()["probe"]["rate"])
self.assertEqual(12.0, Source.objects.get(id=first.json()["id"]).probe["fps"])
def test_the_proxy_is_re_encoded_rather_than_the_upload_re_served(self):
# The footage's identity is the proxy's digest, and the proxy is produced
# by one ffmpeg invocation whatever the upload was. If the upload were
@ -955,6 +986,73 @@ class UploadTests(TestCase):
self.assertTrue(facts["vfr"], "the disagreement is still recorded, just not fatal")
self.assertTrue(facts["has_audio"])
def test_a_declared_rate_the_timestamps_do_not_bear_out_is_not_resampled_to(self):
# THE FOUR-TIMES. An iPhone container declares `r_frame_rate` 120 over a
# stream whose frames are 1/30s apart, and taking the declaration at its
# word turned an 11-second clip into 1293 proxy frames instead of 323:
# four times the encode, four times the tracing stills, four times the
# blobs and the rows, for 970 frames that are copies of their neighbours.
# The timestamps are the evidence and they say 30.
streams = json.dumps({"streams": [
{"codec_type": "video", "r_frame_rate": "120/1",
"avg_frame_rate": "96900/3233", "nb_frames": "323",
"width": 1920, "height": 1440},
{"codec_type": "audio"}],
"format": {"duration": "10.775"}})
# IN DECODE ORDER, which is how an HEVC stream really arrives — the first
# packets of the fixture this was found on come out 0, 0.133, 0.067,
# 0.033. Differencing that order unsorted measures the reordering delay
# and not the rate, so the fixture keeps the hazard in it.
shuffled = [0, 4, 2, 1, 3, 8, 6, 5, 7, 12, 10, 9, 11]
packets = json.dumps({"packets": [{"pts_time": f"{i / 30:.6f}"} for i in shuffled]})
with patch("clips.extraction._command", side_effect=[streams, packets]):
facts = extraction.probe(Path("phone.mov"))
self.assertEqual("96900/3233", facts["rate"], "resampled to the declared 120")
self.assertAlmostEqual(30.0, facts["measured_fps"], places=2)
def test_a_genuine_high_rate_capture_is_still_taken_at_its_own_rate(self):
# The other half of the same decision, and the one that would be easy to
# break: a real 120fps capture must not be dragged down to anything. Its
# declaration and its timestamps agree, so the declaration — the exact
# rational the stream was authored at — is what is used.
streams = json.dumps({"streams": [
{"codec_type": "video", "r_frame_rate": "120/1", "avg_frame_rate": "120/1",
"width": 640, "height": 480}],
"format": {"duration": "2"}})
packets = json.dumps({"packets": [{"pts_time": f"{i / 120:.6f}"} for i in range(13)]})
with patch("clips.extraction._command", side_effect=[streams, packets]):
facts = extraction.probe(Path("slowmo.mov"))
self.assertEqual("120/1", facts["rate"])
def test_too_few_timestamps_to_measure_leaves_the_declaration_standing(self):
# A clip with nine-ish frames cannot outvote one odd timestamp, so the
# measurement declines to have an opinion and the nominal rate — the one
# that drops no distinct frame — is used exactly as it was before.
streams = json.dumps({"streams": [
{"codec_type": "video", "r_frame_rate": "30/1", "avg_frame_rate": "24/1",
"width": 640, "height": 480}],
"format": {"duration": "0.1"}})
packets = json.dumps({"packets": [{"pts_time": f"{i / 30:.6f}"} for i in range(3)]})
with patch("clips.extraction._command", side_effect=[streams, packets]):
facts = extraction.probe(Path("tiny.mov"))
self.assertEqual("30/1", facts["rate"])
self.assertIsNone(facts["measured_fps"])
def test_a_rate_measurement_that_fails_outright_cannot_refuse_an_upload(self):
# The measurement is an optimisation. If ffprobe cannot read the packets
# of a file whose streams it just read happily, the upload still has to be
# accepted on its metadata — an optimisation that can reject work is worse
# than no optimisation.
streams = json.dumps({"streams": [
{"codec_type": "video", "r_frame_rate": "25/1", "avg_frame_rate": "25/1",
"width": 640, "height": 480}],
"format": {"duration": "4"}})
with patch("clips.extraction._command",
side_effect=[streams, ValueError("ffprobe fell over")]):
facts = extraction.probe(Path("awkward.mov"))
self.assertEqual("25/1", facts["rate"])
self.assertIsNone(facts["measured_fps"])
def test_the_proxy_rate_is_exact_rather_than_a_rounded_float(self):
# 30000/1001 is not a float. Handing ffmpeg's -r a rounded one is how a
# long take drifts out of sync with its own audio.