[elpa] externals/emms 643284a952 14/42: Streamline Ogg page reading and decoding

[ELPA Syncer]

branch: externals/emms
commit 643284a9529a73718015cc743a60f4f54beccd53
Author: Petteri Hintsanen <petterih@iki.fi>
Commit: Petteri Hintsanen <petterih@iki.fi>

    Streamline Ogg page reading and decoding
    
    This does not make decoding more memory efficient, but the code is
    more clear now.
---
 emms-info-ogg.el            | 43 +++++++++++++------------------------------
 test/emms-info-ogg-tests.el | 13 +++++++------
 2 files changed, 20 insertions(+), 36 deletions(-)

diff --git a/emms-info-ogg.el b/emms-info-ogg.el
index 165cb8a1be..1db154ecd0 100644
--- a/emms-info-ogg.el
+++ b/emms-info-ogg.el
@@ -179,25 +179,25 @@ See `emms-info-vorbis--split-comment' for details."
 Read in data from the start of FILENAME, remove Ogg packet
 frames, and concatenate payloads until at least PACKETS number of
 packets have been decoded.  Return the decoded packets in a
-vector, concatenated.
+string, concatenated.
 
-Data is read in `emms-info-ogg--page-size' chunks.  If the total
-length of concatenated packets becomes greater than
-`emms-info-ogg--max-peek-size', an error is signaled.
+Read data in `emms-info-ogg--page-size' chunks.  If more than
+`emms-info-ogg--max-peek-size' bytes of data would be read,
+signal an error.
 
 Only elementary streams are supported, that is, FILENAME should
 contain only a single logical stream.  Note that this assumption
 is not verified: with non-elementary streams packets from
 different streams will be mixed together without an error."
-  (let ((num-packets 0) (offset 0) (stream (string)))
+  (let ((num-packets 0) (offset 0) (stream (list)))
     (while (< num-packets packets)
+      (when (> offset emms-info-ogg--max-peek-size)
+        (error "Ogg payload is too large"))
       (let ((page (emms-info-ogg--read-and-decode-page filename offset)))
-        (cl-incf num-packets (or (plist-get page :num-packets) 0))
-        (cl-incf offset (plist-get page :num-bytes))
-        (setq stream (concat stream (plist-get page :stream)))
-        (when (> (length stream) emms-info-ogg--max-peek-size)
-          (error "Ogg payload is too large"))))
-    stream))
+        (cl-incf num-packets (emms-info-ogg--num-packets page))
+        (cl-incf offset (bindat-length emms-info-ogg--page-bindat-spec page))
+        (push (bindat-get-field page 'payload) stream)))
+    (reverse (mapconcat #'nreverse stream))))
 
 (defun emms-info-ogg--read-and-decode-page (filename offset)
   "Read and decode a single Ogg page from FILENAME.
@@ -208,25 +208,8 @@ Return the plist from `emms-info-ogg--decode-page'."
     (set-buffer-multibyte nil)
     (insert-file-contents-literally
      filename nil offset (+ offset emms-info-ogg--page-size))
-    (emms-info-ogg--decode-page (buffer-string))))
-
-(defun emms-info-ogg--decode-page (bytes)
-  "Decode a single Ogg page from a sequence of BYTES.
-Return a plist (:num-packets N :num-bytes B :stream S), where N
-is the number of packets in the page, B is the size of the page
-in bytes, and S is the unframed logical bitstream in a vector.
-Note that N can be zero."
-  (let* ((page
-          (bindat-unpack emms-info-ogg--page-bindat-spec bytes))
-         (num-packets
-          (emms-info-ogg--num-packets page))
-         (num-bytes
-          (bindat-length emms-info-ogg--page-bindat-spec page))
-         (stream
-          (bindat-get-field page 'payload)))
-    (list :num-packets num-packets
-          :num-bytes num-bytes
-          :stream stream)))
+    (bindat-unpack emms-info-ogg--page-bindat-spec
+                   (buffer-string))))
 
 (defun emms-info-ogg--num-packets (page)
   "Return the number of packets in Ogg page PAGE.
diff --git a/test/emms-info-ogg-tests.el b/test/emms-info-ogg-tests.el
index e0e1a14e2d..ea1c325eb3 100644
--- a/test/emms-info-ogg-tests.el
+++ b/test/emms-info-ogg-tests.el
@@ -27,11 +27,12 @@
 (require 'ert)
 
 (ert-deftest emms-ogg-test-decode-page ()
-  (let ((bytes [79 103 103 83 0 2 0 0 0 0 0 0 0 0 134 209 158 23 0 0 0 0 53 82 
251 136 1 30 1 118 111 114 98 105 115 0 0 0 0 1 68 172 0 0 0 0 0 0 128 56 1 0 0 
0 0 0 184 1]))
-    (should (equal (emms-info-ogg--decode-page bytes)
-                   (list :num-packets 1
-                         :num-bytes 58
-                         :stream [1 118 111 114 98 105 115 0 0 0 0 1 68 172 0 
0 0 0 0 0 128 56 1 0 0 0 0 0 184 1])))))
+  (let* ((bytes [79 103 103 83 0 2 0 0 0 0 0 0 0 0 134 209 158 23 0 0 0 0 53 
82 251 136 1 30 1 118 111 114 98 105 115 0 0 0 0 1 68 172 0 0 0 0 0 0 128 56 1 
0 0 0 0 0 184 1])
+         (page (bindat-unpack emms-info-ogg--page-bindat-spec bytes)))
+    (should (= (emms-info-ogg--num-packets page) 1))
+    (should (= (bindat-length emms-info-ogg--page-bindat-spec page) 58))
+    (should (equal (bindat-get-field page 'payload)
+                   (unibyte-string 1 118 111 114 98 105 115 0 0 0 0 1 68 172 0 
0 0 0 0 0 128 56 1 0 0 0 0 0 184 1)))))
 
 (ert-deftest emms-ogg-test-decode-vorbis-headers ()
   "Test `emms-info-ogg--decode-headers' with Vorbis data."
@@ -100,7 +101,7 @@ This is a helper function for 
`emms-ogg-test-decode-last-page'."
         (notlast [#x01 #x02 #x03 #x04 #x4f #x67 #x67 #x53 #x00 #x00 #x00 #x24 
#x08 #x01 #x00 #x00 #x00 #x00 #x9c #x39 #x6e #x47 #x40 #x08 #x00 #x00 #x19 #x4e 
#xac #xa3 #x01 #x0a #x4f #x67 #x67 #x53 #x31 #x32 #x33 #x34 #x35 #x36])
         (invalid [#x01 #x02 #x03 #x04 #x4f #x67 #x67 #x53 #x00 #x04 #x00 #x24 
#x08 #x01 #x00 #x00 #x00 #x00 #x9c #x39 #x6e #x47 #x40 #x08 #x00 #x00 #x01 #x02 
#x03 #x04 #x01 #x0a #x4f #x67 #x67 #x53 #x31 #x32 #x33 #x34 #x35 #x36]))
     (should (equal (emms-ogg-test--decode-last-page valid)
-                   '((payload . [79 103 103 83 49 50 51 52 53 54])
+                   '((payload . "OggS123456")
                      (segment-table . [10])
                      (page-segments . 1)
                      (page-checksum . 2745978393)