premature flac decode deletion solved

audio-placed-player.rkt

@@ -67,6 +67,7 @@
         (player-state 'stopped)
         (decoder-buf-info #f)
         (decoder-meta #f)
+        (playback-error #f)
         (feeding-audio #f)
         (feed-interrupted #f)
         )
@@ -114,6 +115,7 @@
                                          (set! feeding-audio #f)
                                          (set! feed-interrupted #f)
                                          (set! player-state 'stopped)
+                                         (set! playback-error (exn-message e))
                                          (let ((code (if (exn:fail:audio? e) (exn:fail:audio-code e) 'no-audio-error)))
                                            (state (format "audio-read-worker: exception ~a" code) evt)
                                            (evt (list 'exception (exn-message e) code)))
@@ -330,12 +332,15 @@
       
       (cleanup)
       
+      (set! playback-error #f)
       (state "stop-and-cleanup: stopped/cleaned" evt 'force)
       
       player-state
       )
 
     (define (start file)
+      (when playback-error (stop-and-cleanup))
+      (set! playback-error #f)
       (dbg-sound "starting ~a" file)
       (when feeding-audio
         (dbg-sound "interrupting feed")
@@ -361,6 +366,8 @@
         )
 
       
+      ;; A failed earlier Stop must not suppress this track's EOF notification.
+      (set! feed-interrupted #f)
       ;(set! current-file-id (+ current-file-id 1))
       (set! current-file-id (+ (* (get-current-seconds) 10000) (random 1000)))
       (let ((f (build-path file)))
@@ -443,6 +450,7 @@
             (hash-set! h 'at-music-id m-id)
             (hash-set! h 'volume current-volume)
             (hash-set! h 'buf-size (if (ao-valid? ao-h) (ao-bufsize-async ao-h) 0))
+            (hash-set! h 'error playback-error)
             (hash-set! h 'reuse-buf-len (if (ao-valid? ao-h)
                                             (ao-reuse-buf-len-async ao-h)
                                             #f))

flac-decoder.rkt

@@ -92,6 +92,15 @@
       )
     )
 
+  ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
+  ; goal   : Decode FLAC frames and release the decoder on every exit.
+  ; pre    : Handle is open and is not already being read.
+  ; post   : The native decoder is released and reading is false.
+  ; result : Void on completion; decoding and callback errors propagate.
+  ; internals: reader processes frames and callbacks. dynamic-wind deletes
+  ;            the native decoder before clearing reading, so flac-stop
+  ;            cannot return while decoder cleanup is still in progress.
+  ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
   (define (flac-read handle)
     (let* ((ffi-handler (flac-handle-ffi-decoder-handler handle))
            (state (ffi-handler 'state))
@@ -103,10 +112,14 @@
                          (if (flac-handle-stop-reading handle)
                              (begin
                                (dbg-sound "handling stop at: ~a" (current-milliseconds))
-                               (set-flac-handle-reading! handle #f)
                                'stopped-reading)
                              (let* ((st (ffi-handler 'state)))
-                               (ffi-handler 'process-single)
+                               (when (memq st '(ogg-error seek-error aborted
+                                               memory-allocation-error uninitialized))
+                                 (error 'flac-read "Decoder cannot continue in state ~a" st))
+                               (unless (ffi-handler 'process-single)
+                                 (error 'flac-read "Could not decode FLAC frame ~a (state ~a)"
+                                        frame-nr (ffi-handler 'state)))
                                (unless (eq? state st)
                                  (set! state st)
                                  (dbg-sound "Now in state ~a (frame-nr = ~a) (int-state = ~a)"
@@ -131,14 +144,17 @@
                                  )
                                
                                (if (eq? st 'end-of-stream)
-                                   (begin
+                                   st
-                                     (set-flac-handle-reading! handle #f)
-                                     st)
                                    (reader (+ frame-nr 1))))))
                        ))
-        (reader 0)
+        (dynamic-wind
-        ; done reading, delete flac encoder
+          void
-        (ffi-handler 'delete)
+          (λ () (reader 0) (void))
+          (λ ()
+            (dynamic-wind
+              void
+              (λ () (ffi-handler 'delete))
+              (λ () (set-flac-handle-reading! handle #f)))))
         )
       )
     )
@@ -175,6 +191,14 @@
              (ffi-handler 'seek-to-sample sample)))))
 
   
+  ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
+  ; goal   : Stop decoding and safely release an idle FLAC decoder.
+  ; pre    : Handle was returned by flac-open.
+  ; post   : Reading and native decoder cleanup have finished.
+  ; result : Void; repeated stops and stops after EOF are harmless.
+  ; internals: An active reader observes stop-reading and owns cleanup.
+  ;            Otherwise the FFI handler performs idempotent deletion.
+  ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
   (define (flac-stop handle)
     (let ((ct (current-milliseconds)))
       (dbg-sound "requesting stop at: ~a" ct)

libflac-ffi.rkt

@@ -615,17 +615,16 @@
       (set! flac-file file)
       r))
 
+  ;;; Releases the decoder once; stopping after EOF or an earlier stop is safe.
   (define (delete)
+    (when fl
       (dbg-sound "flac-ffi 'delete")
-    (if (eq? fl #f)
-        (error "flac handler has already been deleted")
-        (begin
       (FLAC__stream_decoder_finish fl)
       (FLAC__stream_decoder_delete fl)
       (set! fl #f)
       ;; libFLAC cannot call the callbacks anymore after finish/delete,
       ;; so the generated callback stubs can now be released.
-          (set-box! callback-keepers null))))
+      (set-box! callback-keepers null)))
 
   (define (process-single)
     (FLAC__stream_decoder_process_single fl))

scrbl/audio-player.scrbl

@@ -85,7 +85,11 @@ The @racket[state-hash] contains the detailed playback state reported by the
 worker.  It includes values such as the current playback position, stream
 duration, buffer status, music id, and libao handle validity.  Code that only
 needs the logical playback state should use @racket[current-player-state]
-instead of extracting it from the hash.
+instead of extracting it from the hash. The @racket['error] entry is
+@racket[#f] during normal playback or an error message when decoding fails.
+A failed read reports @racket['stopped]; starting another track cleans up the
+failed playback before opening a new decoder. Each new track also resets the
+interruption flag, so an earlier failed Stop cannot suppress its EOF event.
 
 The @racket[cb-eof-stream] callback is called as:
 

scrbl/flac-decoder.scrbl

@@ -66,17 +66,17 @@ handler.
 }
 
 @defproc[(flac-read [handle flac-handle?])
-         (or/c 'stopped-reading
+         void?]{
-               'end-of-stream)]{
 
 Reads the stream by repeatedly calling the native decoder with
 @racket['process-single].
 
 Before reading starts, the handle fields @racket[stop-reading]
 and @racket[reading] are set to @racket[#f] and @racket[#t]. If a
-stop has been requested with @racket[flac-stop], reading ends
+stop has been requested with @racket[flac-stop], the read loop ends.
-with @racket['stopped-reading] and @racket[reading] is reset to
+Native decoder cleanup runs on normal completion, exceptions and breaks; only
-@racket[#f].
+after cleanup does @racket[reading] become @racket[#f]. Decoder failures and
+errors raised by the audio consumer propagate to the caller.
 
 Whenever pending metadata is available, it is processed with
 @racket[process-meta]. For metadata of type
@@ -121,8 +121,10 @@ The procedure then waits until the handle field
 @racket[reading] becomes @racket[#f], sleeping for 10 ms between
 checks.
 
-The procedure prints timing information before and after the
+If reading has already finished or never started, the native decoder is
-wait.
+released directly. Calling @racket[flac-stop] repeatedly, including after EOF
+or a failed read, is safe. The procedure logs timing information before and
+after the wait.
 }
 
 @defproc[(flac-duration [handle flac-handle?])

tests/flac-lifecycle-test.rkt

@@ -0,0 +1,76 @@
+#lang racket/base
+
+(require rackunit
+         racket/file
+         "../flac-decoder.rkt"
+         "../flac-encoder.rkt")
+
+;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
+;; Tests for module flac-decoder.rkt
+;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
+
+;; Generate a small native FLAC fixture without external programs or test data.
+(let* ((directory (make-temporary-file "flac-lifecycle-~a" 'directory))
+       (file (build-path directory "silence.flac"))
+       (format (hasheq 'sample-rate 44100 'channels 2 'bits-per-sample 24)))
+  (dynamic-wind
+    void
+    (λ ()
+      (let ((encoder (flac-encoder-open file (hasheq) format))
+            (samples (make-bytes (* 256 2 3))))
+        (flac-encoder-write encoder format samples (bytes-length samples))
+        (flac-encoder-finish encoder))
+
+      (test-case "repeated stop before reading is harmless"
+        (let ((handle (flac-open file void void)))
+          (check-not-exn (λ () (flac-stop handle)))
+          (check-not-exn (λ () (flac-stop handle)))))
+
+      (test-case "stop after decoder EOF is harmless"
+        (let ((handle (flac-open file void void)))
+          (flac-read handle)
+          (check-false (flac-handle-reading handle))
+          (check-not-exn (λ () (flac-stop handle)))))
+
+      (test-case "callback failure releases the decoder and allows stop"
+        (let ((handle (flac-open file void
+                                 (λ (_header _samples _size)
+                                   (error 'test "audio consumer failed")))))
+          (check-exn #rx"audio consumer failed" (λ () (flac-read handle)))
+          (check-false (flac-handle-reading handle))
+          (let ((stopper (thread (λ () (flac-stop handle)))))
+            (dynamic-wind
+              void
+              (λ () (check-not-false (sync/timeout 2 stopper)))
+              (λ () (unless (thread-dead? stopper) (kill-thread stopper)))))))
+
+      (test-case "invalid FLAC data reports failure and remains stoppable"
+        (let ((broken (build-path directory "broken.flac")))
+          (display-to-file #"fLaC" broken)
+          (let ((handle (flac-open broken void void)))
+            (check-exn #rx"Could not decode FLAC frame"
+                       (λ () (flac-read handle)))
+            (check-false (flac-handle-reading handle))
+            (check-not-exn (λ () (flac-stop handle))))))
+
+      (test-case "a stop request waits for active decoder cleanup"
+        (letrec ((entered (make-semaphore 0))
+               (handle
+                (flac-open file void
+                           (λ (_header _samples _size)
+                             (semaphore-post entered)
+                             (let loop ()
+                               (unless (flac-handle-stop-reading handle)
+                                 (sleep 0.001)
+                                 (loop))))))
+               (reader (thread (λ () (flac-read handle)))))
+          (dynamic-wind
+            void
+            (λ ()
+              (check-not-false (sync/timeout 2 entered))
+              (flac-stop handle)
+              (check-false (flac-handle-reading handle))
+              (check-not-false (sync/timeout 2 reader))
+              (check-not-exn (λ () (flac-stop handle))))
+            (λ () (unless (thread-dead? reader) (kill-thread reader)))))))
+    (λ () (delete-directory/files directory))))