beetbox / beets

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

Do not consider [silence] as "missing tracks" #1089

Open bkanuka opened 10 years ago

bkanuka commented 10 years ago

Many albums have silent tracks, labelled as "[silence]" in musicbrainz. If these tracks are missing, it should be ignored.

sampsyo commented 10 years ago

Sounds like a reasonable extension. Do you know if there's a way to unambiguously identify this kind of track with MusicBrainz metadata, or should we just look for the string "[silence]"?

bkanuka commented 10 years ago

Just look for the string - it'll be the track title. The musicbrainz spec is here: https://beta.musicbrainz.org/doc/Style/Unknown_and_untitled/Special_purpose_track_title

sampsyo commented 10 years ago

Cool. I'm not 100% sure how we should implement this—maybe we need an "inessential" flag on TrackInfo objects? Some help with the design would not go amiss.

bkanuka commented 10 years ago

Ha I'm not sure either. I wish it could be as simple as if title == '[silence]': ignore but I actually know of two tracks that would be "inessential": [silence] and [crowd noise]. And therefore this probably requires a config setting. I'm probably capable enough to help, but I currently see no end to my AFK to do list :-(

sampsyo commented 10 years ago

Happens to the best of us. :smiley: Check back in if you get a moment to hack.

sampsyo commented 8 years ago

For opatel99 in #metabrainz on IRC:

This should be fixed in a few different steps.

  1. First, we'll need to detect silence tracks. This will happen in the autotag.mb module. The challenging part here is to decide exactly what criterion determines silence. Is there a specific MB ID we can use? Or do we just detect the title string "[silence]", as suggested above?
  2. Next, we need to record the fact that the track is "ignorable" somewhere. This will require extending the TrackInfo class in the autotag.hooks module with a Boolean flag called ignorable or inessential or something. It will be important to make sure we don't break anything with this change by running the tests again after this step.
  3. Finally, we need to tell the matching logic in autotag.match that it's OK to ignore TrackInfo objects with that flag set.

We can get into more detail on the specific steps when we come to them, but that's the high-level outline.

Freso commented 8 years ago

As I wrote on IRC:

The style guidelines also says that artist intent overrules all, so not all silent tracks are named "[silence]" and some artist out there may have made a track called "[silence]" that isn't silence.

I'm not saying it's a bad idea to filter on "[silence]" though, as that seems to be the most reliable thing to do detect silent tracks on MB regardless of reservations above - just as long as those reservations are kept in mind.

bkanuka commented 8 years ago

@sampsyo I agree with most of what you said, however the original intent of my bug was not to detect silence tracks - I think they are already labeled as such in Musicbrainz.

To clarify, occasionally there is an album with CD tracks that are silent and unnamed (don't appear on the album art, but exist as tracks when played on a CD player). The Musicbrainz spec is to give these a title of [silence].

The issue arises when you rip the CD and delete these silent tracks. Currently beets will say "missing tracks", but some may argue that there's no harm in these being missing - and will not want to "punish" or risk mis-tagging due to their absence.

Sorry if this was clear to everyone and I am beating a horse - just wanted to be on the same page. Merry Christmas or Festivus or whatever everyone :-)

sampsyo commented 8 years ago

Yep, that's the goal. The first step is detecting the silence; then we can ignore those tracks. Thanks for checking back in!

tigranl commented 7 years ago

So, the point is to detect [silence] title, and then raise ignorable flag?

sampsyo commented 7 years ago

Yep, that's basically the idea—but we don't currently have an "ignorable" flag, so you'd need to implement that too.

tigranl commented 7 years ago

What is the Distance class? Is it a graph description? I can't figure it out from the code.

sampsyo commented 7 years ago

A Distance is just a weighted set of numbers. The place you'll want to look is the beets.autotag.match module, with all those add and add_string calls. Those add components of the overall similarity score, named with a keyword that lets us determine the weight compared with other components.

Does that help?

tigranl commented 7 years ago

@sampsyo do you have any [silence] examples?

sampsyo commented 7 years ago

Here's one linked from that MusicBrainz wikipedia page: https://musicbrainz.org/release/9c0b5a23-ca6e-4b4e-be2f-98280cf56c88

tigranl commented 7 years ago

What exactly should I look for in match file?

sampsyo commented 7 years ago

Exactly the point you pointed toward in this comment: https://github.com/beetbox/beets/pull/2315#issuecomment-266825622

That is, where we apply the penalty for missing tracks.