clips/extraction.py

@@ -159,16 +159,6 @@ def _extract_stills(job, proxy_path, frames_dir, frames, root):
 
 
 MAX_RATE = 120  # a capture rate; past this the container is describing something else
-# How many packet timestamps `_measured_rate` reads, and the fewest intervals it
-# will draw a conclusion from. 300 is a flat cost on a long take and still a
-# wide enough sample for a median; below 8 intervals there is not enough of a
-# stream to outvote one odd timestamp, so the metadata is left to speak.
-RATE_SAMPLE = 300
-RATE_MINIMUM = 8
-# How far a declared rate may sit from the measured one and still be taken as
-# what the stream is: 2% covers 30 against 30000/1001 and nothing like 120
-# against 30.
-RATE_TOLERANCE = 0.02
 
 
 def probe_image(path):
@@ -194,75 +184,6 @@ def probe_audio(path):
     return duration
 
 
-def _measured_rate(path):
-    """The rate the stream's own packet timestamps imply, or None.
-
-    THE CONTAINER'S SUMMARY OF ITSELF IS NOT EVIDENCE, and this is the function
-    that goes and looks. An iPhone's `r_frame_rate` is 120 on footage whose
-    timestamps are 1/30s apart, which is the difference between 323 frames and
-    1293 — four times the encode, four times the tracing stills, four times the
-    blobs, for 970 frames that are copies of their neighbours.
-
-    It reads TIMESTAMPS, not frames: `-show_entries packet=pts_time` demuxes
-    without decoding, so this costs a file read and no pixels. The times are
-    SORTED before differencing because a stream with B-frames arrives in decode
-    order — an HEVC clip's first packets come out 0, 0.133, 0.067, 0.033 — and
-    differencing that order measures the reordering rather than the rate.
-
-    THE MEDIAN INTERVAL, which is what makes this safe on genuinely variable
-    input. It answers "how far apart are two frames normally", so a take held on
-    one frame for a second still reports the rate of the parts that move, and
-    choosing it keeps every distinct frame — the property `probe` used to reach
-    for by taking the nominal rate. Only the last few intervals of the sample are
-    unreliable (a frame whose turn comes after the window is missing from it), and
-    a median does not care.
-
-    Returning None is the honest answer for a clip too short to sample, and this
-    also swallows a probe that fails outright: the rate the metadata declares is
-    the documented fallback, so an optimisation must not be able to refuse an
-    upload that would otherwise have been accepted.
-    """
-    try:
-        text = _command(["ffprobe", "-v", "error", "-select_streams", "v:0",
-                         "-show_entries", "packet=pts_time", "-of", "json",
-                         "-read_intervals", f"%+#{RATE_SAMPLE}", str(path)])
-        packets = json.loads(text).get("packets") or []
-        times = sorted(float(packet["pts_time"]) for packet in packets
-                       if (packet.get("pts_time") or "N/A") != "N/A")
-    except (ValueError, OSError):
-        return None
-    intervals = sorted(b - a for a, b in zip(times, times[1:]) if b > a)
-    if len(intervals) < RATE_MINIMUM:
-        return None
-    median = intervals[len(intervals) // 2]
-    return 1.0 / median if median > 0 else None
-
-
-def _choose_rate(nominal, average, measured):
-    """The rate to resample onto, as an exact Fraction.
-
-    A DECLARED RATE IS PREFERRED WHEN IT AGREES WITH THE TIMESTAMPS, because it is
-    the exact rational the stream was authored at — 30000/1001 is not a float, and
-    `limit_denominator` on a measured 29.97 is a guess at a number the container
-    already states. So the measured rate is used to CHOOSE between what the
-    container declares, and only stands in itself when neither declaration
-    describes the stream.
-    """
-    candidates = [rate for rate in (nominal, average) if 0 < rate <= MAX_RATE]
-    if measured:
-        agreeing = [rate for rate in candidates
-                    if abs(float(rate) - measured) <= RATE_TOLERANCE * measured]
-        if agreeing:
-            return min(agreeing, key=lambda rate: abs(float(rate) - measured))
-        from_timestamps = Fraction(measured).limit_denominator(1001)
-        if 0 < from_timestamps <= MAX_RATE:
-            return from_timestamps
-    # Nothing to go on but the metadata, and nominal first keeps the rate that
-    # drops no distinct frame. An unusable pair falls through to the refusal
-    # below, which names the rate the file claimed rather than one of these.
-    return nominal if 0 < nominal <= MAX_RATE else average
-
-
 def probe(path):
     """What the upload is, as far as choosing a proxy rate goes.
 
@@ -280,21 +201,12 @@ def probe(path):
     of itself disagreed with the container's own contents, so the guard rejected
     CFR video for being variable.
 
-    THE RATE IS MEASURED AND THE DECLARATIONS ARE VOTED ON, which is the same
+    THE RATE IS THE NOMINAL ONE. `r_frame_rate` is the rate every timestamp in the
-    distrust applied to the one number that still comes from here. This used to
+    stream can be expressed at, which is the rate that keeps every distinct source
-    take `r_frame_rate` outright — the rate every timestamp in the stream can be
+    frame; resampling to the average would drop some. Duration is preserved either
-    expressed at, and so the rate that keeps every distinct source frame. The
+    way — ffmpeg's CFR conversion is driven by timestamps, so the audio stays in
-    trouble is that it is not a claim about frames at all: the file above declares
+    sync at any rate — so this trades a possible duplicated frame against a
-    120 and holds 30, and resampling it up cost four times the encode, four times
+    certainly lost one.
-    the tracing stills and four times the blobs for 970 duplicated frames. So
-    `_measured_rate` reads the timestamps, `_choose_rate` keeps whichever declared
-    rate they bear out, and the nominal rate is believed when it is true rather
-    than because it is nominal.
-
-    Duration is preserved either way — ffmpeg's CFR conversion is driven by
-    timestamps, so the audio stays in sync at any rate — and the median interval
-    keeps the no-distinct-frame-dropped property that taking the nominal rate was
-    reaching for. See `_measured_rate`.
     """
     data = json.loads(_command(["ffprobe", "-v", "error", "-show_streams",
                                 "-show_format", "-of", "json", str(path)]))
@@ -305,8 +217,7 @@ def probe(path):
     average = Fraction(video.get("avg_frame_rate") or "0")
     if nominal <= 0 and average <= 0:
         raise ValueError("the video's frame rate is unknown")
-    measured = _measured_rate(path)
+    rate = nominal if 0 < nominal <= MAX_RATE else average
-    rate = _choose_rate(nominal, average, measured)
     if not 0 < rate <= MAX_RATE:
         raise ValueError(f"the video reports a frame rate of {float(rate):g}, which is "
                          "not a rate footage can be measured at")
@@ -319,11 +230,6 @@ def probe(path):
             # `-r` a rounded one is how a long take drifts out of sync.
             "rate": f"{rate.numerator}/{rate.denominator}",
             "nominal_fps": float(nominal), "average_fps": float(average),
-            # What the timestamps said, and null when there were too few to ask.
-            # Recorded because it is the input to a decision this file used not to
-            # make, and the one number that explains a chosen rate matching
-            # neither declaration.
-            "measured_fps": measured,
             "width": int(video["width"]), "height": int(video["height"]),
             "duration": duration,
             # KEPT, AND NO LONGER TRUSTED AS A COUNT. See the docstring: this is

clips/tests/test_api.py

@@ -908,37 +908,6 @@ 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
@@ -986,73 +955,6 @@ 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.

clips/views.py

@@ -218,17 +218,6 @@ def sources(request):
                                      "media_type": upload.content_type or "video/mp4"})
         row, created = Source.objects.get_or_create(
             blob=blob, defaults={"filename": Path(upload.name).name[:255], "probe": facts})
-        if not created and row.probe != facts:
-            # THE FACTS ARE RE-READ, NOT REMEMBERED. They are a pure function of
-            # the bytes, and the bytes are this row's identity — so a
-            # disagreement means the server reads the file differently now from
-            # whenever it first saw it, and the fresh reading is the one to keep.
-            # Storing the first reading forever pins a source to a rate the code
-            # no longer believes in, and makes it unfixable without deleting the
-            # row: `extraction.probe` got better at phone footage and every
-            # already-uploaded source would have gone on being wrong.
-            row.probe = facts
-            row.save(update_fields=["probe"])
         return JsonResponse({"id": str(row.id), "digest": digest,
                              "filename": row.filename, "probe": row.probe,
                              "created": created}, status=201 if created else 200)

frontend/src/arthur/events/footage.cljs

@@ -334,32 +334,12 @@
       (.catch (fn [error]
                 (rf/dispatch [::failed (or (ex-message error) (str error))])))))
 
-(defn- sending
-  "A progress callback that puts the whole percentage sent in the status line.
-
-  ONLY WHEN THE PERCENTAGE MOVES. The browser fires upload progress as often as
-  it pleases and a status line has something new to say a hundred times at most,
-  and each dispatch here re-renders the pane.
-
-  Worth having at all because the transfer is the longest part of an import on
-  anything but a local server, and it was the part with no number on it:
-  `uploading video…` sat unchanged from the first byte to the last, however many
-  there were, and only the extraction that followed it ever counted. See
-  `arthur.fx.http/POST-form`."
-  [label]
-  (let [reported (atom -1)]
-    (fn [fraction]
-      (let [percent (js/Math.round (* 100 fraction))]
-        (when (not= percent @reported)
-          (reset! reported percent)
-          (rf/dispatch [::progress (str label " " percent "%")]))))))
-
 (rf/reg-fx
  ::upload!
  (fn [file]
    (let [form (js/FormData.)]
      (.append form "file" file)
-     (-> (http/POST-form "/api/sources" form (sending "uploading video…"))
+     (-> (http/POST-form "/api/sources" form)
          (.then (fn [^js source]
                   (rf/dispatch [::progress "queued for extraction…"])
                   (http/POST "/api/extractions" #js {:source (.-id source)
@@ -373,7 +353,7 @@
  (fn [file]
    (let [form (js/FormData.)]
      (.append form "file" file)
-     (-> (http/POST-form "/api/sounds" form (sending "uploading sound…"))
+     (-> (http/POST-form "/api/sounds" form)
          (.then (fn [^js sound] (rf/dispatch [::uploaded (.-id sound) "sound imported"])))
          (.catch (fn [error]
                    (rf/dispatch [::failed (or (ex-message error) (str error))])))))))
@@ -383,7 +363,7 @@
  (fn [file]
    (let [form (js/FormData.)]
      (.append form "file" file)
-     (-> (http/POST-form "/api/images" form (sending "uploading image…"))
+     (-> (http/POST-form "/api/images" form)
          (.then (fn [^js image] (rf/dispatch [::uploaded (.-id image) "image imported"])))
          (.catch (fn [error]
                    (rf/dispatch [::failed (or (ex-message error) (str error))])))))))

frontend/src/arthur/fx/http.cljs

@@ -19,28 +19,17 @@
             (when (= "csrftoken" (str/trim (or k ""))) v)))
         (str/split (or (.-cookie js/document) "") #";")))
 
-(defn- failure
+(defn- fail
   "Turn a non-2xx into an ex-info carrying what the server said.
 
   The server's message is the useful one — \"this clip names tier-2 blocks the
   server does not have\" — and a status code alone would put the interesting half
-  of it in a console nobody is watching.
+  of it in a console nobody is watching."
-
+  [response body]
-  Built rather than thrown, because the two transports below raise it in
+  (throw (ex-info (or (some-> body .-error)
-  different ways: `fetch` throws inside a `.then` and XHR has to reject a promise
+                      (str "the server answered " (.-status response)))
-  by hand."
+                  {:status (.-status response)
-  [status body]
+                   :body   (when body (js->clj body :keywordize-keys true))})))
-  (ex-info (or (some-> body .-error)
-               (str "the server answered " status))
-           {:status status
-            :body   (when body (js->clj body :keywordize-keys true))}))
-
-(defn- parse
-  "A response body as a parsed JS value, or nil. An empty 204 and a 500 whose
-  body is an HTML error page both land here, and neither is worth an exception."
-  [text]
-  (when (seq text)
-    (try (js/JSON.parse text) (catch :default _ nil))))
 
 (defn request!
   ([method url] (request! method url nil))
@@ -58,58 +47,14 @@
        (.then (fn [response]
                 (-> (.text response)
                     (.then (fn [text]
-                             (let [parsed (parse text)]
+                             (let [parsed (when (seq text)
+                                            (try (js/JSON.parse text) (catch :default _ nil)))]
                                (if (.-ok response)
                                  parsed
-                                 (throw (failure (.-status response) parsed)))))))))))))
+                                 (fail response parsed))))))))))))
-
-(defn POST-form
-  "A multipart POST, optionally reporting how much of the body has gone out.
-
-  XMLHttpRequest RATHER THAN `fetch`, for the one thing fetch cannot do: a
-  Request built from a FormData gives no way to observe its own upload, so the
-  promise settles when the response arrives and everything before that is a
-  single unknown. On the one call that sends a whole video that unknown is
-  minutes long, and a spinner with no number on it is most of what \"slow\" means
-  to whoever is waiting — so this is worth the twenty lines XHR costs.
-
-  `on-progress` is called with the fraction sent, 0.0 to 1.0. Only while the
-  browser can say what the total is: a body whose length it cannot compute
-  reports nothing rather than a made-up number.
-
-  CONTENT-TYPE IS DELIBERATELY NOT SET. The browser has to write the multipart
-  boundary into it, and setting it by hand sends a body the server cannot parse."
-  ([url body] (request! "POST" url body))
-  ([url body on-progress]
-   (js/Promise.
-    (fn [resolve reject]
-      (let [xhr (js/XMLHttpRequest.)]
-        (.open xhr "POST" url)
-        (.setRequestHeader xhr "Accept" "application/json")
-        (.setRequestHeader xhr "X-CSRFToken" (or (csrf-token) ""))
-        (set! (.. xhr -upload -onprogress)
-              (fn [^js event]
-                (when (and (.-lengthComputable event) (pos? (.-total event)))
-                  (on-progress (/ (.-loaded event) (.-total event))))))
-        (set! (.-onload xhr)
-              (fn []
-                (let [parsed (parse (.-responseText xhr))]
-                  (if (<= 200 (.-status xhr) 299)
-                    (resolve parsed)
-                    (reject (failure (.-status xhr) parsed))))))
-        ;; The three ways a send produces no response at all. None of them has a
-        ;; status, so none of them can go through `failure` — and a dropped
-        ;; upload has to say so, because it is the one failure here that is
-        ;; routine rather than a bug.
-        (set! (.-onerror xhr)
-              (fn [_] (reject (ex-info "the upload did not reach the server" {}))))
-        (set! (.-onabort xhr)
-              (fn [_] (reject (ex-info "the upload was cancelled" {}))))
-        (set! (.-ontimeout xhr)
-              (fn [_] (reject (ex-info "the upload timed out" {}))))
-        (.send xhr body))))))
 
 (defn GET [url] (request! "GET" url))
 (defn POST [url body] (request! "POST" url body))
+(defn POST-form [url body] (request! "POST" url body))
 (defn PUT [url body] (request! "PUT" url body))
 (defn PATCH [url body] (request! "PATCH" url body))