Review Request: MusicBrainz and MusicDNS services support implementation

Leo Franchi lfranchi at kde.org
Fri Sep 24 03:11:53 CEST 2010 


> On None, Sergey Ivanov wrote:
> > Looks works for me.
> 
> Leo Franchi wrote:
>     Doesn't compile when KDE4_BUILD_TESTS is enabled:
>     
>     
>     [ 54%] Building CXX object tests/core-impl/collections/proxycollection/CMakeFiles/testproxycollectionmeta.dir/TestProxyCollectionMeta.o
>     /home/leo/kde/src/amarok/tests/core-impl/collections/proxycollection/TestProxyCollectionMeta.cpp: In member function ‘void TestProxyCollectionMeta::testEditableCapabilityOnMultipleTracks()’:                                                                                                 
>     /home/leo/kde/src/amarok/tests/core-impl/collections/proxycollection/TestProxyCollectionMeta.cpp:463:51: error: cannot allocate an object of abstract type ‘MyEditCapability’
>     /home/leo/kde/src/amarok/tests/core-impl/collections/proxycollection/TestProxyCollectionMeta.cpp:433:1: note:   because the following virtual functions are pure within ‘MyEditCapability’:
>     /home/leo/kde/src/amarok/src/core/capabilities/EditCapability.h:57:26: note:    virtual void Capabilities::EditCapability::setUid(const QString&, const QString&)
>     /home/leo/kde/src/amarok/tests/core-impl/collections/proxycollection/TestProxyCollectionMeta.cpp:464:51: error: cannot allocate an object of abstract type ‘MyEditCapability’
>     /home/leo/kde/src/amarok/tests/core-impl/collections/proxycollection/TestProxyCollectionMeta.cpp:433:1: note:   since type ‘MyEditCapability’ has pure virtual functions
>     make[2]: *** [tests/core-impl/collections/proxycollection/CMakeFiles/testproxycollectionmeta.dir/TestProxyCollectionMeta.o] Error 1
>     make[1]: *** [tests/core-impl/collections/proxycollection/CMakeFiles/testproxycollectionmeta.dir/all] Error 2
>
> 
> Leo Franchi wrote:
>     I have some doubts about the mass tagging UI. When I first open the MusicBrainz tagger with multiple files selected, I see this:
>     
>     http://imagebin.ca/view/e25y1kwP.html'
>     
>     I can select files in the left pane, and that populates the fields below. It's not clear if the "Start Searching..." button operates on the currently selected track, or all the tracks. I learned it operates on all the tracks, but I didn't know until I tried it. The text should clarify this. Then I see this:
>     
>     http://imagebin.ca/view/iKtHSXE.html'
>     
>     Now I am just confused. What happened to the tracks on the left? What are the entries on the right? None of them are track names, they are actually album names. It's not good to h ave the left listview be tracks and the right listview be albums without any sort of help for the user to figure that out. Anyway, as a new user i'm curious as to what these entries are, so I expand one or two:
>     
>     http://imagebin.ca/view/jrUBpR3K.html'
>     
>     And so I am extrapolating here, it seems like it is picking albums and bolding the names of tracks in the album that matches one of the tracks I had on the left---but has now disappeared. What are all the non-bold track names for? They are not related to the original tracks in the left hand pane. I can't have an overview of what this dialog is actually going to do.
>     
>     I'm not sure at this point if "Save" operates on the currently selected tracks, or if it saves all the tracks (what tracks? the ones that are bold? all of them? it is confusing). 
>     
>     Also, I am unable to use the MusicDNS finder. No matter what, it never find anything. Here is my debug output:
>     
>     
>     amarok:       BEGIN: MusicDNSFinder::MusicDNSFinder(const Meta::TrackList&, QObject*, const QString&, int, const QString&, const QString&, const QString&) 
>     amarok:         [MusicDNSFinder] Initiating MusicDNS search: 
>     amarok:         [MusicDNSFinder]        Host:            "ofa.musicdns.org" 
>     amarok:         [MusicDNSFinder]        Port:            80 
>     amarok:         [MusicDNSFinder]        Path Prefix:     "/ofa/1" 
>     amarok:         [MusicDNSFinder]        Client ID:       "0c6019606b1d8a54d0985e448f3603ca" 
>     amarok:         [MusicDNSFinder]        Client version:  "2.3-GIT" 
>     amarok:         BEGIN: MusicDNSAudioDecoder::MusicDNSAudioDecoder(const Meta::TrackList&, int) 
>     amarok:         END__: MusicDNSAudioDecoder::MusicDNSAudioDecoder(const Meta::TrackList&, int) - Took 0.085s 
>     amarok:       END__: MusicDNSFinder::MusicDNSFinder(const Meta::TrackList&, QObject*, const QString&, int, const QString&, const QString&, const QString&) - Took 0.086s 
>     amarok:     END__: void MusicBrainzTagger::searchDone() - DELAY Took (quite long) 10s 
>     amarok:   END__: void MusicBrainzFinder::sendNewRequest() - DELAY Took (quite long) 10s 
>     amarok:   BEGIN: void MusicDNSFinder::decodingDone(ThreadWeaver::Job*) 
>     amarok:   END__: void MusicDNSFinder::decodingDone(ThreadWeaver::Job*) - Took 0.00011s 
>     amarok:   BEGIN: void MusicDNSFinder::sendNewRequest() 
>     amarok:     [MusicDNSFinder] There is no any queued requests. Stopping timer. 
>     amarok:     BEGIN: void MusicBrainzTagger::mdnsSearchDone() 
>     amarok:     END__: void MusicBrainzTagger::mdnsSearchDone() - Took 6.9e-05s 
>     amarok:   END__: void MusicDNSFinder::sendNewRequest() - Took 0.00026s 
>     amarok:   BEGIN: void MusicBrainzFinder::sendNewRequest() 
>     amarok:     [MusicBrainzFinder] There is no any queued requests. Stopping timer. 
>     amarok:     BEGIN: void MusicBrainzTagger::searchDone() 
>     amarok:     END__: void MusicBrainzTagger::searchDone() - Took 8.5e-05s 
>     amarok:   END__: void MusicBrainzFinder::sendNewRequest() - Took 0.00029s 
>     
>     am I doing something wrong?
> 
> Sergey Ivanov wrote:
>     This UI made to be like on Picard (original musicbrainz tagger). On a left side we have tracks to search, on a right side - search result. When a track found we move it from left side to the right one. On the right table complete album's track list is shown. Albums, that contains a track has a bold text, the same to tracks - all tracks in a album linked to real tracks has a bold text. BTW all tracks are drag-able. So if don't wanna save search result for some tracks, you can drop this tracks to left panel. 
>     Could you send one of this track on 123kash at gmail.com, i didn't have such troubles with my "test samples".

Sorry for the delay. Work has been keeping me busy.

I've never used Picard, so that's why I was confused. But I think this points to a valid criticism---the use of Picard is not necessary to the use of Amarok. Users should not be expected to be familiar with the user interface of another program to feel comfortable with Amarok's user interface. For example, I had no idea i could drag tracks---there are no visual indicators that dragging is possible, nor do I know what dragging would do exactly (why is dragging right->left "choosing not to save results"?). My point with this is that I think we need a better explanation than "it is how Picard does it"---we want Amarok to be as intuitive and user-friendly as possible, regardless of how good or bad other programs are :) I think a few things at least need to be done:

1) Workflow clarified---what is the user supposed to do? What buttons do what, and how is the user informed of that?
2) Text to explain to the user what to do. Tooltips to explain what buttons do

Re: files:

What sort of files are supposed to 'work'? I tried some popular bands (the beatles, the who, queen, led zeppelin) that they definitely should have in their records, but got 0 results. I can send you some if you want, or come find me on irc (nick lfranchi, #amarok on Freenode)


- Leo


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/100000/#review4
-----------------------------------------------------------


On 2010-09-22 09:23:42, Sergey Ivanov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/100000/
> -----------------------------------------------------------
> 
> (Updated 2010-09-22 09:23:42)
> 
> 
> Review request for amarok.
> 
> 
> Summary
> -------
> 
> MusicBrainz and MusicDNS services support implementation.
> There are three independent parts:
> 1. MusicBrainzFinder class - used to make requests to MusicBrainz server and process replies. All relies process in separate threads by MusicBrainzXmlParser class. For a search uses guessed from a file name track information.  No external dependences required.
> 2. MusicDNSFinder class - used for the same purpose as MusicBrainzFinder, but i communicate with musicdns server and receives track's PUID as a reply. Replies ether process in separate threads by MusicDNSXmlParser class. Fingerprints generated by libofa (the only external dependence in entire patch). For track decompressing (MusicDNSAudioDecoder class) used xine engine (I'm not sure is It a good choice, but amarok based on phonon media-engine, that uses xine. So we don't deed to pull any other dependences). Received PUIDs sends to MusicBrainzFinder class, for a search routine.
> 3. View. All classes used for store (MusicBrainzTagsModel, MusicBrainzTrackListModel) and display (MusicBrainzTagsModelDelegate) purposes.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 191b02e 
>   cmake/modules/FindLibOFA.cmake PRE-CREATION 
>   config-amarok.h.cmake 981b7b7 
>   src/CMakeLists.txt 42614ff 
>   src/core-impl/capabilities/timecode/TimecodeEditCapability.h 6e15303 
>   src/core-impl/capabilities/timecode/TimecodeEditCapability.cpp 8205d45 
>   src/core-impl/collections/mediadevicecollection/MediaDeviceMeta.h e35b57f 
>   src/core-impl/collections/mediadevicecollection/MediaDeviceMeta.cpp 9be62d9 
>   src/core-impl/collections/proxycollection/ProxyCollectionMeta.cpp 8bdbf75 
>   src/core-impl/collections/sqlcollection/CapabilityDelegateImpl.cpp b5cb083 
>   src/core-impl/collections/sqlcollection/SqlMeta.h ee3ec21 
>   src/core-impl/collections/sqlcollection/SqlMeta.cpp 2da0333 
>   src/core-impl/meta/file/File.h 6d4d395 
>   src/core-impl/meta/file/File.cpp 30cd2ff 
>   src/core-impl/meta/file/TagLibUtils.cpp 15b64a4 
>   src/core-impl/meta/proxy/MetaProxy.h 2ef3805 
>   src/core-impl/meta/proxy/MetaProxy.cpp 341e076 
>   src/core/capabilities/EditCapability.h 79344bd 
>   src/dialogs/MusicBrainzTagger.h PRE-CREATION 
>   src/dialogs/MusicBrainzTagger.cpp PRE-CREATION 
>   src/dialogs/MusicBrainzTagger.ui PRE-CREATION 
>   src/dialogs/TagDialog.h 50cd801 
>   src/dialogs/TagDialog.cpp 68fc5e3 
>   src/dialogs/TagDialogBase.ui 9974d0b 
>   src/musicbrainz/MusicBrainzFinder.h PRE-CREATION 
>   src/musicbrainz/MusicBrainzFinder.cpp PRE-CREATION 
>   src/musicbrainz/MusicBrainzMetaClasses.h PRE-CREATION 
>   src/musicbrainz/MusicBrainzMetaClasses.cpp PRE-CREATION 
>   src/musicbrainz/MusicBrainzTagsModel.h PRE-CREATION 
>   src/musicbrainz/MusicBrainzTagsModel.cpp PRE-CREATION 
>   src/musicbrainz/MusicBrainzTagsModelDelegate.h PRE-CREATION 
>   src/musicbrainz/MusicBrainzTagsModelDelegate.cpp PRE-CREATION 
>   src/musicbrainz/MusicBrainzTrackListModel.h PRE-CREATION 
>   src/musicbrainz/MusicBrainzTrackListModel.cpp PRE-CREATION 
>   src/musicbrainz/MusicBrainzXmlParser.h PRE-CREATION 
>   src/musicbrainz/MusicBrainzXmlParser.cpp PRE-CREATION 
>   src/musicbrainz/MusicDNSAudioDecoder.h PRE-CREATION 
>   src/musicbrainz/MusicDNSAudioDecoder.cpp PRE-CREATION 
>   src/musicbrainz/MusicDNSFinder.h PRE-CREATION 
>   src/musicbrainz/MusicDNSFinder.cpp PRE-CREATION 
>   src/musicbrainz/MusicDNSXmlParser.h PRE-CREATION 
>   src/musicbrainz/MusicDNSXmlParser.cpp PRE-CREATION 
>   tests/core-impl/collections/proxycollection/TestProxyCollectionMeta.cpp 55d1914 
> 
> Diff: http://git.reviewboard.kde.org/r/100000/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/amarok-devel/attachments/20100924/7a9a310d/attachment-0001.htm

More information about the Amarok-devel mailing list