beetbox / beets

music library manager and MusicBrainz tagger
http://beets.io/
MIT License
12.74k stars 1.82k forks source link

beets keeps writing data #2620

Closed yacoob closed 7 years ago

yacoob commented 7 years ago

Problem

I've reran beet write after 1.4.4 update and beet decided to write some more metadata to files. That's OK, perhaps something has changed and more metadata from database can now be serialised into files. Problem is, the write doesn't seem to be succeeding, and beets wants to write it next time, again and again

$ beet -vv write noisetest
user configuration: /home/yacoob/.config/beets/config.yaml
data directory: /home/yacoob/.config/beets
plugin paths:
Sending event: pluginload
lyrics: Disabling google source: no API key configured.
inline: adding item field multidisc
library database: /home/yacoob/.beetsdb
library directory: /mnt/lcl/geofront/worek/music
Sending event: library_opened
65daysofstatic - No Man’s Sky: Music for an Infinite Universe - Departure / Shortwave / Noisetest
  original_day: 00 -> 05
  artist_sort:  -> 65daysofstatic
  original_month: 00 -> 08
  disctitle:  -> Soundscapes
Sending event: write
Sending event: after_write
Sending event: database_change
Sending event: cli_exit

but

$ beet info noisetest | ag original
     original_date: 2016-01-01
     original_year: 2016

If I rerun beet write, I get same output as above. This happens for a large number of songs in the library, so I don't think it's file-specific. File permissions seem to be OK:

$ ls -l "$(beet ls -p noisetest)"
-rwxrwx---+ 1 yacoob pierniks 31799157 Jul  2 00:26 /mnt/lcl/geofront/worek/music/65daysofstatic/No Man’s Sky_ Music for an Infinite Universe/02-03 Departure _ Shortwave _ Noisetest.mp3

Setup

My configuration (output of beet config) is:

lyrics:
    bing_lang_from: []
    auto: yes
    sources: [google, lyricwiki, musixmatch]
    force: no
    bing_lang_to:
    bing_client_secret: REDACTED
    genius_api_key: REDACTED
    google_engine_ID: REDACTED
    fallback:
    google_API_key: REDACTED

paths:
    default: $albumartist/$album%aunique{}/%if{$multidisc,$disc-}$track $title
    comp: Compilations/$album%aunique{}/%if{$multidisc,$disc-}$track $title
lastgenre:
    prefer_specific: yes
    canonical: yes
    fallback:
    auto: yes
    min_weight: 10
    force: yes
    source: album
    separator: ', '
    count: 1
    whitelist: yes
original_date: yes

match:
    strong_rec_thresh: 0.1
    preferred:
        countries: [US, GB|UK]
        media: [CD, Digital Media|File]
        original_year: yes

plugins: mbsync fetchart metasync duplicates missing scrub embedart lyrics lastgenre discogs web info inline
per_disc_numbering: yes
directory: /mnt/lcl/geofront/worek/music
item_fields:
    multidisc: 1 if disctotal > 1 else 0
fetchart:
    cautious: yes
    cover_names: front covert art
    auto: yes
    minwidth: 0
    google_engine: 001442825323518660753:hrh5ch1gjzm
    fanarttv_key: REDACTED
    google_key: REDACTED
    maxwidth: 0
    enforce_ratio: no
    sources:
    - filesystem
    - coverart
    - itunes
    - amazon
    - albumart
    store_source: no

import:
    copy: yes
    write: yes
    resume: yes
    languages: en
library: ~/.beetsdb
metasync:
    source: itunes
    itunes:
        library: /home/yacoob/workarea/itunes.xml
id3v23: yes
chroma:
    auto: no
discogs:
    apikey: REDACTED
    apisecret: REDACTED
    user_token: REDACTED
    source_weight: 0.5
    tokenfile: discogs_token.json
web:
    host: 127.0.0.1
    cors: ''
    port: 8337
    include_paths: no
    reverse_proxy: no
pathfields: {}
album_fields: {}
missing:
    count: no
    album: no
    total: no
scrub:
    auto: yes
embedart:
    auto: yes
    compare_threshold: 0
    remove_art_file: no
    ifempty: no
    maxwidth: 0
duplicates:
    copy: ''
    keys: []
    checksum: ''
    album: no
    tiebreak: {}
    full: no
    merge: no
    strict: no
    delete: no
    format: ''
    tag: ''
    path: no
    move: ''
    count: no
sampsyo commented 7 years ago

Hi! These sorts of things can happen. To help narrow down what's wrong, can you provide a file and instructions so someone can reproduce the problem locally?

yacoob commented 7 years ago

Here's the file mentioned above, together with copy of my beets db: https://drive.google.com/open?id=0B82HDkmPy51dZU1obmJheGp0WHM

Mind you, similar treatment is applied to a number of files in my library, so I'm not sure how much of a representative sample this is. Is there any debug mode I can run to give you more information? Strace it?

sampsyo commented 7 years ago

OK! And, as far as instructions go, should I be able to reproduce this by importing the file with a blank configuration into a blank library, presumably with -s, and then invoking beet write? (I'm afraid it won't quite work to reuse your database file because it will have your absolute paths in it.)

yacoob commented 7 years ago

Well, let's try that. I'm really curious whether this problem is an artifact of something in the db (that makes beets think that it always needs a sync) or the write not getting through because something on my fs.

sampsyo commented 7 years ago

Hmm, it's a bit different here:

$ beet imp -s 02-03\ Departure\ _\ Shortwave\ _\ Noisetest.mp3 

[...]/02-03 Departure _ Shortwave _ Noisetest.mp3
Tagging track: 65daysofstatic - Departure / Shortwave / Noisetest
URL:
    https://musicbrainz.org/recording/01dde02c-59ef-490c-9536-0700e4808764
(Similarity: 100.0%)
$ beet write noisetest
65daysofstatic - No Man’s Sky: Music for an Infinite Universe - Departure / Shortwave / Noisetest
  artist_sort:  -> 65daysofstatic
  initial_key:  -> C
  bpm: 0 -> 143.620941162
$ beet write noisetest
65daysofstatic - No Man’s Sky: Music for an Infinite Universe - Departure / Shortwave / Noisetest
  artist_sort:  -> 65daysofstatic
  bpm: 143 -> 143.620941162

artist_sort is indeed "stuck", but the other fields are not. Here are some ways you can help out by digging a little deeper into the problem:

yacoob commented 7 years ago

After comparing beet info noisetest and beet info -l noisetest: artist_sort is present in the library data, but not in the file tags. See the attachements. beetinfo-l.txt beet_info.txt

yacoob commented 7 years ago

Played with config, and the culprit is id3v23: yes. With that commented out, writes go through. Without it, they're stuck in the way I've described previously. Once the write goes through (with id3v23: no or commented out), output of beet info contains the missing fields.

There was something that made me enable this option. That "something" might have been iTunes, which I've stopped using, although I'm not sure. :D

Are the problematic fields present in 2.3 spec? If they are not, it'd explain the problem - they get dropped silently during write, and next time beets find them missing again.

yacoob commented 7 years ago

Well, now I know why I've enabled this:

$ id3v2 -l 02-03\ Departure\ _\ Shortwave\ _\ Noisetest.mp3
02-03 Departure _ Shortwave _ Noisetest.mp3: No ID3 tag

and I was using id3v2 for debugging tagging problems.

sampsyo commented 7 years ago

Aha; that makes sense. Thanks for investigating. Yes, the old version of ID3 doesn't support a number of modern tags. These, as regrettable as it is, just need to be silently dropped.

And also yes, the id3v2 tool is sadly outdated.

yacoob commented 7 years ago

Do you think this can be improved on beets side? With id3v23: yes it could conceivably check whether fields to be written are present in 2.3 set, and drop those that are not.

sampsyo commented 7 years ago

Yeah, that's certainly a possibility—or we could just warn that some fields won't be written? Actually hiding the changes would require a somewhat funky violation of our layers of abstraction, however. Currently, MediaFile (or on-disk tag abstraction module) tries to isolate the rest of the application from the low-level, nitty-gritty details of different tag formats—exposing the details of a specific configuration (ID3v2.3) would go against that. But if we can figure out a clean way to do this introspection, it would clearly be useful!

tweitzel commented 7 years ago

This is the same issue I raised in #2189, just for completion's sake. I think we backburnered it at the time because the dropping of fields took place in the bowels of MediaFile and getting it to spit out a warning in an agnostic way was difficult.

sampsyo commented 7 years ago

Good point, @tweitzel; I knew I had seen this set of issues before somewhere. :fish: