Skip to content
New issue

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

#

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

Already on GitHub? # 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>
# for free to join this conversation on GitHub. Already have an account? # 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