GSoC proposal draft: Audio СD collection refactoring.

[Matěj Laitl]

On 30. 4. 2013 Tatjana Gornak wrote:
> Hi all,
> 
> I am looking forward to get some feedback on my proposal. Thanks in advance.

Glad to see your proposal. Sorry for complications caused by me not publicly 
communicating my MTP intents early enough.

> ==Motivation and Goals==
> Current implementation of AudioCd collection suffers from following
> drawbacks: it uses deprecated MediaDevice framework, does not support
> CDs playback on Windows, uses not best code solutions (for example,
> see comments in [3]).

I'd be even more harsh and say "ugly and hacky" or "convoluted" code.

> Amarok has several bugs in AudioCd collection components [1]. Since
> successful accomplishment of this project will lead to fully rewritten
> AudioCd collection, these bugs should be taken into account and if
> possible do not appear in new code.

You're too humble, I'm sure you'll fix all these bugs along with the rewrite. 
:-) (if they are actually assigned to the right component)

I think that you should also explicitly state the goals, perhaps dependable 
audio cd detection using KDE Solid, track enumeration that doesn't depend on 
cddb, asynchronous CDDB info retrieval, using more up-to-date technologies..

> ==Implementation Details==
> Following classes should be implemented or revised (in case they exist).

They do exist (and you know it). This may be interpreted as not knowing enough 
the current state (not by me), so you may want to reword it so that it is 
clear that you understand what the current situation is.

> ===AudioCdCollectionFactory===
> This class should react on Solid device detection and Solid device
> ejection. If new device is CD [10], then it should create
> AudioCdCollection.
> Thus, in initialization of AudioCdCollectionFactory following steps
> should be implemented:
> 1) checked if some AudioCd devices were already inserted on startup;
> 2) solid device detection and solid device ejection events should be
> connected to proper slots of AudioCdCollectionFactory: for this
> purpose Solid::DeviceNotifier and its deviceAdded and deviceRemoved
> signals should be used.

Exactly.

> ===AudioCdCollection===
> AudioCdCollection should be inherited from Collection and should
> implement its methods. Collection initialization: fetches and stores
> metadata as described below.
> 
> ====Metadata fetching====
> 1. Amarok uses kioslave audiocd:/ to browse data on audio CDs (see
> AudioCdCollection::audioCdEntries), that could be a reason why CD
> playback does not work on Windows [4]. The problem which should be
> solved: is there any Windows-compatible solution which can be used for
> metadata fetching or is it possible to add this feature to kioslave
> audiocd. As alternative to kioslave audiocd:/ libk3bdevice can be
> considered. This part of K3B is cross-platform (this is achieved by
> using WinAPI in some routines).

Sounds sane. I was hoping to use Solid for actual track enumeration, but it 
seems that it doesn't provide the information.

There is also libkcompactdisc, but that seems to be just for playback.

libk3bdevice could be the solution, although that would include some issues to 
solve:
 * would Amarok depend on k3b for AudioCD playback then?
 * binary compatibility issues

Another possibility would be to employ Phonon for this, see for example 
/usr/include/phonon/addoninterface.h, namely TitleInterface. You could 
construct a Phonon::MediaObject and pass it an AudioCD Phonon::MediaSource. 
Then you could use the TitleInterface to get total number of titles and then 
setCurrentTitle (see also DelayedSeeker class in Amarok, although not directly 
reusable by you) followed by totalTime() to get times of individual tracks.

^^^ OTOH the above looks rather fragile, but is worth a test. The advantage is 
that Phonon support is needed for actual audio playback, so it is most natural 
to use the same framework to do the enumeration.

> 2. For each track CDDB information should be requested (BR279485
> should be taken into account while working on CDDB information
> fetching).

unfinished sentence?

> 3. AudioCdCollection should use MemoryMeta framework to represent
> tracks: MemoryCollection should be used to store tracks, MapChanger
> should be used to add new tracks to MemoryCollection, code of AudioCd
> meta classes should be polished.

Right.

> 4. Track enumeration, CDDB information fetching should work  asynchronously.
> 5. BR289294 should be taken into account while working on metadata
> fetching.

I propose you add [Bug Title After Number] so that this can be read without 
switching contexts.

> ===AudioCdCollectionLocation===
> AudioCdCollectionLocation copies tracks out of the CD and supports
> transcoding capability.

This will be another interesting issue to solve - but I see that the research 
and testing needed is immense, so it is not realistic to require a firm plan in 
proposal time.

> ===Miscellaneous===
> Current AudioCd collection implementation suffers from a lack of
> interaction with the user. For example, there is no any feedback in
> following cases: if transcoding fails or if track playback fails.

I guess you'll fix this; do mention it. ;)

> ==Tentative Timeline==
> Week 1 (17.06 -- 23.06), Week 2 (24.06 -- 30.06)
> New implementation of AudioCdCollectionFactory and AudioCdCollection.
> After  this step is completed, amarok should support AudioCd in a same
> way as before, since code which uses kio, solid and cddb will not have
> been rewritten at this point, but just reorganized.

Right, although you may want to split these into week-long bits to fulfil the 
formal requirement.

> Week 3 (01.07 -- 07.07), Week 4 (08.07 -- 14.07)
> Integration of MemoryMeta framework for track representation, BR289294.

This also means getting rid of the MediaDeviceFramework - please to mention 
it; it will allow you to break it down to 2 items for each week.

> Week 5 (15.07 -- 21.07)
> Work on CBBD metadata fetching routine, BR279485.
> 
> Week 7 (22.07 -- 28.07), Week 6 (29.07 -- 04.08)
> Decision about metadata fetching routine should be done and
> implemented: leave kioslave or switch to another solution.

Oh, for metadata please don't use the kio, libkcddb should be easy to use 
directly when you have the tracks (and lengths) enumerated.

> Week 8 (05.08 -- 11.08)
> Fix Bug 279188
> Implement correct behavior on launching amarok with --cdplay in a way
> it was suggested in review request [10].
> 
> Week 9 (12.08 -- 18.08)
> Work on AudioCdCollectionLocation.
> 
> Week 10 (19.08 -- 25.08)
> CD support testing and bug fixing stage.
> After this step is completed, amarok should support CD not worse than
> before.
> 
> Week 11 (26.08 -- 01.09)
> AudioCd Collection performance evaluation.
> BR311329 and BR319036 should be taken into account on this step
> 
> Week 12 (02.09 -- 08.09), Week 13 (09.09 -- 15.09)
> Code polishing and documentation writing phase.

Please write in-code documentation right with the implementation and/or 
changes. User documentation (handbook) can be updated in this phase.

> Week 14 (16.09 -- 22.09), Week 15 (23.09 -- 29.09)
> Pencils-down stage. Code should be prepared to be merged in master.
> 
> ==About me==
> I’m 24 year old Phd student of TU Kaiserslautern. My research lies in
> area of computational mathematics. Some additional information about
> me can be found on my page [9]. I have some experience of working with
> amarok source code [6-8].

Oh, you're very humble! :-) You're basically rewritten all our playlist file 
support.

> I will be able to work 40 hours a week during GSoC period. But it is
> possible that I'll have a week vacation (I haven't planned it yet), if
> so I will compensate it by rearranging work for weeks before vacation
> and after.
> 
> Why AudioCd refactoring? My opinion is that open source project code
> should be clean and advanced, in order to attract new developers and
> AudioCd refactoring will help improve quality of Amarok code base.

You can also mention that the MediaDeviceFramework can be removed when this 
project is completed (assuming MTP rewrite happens) as another advantage.

Anyways, very high standard proposal, if you have a look at the (mostly minor) 
remarks, this will be a very nice one.

Cheers,
		Matěj