Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Use the track title instead the recoding title (MusicBrainz related) #192

Closed
gorgobacka opened this issue Sep 17, 2017 · 6 comments
Closed

Use the track title instead the recoding title (MusicBrainz related) #192

gorgobacka opened this issue Sep 17, 2017 · 6 comments
Labels
Accepted Accepted issue on our roadmap Bug Generic bug: can be used together with more specific labels Good First Issue Good for new contributors Needed: patch A pull request is required Sprintable Small enough to sprint on
Milestone
2.0

Comments

@gorgobacka
Copy link
Contributor

Currently, whipper is using the recording title as "track title". However, this can be different from the correct track title.

This is similar to: #156
@RecursiveForest RecursiveForest self-assigned this Mar 2, 2018
@MerlijnWajer
Copy link
Collaborator

I am assuming that this is for the track filename, is that correct?

@gorgobacka
Copy link
Contributor Author

I need to check it again, but if I remember correctly, the 'recording title' for tagging, and the same title is later also used for the filename. Therefore, yes, it's also related to the filename.

(Maybe I will solve the issue by myself, if I find some time in the coming days.)

@MerlijnWajer
Copy link
Collaborator

It's also a task for GCI, btw. So perhaps a student will pick it up.

@gorgobacka
Copy link
Contributor Author

gorgobacka commented Oct 22, 2018 •
edited
Loading

No problem. It should be easy to fix. Check also this PR #156. It was a quite similar task.

@JoeLametta JoeLametta added Bug Generic bug: can be used together with more specific labels Accepted Accepted issue on our roadmap Good First Issue Good for new contributors Sprintable Small enough to sprint on Needed: patch A pull request is required and removed enhancement labels Nov 12, 2018
@JoeLametta JoeLametta added this to the 2.0 milestone Nov 12, 2018
@ultra-programmer
Copy link

Hello! I am a GCI student hoping to help out with this problem. In what specific section(s) of the code do you believe the problem is occurring in?

@JoeLametta
Copy link
Collaborator

Welcome!
You can start looking at function _getMetadata in /whipper/common/mbngs.py:

whipper/whipper/common/mbngs.py

Line 250 in d5bf83e

track.title = t['recording']['title']

😉

@ABCbum ABCbum mentioned this issue Dec 12, 2019
Merged
ABCbum pushed a commit to ABCbum/whipper that referenced this issue Dec 13, 2019
Enable whipper to use track title if possible
758f854 
track.title = t.get('title', t['recording']['title']).

Since if a track itself doesn't have a title then the track
title is the same with the recording title. Otherwise, a track
has its own title then t['title'] is different from
t['recording']['title'] and whipper chooses t['title'].

[Fixes whipper-team#192]
Signed-off-by: ABCbum <kimlong221002@gmail.com>
hydrian pushed a commit to hydrian/whipper that referenced this issue Dec 18, 2019
@ABCbum @hydrian
Enable whipper to use track title (whipper-team#430)
ab6b886 
* Enable whipper to use track title if possible

track.title = t.get('title', t['recording']['title']).

Since if a track itself doesn't have a title then the track
title is the same with the recording title. Otherwise, a track
has its own title then t['title'] is different from
t['recording']['title'] and whipper chooses t['title'].

[Fixes whipper-team#192]
Signed-off-by: ABCbum <kimlong221002@gmail.com>

* Add test case to check for track title

Using an existing JSON release file

Signed-off-by: ABCbum <kimlong221002@gmail.com>
Signed-off-by: hydrian <ben.tyger@tygerclan.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Accepted Accepted issue on our roadmap Bug Generic bug: can be used together with more specific labels Good First Issue Good for new contributors Needed: patch A pull request is required Sprintable Small enough to sprint on
Projects
None yet
Milestone
2.0
Development

No branches or pull requests
5 participants
@RecursiveForest @MerlijnWajer @gorgobacka @JoeLametta @ultra-programmer