#525 RZX 'repeat last frame' resolves to the wrong frame -> libspectrum_rzx_playback error

[AJ B]

libspectrum_rzx_playback_frame() resolves an IN count of 0xFFFF to the last non-repeat frame. It should be the last frame whose IN values were stored — frames with no INs store nothing, so they have to be skipped:

  if( !rzx->current_input->frames[ rzx->current_frame ].repeat_last )
    rzx->data_frame = &rzx->current_input->frames[ rzx->current_frame ];

A repeat following an empty frame is therefore handed no values, and playback dies on the frame's first IN:

libspectrum_rzx_playback: more INs during frame 390 than stored in RZX file (0)

Reproducing

curl -sSLO https://web.archive.org/web/20070106082932id_/http://www.ramsoft.bbk.org/rzx/rtype.zip
unzip -j rtype.zip
fuse rtype.rzx        # stops at frame 390

Fix

Track the last frame whose IN bytes were stored; resolve a repeat against that.

--- a/rzx.c
+++ b/rzx.c
@@ -119,6 +119,10 @@
   size_t current_frame;

   libspectrum_rzx_frame_t *data_frame;
+

+  /* The last frame whose IN bytes were stored; what a repeat refers back to */
+  libspectrum_rzx_frame_t *stored_frame;
+
   size_t in_count;

   /* Signature parameters */
@@ -499,6 +503,7 @@

     rzx->current_frame = 0; rzx->in_count = 0;
     rzx->data_frame = rzx->current_input->frames;

+    rzx->stored_frame = rzx->data_frame->count ? rzx->data_frame : NULL;

     /* If the previous frame was a snap, return that as well */
     if( previous ) {
@@ -525,6 +530,8 @@
 libspectrum_rzx_playback_frame( libspectrum_rzx *rzx, int *finished,
                libspectrum_snap **snap )
 {

+  libspectrum_rzx_frame_t *frame;
+
   *snap = NULL;
   *finished = 0;

@@ -565,6 +572,7 @@

       rzx->current_frame = 0; rzx->in_count = 0;
       rzx->data_frame = rzx->current_input->frames;

+      rzx->stored_frame = rzx->data_frame->count ? rzx->data_frame : NULL;

     } else {
       *finished = 1;
@@ -575,9 +583,16 @@

   /* Move the data frame pointer along, unless we're supposed to be
      repeating the last frame */

-  if( !rzx->current_input->frames[ rzx->current_frame ].repeat_last )
-    rzx->data_frame = &rzx->current_input->frames[ rzx->current_frame ];
+  frame = &rzx->current_input->frames[ rzx->current_frame ];


+  if( frame->repeat_last ) {
+    /* Frames with no INs store nothing, so they are not what is repeated */
+    if( rzx->stored_frame ) rzx->data_frame = rzx->stored_frame;
+  } else {
+    rzx->data_frame = frame;
+    if( frame->count ) rzx->stored_frame = frame;
+  }
+
   /* And start with the first byte of the new frame */
   rzx->in_count = 0;

Files libspectrum writes are unaffected, libspectrum_rzx_store_frame() emits a repeat only when count != 0 matches frames[non_repeat].count, so the frame it points back to is always both the last non-repeat and the last stored one. make check stays 367/367.

[AJ B]

Please close this ticket as closed-invalid. This issue is invalid and the patch will break things . I tested with 2 different patches in my tree, the poluted build & test environment led to invalid findings.

[Fredrick Meunier]

Thanks for investigating.