Re: New emms-info-native

[Yoni Rabkin-2]

Petteri Hintsanen <[hidden email]> writes:

> Nonetheless, please try the new emms-info-native.  I do not have too
> many MP3s around so bugs are likely.  As before, let me know of any
> issues you encounter.

Thank you for you work.

I got this error on the very first file I tried:
(error "id3v2 tag/header/frame size [0 68 75 12] is invalid")

exiftool reports that the id3 size is a wopping:
ID3 Size                        : 1123734

so if I run:
$ head -c 2000000 01\ Contrapunctus_1.mp3 > contra.mp3

...exiftool can still extract the id3 information, and

(emms-info-native--decode-info-fields "/home/yrk/devel/emms-hacking/info/contra.mp3")

...still produces the same error.

--
   "Cut your own wood and it will warm you twice"

[Petteri Hintsanen-2]

Yoni Rabkin <[hidden email]> writes:

> I got this error on the very first file I tried:
> (error "id3v2 tag/header/frame size [0 68 75 12] is invalid")
>
> exiftool reports that the id3 size is a wopping:
> ID3 Size                        : 1123734

That’s correct, you can find that out also by evaling

  (emms-info-native--decode-id3v2-size [0 68 75 12])

It gives 1123724; that does include the tag header which is always 10
bytes.

This means your file has some big metadata chunk, perhaps cover art?

Try to increase emms-info-native--max-peek-size, which is the maximum
allowed size for the tag.  The default value is 512 kiB.  If you set it
to e.g. 2 MiB you should get rid of invalid size errors.

The reason for that limit is to protect against cases where the tag size
is unreasonably big (e.g. due to corrupt data).  Those could easily
cause Emacs to run out of memory, because the decoder reserves memory
according to decoded sizes.

It looks like the default limit may be too small.

Thanks,
Petteri

[Petteri Hintsanen-2]

Petteri Hintsanen <[hidden email]> writes:

> It gives 1123724; that does include the tag header which is always 10
> bytes.

Should have been "that does *not* include the tag header...", sorry.

Petteri

[Yoni Rabkin-2]

Petteri Hintsanen <[hidden email]> writes:

> Yoni Rabkin <[hidden email]> writes:
>
>> I got this error on the very first file I tried:
>> (error "id3v2 tag/header/frame size [0 68 75 12] is invalid")
>>
>> exiftool reports that the id3 size is a wopping:
>> ID3 Size                        : 1123734
>
> That’s correct, you can find that out also by evaling
>
>   (emms-info-native--decode-id3v2-size [0 68 75 12])
>
> It gives 1123724; that does include the tag header which is always 10
> bytes.
>
> This means your file has some big metadata chunk, perhaps cover art?
>
> Try to increase emms-info-native--max-peek-size, which is the maximum
> allowed size for the tag.  The default value is 512 kiB.  If you set it
> to e.g. 2 MiB you should get rid of invalid size errors.
>
> The reason for that limit is to protect against cases where the tag size
> is unreasonably big (e.g. due to corrupt data).  Those could easily
> cause Emacs to run out of memory, because the decoder reserves memory
> according to decoded sizes.
>
> It looks like the default limit may be too small.

Probably, I set it to (expt 2 21) and that error went away.

However, the same file now shows:
(error "id3v2 unsynchronisation scheme is not supported")

I will admit ignorance to what an "id3v2 unsynchronisation scheme" is.


--
   "Cut your own wood and it will warm you twice"

[Petteri Hintsanen-2]

On Wed, Feb 10, 2021 at 08:02:49PM -0500, Yoni Rabkin wrote:

> I will admit ignorance to what an "id3v2 unsynchronisation scheme" is.

"Unsychronization scheme" is a compatibility feature for older software that
does not understand id3 tags and looks for MP3 synchronization frames
(certain sequences of adjacent 1-bits) to find out where the actual stream
starts.  This scheme is a simple escaping for such sync frames that may
occur in tags and hence fail id3-unaware software.  I don't have any files
that use this scheme, so I did not bother to implement it.  I also thought
that maybe we can do without it...but now it looks like we can't.

Implementation is not difficult to do, the only problem is that I don't have
any test data readily available.  But it shouldn't be too hard to find some.

Thanks for your efforts.
Petteri

[Petteri Hintsanen-2]

Yoni Rabkin <[hidden email]> writes:

> However, the same file now shows:
> (error "id3v2 unsynchronisation scheme is not supported")

I just pushed a new version to emms.git/info-native.  Please try if it
gives any better results.

Thanks,
Petteri

[Yoni Rabkin-2]

Petteri Hintsanen <[hidden email]> writes:

> Yoni Rabkin <[hidden email]> writes:
>
>> However, the same file now shows:
>> (error "id3v2 unsynchronisation scheme is not supported")
>
> I just pushed a new version to emms.git/info-native.  Please try if it
> gives any better results.

That works very nicely with about 1,000 classical tracks; thank you.

Can you please merge this so that people living with the main branch of
the git repo can give it a go?

--
   "Cut your own wood and it will warm you twice"

[Petteri Hintsanen-2]

Yoni Rabkin <[hidden email]> writes:

> Can you please merge this so that people living with the main branch of
> the git repo can give it a go?

Merged and pushed.

Thanks,
Petteri

[Fran Burstall (Gmail)]

Hi,

I did 

(benchmark-run

    (dolist (filename (map-keys emms-cache-db))

      (condition-case nil

        (emms-info-native--decode-info-fields filename)

      (error (message "Error while processing %s" filename)))))

which read tags from 14051 files (mostly mp3 with some flac and ogg) in 710 seconds and only errored out on 75 files.  Not too bad!

The errors were of the form 

id3v2 tag or frame size 3832965 is invalid

Let me know what more data I can provide to help debug...

---Fran

On Wed, 17 Feb 2021 at 17:31, Petteri Hintsanen <[hidden email]> wrote:

Yoni Rabkin <[hidden email]> writes:

> Can you please merge this so that people living with the main branch of
> the git repo can give it a go?

Merged and pushed.

Thanks,
Petteri

[Fran Burstall (Gmail)]

On Wed, 17 Feb 2021 at 21:56, Yoni Rabkin <[hidden email]> wrote:

Can you please try again with a larger frame size limit?

    (setq emms-info-native--max-peek-size (expt 2 22))

(setq emms-info-native--max-peek-size (expt 2 22)) reduces the fails from 75 to 15 at the cost of a massive slowdown (52sec for 75 files)

(setq emms-info-native--max-peek-size (expt 2 24)) reduces the fails from 15 to zero at the cost of another massive slowdown (69sec for 15 files)

I wonder if one could dynamically and temporarily increase peek size in reaction to frame size errors?

---Fran

[Yoni Rabkin-2]

"Fran Burstall (Gmail)" <[hidden email]> writes:

> On Wed, 17 Feb 2021 at 21:56, Yoni Rabkin <[hidden email]> wrote:
>
>>
>>
>> Can you please try again with a larger frame size limit?
>>
>>     (setq emms-info-native--max-peek-size (expt 2 22))
>>
>>
>>
> (setq emms-info-native--max-peek-size (expt 2 22)) reduces the fails from
> 75 to 15 at the cost of a massive slowdown (52sec for 75 files)
>
> (setq emms-info-native--max-peek-size (expt 2 24)) reduces the fails from
> 15 to zero at the cost of another massive slowdown (69sec for 15 files)
>
> I wonder if one could dynamically and temporarily increase peek size in
> reaction to frame size errors?

I was thinking the exact same thing when I read the first line you
wrote. The frame-size error needs to be caught and then the read retried
with a higher value until either a success or a critical limit is
reached.

To avoid trashing Emacs and the user's memory the process should
critically terminate when it gets to, say, half the size of your average
mp3, or half the file size if we that on hand. This is since I can go
ahead and assume that there aren't many mp3s out there where half of the
file is metadata.

Petteri, what do you think of the dynamic-scaling idea?

--
   "Cut your own wood and it will warm you twice"

[tumashu]

I have tested with this benchmark,  find three problem:
Maybe emms-info-native should ignore errors and use message to tell user which file can not be handle properly.

1. if  mp3 file can't be readed for control access.  info-native will error.

2.
------------------------
Debugger entered--Lisp error: (args-out-of-range "" 0)
  bindat--unpack-u8()
  bindat--unpack-item(u8 1 nil)
  bindat--unpack-item(vec 3 nil)
  bindat--unpack-group(((file-identifier vec 3) (eval (unless (equal last emms-info-native--id3v2-magic-array) (error "id3v2 framing mismatch: expected ‘%s’, got ‘%s’" emms-info-native--id3v2-magic-array last))) (version u8) (eval (setq emms-info-native--id3v2-version last)) (revision u8) (flags bits 1) (size-bytes vec 4) (size eval (emms-info-native--checked-id3v2-size 'tag last))))
  bindat-unpack(((file-identifier vec 3) (eval (unless (equal last emms-info-native--id3v2-magic-array) (error "id3v2 framing mismatch: expected ‘%s’, got ‘%s’" emms-info-native--id3v2-magic-array last))) (version u8) (eval (setq emms-info-native--id3v2-version last)) (revision u8) (flags bits 1) (size-bytes vec 4) (size eval (emms-info-native--checked-id3v2-size 'tag last))) "")
  (progn (set-buffer-multibyte nil) (insert-file-contents-literally filename nil 0 10) (bindat-unpack emms-info-native--id3v2-header-bindat-spec (buffer-string)))
  (unwind-protect (progn (set-buffer-multibyte nil) (insert-file-contents-literally filename nil 0 10) (bindat-unpack emms-info-native--id3v2-header-bindat-spec (buffer-string))) (and (buffer-name temp-buffer) (kill-buffer temp-buffer)))
  (save-current-buffer (set-buffer temp-buffer) (unwind-protect (progn (set-buffer-multibyte nil) (insert-file-contents-literally filename nil 0 10) (bindat-unpack emms-info-native--id3v2-header-bindat-spec (buffer-string))) (and (buffer-name temp-buffer) (kill-buffer temp-buffer))))
  (let ((temp-buffer (generate-new-buffer " *temp*" t))) (save-current-buffer (set-buffer temp-buffer) (unwind-protect (progn (set-buffer-multibyte nil) (insert-file-contents-literally filename nil 0 10) (bindat-unpack emms-info-native--id3v2-header-bindat-spec (buffer-string))) (and (buffer-name temp-buffer) (kill-buffer temp-buffer)))))
  emms-info-native--decode-id3v2-header("/home/feng/音乐/李玉刚/逐梦令 四美图/月光.mp3")
  (let* (emms-info-native--id3v2-version (header (emms-info-native--decode-id3v2-header filename)) (tag-size (bindat-get-field header 'size)) (unsync (memq 7 (bindat-get-field header 'flags))) (offset 10)) (if (memq 6 (bindat-get-field header 'flags)) (progn (setq offset (+ offset (emms-info-native--decode-id3v2-ext-header-size filename))))) (emms-info-native--decode-id3v2-frames filename offset (+ tag-size 10) unsync))
  emms-info-native--decode-id3v2("/home/feng/音乐/李玉刚/逐梦令 四美图/月光.mp3")
  (cond ((or (eq stream-type 'vorbis) (eq stream-type 'opus)) (emms-info-native--decode-ogg-comments filename stream-type)) ((eq stream-type 'flac) (emms-info-native--decode-flac-comments filename)) ((eq stream-type 'mp3) (emms-info-native--decode-id3v2 filename)) (t nil))
  (let ((stream-type (emms-info-native--find-stream-type filename))) (cond ((or (eq stream-type 'vorbis) (eq stream-type 'opus)) (emms-info-native--decode-ogg-comments filename stream-type)) ((eq stream-type 'flac) (emms-info-native--decode-flac-comments filename)) ((eq stream-type 'mp3) (emms-info-native--decode-id3v2 filename)) (t nil)))
  emms-info-native--decode-info-fields("/home/feng/音乐/李玉刚/逐梦令 四美图/月光.mp3")
  eval((emms-info-native--decode-info-fields "/home/feng/音乐/李玉刚/逐梦令 四美图/月光.mp3") nil)
  elisp--eval-last-sexp(nil)
  #f(compiled-function (eval-last-sexp-arg-internal) "Evaluate sexp before point; print value in the echo area.\nInteractively, with a non `-' prefix argument, print output into\ncurrent buffer.\n\nThis commands handles `defvar', `defcustom' and `defface' the\nsame way that `eval-defun' does.  See the doc string of that\nfunction for details.\n\nNormally, this function truncates long output according to the\nvalue of the variables `eval-expression-print-length' and\n`eval-expression-print-level'.  With a prefix argument of zero,\nhowever, there is no such truncation.\nInteger values are printed in several formats (decimal, octal,\nand hexadecimal).  When the prefix argument is -1 or the value\ndoesn't exceed `eval-expression-print-maximum-character', an\ninteger value is also printed as a character of that codepoint.\n\nIf `eval-expression-debug-on-error' is non-nil, which is the default,\nthis command arranges for all errors to enter the debugger." (interactive "P") #<bytecode 0x1d1337931c73bf61>)(nil)
  apply(#f(compiled-function (eval-last-sexp-arg-internal) "Evaluate sexp before point; print value in the echo area.\nInteractively, with a non `-' prefix argument, print output into\ncurrent buffer.\n\nThis commands handles `defvar', `defcustom' and `defface' the\nsame way that `eval-defun' does.  See the doc string of that\nfunction for details.\n\nNormally, this function truncates long output according to the\nvalue of the variables `eval-expression-print-length' and\n`eval-expression-print-level'.  With a prefix argument of zero,\nhowever, there is no such truncation.\nInteger values are printed in several formats (decimal, octal,\nand hexadecimal).  When the prefix argument is -1 or the value\ndoesn't exceed `eval-expression-print-maximum-character', an\ninteger value is also printed as a character of that codepoint.\n\nIf `eval-expression-debug-on-error' is non-nil, which is the default,\nthis command arranges for all errors to enter the debugger." (interactive "P") #<bytecode 0x1d1337931c73bf61>) nil)
  eval-last-sexp(nil)
  funcall-interactively(eval-last-sexp nil)
  command-execute(eval-last-sexp)
--------------------------

3.
--------------------------------------------------------------------------------------------------------------------------------------
Debugger entered--Lisp error: (error "id3v2 framing mismatch: expected ‘[73 68 51]’, got...")
  error("id3v2 framing mismatch: expected ‘%s’, got ‘%s’" [73 68 51] [255 251 224])
  (if (equal last emms-info-native--id3v2-magic-array) nil (error "id3v2 framing mismatch: expected ‘%s’, got ‘%s’" emms-info-native--id3v2-magic-array last))
  (unless (equal last emms-info-native--id3v2-magic-array) (error "id3v2 framing mismatch: expected ‘%s’, got ‘%s’" emms-info-native--id3v2-magic-array last))
  bindat--unpack-group(((file-identifier vec 3) (eval (unless (equal last emms-info-native--id3v2-magic-array) (error "id3v2 framing mismatch: expected ‘%s’, got ‘%s’" emms-info-native--id3v2-magic-array last))) (version u8) (eval (setq emms-info-native--id3v2-version last)) (revision u8) (flags bits 1) (size-bytes vec 4) (size eval (emms-info-native--checked-id3v2-size 'tag last))))
  bindat-unpack(((file-identifier vec 3) (eval (unless (equal last emms-info-native--id3v2-magic-array) (error "id3v2 framing mismatch: expected ‘%s’, got ‘%s’" emms-info-native--id3v2-magic-array last))) (version u8) (eval (setq emms-info-native--id3v2-version last)) (revision u8) (flags bits 1) (size-bytes vec 4) (size eval (emms-info-native--checked-id3v2-size 'tag last))) "\377\373\340\4\0\0\0\0\0\0")
  (progn (set-buffer-multibyte nil) (insert-file-contents-literally filename nil 0 10) (bindat-unpack emms-info-native--id3v2-header-bindat-spec (buffer-string)))
  (unwind-protect (progn (set-buffer-multibyte nil) (insert-file-contents-literally filename nil 0 10) (bindat-unpack emms-info-native--id3v2-header-bindat-spec (buffer-string))) (and (buffer-name temp-buffer) (kill-buffer temp-buffer)))
  (save-current-buffer (set-buffer temp-buffer) (unwind-protect (progn (set-buffer-multibyte nil) (insert-file-contents-literally filename nil 0 10) (bindat-unpack emms-info-native--id3v2-header-bindat-spec (buffer-string))) (and (buffer-name temp-buffer) (kill-buffer temp-buffer))))
  (let ((temp-buffer (generate-new-buffer " *temp*" t))) (save-current-buffer (set-buffer temp-buffer) (unwind-protect (progn (set-buffer-multibyte nil) (insert-file-contents-literally filename nil 0 10) (bindat-unpack emms-info-native--id3v2-header-bindat-spec (buffer-string))) (and (buffer-name temp-buffer) (kill-buffer temp-buffer)))))
  emms-info-native--decode-id3v2-header("/home/feng/音乐/未整理/Jambalaya (On The Bayou).mp3")
  (let* (emms-info-native--id3v2-version (header (emms-info-native--decode-id3v2-header filename)) (tag-size (bindat-get-field header 'size)) (unsync (memq 7 (bindat-get-field header 'flags))) (offset 10)) (if (memq 6 (bindat-get-field header 'flags)) (progn (setq offset (+ offset (emms-info-native--decode-id3v2-ext-header-size filename))))) (emms-info-native--decode-id3v2-frames filename offset (+ tag-size 10) unsync))
  emms-info-native--decode-id3v2("/home/feng/音乐/未整理/Jambalaya (On The Bayou).mp3")
  (cond ((or (eq stream-type 'vorbis) (eq stream-type 'opus)) (emms-info-native--decode-ogg-comments filename stream-type)) ((eq stream-type 'flac) (emms-info-native--decode-flac-comments filename)) ((eq stream-type 'mp3) (emms-info-native--decode-id3v2 filename)) (t nil))
  (let ((stream-type (emms-info-native--find-stream-type filename))) (cond ((or (eq stream-type 'vorbis) (eq stream-type 'opus)) (emms-info-native--decode-ogg-comments filename stream-type)) ((eq stream-type 'flac) (emms-info-native--decode-flac-comments filename)) ((eq stream-type 'mp3) (emms-info-native--decode-id3v2 filename)) (t nil)))
  emms-info-native--decode-info-fields("/home/feng/音乐/未整理/Jambalaya (On The Bayou).mp3")
  eval((emms-info-native--decode-info-fields "/home/feng/音乐/未整理/Jambalaya (On The Bayou).mp3") nil)
  elisp--eval-last-sexp(nil)
  #f(compiled-function (eval-last-sexp-arg-internal) "Evaluate sexp before point; print value in the echo area.\nInteractively, with a non `-' prefix argument, print output into\ncurrent buffer.\n\nThis commands handles `defvar', `defcustom' and `defface' the\nsame way that `eval-defun' does.  See the doc string of that\nfunction for details.\n\nNormally, this function truncates long output according to the\nvalue of the variables `eval-expression-print-length' and\n`eval-expression-print-level'.  With a prefix argument of zero,\nhowever, there is no such truncation.\nInteger values are printed in several formats (decimal, octal,\nand hexadecimal).  When the prefix argument is -1 or the value\ndoesn't exceed `eval-expression-print-maximum-character', an\ninteger value is also printed as a character of that codepoint.\n\nIf `eval-expression-debug-on-error' is non-nil, which is the default,\nthis command arranges for all errors to enter the debugger." (interactive "P") #<bytecode 0x1d1337931c73bf61>)(nil)
  apply(#f(compiled-function (eval-last-sexp-arg-internal) "Evaluate sexp before point; print value in the echo area.\nInteractively, with a non `-' prefix argument, print output into\ncurrent buffer.\n\nThis commands handles `defvar', `defcustom' and `defface' the\nsame way that `eval-defun' does.  See the doc string of that\nfunction for details.\n\nNormally, this function truncates long output according to the\nvalue of the variables `eval-expression-print-length' and\n`eval-expression-print-level'.  With a prefix argument of zero,\nhowever, there is no such truncation.\nInteger values are printed in several formats (decimal, octal,\nand hexadecimal).  When the prefix argument is -1 or the value\ndoesn't exceed `eval-expression-print-maximum-character', an\ninteger value is also printed as a character of that codepoint.\n\nIf `eval-expression-debug-on-error' is non-nil, which is the default,\nthis command arranges for all errors to enter the debugger." (interactive "P") #<bytecode 0x1d1337931c73bf61>) nil)
  eval-last-sexp(nil)
  funcall-interactively(eval-last-sexp nil)
  command-execute(eval-last-sexp)
-----------------------------------------------------------













At 2021-02-18 05:16:06, "Fran Burstall (Gmail)" <[hidden email]> wrote:


Hi,


I did 



(benchmark-run

    (dolist (filename (map-keys emms-cache-db))
      (condition-case nil
        (emms-info-native--decode-info-fields filename)
      (error (message "Error while processing %s" filename)))))


which read tags from 14051 files (mostly mp3 with some flac and ogg) in 710 seconds and only errored out on 75 files.  Not too bad!


The errors were of the form 



id3v2 tag or frame size 3832965 is invalid

Let me know what more data I can provide to help debug...


---Fran



On Wed, 17 Feb 2021 at 17:31, Petteri Hintsanen <[hidden email]> wrote:

Yoni Rabkin <[hidden email]> writes:



> Can you please merge this so that people living with the main branch of

> the git repo can give it a go?



Merged and pushed.



Thanks,

Petteri

[Yoni Rabkin-2]

tumashu  <[hidden email]> writes:

> I have tested with this benchmark, find three problem: Maybe
> emms-info-native should ignore errors and use message to tell user
> which file can not be handle properly.

Thank you for testing.

I agree that emms-info-native should ultimately be kind about erroring
out. Perhaps a message at the end that says "the following files could
not be read..." for the release version.

But hard errors are good for us now for testing the code.

--
   "Cut your own wood and it will warm you twice"

[Petteri Hintsanen-2]

(apologies it you get this message twice, my mailer is complaining for
some reason)

Yoni Rabkin <[hidden email]> writes:

> Petteri, what do you think of the dynamic-scaling idea?

It could help but I think it’s not the best way to go forward.  I am
afraid it is the design itself that is failing here.

Namely, the current implementation reads in the whole tag (which can be
big, as we’ve seen) before parsing, even though we are not interested in
most of it.  At the end, we need only a few, relatively small textual
frames (album, artist, etc.) from the complete tag.

I hacked together an "incremental" parser that goes through the tag one
frame at a time, and skips over those that EMMS does not care about.
Hopefully it saves memory and, who knows, it may also give some
performance boost.  (I didn’t have time to do any benchmarking.)  Due to
id3v2 peculiarities, the code is quite ugly and regressions are likely.

But please test if it gives any better results.  Code is in info-native
branch.

Thank you all for your feedback.

Regards,
Petteri

[Yoni Rabkin-2]

Petteri Hintsanen <[hidden email]> writes:

> (apologies it you get this message twice, my mailer is complaining for
> some reason)
>
> Yoni Rabkin <[hidden email]> writes:
>
>> Petteri, what do you think of the dynamic-scaling idea?
>
> It could help but I think it’s not the best way to go forward.  I am
> afraid it is the design itself that is failing here.
>
> Namely, the current implementation reads in the whole tag (which can be
> big, as we’ve seen) before parsing, even though we are not interested in
> most of it.  At the end, we need only a few, relatively small textual
> frames (album, artist, etc.) from the complete tag.
>
> I hacked together an "incremental" parser that goes through the tag one
> frame at a time, and skips over those that EMMS does not care about.
> Hopefully it saves memory and, who knows, it may also give some
> performance boost.  (I didn’t have time to do any benchmarking.)  Due to
> id3v2 peculiarities, the code is quite ugly and regressions are likely.
>
> But please test if it gives any better results.  Code is in info-native
> branch.

This gives me no errors on about 1000 classical tracks and runs close to
4 seconds; 250 tracks a second is just fine.

When I try it on a wider selection of tracks I get a framing mismatch
early on. Attached are the first 10k of that file.




--
   "Cut your own wood and it will warm you twice"

[Fran Burstall (Gmail)]

I tried last night's test on the new info-native branch version and got zero errors in 14051 files in 371 secs.  This is twice as fast as last night but nowhere near the 250 tracks a second that Yoni is seeing!

---Fran

On Thu, 18 Feb 2021 at 22:12, Yoni Rabkin <[hidden email]> wrote:

Petteri Hintsanen <[hidden email]> writes:

> (apologies it you get this message twice, my mailer is complaining for
> some reason)
>
> Yoni Rabkin <[hidden email]> writes:
>
>> Petteri, what do you think of the dynamic-scaling idea?
>
> It could help but I think it’s not the best way to go forward.  I am
> afraid it is the design itself that is failing here.
>
> Namely, the current implementation reads in the whole tag (which can be
> big, as we’ve seen) before parsing, even though we are not interested in
> most of it.  At the end, we need only a few, relatively small textual
> frames (album, artist, etc.) from the complete tag.
>
> I hacked together an "incremental" parser that goes through the tag one
> frame at a time, and skips over those that EMMS does not care about.
> Hopefully it saves memory and, who knows, it may also give some
> performance boost.  (I didn’t have time to do any benchmarking.)  Due to
> id3v2 peculiarities, the code is quite ugly and regressions are likely.
>
> But please test if it gives any better results.  Code is in info-native
> branch.

This gives me no errors on about 1000 classical tracks and runs close to
4 seconds; 250 tracks a second is just fine.

When I try it on a wider selection of tracks I get a framing mismatch
early on. Attached are the first 10k of that file.



--
   "Cut your own wood and it will warm you twice"

[Yoni Rabkin-2]

"Fran Burstall (Gmail)" <[hidden email]> writes:

> I tried last night's test on the new info-native branch version and got
> zero errors in 14051 files in 371 secs.  This is twice as fast as last
> night but nowhere near the 250 tracks a second that Yoni is seeing!

I saw that speed on the version just released on the info-native branch
about 3 hours ago.


--
   "Cut your own wood and it will warm you twice"

[tumashu]

(let* ((n 1)
       (bm (benchmark-run
               (dolist (filename (map-keys emms-cache-db))
                 (setq n (+ n 1))
                 (condition-case nil
                     (emms-info-native--decode-info-fields filename)
                   (error (message "Error while processing %s" filename)))))))
  (message "Totel %s files, speed is %s files per second." n  (/ n (car bm))))

I use emacs 28 gccemacs branch.

1. master speed

Error while processing /home/feng/音乐/未整理/Jambalaya (On The Bayou).mp3
Error while processing /home/feng/音乐/未整理/Mean.mp3
Error while processing /home/feng/音乐/李玉刚/逐梦令 四美图/月光.mp3
Totel 1261 files, speed is 405.9625992748337 files per second.

2.  info-native speed

Error while processing /home/feng/音乐/未整理/Jambalaya (On The Bayou).mp3
Error while processing /home/feng/音乐/未整理/Mean.mp3
Error while processing /home/feng/音乐/李玉刚/逐梦令 四美图/月光.mp3
Totel 1261 files, speed is 808.5052910067327 files per second.


by the way, is it possible add a function, which can read a problem mp3 file, and
generate a small test mp3 file?








At 2021-02-19 05:46:35, "Petteri Hintsanen" <[hidden email]> wrote:

>(apologies it you get this message twice, my mailer is complaining for
>some reason)
>
>Yoni Rabkin <[hidden email]> writes:
>
>> Petteri, what do you think of the dynamic-scaling idea?
>
>It could help but I think it’s not the best way to go forward.  I am
>afraid it is the design itself that is failing here.
>
>Namely, the current implementation reads in the whole tag (which can be
>big, as we’ve seen) before parsing, even though we are not interested in
>most of it.  At the end, we need only a few, relatively small textual
>frames (album, artist, etc.) from the complete tag.
>
>I hacked together an "incremental" parser that goes through the tag one
>frame at a time, and skips over those that EMMS does not care about.
>Hopefully it saves memory and, who knows, it may also give some
>performance boost.  (I didn’t have time to do any benchmarking.)  Due to
>id3v2 peculiarities, the code is quite ugly and regressions are likely.
>
>But please test if it gives any better results.  Code is in info-native
>branch.
>
>Thank you all for your feedback.
>
>Regards,
>Petteri

[tumashu]

maybe I am wrong,  I test again with emacs28 gccemacs branch, and find that:

1. master

Totel 1261 files, speed is 1308.8998360985452 files per second.

2. info-native

Totel 1261 files, speed is 899.9466389450938 files per second.

both are  fast enough :-)

在 2021-02-19 08:52:44，"tumashu" <[hidden email]> 写道：
>(let* ((n 1)
>       (bm (benchmark-run
>               (dolist (filename (map-keys emms-cache-db))
>                 (setq n (+ n 1))
>                 (condition-case nil
>                     (emms-info-native--decode-info-fields filename)
>                   (error (message "Error while processing %s" filename)))))))
>  (message "Totel %s files, speed is %s files per second." n  (/ n (car bm))))
>
>I use emacs 28 gccemacs branch.
>
>1. master speed
>
>Error while processing /home/feng/音乐/未整理/Jambalaya (On The Bayou).mp3
>Error while processing /home/feng/音乐/未整理/Mean.mp3
>Error while processing /home/feng/音乐/李玉刚/逐梦令 四美图/月光.mp3
>Totel 1261 files, speed is 405.9625992748337 files per second.
>
>2.  info-native speed
>
>Error while processing /home/feng/音乐/未整理/Jambalaya (On The Bayou).mp3
>Error while processing /home/feng/音乐/未整理/Mean.mp3
>Error while processing /home/feng/音乐/李玉刚/逐梦令 四美图/月光.mp3
>Totel 1261 files, speed is 808.5052910067327 files per second.
>
>
>by the way, is it possible add a function, which can read a problem mp3 file, and
>generate a small test mp3 file?
>
>
>
>
>
>
>
>
>At 2021-02-19 05:46:35, "Petteri Hintsanen" <[hidden email]> wrote:
>>(apologies it you get this message twice, my mailer is complaining for
>>some reason)
>>
>>Yoni Rabkin <[hidden email]> writes:
>>
>>> Petteri, what do you think of the dynamic-scaling idea?
>>
>>It could help but I think it’s not the best way to go forward.  I am
>>afraid it is the design itself that is failing here.
>>
>>Namely, the current implementation reads in the whole tag (which can be
>>big, as we’ve seen) before parsing, even though we are not interested in
>>most of it.  At the end, we need only a few, relatively small textual
>>frames (album, artist, etc.) from the complete tag.
>>
>>I hacked together an "incremental" parser that goes through the tag one
>>frame at a time, and skips over those that EMMS does not care about.
>>Hopefully it saves memory and, who knows, it may also give some
>>performance boost.  (I didn’t have time to do any benchmarking.)  Due to
>>id3v2 peculiarities, the code is quite ugly and regressions are likely.
>>
>>But please test if it gives any better results.  Code is in info-native
>>branch.
>>
>>Thank you all for your feedback.
>>
>>Regards,
>>Petteri

[Yoni Rabkin-2]

tumashu  <[hidden email]> writes:

> by the way, is it possible add a function, which can read a problem mp3 file, and
> generate a small test mp3 file?

Shouldn't be too hard, but would require a bit of work. You would call a
process on the mp3 file that does:

    $ head -c 1000 file.mp3 > partial.mp3

...and so partial.mp3 would be the first 1k of file.mp3.

The question is, would that be helpful to Petteri?

--
   "Cut your own wood and it will warm you twice"