z80 branch, but with original (slightly more accurate) prob update

src/context_state.rs

@@ -33,13 +33,12 @@ impl<'a> Context<'a> {
 
     pub fn update(&mut self, bit: bool) {
         let old = self.state.contexts[self.index];
-        let offset = if !bit {
-            ONE_PROB as i32 >> UPDATE_RATE
-        } else {
-            0
-        };
-
+        if bit {
             self.state.contexts[self.index] =
-            (offset + old as i32 - ((old as i32 + UPDATE_ADD) >> UPDATE_RATE)) as u8;
+                old - ((old as i32 + UPDATE_ADD) >> UPDATE_RATE) as u8;
+        } else {
+            self.state.contexts[self.index] =
+                old + (((ONE_PROB as i32 - old as i32) + UPDATE_ADD) >> UPDATE_RATE) as u8;
+        }
     }
 }

src/main.rs

@@ -10,20 +10,16 @@ fn main() -> Result<()> {
         Some("pack") => {
             let level = args.opt_value_from_str(["-l", "--level"])?.unwrap_or(2u8);
             let use_bitstream = args.contains(["-b", "--bitstream"]);
-            let reverse = args.contains(["-r", "--reverse"]);
 
             let infile = args.free_from_os_str::<PathBuf, bool>(|s| Ok(s.into()))?;
             let outfile = args.free_from_os_str::<PathBuf, bool>(|s| Ok(s.into()))?;
 
             let mut data = vec![];
             File::open(infile)?.read_to_end(&mut data)?;
-            if reverse {
-                data.reverse();
-            }
 
             let mut pb = pbr::ProgressBar::new(data.len() as u64);
             pb.set_units(pbr::Units::Bytes);
-            let mut packed_data = upkr::pack(
+            let packed_data = upkr::pack(
                 &data,
                 level,
                 use_bitstream,
@@ -33,10 +29,6 @@ fn main() -> Result<()> {
             );
             pb.finish();
 
-            if reverse {
-                packed_data.reverse();
-            }
-
             println!(
                 "Compressed {} bytes to {} bytes ({}%)",
                 data.len(),
@@ -47,21 +39,14 @@ fn main() -> Result<()> {
         }
         Some("unpack") => {
             let use_bitstream = args.contains(["-b", "--bitstream"]);
-            let reverse = args.contains(["-r", "--reverse"]);
 
             let infile = args.free_from_os_str::<PathBuf, bool>(|s| Ok(s.into()))?;
             let outfile = args.free_from_os_str::<PathBuf, bool>(|s| Ok(s.into()))?;
 
             let mut data = vec![];
             File::open(infile)?.read_to_end(&mut data)?;
-            if reverse {
-                data.reverse();
-            }
-            let mut unpacked_data = upkr::unpack(&data, use_bitstream);
-            if reverse {
-                unpacked_data.reverse();
-            }
-            File::create(outfile)?.write_all(&unpacked_data)?;
+            let packed_data = upkr::unpack(&data, use_bitstream);
+            File::create(outfile)?.write_all(&packed_data)?;
         }
         Some(other) => {
             bail!("Unknown subcommand '{}'", other);

z80_unpacker/.gitignore

@@ -1,3 +1,4 @@
 *.bin
 *.tap
+*.sna
 *.lst

z80_unpacker/example/example.asm

@@ -3,8 +3,7 @@
     DEVICE ZXSPECTRUM48,$8FFF
 
     ORG     $9000
-  ;; forward example data
-compressed_scr_files.fwd:               ; border color byte + upkr-packed .scr file
+compressed_scr_files:       ; border color byte + upkr-packed .scr file
     DB      1
     INCBIN  "screens/Grongy - ZX Spectrum (2022).scr.upk"
     DB      7
@@ -14,87 +13,37 @@ compressed_scr_files.fwd:               ; border color byte + upkr-packed .scr f
     DB      6
     INCBIN  "screens/diver - Back to Bjork (2015).scr.upk"
 .e:
-  ;; backward example data (unpacker goes from the end of the data!)
-compressed_scr_files.rwd.e: EQU $-1     ; the final IX will point one byte ahead of "$" here
-    INCBIN  "screens.reversed/diver - Back to Bjork (2015).scr.upk"
-    DB      6
-    INCBIN  "screens.reversed/diver - Mercenary 4. The Heaven's Devil (2014) (Forever 2014 Olympic Edition, 1).scr.upk"
-    DB      0
-    INCBIN  "screens.reversed/Schafft - Poison (2017).scr.upk"
-    DB      7
-    INCBIN  "screens.reversed/Grongy - ZX Spectrum (2022).scr.upk"
-compressed_scr_files.rwd:               ; border color byte + upkr-packed .scr file (backward)
-    DB      1
 
 start:
     di
 ;     OPT --zxnext
 ;     nextreg 7,3                 ; ZX Next: switch to 28Mhz
-
-  ;;; FORWARD packed/unpacked data demo
-    ld      ix,compressed_scr_files.fwd
-.slideshow_loop.fwd:
+    ld      ix,compressed_scr_files
+.slideshow_loop
   ; set BORDER for next image
-    ld      a,(ix)
-    inc     ix
+    ldi     a,(ix)              ; fake: ld a,(ix) : inc ix
     out     (254),a
   ; call unpack of next image directly into VRAM
     ld      de,$4000            ; target VRAM
     exx
   ; IX = packed data, DE' = destination ($4000)
   ; returned IX will point right after the packed data
-    call    fwd.upkr.unpack
+    call    upkr.unpack
   ; do some busy loop with CPU to delay between images
-    call    delay
-  ; check if all images were displayed, loop around from first one then
-    ld      a,ixl
-    cp      low compressed_scr_files.fwd.e
-    jr      nz,.slideshow_loop.fwd
-
-  ;;; BACKWARD packed/unpacked data demo
-    ld      ix,compressed_scr_files.rwd
-.slideshow_loop.rwd:
-  ; set BORDER for next image
-    ld      a,(ix)
-    dec     ix
-    out     (254),a
-  ; call unpack of next image directly into VRAM
-    ld      de,$5AFF                    ; target VRAM
-    exx
-  ; IX = packed data, DE' = destination
-  ; returned IX will point right ahead of the packed data
-    call    rwd.upkr.unpack
-  ; do some busy loop with CPU to delay between images
-    call    delay
-  ; check if all images were displayed, loop around from first one then
-    ld      a,ixl
-    cp      low compressed_scr_files.rwd.e
-    jr      nz,.slideshow_loop.rwd
-
-    jr      start
-
-delay:
     ld      bc,$AA00
 .delay:
     .8 ex      (sp),ix
     dec     c
     jr      nz,.delay
     djnz    .delay
-    ret
+  ; check if all images were displayed, loop around from first one then
+    ld      a,ixl
+    cp      low compressed_scr_files.e
+    jr      z,start
+    jr      .slideshow_loop
 
   ; include the depacker library, optionally putting probs array buffer near end of RAM
     DEFINE  UPKR_PROBS_ORIGIN $FA00   ; if not defined, array will be put after unpack code
-
-    MODULE fwd
     INCLUDE "../unpack.asm"
-    ENDMODULE
-
-    MODULE rwd
-        DEFINE BACKWARDS_UNPACK         ; defined to build backwards unpack
-                ; initial IX points at last byte of compressed data
-                ; initial DE' points at last byte of unpacked data
-
-        INCLUDE "../unpack.asm"
-    ENDMODULE
 
     SAVESNA "example.sna",start

z80_unpacker/readme.txt

@@ -10,23 +10,10 @@ may be incompatible with files you will produce with current version)
 
 Asm syntax is z00m's sjasmplus: https://github.com/z00m128/sjasmplus
 
-Backward direction unpacker added as compile-time option, see example for both forward/backward
-depacker in action.
-
-The packed/unpacked data-overlap has to be tested per-case, in worst case the packed data
-may need even more than 7 bytes to unpack final byte, but usually 1-4 bytes may suffice.
-
 TODO:
-- build bigger corpus of test data to benchmark future changes in algorithm/format (example and zx48.rom was used to do initial tests)
-- maybe try to beat double-loop `decode_number` with different encoding format
+- build base corpus of test data to benchmark future changes in algorithm/format
+- review first implementation to identify weak spots where the implementation can be shorter+faster
+with acceptable small changes to the format
+- review non-bitstream variant, if it's feasible to try to implement it with Z80
 - (@ped7g) Z80N version of unpacker for ZX Next devs
 - (@exoticorn) add Z80 specific packer (to avoid confusion with original MicroW8 variant), and land it all to master branch, maybe in "z80" directory or something? (and overall decide how to organise+merge this upstream into main repo)
-- (@exoticorn) add to packer output with possible packed/unpacked region overlap
-
-DONE:
-* review non-bitstream variant, if it's feasible to try to implement it with Z80
-    - Ped7g: IMHO nope, the 12b x 8b MUL code would probably quickly cancel any gains from the simpler state update
-* review first implementation to identify weak spots where the implementation can be shorter+faster
-with acceptable small changes to the format
-    - Ped7g: the decode_bit settled down and now doesn't feel so confused and redundant, the code seems pretty on point to me, no obvious simplification from format change
-    - Ped7g: the decode_number double-loop is surprisingly resilient, especially in terms of code size I failed to beat it, speed wise only negligible gains

z80_unpacker/unpack.asm

@@ -4,7 +4,7 @@
 ;; initial version by Peter "Ped" Helcmanovsky (C) 2022, licensed same as upkr project ("unlicensed")
 ;; to assemble use z00m's sjasmplus: https://github.com/z00m128/sjasmplus
 ;;
-;; you can define UPKR_PROBS_ORIGIN to specific 256 byte aligned address for probs array (320 bytes),
+;; you can define UPKR_PROBS_ORIGIN to specific 256 byte aligned address for probs array (386 bytes),
 ;; otherwise it will be positioned after the unpacker code (256 aligned)
 ;;
 ;; public API:
@@ -12,24 +12,12 @@
 ;;     upkr.unpack
 ;;         IN: IX = packed data, DE' (shadow DE) = destination
 ;;         OUT: IX = after packed data
-;;         modifies: all registers except IY, requires 10 bytes of stack space
+;;         modifies: all registers except IY, requires 14 bytes of stack space
 ;;
 
-;     DEFINE BACKWARDS_UNPACK         ; uncomment to build backwards depacker (write_ptr--, upkr_data_ptr--)
-            ; initial IX points at last byte of compressed data
-            ; initial DE' points at last byte of unpacked data
-
-;     DEFINE UPKR_UNPACK_SPEED        ; uncomment to get larger but faster unpack routine
-
-; code size hint: if you put probs array just ahead of BASIC entry point, you will get BC
-; initialised to probs.e by BASIC `USR` command and you can remove it from unpack init (-3B)
-
     OPT push reset --syntax=abf
     MODULE upkr
 
-NUMBER_BITS     EQU     16+15       ; context-bits per offset/length (16+15 for 16bit offsets/pointers)
-    ; numbers (offsets/lengths) are encoded like: 1a1b1c1d1e0 = 0000'0000'001e'dbca
-
 /*
 u8* upkr_data_ptr;
 u8 upkr_probs[1 + 255 + 1 + 2*32 + 2*32];
@@ -93,8 +81,8 @@ unpack:
     ; BC = probs (context_index 0), state HL = 0, A' = 0x80 (no source bits left in upkr_current_byte)
 
   ; ** main loop to decompress data
-    ; D = prev_was_match = uninitialised, literal is expected first => will reset D to "false"
-    ; values for false/true of prev_was_match are: false = high(probs), true = 1 + high(probs)
+.decompress_data_reset_match:
+    ld      d,0                 ; prev_was_match = 0;
 .decompress_data:
     ld      c,0
     call    decode_bit          ; if(upkr_decode_bit(0))
@@ -109,23 +97,23 @@ unpack:
     ld      a,c
     exx
     ld      (de),a              ; *write_ptr++ = byte;
-    IFNDEF BACKWARDS_UNPACK : inc de : ELSE : dec de : ENDIF
+    inc     de
     exx
-    ld      d,b                 ; prev_was_match = false
-    jr      .decompress_data
+    jr      .decompress_data_reset_match
 
   ; * copy chunk of already decompressed data (match)
 .copy_chunk:
-    ld      a,b
     inc     b                   ; context_index = 256
         ;             if(prev_was_match || upkr_decode_bit(256)) {
         ;                 offset = upkr_decode_length(257) - 1;
         ;                 if (0 == offset) break;
         ;             }
+    xor     a
     cp      d                   ; CF = prev_was_match
     call    nc,decode_bit       ; if not prev_was_match, then upkr_decode_bit(256)
     jr      nc,.keep_offset     ; if neither, keep old offset
-    call    decode_number       ; context_index is already 257-1 as needed by decode_number
+    inc     c
+    call    decode_length
     dec     de                  ; offset = upkr_decode_length(257) - 1;
     ld      a,d
     or      e
@@ -138,27 +126,18 @@ unpack:
         ;                 ++write_ptr;
         ;             }
         ;             prev_was_match = 1;
-    ld      c,low(257 + NUMBER_BITS - 1)    ; context_index to second "number" set for lengths decoding
-    call    decode_number       ; length = upkr_decode_length(257 + 64);
+    ld      c,low(257+64)       ; context_index = 257+64
+    call    decode_length       ; length = upkr_decode_length(257 + 64);
     push    de
     exx
-    IFNDEF BACKWARDS_UNPACK
-        ; forward unpack (write_ptr++, upkr_data_ptr++)
     ld      h,d                 ; DE = write_ptr
     ld      l,e
 .offset+*:  ld      bc,0
-        sbc     hl,bc           ; CF=0 from decode_number ; HL = write_ptr - offset
+    sbc     hl,bc               ; CF=0 from decode_length ; HL = write_ptr - offset
     pop     bc                  ; BC = length
     ldir
-    ELSE
-        ; backward unpack (write_ptr--, upkr_data_ptr--)
-.offset+*:  ld  hl,0
-        add     hl,de           ; HL = write_ptr + offset
-        pop     bc              ; BC = length
-        lddr
-    ENDIF
     exx
-    ld      d,b                 ; prev_was_match = true
+    ld      d,b                 ; prev_was_match = non-zero
     djnz    .decompress_data    ; adjust context_index back to 0..255 range, go to main loop
 
 /*
@@ -190,9 +169,6 @@ int upkr_decode_bit(int context_index) {
     return bit;
 }
 */
-inc_c_decode_bit:
-  ; ++low(context_index) before decode_bit (to get -1B by two calls in decode_number)
-    inc     c
 decode_bit:
   ; HL = upkr_state
   ; IX = upkr_data_ptr
@@ -213,7 +189,7 @@ decode_bit:
     jr      nz,.has_bit             ; CF=data, ZF=0 -> some bits + stop bit still available
   ; CF=1 (by stop bit)
     ld      a,(ix)
-    IFNDEF BACKWARDS_UNPACK : inc ix : ELSE : dec ix : ENDIF    ; upkr_current_byte = *upkr_data_ptr++;
+    inc     ix                      ; upkr_current_byte = *upkr_data_ptr++;
     adc     a,a                     ; CF=data, b0=1 as new stop bit
 .has_bit:
     adc     hl,hl                   ; upkr_state = (upkr_state << 1) + (upkr_current_byte >> 7);
@@ -226,8 +202,9 @@ decode_bit:
     cp      l                       ; CF = bit = prob-1 < (upkr_state & 255) <=> prob <= (upkr_state & 255)
     inc     a
   ; ** adjust state
-    push    bc
-    ld      c,l                     ; C = (upkr_state & 255); (preserving the value)
+    push    af
+    push    af
+    push    hl
     push    af
     jr      nc,.bit_is_0
     neg                             ; A = -prob == (256-prob), CF=1 preserved
@@ -235,52 +212,38 @@ decode_bit:
     ld      d,0
     ld      e,a                     ; DE = state_scale ; prob || (256-prob)
     ld      l,d                     ; H:L = (upkr_state>>8) : 0
-
-  IFNDEF UPKR_UNPACK_SPEED
-
-    ;; looped MUL for minimum unpack size
-    ld      b,8                     ; counter
+    ld      a,8                     ; counter
 .mulLoop:
     add     hl,hl
     jr      nc,.mul0
     add     hl,de
 .mul0:
-    djnz    .mulLoop                ; until HL = state_scale * (upkr_state>>8), also BC becomes (upkr_state & 255)
-
-  ELSE
-
-    ;;; unrolled MUL for better performance, +25 bytes unpack size
-    ld      b,d
-    DUP     8
-        add     hl,hl
-        jr      nc,0_f
-        add     hl,de
-0:
-    EDUP
-
-  ENDIF
-
-    add     hl,bc                   ; HL = state_scale * (upkr_state >> 8) + (upkr_state & 255)
-    pop     af                      ; restore prob and CF=bit
+    dec     a
+    jr      nz,.mulLoop             ; until HL = state_scale * (upkr_state>>8)
+    pop     af
     jr      nc,.bit_is_0_2
-    dec     d                       ; DE = -prob (also D = bit ? $FF : $00)
+    dec     d                       ; D = 0xFF (DE = -prob)
     add     hl,de                   ; HL += -prob
-    ; ^ this always preserves CF=1, because (state>>8) >= 128, state_scale: 7..250, prob: 7..250,
-    ; so 7*128 > 250 and thus edge case `ADD hl=(7*128+0),de=(-250)` => CF=1
-.bit_is_0_2:
+.bit_is_0_2:                        ; HL = state_offset + state_scale * (upkr_state >> 8)
+    pop     de
+    ld      d,0                     ; DE = (upkr_state & 255)
+    add     hl,de                   ; HL = state_offset + state_scale * (upkr_state >> 8) + (upkr_state & 255) ; new upkr_state
  ; *** adjust probs[context_index]
-    rra                             ; + (bit<<4) ; part of -prob_offset, needs another -16
-    and     $FC                     ; clear/keep correct bits to get desired (prob>>4) + extras, CF=0
-    rra
-    rra
-    rra                             ; A = (bit<<4) + (prob>>4), CF=(prob & 8)
-    adc     a,-16                   ; A = (bit<<4) - 16 + ((prob + 8)>>4) ; -prob_offset = (bit<<4) - 16
+    pop     af                      ; restore prob and bit
     ld      e,a
-    pop     bc
-    ld      a,(bc)                  ; A = prob (cheaper + shorter to re-read again from memory)
-    sub     e                       ; A = 16 - (bit<<4) + prob - ((prob + 8)>>4) ; = prob_offset + prob - ((prob + 8)>>4)
-    ld      (bc),a                  ; probs[context_index] = prob_offset + prob - ((prob + 8) >> 4);
-    add     a,d                     ; restore CF = bit (D = bit ? $FF : $00 && A > 0)
+    jr      c,.bit_is_1
+    ld      d,-16                   ; 0xF0
+.bit_is_1:                          ; D:E = -prob_offset:prob, A = prob
+    and     $F8
+    rra
+    rra
+    rra
+    rra
+    adc     a,d                     ; A = -prob_offset + ((prob + 8) >> 4)
+    neg
+    add     a,e                     ; A = prob_offset + prob - ((prob + 8) >> 4)
+    ld      (bc),a                  ; update probs[context_index]
+    pop     af                      ; restore resulting CF = bit
     pop     de
     ret
 
@@ -295,19 +258,22 @@ int upkr_decode_length(int context_index) {
     return length | (1 << bit_pos);
 }
 */
-decode_number:
+decode_length:
   ; HL = upkr_state
   ; IX = upkr_data_ptr
-  ; BC = probs+context_index-1
+  ; BC = probs+context_index
   ; A' = upkr_current_byte (!!! init to 0x80 at start, not 0x00)
   ; return length in DE, CF=0
-    ld      de,$FFFF            ; length = 0 with positional-stop-bit
-    or      a                   ; CF=0 to skip getting data bit and use only `rr d : rr e` to fix init DE
+    ld      de,$7FFF            ; length = 0 with positional-stop-bit
+    jr      .loop_entry
 .loop:
-    call    c,inc_c_decode_bit  ; get data bit, context_index + 1 / if CF=0 just add stop bit into DE init
+    inc     c                   ; context_index + 1
+    call    decode_bit
     rr      d
     rr      e                   ; DE = length = (length >> 1) | (bit << 15);
-    call    inc_c_decode_bit    ; context_index += 2
+    inc     c                   ; context_index += 2
+.loop_entry:
+    call    decode_bit
     jr      c,.loop
 .fix_bit_pos:
     ccf                         ; NC will become this final `| (1 << bit_pos)` bit
@@ -321,61 +287,15 @@ decode_number:
     ; reserve space for probs array without emitting any machine code (using only EQU)
 
     IFDEF UPKR_PROBS_ORIGIN     ; if specific address is defined by user, move probs array there
-probs:      EQU ((UPKR_PROBS_ORIGIN) + 255) & -$100     ; probs array aligned to 256
-    ELSE
-probs:      EQU ($ + 255) & -$100                       ; probs array aligned to 256
+    ORG UPKR_PROBS_ORIGIN
     ENDIF
-.real_c:    EQU 1 + 255 + 1 + 2*NUMBER_BITS             ; real size of probs array
+
+probs:      EQU ($+255) & -$100                 ; probs array aligned to 256
+.real_c:    EQU 1 + 255 + 1 + 2*32 + 2*32       ; real size of probs array
 .c:         EQU (.real_c + 1) & -2              ; padding to even size (required by init code)
 .e:         EQU probs + .c
 
     DISPLAY "upkr.unpack probs array placed at: ",/A,probs,",\tsize: ",/A,probs.c
 
-/*
- archived: negligibly faster but +6B longer decode_number variant using HL' and BC' to
- do `number|=(1<<bit_pos);` type of logic in single loop.
-*/
-; decode_number:
-;     exx
-;     ld      bc,1
-;     ld      l,b
-;     ld      h,b                 ; HL = 0
-; .loop
-;     exx
-;     inc     c
-;     call    decode_bit
-;     jr      nc,.done
-;     inc     c
-;     call    decode_bit
-;     exx
-;     jr      nc,.b0
-;     add     hl,bc
-; .b0:
-;     sla     c
-;     rl      b
-;     jr      .loop
-; .done:
-;     exx
-;     add     hl,bc
-;     push    hl
-;     exx
-;     pop     de
-;     ret
-
-/*
- archived: possible LUT variant of updating probs value, requires 512-aligned 512B table (not tested)
-*/
-; code is replacing decode_bit from "; *** adjust probs[context_index]", followed by `ld (bc),a : add a,d ...`
-;     ld      c,a
-;     ld      a,high(probs_update_table)/2    ; must be 512 aligned
-;     rla
-;     ld      b,a
-;     ld      a,(bc)
-;     pop     bc
-; -------------------------------------------
-; probs_update_table: EQU probs-512
-; -------------------------------------------
-; table generator is not obvious and probably not short either, 20+ bytes almost for sure, maybe even 30-40
-
     ENDMODULE
     OPT pop