GNOME Bugzilla – Bug 706352
Add support for editing song tags
Last modified: 2018-01-10 14:33:46 UTC
I think this is a nice feature that a music management application could have. We could create something similar to gnome-documents' Document Properties (shows up when you select a single document in selection mode then select the properties icon in the OSD toolbar). In my opinion, a simple implementation is all we need in gnome-music. Some of the properties/tags we could add here are: - Track Title - Artist Name - Album Name - Year Published - Album Art We could change the tags through gstreamer to change the files directly, then tell tracker to re-extract the new tags. Or we could just change tracker's data, without changing the file's metadata.
I agree that we should just change tracker's data, without changing the file's metadata.
I think that the expected functionality of changing a tag would be to change the tag, not to display a value other than the tag. I would expect the newly corrected property to appear in Rhythmbox as well. I would expect to be able to copy my files to another computer and have the values - that I may have tediously corrected one by one - appear on the second computer.
This'll need design. :)
Shouldn't tag editing be implemented in Nautilus instead? I think music player should just play music and not much more, certainly not manage music collection (considering part of it might be pulled from online services like soundcloud, so it wouldn't even always work). It would make sense to make it a separate app, but even then there already are a few great music management apps like gMusicBrowser or Picard. Is there a place to discuss this issue?
(In reply to comment #4) > Shouldn't tag editing be implemented in Nautilus instead? We're not planning to update file's tags at this stage, not sure if there is way for tracker to write updated tags to the file after the internal tracker record has been updated - this needs investigation. > Is there a place to discuss this issue? Generally - yes, but we should keep it short. If the discussion will be rather long we can postpone it for BoF during any conference. Though this still needs design I think that would be a nice feature of 3.16 release
(In reply to Vadim Rutkovsky from comment #5) > (In reply to comment #4) > > Shouldn't tag editing be implemented in Nautilus instead? > We're not planning to update file's tags at this stage, not sure if there is > way for tracker to write updated tags to the file after the internal tracker > record has been updated - this needs investigation. FWIW there is, the tracker-writeback service takes care of mirroring changes to the Tracker database back into files. Relevant to gnome-music, there is support for a bunch of audio formats (https://git.gnome.org/browse/tracker/tree/src/tracker-writeback/tracker-writeback-taglib.c#n83) and playlists (https://git.gnome.org/browse/tracker/tree/src/tracker-writeback/tracker-writeback-playlist.c#n84) AFAICS tracker doesn't currently install a .desktop file to autostart the service, but that can be revised. All that would be left to gnome-music is doing the relevant sparql updates.
3.18 is over - removing the target-milestone.
It would be cool to make usage of AcoustID [0] source in someway to help user identify and tag the media. [0] https://acoustid.org/ We have this Grilo plugin (bug 732879) that could provide most if not all the metadata mentioned in comment 0. As bonus, we also receive musicbrainz identifications if more info is needed :) * Tracker could be the one generating and storing the music's fingerprint and gnome-music could use it to fetch AcoustID for metadata; * Or gnome-music could generate the fingerprint and store it with tracker once the user wants to edit the music Note that in order to generate the fingerprint we need to read the whole music file as input for chromaprint
Linking here the current mockups for the feature: https://raw.githubusercontent.com/gnome-design-team/gnome-mockups/master/music/edit-details-wire.png * This feature is going to be implemented by the selected student in the 2016 round of Google Summer of Code *
Created attachment 323864 [details] Suggested mockup for tag-editor Attached mockup of how tag-editor will work. Inspired from Allan's mockup for the same: https://github.com/gnome-design-team/gnome-mockups/blob/master/music/edit-details-wire.svg Open to any suggestions to make this better. :)
(In reply to Saiful B. Khan from comment #10) > Created attachment 323864 [details] > Suggested mockup for tag-editor > > Attached mockup of how tag-editor will work. Inspired from Allan's mockup > for the same: > https://github.com/gnome-design-team/gnome-mockups/blob/master/music/edit- > details-wire.svg > > Open to any suggestions to make this better. :) Great. My opinions on the designs are: 1. Allan's design still does not show how this dialog would be launched, but I would avoid using right-click. I would go for a action bar instead; 2. No need to have a button triggering the fetch metadata operation. Considering Victor's idea, we would query on the background (in a non-blocking manner) for the metadata from the web. The user can always manually change it, we just offer the pre-filled options from the web. 3. The same as the metadata, we would fetch the cover image by default. We could offer the manual editing of it by clicking on the image (launching a file chooser). 4. We usually don't ~apply~ changes in a batch. We automatically apply them as we edit it.
Would this be based off id3tag?
(In reply to Christopher Lee Murray from comment #12) > Would this be based off id3tag? Christoper yes! tracker-writeback uses ID3 format for tagging music files. https://mail.gnome.org/archives/commits-list/2009-November/msg06020.html
Created attachment 324682 [details] Updated mockup An updated mockup addressing Felipe's suggestions in the earlier comment.
(In reply to Saiful B. Khan from comment #13) > (In reply to Christopher Lee Murray from comment #12) > > Would this be based off id3tag? > > Christoper yes! tracker-writeback uses ID3 format for tagging music files. > https://mail.gnome.org/archives/commits-list/2009-November/msg06020.html Nice! This is very doable. Also, I looked at the proposed mockups. I think it would be more minimal to have the play button and shuffle button side by side, and appear when the cursor hovers over the cover art(the cover art would shade). Either that or they are evenly distributed below the cover art in a similar fashion to that in the mockups. I have proposed to work on this from GSOC. I hope I get the opportunity. It seems very doable from my experience and I am pretty excited to do it. I have forked gnome-music and started to build it out, but on the latest branch so I ran into some dependency issues. I will use a more feasible branch and start looking at how to implement this along with the audio tag edition support.
(In reply to Christopher Lee Murray from comment #15) > (In reply to Saiful B. Khan from comment #13) > > (In reply to Christopher Lee Murray from comment #12) > > > Would this be based off id3tag? > > > > Christoper yes! tracker-writeback uses ID3 format for tagging music files. > > https://mail.gnome.org/archives/commits-list/2009-November/msg06020.html > > Nice! This is very doable. Also, I looked at the proposed mockups. I think > it would be more minimal to have the play button and shuffle button side by > side, and appear when the cursor hovers over the cover art(the cover art > would shade). Either that or they are evenly distributed below the cover art > in a similar fashion to that in the mockups. > > I have proposed to work on this from GSOC. I hope I get the opportunity. It > seems very doable from my experience and I am pretty excited to do it. I > have forked gnome-music and started to build it out, but on the latest > branch so I ran into some dependency issues. I will use a more feasible > branch and start looking at how to implement this along with the audio tag > edition support. Hey Christopher! Actually I myself am an applicant for GSoC. The above mockup is just suggestive of what I think it should be like (and included it in my proposal). But you're free to use it as an inspiration. Glad you liked it :)
That's pretty cool! It's definitely doable. Good luck man!
Created attachment 329186 [details] [review] Patches for partial manual editing ability The patch contains two large commits. One for the new UI xml and one for providing write support through the UI
Review of attachment 329186 [details] [review]: Good to have such good work in the beginning of your internship. Congrats! This is a first round of reviews, therefore I will get back to these patches after you address my current review. ::: data/SelectionToolbar.ui @@ +28,3 @@ </child> + <child> + <object class="GtkButton" id="button3"> I know we don't identify the buttons properly elsewhere, but we could start by doing it now. "edit-details-button" sounds clearer. ::: data/TagEditingDialog.ui @@ +3,3 @@ +<interface> + <requires lib="gtk+" version="3.16"/> + <object class="GtkHeaderBar" id="headerbar"> make it an internal child of the dialog. <child internal-child="headerbar"> @@ +11,3 @@ + <object class="GtkFileFilter" id="imageFilter"> + <mime-types> + <mime-type>image/png</mime-type> wouldn't it be better to cover more mime-types? <mime-type>image/*</mime-type> @@ +51,3 @@ + <property name="column_spacing">12</property> + <child> + <object class="GtkBox" id="box1"> if you are not referencing this object elsewhere, you don't need to set an identifier. <object class="GtkBox"> should be enough. @@ +70,3 @@ + </child> + <child> + <object class="GtkLabel" id="suggestedTitle"> Instead of having the label under the GtkEntry, I would like to pre-fill the GtkEntry with the suggested title. GtkEntry has a property called "placeholder-text" that you might be interested. The UX behavior in this case would be: 1. gnome-music sets the "placeholder-text" with the default values extracted from the files (from tracker). 2. gnome-music tries to set the "placeholder-text" with the values obtained from the AcousticId grilo plugin. 3. If the user types something different in the entry, then we save what is on the entry. @@ +77,3 @@ + <property name="xalign">0.019999999552965164</property> + <attributes> + <attribute name="scale" value="0.90000000000000002"/> no need for that much precision. 0.9 shall be ok. @@ +78,3 @@ + <attributes> + <attribute name="scale" value="0.90000000000000002"/> + <attribute name="foreground" value="#88888a8a8585"/> go for already existent style class, <style><class name="dim-label"/></style> @@ +102,3 @@ + <attributes> + <attribute name="weight" value="bold"/> + <attribute name="foreground" value="#555557575353"/> ditto. @@ +270,3 @@ + <property name="spacing">6</property> + <child> + <object class="GtkImage" id="cover"> I'd like to have the cover positioned in the topleft corner, just like in Allan's mockups: https://github.com/gnome-design-team/gnome-mockups/blob/master/music/edit-details-wire.svg @@ +288,3 @@ + </child> + <child> + <object class="GtkFileChooserButton" id="filechooserbutton1"> I would rather launch the File Chooser by clicking in the cover image, just like the avatar selector in gnome-control-center/user-accounts. @@ +654,3 @@ + </child> + <child> + <object class="GtkButtonBox" id="buttonbox2"> I would rather have a more automated behavior for this (instead of buttons). See my previous comment about Suggested entries. @@ +677,3 @@ + </child> + <child> + <object class="GtkButton" id="undoChangesButton"> Instead of having an Undo Button, we could do exactly like we do when deleting a playlist (using the in app notifications to Undo). ::: configure.ac @@ +41,3 @@ AC_SUBST(GLIB_COMPILE_RESOURCES) +GRL_MIN_VERSION=0.3.1 would be good to have a patch just for this change, explaining which feature made you want to bump de grilo version. ::: gnomemusic/view.py @@ +186,3 @@ self.selection_toolbar._remove_from_playlist_button.\ set_sensitive(len(items) > 0) + if self.name is 'songs': I'd rather enumerate the view container instead of using hardcoded strings like this.
Created attachment 329289 [details] [review] Bump grilo dependency to v0.3.1 Standalone patch to just raise grilo minimum requirement to grilo-0.3.1
Created attachment 329290 [details] [review] Rework dialog UI Relevant changes made in response to the last patch review.
Created attachment 329635 [details] [review] Provide proper implementation of cover selection The cover selection works as expected, locally for now. Embedding cover art into music files is not yet possible with existing frameworks/libraries. This can be improved later on. For now the user can store custom media art in cache for gnome-music.
Created attachment 333959 [details] [review] Added chromaprint generation and resolution of relevant data The acoustID source and chromaprint-plugin from, grilo now help us to get automatic matches for any song and also provide its MusicBrainz recording-id, which can be further used in conjunction with the grl-musicbrainz plugin to get the rest of the relevant metadata. Since the mb plugin is still a WIP, development on this feature will be continued after grl-musicbrainz is capable enough.
Created attachment 333960 [details] [review] Tag editor: Automatic metadata overwrites the text-entries The metadata fetched with musicbrainz plugin can overwrite the existing metadata in tag editor dialog. This seems somewhat abrupt and non undoable at the moment.
Created attachment 333961 [details] [review] Tag editor: Add suggest labels, confirm and reset buttons The fetched metadata will not be directly applied but instead shown as labels below each entry after which the user can apply them using the provided button. A button to reset all fields back to their original values was also added. The click response to both the buttons are just stubs for now.
Created attachment 333962 [details] [review] Tag editor: Provide functionality for the 'Confirm' button The 'Use suggestions' button will now expectedly write the suggested data (if available) to the music file. Unavailable entries will not overwrite and the previous value will remain.
Created attachment 333963 [details] [review] Tag editor: Modified write queries to support metadata deletion To support undoing edits and removing other wrong metadata the queries needed some modifications. The same functions are being made to return different update queries based on the 'remove_only' flag.
Created attachment 333964 [details] [review] Tag editor: Empty entries when activated now delete metadata Earlier when text entries in the UI were activated without any value they would skip any write operation but now instead perform metadata deletion for those entries.
Created attachment 333965 [details] [review] Tag editor: Implement clean undo option for the reset button. The Undo/Reset button for all edits will now reset the song to its original state of values. Also corrected some PEP8 inconsistencies.
Created attachment 333966 [details] [review] Tag editor: Fix some issues Rectify some logical issues that would prevent correct tag update through the previous patches. There still remain a few unnoticed or unfound bugs that will be corrected as they're discovered.
Created attachment 333967 [details] [review] Tag editor: Use Tracker's string escape function instead of GLib's The current string escaping to prevent sparql injection was being done by GLib.strescape(). This produced some unrecognised escape sequences for SPARQL queries where the string contained unicode characters. Tracker has its own sparql_escape_string() function that is meant for use in SPARQL itself. This utility is now being used for all new music entity creations.
Created attachment 333968 [details] [review] Tag editor: Remove @log wrapper from __repr__ of TagEditorDialog The log decorator around __repr__() function of the editor class was causing an infinite recursion depth issue in '-d' (debug) mode. Once removed the debug log represents the class correctly for all the events happening within it.
Created attachment 333969 [details] [review] Tag editor: Handle source unavailablity & correct Songs view update The automatic fetching functions were relying on the assumption that both the sources (AcoustID and MusicBrainz) are available which may be unavailable without the internet or any other reason. This is now being checked before proceeding. The 'Songs' view is updated after closing editor dialog by removing the original song that was edited. For this instead of just checking for title, now album and artist is also checked before removing it from the list.
Created attachment 333970 [details] [review] Tag editor: Use GNotification instead of 'Undo' button To maintain the coherence in undoing changes made through the app the GNotification will now be used instead of the 'undo' button. For now the notification prompts an undo to original state after each edit but this will change accordingly when the new design is decided upon.
Created attachment 333971 [details] [review] Tag editor: Add documentation strings for TagEditorWidget class
Created attachment 333972 [details] [review] Tag editor: Privatize all class limited functions and variables. Most functions had not been yet considered for classification as private if they are not meant to be used outside the tag editor class. This was corrected for functions as well as variables with wrongly defined scopes.
Created attachment 334167 [details] [review] Tag editor: Rework UI according to new designs. The suggested tag labels are now displayed beside the corresponding property labels, which themselves have now been positioned over the entries. A new 'Use Suggestions' button is being used to apply automatic tags instead of the earlier symbolic button in headerbar.
Created attachment 334669 [details] [review] Tag editor: Remove some repetitive code and control structures. Instead of using seperate variables for each entry and label element, a dictionary with those elements is now stored and iterated upon whenever an overall change/activation is needed. This makes functions more compact and general.
Created attachment 334670 [details] [review] Tag editor: Fix undo option appearing on 'composer' entry. The 'perform_update' function was being called when we lost focus of the composer entry even if no changes were made. This has been fixed by putting a seperate check for this entry being modified.
This patchset needs to be rebased with the recent widgets/views split. This cannot be reviewed this way. For review it should be rebased into a few logical units: we don't have to see every step of the way (thats what your github branch is fine for), we just need the finished code in sensible working chunks. So you should really trickle this down to 2 or 3 commits that add specific functionality (like the UI/the backend logic/etc. ).
Created attachment 337666 [details] [review] Tag-editor: Add user interface Added UI for tag editor dialog for single song edits. Selected songs now show up an 'Edit Details' button on the actionbar. This will be used to bring up the dialog once the implementations are written.
Created attachment 337667 [details] [review] Tag-editor: Add queries for creating and updating music entities Added various queries which when sent to the tracker-writeback daemon can modify or add music metadata and create new music entities in the tracker-db as well. These SPARQL updates are fundamentally capable of being used for batch edits as well when the feature is implemented in the future.
Created attachment 337668 [details] [review] Tag-editor: Add tag writing functions through a class Add bundled functions to write metadata to songs in the form of TagWriter class. The class provides general functions that may be used by different dialogs, from whom it inherits a few attributes, to perform write operations.
Created attachment 337669 [details] [review] Tag-editor: Add final dialog and interaction for single edits Provide a dialog for interactively editing common song tags. Yet to do media art tagging and album editing. The update after each dialog close is a bit noticable, though this should remain same even when multiple songs are edited at once (in future).
The above patches were rebased over the recent structural changes. Please consider earlier patches as obsolete (I'll mark them when I'm more awake). There is an issue where the 'Albums' view does not update after an edit (dialog closed). It should get re-populated after a 'grilo.emit_change_signal()' call along with the other views (which actually do update). This particular call is a bit costly as well. I could feel a small freeze on my system when this happens. Let me know how they can be better.
*** Bug 786808 has been marked as a duplicate of this bug. ***
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/gnome-music/issues/6.