fix(timing): advertise the emulated field rate (#392); test(cd): pin Q BCD encoding (#330) (#448)

* test(cd): pin Q-subcode BCD encoding at a double-digit position (#330) Stubbing SubQBCD() to `return (uint8_t)v;` -- emitting binary where the Red Book requires BCD -- left test_cd_synth_subq fully green, 6/6. The existing tests decode through from_bcd() and collect within a few sectors of a low LBA, so every position field is single-digit, and BCD and binary agree below 10. The CRC cannot help: CDROMBuildSubQ() computes it over whatever bytes are present, so it stays self-consistent under either encoding. Add one test that seeks 50 sectors into track 3, where absolute time is 00:10:50, and compares the RAW BYTES against ref_bcd() directly. from_bcd() appears nowhere in it -- round-tripping through the decoder is exactly what hides the bug. Two independent discriminators: * ASEC is exactly 10 by construction, so BCD $10 != binary $0A. * AFRAME and the track-relative FRAME land in [50, 70]; the BCD images of that window ($50..$70) and its binary images ($32..$46) are disjoint. A runtime tightness guard asserts that disjointness instead of merely claiming it in a comment, and a precondition guard fails loudly if the fixture geometry ever stops reaching a double-digit field -- the exact hole #330 documents. Expected values are derived from the seek target the test computes from p_track_info/track_data_lba, not hardcoded. Existing tests 1-6 are untouched and no bound is loosened. Negative controls, both run: SubQBCD() -> `return (uint8_t)v;` -> 6 pass / 1 fail (the new test, on the ASEC byte: "$0A is the BINARY encoding"). CDROMBuildSubQ() stubbed after the position fetch, subQValid forced true -> 3 pass / 4 fail. Reverted and rebuilt after each: back to 7/7. Refs #330 Co-Authored-By: Claude Opus 5 <[email protected]> * fix(timing): advertise the emulated field rate, not 60/50 (#392) retro_get_system_av_info hard-coded fps = 60 (NTSC) / 50 (PAL). The core paces a field of VP+1 halflines, which is 60.05445 Hz and 50.08013 Hz, so the frontend matched vsync and ran dynamic rate control against a number the core does not run at. The emulated field length is ALREADY correct and is not touched. JTRM Rev 8 p.15 (VP): "the number of half lines per field ... one more than the value written ... If the number of half lines is odd then the display is interlaced." JTRM Rev 10 p.8 "Video Timings": Vertical Lines (non interlaced) = 524 NTSC / 624 PAL, with 525/625 listed as the interlaced variants. Titles run non-interlaced, so VP+1 is the even member -- which is exactly what TOMReset programs. A non-interlaced NTSC field is therefore 60.05 Hz, not the 59.94 Hz of 262.5-line interlaced video, and #392's suggestion that VP+1 might be 525 is refuted rather than deferred. Note #392's PAL figures are wrong: it says 626 halflines / 49.9201 Hz. The PAL field is 624 halflines -> 50.08013 Hz, so PAL is UNDER-reported like NTSC, not over-reported. The same wrong 626 was the clamp fallback in DACPrepareFrame and is fixed here -- the only behaviour-bearing audio hunk (PAL-only, and only when a title leaves VP outside [200,1200]). Add one source of truth in jaguar.{h,c} -- JaguarGetHalflinePeriodUs(), JaguarGetDefaultFieldHalflines(), JaguarGetFieldRateHz() -- consumed by libretro.c and dac.c, and route jaguar.c's four inline halfline-period ternaries through it (same literal, same double, bit-identical). The declared sample rate has to follow. retro_run hands the frontend a fixed BUFNTSC/BUFPAL batch (800/960 pairs) once per field, so the true output rate is pairs x field rate = 48043.56 Hz NTSC / 48076.92 Hz PAL. 60 x 800 and 50 x 960 both hit 48000 only because both numbers were wrong together; keeping 48000 against the corrected fps misses test/test_audio_rate.c's 5.0 samples/sec bound by 8.7x and would leave a permanent audio-buffer drift. DAC_AUDIO_RATE stays 48000 (internal resample target; it cancels out of the pitch math), and DRIFT_LIMIT was not touched. Making the batch itself fractional so the product lands on a round 48000 is the remaining coherent option and is deliberately left for its own PR -- it is an audio-behaviour change needing a listening test. Also, no behaviour: * jaguar.c comments claimed 525/625 halflines per field. * test/harness/timing_probe.{c,h}: PAL_HALFLINES was 625, the INTERLACED value, making the PAL cycle expectations 0.16% high (diagnostic only). * docs/jtrm-clocks-timing.md: the vertical-lines row was internally inconsistent (524 halflines is 262.0 lines, not 262.5) and gave PAL the interlaced value; it cited src/tom/tom.c as its source, which is the thing the JTRM is meant to check. Now carries both field rates and the Rev 8 / Rev 10 citations. * docs/fmv-drift-notes.md 5.2/6: RESOLVED banner. The "PDFs have no text layer" blocker applied to the chunked 03/04 scans; Technical Reference v8.pdf / v10.pdf do have one, 524 is confirmed correct, and the feared 0.19% field-clock error does not exist. * src/tom/tom.c: the "Bit 0 in VP is interlace flag" comment is CORRECT (tomRam8[VP + 1] is the VP register's low byte, not VP+1 halflines, so the register bit carries the inverse sense) and was NOT inverted. It now carries the JTRM derivation so the misreading is not repeated. Advertise-only: info->timing.fps had exactly one reference in the tree and SAMPLERATE exactly one consumer, so no emulated state moves. Proven, not asserted: screenshot baselines are byte-identical against a separately built develop worktree. Refs #392 Co-Authored-By: Claude Opus 5 <[email protected]> --------- Co-authored-by: Claude Opus 5 <[email protected]>
2026-11-07 04:36:00 -10:00 · 2026-08-14 23:08:40 -04:00
1 parent 5f0a5c5bde
commit 7de96cba98
@@ -309,6 +309,22 @@ than a visible stall. Worth a diagnostic log line at minimum

 ### 5.2 The emulated NTSC field is one halfline short — unverified, out of scope

+> **RESOLVED (2026-08-14, issue #392).** 524 is **correct** and there is no
+> 0.19 % field-clock error. `Technical Reference v8.pdf` and
+> `Technical Reference v10.pdf` in the same directory *do* carry a text
+> layer (only the chunked `03/04 - *.pdf` scans do not), and they settle it:
+> JTRM Rev 8 p.15 (`VP`) — "the number of half lines per field ... one more
+> than the value written ... **If the number of half lines is odd then the
+> display is interlaced**"; JTRM Rev 10 p.8 "Video Timings" — Vertical Lines
+> (non interlaced) = **524 NTSC / 624 PAL**, with 525/625 listed as the
+> *interlaced* variants. Titles run non-interlaced, so 524/624 is the
+> hardware field and 59.94 Hz is the interlaced 262.5-line rate the Jaguar
+> does not use. The hedge below ("a real Jaguar NTSC field may genuinely be
+> 60.05 Hz") was right. The section's own conclusion — do not change `VP` —
+> stands, and the *advertised* rate now follows the emulated field
+> (`JaguarGetFieldRateHz()`, `libretro.c retro_get_system_av_info`).
+> The original text is left in place below for the record.
+
 `src/tom/tom.c:1159` sets `VP = 523`, i.e. 524 halflines per field:

 ```
@@ -381,7 +397,9 @@ playthrough".
 * **Space Ace beyond boot.** Its clock pair was located
  (`$00582E`/`$005830`) and matches Dragon's Lair bit-for-bit at the same
  field, but no long run was made.
-* **VP against the JTRM** — see §5.2, the PDFs have no text layer.
+* **VP against the JTRM** — see §5.2. **Now done (#392):** the chunked
+  `03/04 - *.pdf` scans have no text layer, but `Technical Reference
+  v8.pdf` / `v10.pdf` do; 524/624 non-interlaced is confirmed.
 * **Anything above the core.** Frontend-side pacing (RetroArch audio-sync
  resampling, frame duping/dropping when the host cannot hold 60 fps,
  fast-forward, run-ahead) is invisible to a headless harness and is not
@@ -50,14 +50,31 @@ Source: `src/jerry/jerry.c`
 | Back porch (colour burst to active) | 5.7 us | 5.6 us |
 | Active display | 52.0 us | 52.0 us |
 | Front porch | 1.0 us | 1.7 us |
-| Total vertical lines | 524 (262.5 per field) | 625 (312.5 per field) |
+| Halflines per field (VP+1, non-interlaced) | 524 (262 lines) | 624 (312 lines) |
+| Halflines per field (interlaced) | 525 | 625 |
+| **Field rate (non-interlaced)** | **60.05445 Hz** | **50.08013 Hz** |
 | Vertical sync lines | 6 | 5 |
 | Pre-equalizing pulses | 6 | 5 |
 | Post-equalizing pulses | 6 | 5 |
 | Vertical blanking lines | ~20 | ~24 |
 | Active display lines | ~240 | ~256 |

-Source: `src/tom/tom.c` (scanline timing, VMODE), `src/tom/op.c` (active display)
+Source: JTRM Rev 10 p.8 "Video Timings" (master clock, periods, vertical
+lines); JTRM Rev 8 p.15 (`VP`: "the number of half lines per field ... one
+more than the value written ... If the number of half lines is odd then the
+display is interlaced", and `HP`: "the period of half a display line ... one
+tick longer than the value written"); `src/tom/op.c` (active display).
+
+The field rate follows from the two rows above: 524 x 31.7778 us =
+16651.56 us -> 60.05445 Hz, and 624 x 32.0 us = 19968.0 us -> 50.08013 Hz.
+Titles run non-interlaced, so 59.94 Hz -- the 262.5-line INTERLACED NTSC
+rate -- is NOT the Jaguar's field rate; 525 halflines would select
+interlace. This is what `retro_get_system_av_info` advertises (issue #392).
+
+The previous version of this row read "524 (262.5 per field) | 625 (312.5
+per field)": internally inconsistent (524 halflines is 262.0 lines) and it
+gave PAL the interlaced value. It also cited `src/tom/tom.c` as its source,
+which is the thing the JTRM is supposed to check.

 ### Pixel Clock Divisor (VMODE Register)

@@ -52,7 +52,9 @@ int64_t rfread(void* buffer, size_t elem_size, size_t elem_count, RFILE* stream)
 #include "log.h"
 #include "version.h" /* generated; defines CORE_VERSION */

-#define SAMPLERATE 48000
+/* Samples (not pairs) handed to the frontend once per field.  These are
+ * also the numerator of the advertised sample rate -- see
+ * retro_get_system_av_info(). */
 #define BUFPAL  1920
 #define BUFNTSC 1600
 #define BUFMAX 2048
@@ -1167,8 +1169,32 @@ void retro_get_system_info(struct retro_system_info *info)
 void retro_get_system_av_info(struct retro_system_av_info *info)
 {
   memset(info, 0, sizeof(*info));
-   info->timing.fps            = vjs.hardwareTypeNTSC ? 60 : 50;
-   info->timing.sample_rate    = SAMPLERATE;
+   /* Advertised timing (issue #392).  The field rate is derived from the
+    * field the core actually paces -- JaguarGetFieldRateHz(), defined
+    * once in jaguar.h with its JTRM citations -- instead of the rounded
+    * 60/50 that used to be hard-coded here.  A non-interlaced NTSC field
+    * is 524 halflines x 31.7778 us = 60.05445 Hz; PAL is 624 x 32.0 us =
+    * 50.08013 Hz.  (NOT 59.94 Hz: that is the 262.5-line INTERLACED rate,
+    * and 525 halflines would put TOM into interlace -- JTRM Rev 8 p.15.)
+    *
+    * The declared sample rate has to follow, because retro_run hands the
+    * frontend a FIXED batch of BUFNTSC/BUFPAL samples -- 800 / 960 stereo
+    * pairs -- exactly once per field, so the true output rate IS
+    * pairs x field rate (48043.6 Hz NTSC / 48076.9 Hz PAL).  Advertising
+    * a flat 48000 against the corrected fps would over-deliver by ~0.09%
+    * forever, draining/overfilling the frontend's audio buffer: the
+    * underrun-pop class that test_audio_rate gates at 5 samples/sec.
+    * Before this change, 60 x 800 and 50 x 960 were both exactly 48000 --
+    * self-consistent only because both numbers were wrong together.
+    *
+    * 48000 remains the DAC's internal resample target (DAC_AUDIO_RATE);
+    * it cancels out of the pitch math (the per-output phase step is
+    * ring-samples-captured / output-pairs), so this is an advertised-value
+    * change only, with no effect on emulated state or emitted samples. */
+   info->timing.fps            = JaguarGetFieldRateHz();
+   info->timing.sample_rate    = (double)(vjs.hardwareTypeNTSC
+                                          ? (BUFNTSC / 2) : (BUFPAL / 2))
+                                 * info->timing.fps;
   info->geometry.base_width   = game_width;
   info->geometry.base_height  = game_height;
   /* Hi-res: the maxima scale by the (load-time-fixed) internal
@@ -47,8 +47,8 @@ static bool frameDone;

 /* Frame-pacing instrumentation (no-op unless built with BENCH_PROFILE).
 * Lets the acid runner / benchmark detect timing regressions like the
- * Doom 2x speed bug -- e.g. expected 525 halflines/frame NTSC, 60 vblank
- * IRQs/sec.  See test/acid/README.md and src/core/perf_counters.h.
+ * Doom 2x speed bug -- e.g. expected 524 halflines/field NTSC (JTRM Rev 10
+ * "Video Timings", non-interlaced), 60.05 vblank IRQs/sec.  See test/acid/README.md and src/core/perf_counters.h.
 * Counters that fire from other TUs are declared at their use sites
 * (PERF_COUNTER backs each name with a file-scope static). */
 PERF_COUNTER(timing_halfline_callbacks);
@@ -1192,16 +1192,40 @@ void JaguarInit(void)
 // regardless of whether it's in interlace mode or not.
 // NB2: Seems it doens't always, not sure what the constraint is...)
 //
+/* Video field geometry -- the single source of truth behind the advertised
+ * field rate (issue #392).  Definitions and the JTRM citations live in
+ * jaguar.h; these are the three accessors every consumer uses so the
+ * numbers cannot drift apart between libretro.c and dac.c. */
+double JaguarGetHalflinePeriodUs(void)
+{
+   return vjs.hardwareTypeNTSC ? VJ_HALFLINE_US_NTSC : VJ_HALFLINE_US_PAL;
+}
+
+uint32_t JaguarGetDefaultFieldHalflines(void)
+{
+   return vjs.hardwareTypeNTSC ? VJ_HALFLINES_PER_FIELD_NTSC
+                               : VJ_HALFLINES_PER_FIELD_PAL;
+}
+
+double JaguarGetFieldRateHz(void)
+{
+   return 1000000.0 / ((double)JaguarGetDefaultFieldHalflines()
+                       * JaguarGetHalflinePeriodUs());
+}
+
 // Normally, TVs will render a full frame in 1/30s (NTSC) or 1/25s (PAL) by
 // rendering two fields that are slighty vertically offset from each other.
 // Each field is created in 1/60s (NTSC) or 1/50s (PAL), and every other line
 // is rendered in this mode so that each field, when overlaid on each other,
 // will yield the final picture at the full resolution for the full frame.
 //
-// We execute a half frame in each timeslice (1/60s NTSC, 1/50s PAL).
-// Since the number of lines in a FULL frame is 525 for NTSC, 625 for PAL,
-// it will be half this number for a half frame. BUT, since we're counting
-// HALF lines, we double this number and we're back at 525 for NTSC, 625 for PAL.
+// We execute one field in each timeslice.  A field is VP+1 HALF lines, and
+// per JTRM Rev 8 p.15 an odd half-line count selects interlace -- so a
+// non-interlaced field is the even value from the Rev 10 "Video Timings"
+// table: 524 for NTSC, 624 for PAL (525/625 are the interlaced variants).
+// That makes a field 16651.56 us NTSC / 19968.0 us PAL, i.e. 60.05445 Hz
+// and 50.08013 Hz -- not 60/50, and not NTSC's interlaced 59.94 Hz.
+// See JaguarGetFieldRateHz() above and jaguar.h for the full citations.
 //
 // Scanline times are 63.5555... μs in NTSC and 64 μs in PAL
 // Half line times are, naturally, half of this. :-P
@@ -1251,7 +1275,7 @@ void HalflineCallback(void)
      uint32_t halfclks;
      uint32_t charge;
      halfclks = (uint32_t)USEC_TO_RISC_CYCLES(
-                    vjs.hardwareTypeNTSC ? 31.777777777 : 32.0);
+                    JaguarGetHalflinePeriodUs());
      charge = bus_arbiter_op_take() + bus_arbiter_refresh_clocks(halfclks);
      /* Bus occupancy within a halfline cannot exceed the halfline:
       * the emulated OP walks its whole list instantly, so a
@@ -1267,8 +1291,7 @@ void HalflineCallback(void)

   /* Blitter bus-time window decays with real time: a blit finishes on
    * its own whether or not anyone is watching (blitter_mmio.c). */
-   BlitterTimingTick(USEC_TO_RISC_CYCLES(
-                        vjs.hardwareTypeNTSC ? 31.777777777 : 32.0));
+   BlitterTimingTick(USEC_TO_RISC_CYCLES(JaguarGetHalflinePeriodUs()));

   //Change this to VBB???
   //Doesn't seem to matter (at least for Flip Out & I-War)
@@ -1292,7 +1315,7 @@ void HalflineCallback(void)
      JaguarCDHLEStreamTick();
   }

-   SetCallbackTime(HalflineCallback, (vjs.hardwareTypeNTSC ? 31.777777777 : 32.0), EVENT_MAIN);
+   SetCallbackTime(HalflineCallback, JaguarGetHalflinePeriodUs(), EVENT_MAIN);
 }

 void JaguarReset(void)
@@ -1490,7 +1513,7 @@ void JaguarReset(void)
   m68k_pulse_reset();								// Reset the 68000

   lowerField = false;								// Reset the lower field flag
-   SetCallbackTime(HalflineCallback, (vjs.hardwareTypeNTSC ? 31.777777777 : 32.0), EVENT_MAIN);
+   SetCallbackTime(HalflineCallback, JaguarGetHalflinePeriodUs(), EVENT_MAIN);
 }


@@ -59,6 +59,45 @@ extern uint32_t jaguarLoadedRAMStart, jaguarLoadedRAMEnd;
 #define SYSTEM_CLOCK_RATE		(vjs.hardwareTypeNTSC ? RISC_CLOCK_RATE_NTSC : RISC_CLOCK_RATE_PAL)
 #define M68K_CLOCK_RATE			(vjs.hardwareTypeNTSC ? M68K_CLOCK_RATE_NTSC : M68K_CLOCK_RATE_PAL)

+/* Video field geometry -- ONE definition, so the rate we advertise to the
+ * frontend can never drift from the field the core actually paces.
+ *
+ * JTRM Revision 8 p.15 (VP -- Vertical Period, $F0003E): "This eleven bit
+ * register determines the number of half lines per field.  The number is
+ * one more than the value written into this register.  If the number of
+ * half lines is odd then the display is interlaced."
+ * JTRM Revision 10 p.8 ("Video Timings"): Vertical Lines (non interlaced)
+ * = 524 NTSC / 624 PAL (the interlaced variants are 525 / 625).
+ *
+ * Jaguar titles run non-interlaced, so VP+1 is the EVEN member: 524 / 624
+ * -- which is exactly what TOMReset programs (VP = 523 / 623).  525 would
+ * put TOM into interlace, so a non-interlaced NTSC field is 60.05 Hz, NOT
+ * the 59.94 Hz of 262.5-line interlaced video (that arithmetic is the same
+ * one that puts a 262-line NES/SNES field at ~60.1 Hz).
+ *
+ * The halfline periods are (HP+1)/master clock, with the HP values
+ * TOMReset writes (844 NTSC / 850 PAL): 845/26.590906 MHz = 31.77778 us
+ * and 851/26.593900 MHz = 31.99972 us, matching the Rev 10 line periods
+ * of 63.5555 us / 64.0 us.
+ *
+ * These are the RESET DEFAULTS.  VP is game-programmable and
+ * HalflineCallback follows the live register, so the emulated field
+ * always tracks whatever a title programs; these are what the machine
+ * runs unless a title says otherwise, and are therefore what gets
+ * advertised (chasing a live VP would mean firing SET_SYSTEM_AV_INFO
+ * mid-session, which tears down the frontend's audio pipeline). */
+#define VJ_HALFLINES_PER_FIELD_NTSC	524u
+#define VJ_HALFLINES_PER_FIELD_PAL	624u
+#define VJ_HALFLINE_US_NTSC		31.777777777	/* (HP+1)/26.590906 MHz */
+#define VJ_HALFLINE_US_PAL		32.0		/* (HP+1)/26.593900 MHz */
+
+/* 31.777777777 (NTSC) / 32.0 (PAL), per vjs.hardwareTypeNTSC. */
+double JaguarGetHalflinePeriodUs(void);
+/* 524 (NTSC) / 624 (PAL): the reset-default field length. */
+uint32_t JaguarGetDefaultFieldHalflines(void);
+/* 1e6 / (halflines * halfline_us): 60.05445 Hz NTSC / 50.08013 Hz PAL. */
+double JaguarGetFieldRateHz(void);
+
 /* Clock-scale enhancement levers (issue #314), in percent of the stock
 * rate (100 = stock).  Config, not state: set from the core options in
 * libretro.c::check_variables(), never serialized.  Applied ONLY where
@@ -86,12 +86,14 @@ static uint32_t i2sResyncCount = 0;	/* gross-drift resyncs this session (diagnos
 *
 * The batch is a fixed `length` (800 NTSC / 960 PAL pairs) but the
 * emulated field is NOT 800 sample periods long: it is VP+1 halflines
- * (799.275 periods at 48 kHz for the standard NTSC VP=523, 961.536 for
- * PAL VP=625).  Scheduling the chain at a flat 1/48000 s landed only
- * 799 (NTSC) / 960 (PAL) callbacks inside the frame while ring capture
- * (word strobes) tracked the full field, so the read cursor fell
- * behind by 0.275 x ratio (NTSC) / 1.54 x ratio (PAL) ring samples per
- * frame.  The lag climbed from I2S_TARGET_LAG to I2S_RESYNC_LAG and
+ * (799.275 periods at 48 kHz for the standard NTSC VP=523, 958.464 for
+ * PAL VP=623).  Scheduling the chain at a flat 1/48000 s landed only
+ * 799 (NTSC) callbacks inside the frame while ring capture (word
+ * strobes) tracked the full field, so the read cursor fell behind by
+ * 0.275 x ratio ring samples per frame.  PAL drifts the other way by
+ * the same magnitude: a 624-halfline field is 958.464 periods but the
+ * flat schedule consumed 960, so the cursor gained 1.536 x ratio ring
+ * samples per frame.  The lag climbed from I2S_TARGET_LAG to I2S_RESYNC_LAG and
 * the gross-drift resync snapped it back, discarding ~254 ring samples
 * (~12 ms) in one hop -- an audible skip every ~36 s NTSC / ~8 s PAL
 * in every title with continuous DSP audio (issue #393).
@@ -343,8 +345,8 @@ void DACPrepareFrame(int length)
    * zero, mid-init writes) to the hardware default field. */
   vp1 = (uint32_t)(TOMReadWord(0xF0003E, JAGUAR) & 0x7FF) + 1;
   if (vp1 < 200 || vp1 > 1200)
-      vp1 = vjs.hardwareTypeNTSC ? 524 : 626;
-   halfline_us = vjs.hardwareTypeNTSC ? 31.777777777 : 32.0;
+      vp1 = JaguarGetDefaultFieldHalflines();
+   halfline_us = JaguarGetHalflinePeriodUs();
   frame_us = (double)vp1 * halfline_us;
   out_pairs = length / 2;
   pairs = (double)out_pairs;
@@ -1341,7 +1341,11 @@ void TOMExecHalfline(uint16_t halfline, bool render)
       * the stock path is unchanged (see shadowfb.h). */
      uint32_t hiresRowScale = (uint32_t)shadowHiresN;

-      // Bit 0 in VP is interlace flag. 0 = interlace, 1 = non-interlaced
+      // Bit 0 in VP is interlace flag. 0 = interlace, 1 = non-interlaced.
+      // (JTRM Rev 8 p.15: half lines per field = VP+1, and an ODD half-line
+      // count is interlaced -- so the REGISTER's bit 0 carries the inverse
+      // sense: VP odd -> VP+1 even -> non-interlaced.  tomRam8[VP + 1] is
+      // the low byte of the 16-bit VP register, not VP+1 halflines.)
      if (tomRam8[VP + 1] & 0x01)
         writtenRow = (halfline - topVisible) / 2;//non-interlace
      else
@@ -29,9 +29,12 @@ static const char *counter_labels[TIMING_NUM_COUNTERS] = {
    "Blitter calls"
 };

-/* NTSC: 524 halflines/frame, PAL: 625 halflines/frame */
+/* Halflines per non-interlaced field (VP+1): JTRM Rev 10 p.8 "Video
+ * Timings", Vertical Lines (non interlaced) = 524 NTSC / 624 PAL.
+ * PAL read 625 here -- the INTERLACED value -- which made the PAL
+ * cycle expectations below 0.16% high. */
 #define NTSC_HALFLINES  524
-#define PAL_HALFLINES   625
+#define PAL_HALFLINES   624

 /* Halfline period in microseconds */
 #define NTSC_HALFLINE_US  31.777777777
@@ -24,7 +24,7 @@
 * ======================================================================
 *
 *   Per frame (via perf_counters deltas):
- *   - Halfline callbacks fired (expected: 524 NTSC, 625 PAL)
+ *   - Halfline callbacks fired (expected: 524 NTSC, 624 PAL)
 *   - VBlank IRQs delivered (expected: 1)
 *   - JERRY IRQs delivered
 *   - GPU-to-68K IRQs delivered
@@ -49,6 +49,15 @@
 *      test 5 fails (position frozen).
 *   6. Stop mutes the window -- after $0200 Stop, reads fall back to
 *      $0000 (subcode exists only while the disc streams).
+ *   7. The position bytes are really BCD -- tests 3..5 collect within a
+ *      few sectors of a low LBA, so every position field is single
+ *      digit, and BCD == binary below 10.  Test 7 collects 50 sectors
+ *      into track 3, where absolute time is 00:10:50, and compares the
+ *      RAW BYTES against ref_bcd() directly -- never round-tripped
+ *      through from_bcd(), which is exactly what hides the bug.
+ *      NEGATIVE CONTROL: stub SubQBCD() (src/cd/cdrom.c) to
+ *      `return (uint8_t)v;` -> test 7 fails, and only test 7 (issue
+ *      #330: before this test that sabotage left the file 6/6 green).
 *
 * Build: make test/test_cd_synth_subq      (needs TEST_EXPORTS=1)
 * Run:   DYLD_LIBRARY_PATH=. test/test_cd_synth_subq
@@ -627,6 +636,118 @@ TEST(stop_returns_subdata_to_zero)
    }
 }

+/* ------------------------------------------------------------------ */
+/* 7. BCD is real: raw bytes at a DOUBLE-DIGIT position (issue #330)    */
+/*                                                                      */
+/* Tests 3-5 collect within a few sectors of a low LBA, so every        */
+/* position field is single-digit -- and BCD == binary below 10.        */
+/* Stubbing SubQBCD() to `return (uint8_t)v;` therefore left all six    */
+/* green.  This test seeks 50 sectors into track 3, where absolute      */
+/* time is 00:10:50, and compares the RAW BYTE against ref_bcd()        */
+/* DIRECTLY.  from_bcd() is deliberately not used anywhere below --     */
+/* round-tripping through the decoder is what hides the bug.            */
+/*                                                                      */
+/* Two independent discriminators:                                      */
+/*   ASEC is exactly 10 by construction -> BCD $10 vs binary $0A.       */
+/*   AFRAME and the track-relative FRAME land in [50, 50+SLOP]; the     */
+/*   BCD images of that window ($50..$70) and its binary images         */
+/*   ($32..$46) are disjoint, which the tightness guard below asserts   */
+/*   at runtime rather than merely claiming in a comment.               */
+/* NEGATIVE CONTROL: SubQBCD() -> `return (uint8_t)v;` -> this fails.   */
+/* ------------------------------------------------------------------ */
+
+/* 50 sectors past track 3's INDEX 01 -- absolute 00:10:50, and the
+ * frame fields stay clear of the 75-frame second boundary. */
+#define BCD_PROBE_OFFSET 50u
+/* Alignment drift budget for collect_frame() (tests 4 and 5 observe
+ * 5-20 and 5-9 frames).  50+20 = 70 < 75, so no second rollover, and
+ * 800+20 = 820 keeps ASEC on 10 (825 would be the boundary). */
+#define BCD_PROBE_SLOP   20u
+
+TEST(raw_bytes_are_bcd_at_double_digit_position)
+{
+    uint8_t  q[SUBQ_BYTES];
+    uint32_t target, abs_lo, exp_amin, exp_asec, v, w;
+    bool     framed, relmatch;
+
+    target   = track_data_lba(3u) + BCD_PROBE_OFFSET;
+    abs_lo   = target + 150u;
+    exp_amin = abs_lo / 4500u;
+    exp_asec = (abs_lo / 75u) % 60u;
+
+    /* PRECONDITION: if the fixture ever stops reaching a double-digit
+     * field, this test has silently stopped testing anything -- which
+     * is the exact hole #330 documents.  Fail loudly instead. */
+    if (exp_asec < 10u)
+        FAIL("fixture no longer reaches a double-digit field (ASEC=%u); "
+             "this test can no longer distinguish BCD from binary",
+             exp_asec);
+    /* exp_asec is exact by construction (computed from `target`).  This
+     * guard says only that alignment slop cannot carry the collected
+     * frame into the NEXT second, which would break both the ASEC
+     * equality and the AFRAME window below. */
+    if (((abs_lo + BCD_PROBE_SLOP) / 75u) % 60u != exp_asec)
+        FAIL("alignment slop of %u frames can cross a second boundary "
+             "(ASEC %u -> %u)", BCD_PROBE_SLOP, exp_asec,
+             ((abs_lo + BCD_PROBE_SLOP) / 75u) % 60u);
+    /* TIGHTNESS: the admissible BCD images and the binary images of the
+     * same window must be disjoint, or a matching byte would prove
+     * nothing about the encoding. */
+    for (v = BCD_PROBE_OFFSET; v <= BCD_PROBE_OFFSET + BCD_PROBE_SLOP; v++)
+        for (w = BCD_PROBE_OFFSET; w <= BCD_PROBE_OFFSET + BCD_PROBE_SLOP; w++)
+            if (ref_bcd(v) == (uint8_t)w)
+                FAIL("frame-field window %u..%u cannot distinguish BCD from "
+                     "binary: ref_bcd(%u) == $%02X == binary %u",
+                     BCD_PROBE_OFFSET, BCD_PROBE_OFFSET + BCD_PROBE_SLOP,
+                     v, ref_bcd(v), w);
+
+    if (!prime_stream(target))
+        FAIL("could not prime the stream %u sectors into track 3",
+             BCD_PROBE_OFFSET);
+    arm_subcode();
+    if (!collect_frame(q))
+        FAIL("no aligned Q frame within the collection window");
+
+    /* Frame integrity first, so a raw-byte mismatch below means the
+     * ENCODING is wrong, not that we grabbed a torn frame. */
+    if (ref_crc16(q, SUBQ_BYTES) != 0x1D0Fu)
+        FAIL("CRC residual $%04X != $1D0F", ref_crc16(q, SUBQ_BYTES));
+    if (q[1] != ref_bcd(3u))
+        FAIL("collected a frame for track byte $%02X, expected $03 -- the "
+             "seek did not land inside track 3", q[1]);
+
+    /* --- the assertions that fail under the sabotage --- */
+
+    /* AMIN: exact.  ASEC: exact AND >= 10, so BCD($10) != binary($0A). */
+    if (q[7] != ref_bcd(exp_amin))
+        FAIL("AMIN byte $%02X, expected $%02X (ref_bcd(%u)) -- raw byte, "
+             "not decoded", q[7], ref_bcd(exp_amin), exp_amin);
+    if (q[8] != ref_bcd(exp_asec))
+        FAIL("ASEC byte $%02X, expected $%02X (ref_bcd(%u)); $%02X is the "
+             "BINARY encoding -- SubQBCD() is not encoding",
+             q[8], ref_bcd(exp_asec), exp_asec, (uint8_t)exp_asec);
+
+    /* AFRAME and the track-relative FRAME both sit in
+     * [OFFSET, OFFSET+SLOP], every member >= 10.  Match the raw byte
+     * against the ref_bcd() of each admissible value -- still a direct
+     * raw-byte comparison, never a decode. */
+    framed = false; relmatch = false;
+    for (v = BCD_PROBE_OFFSET; v <= BCD_PROBE_OFFSET + BCD_PROBE_SLOP; v++)
+    {
+        if (q[9] == ref_bcd(v))
+            framed = true;
+        if (q[5] == ref_bcd(v))
+            relmatch = true;
+    }
+    if (!framed)
+        FAIL("AFRAME byte $%02X is not ref_bcd(v) for any v in %u..%u",
+             q[9], BCD_PROBE_OFFSET, BCD_PROBE_OFFSET + BCD_PROBE_SLOP);
+    if (!relmatch)
+        FAIL("relative FRAME byte $%02X is not ref_bcd(v) for any v in "
+             "%u..%u", q[5], BCD_PROBE_OFFSET,
+             BCD_PROBE_OFFSET + BCD_PROBE_SLOP);
+}
+
 /* ------------------------------------------------------------------ */
 /* Symbol resolution                                                    */
 /* ------------------------------------------------------------------ */
@@ -703,6 +824,7 @@ int main(int argc, char *argv[])
    RUN_TEST(pregap_frame_has_index_zero_and_audio_control);
    RUN_TEST(q_position_advances_one_sector_per_588_samples);
    RUN_TEST(stop_returns_subdata_to_zero);
+    RUN_TEST(raw_bytes_are_bcd_at_double_digit_position);

    rc = TEST_REPORT();