Review Request: Get FileType of SQLTracks from Database instead of File Extension

Dan Meltzer parallelgrapefruit at gmail.com
Tue Sep 28 07:06:34 CEST 2010


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


For the most part I like this idea.  If we are determining the information already, then we should use it, rather than solving it again.  That being said, I think that even as deep as taglib, the extension is used when scanning the file for metadata, so this isn't really going to solve the problem that you wanted to solve.


src/core-impl/collections/sqlcollection/SqlMeta.h
<http://git.reviewboard.kde.org/r/100004/#comment19>

    This should be initialized somewhere, just to prevent oddities if the db doesn't have filetype information for a given track.



src/core-impl/collections/sqlcollection/SqlMeta.cpp
<http://git.reviewboard.kde.org/r/100004/#comment18>

    I think this whole section should be moved to a helper method in FileType.h.  This is the type of thing that will be duplicated many times over, and it makes more sense to just put it where it belongs now.  Also, translate the unknown, there are way too many file types out there that people might throw at amarok for us to assume that the four will cover everything.  


- Dan


On 2010-09-27 22:59:50, Stefan Derkits wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/100004/
> -----------------------------------------------------------
> 
> (Updated 2010-09-27 22:59:50)
> 
> 
> Review request for amarok.
> 
> 
> Summary
> -------
> 
> -) Added a new Class Variable to SqlTrack to hold the FileType of a Track
> -) Retrieve FileType from Database and save it in the Class Variable
> -) Return a QString Representation of the Amarok::FileType when calling the Method SqlTrack::type()
> 
> 
> Diffs
> -----
> 
>   src/core-impl/collections/sqlcollection/SqlMeta.h ee3ec21 
>   src/core-impl/collections/sqlcollection/SqlMeta.cpp 2da0333 
> 
> Diff: http://git.reviewboard.kde.org/r/100004/diff
> 
> 
> Testing
> -------
> 
> Hard to test, but the TagDialog displays the correct Filetypes
> Wanted to test it with a Testcase where FileType in Database is different then the FileExtension but realized that only certain FileExtensions are read by the CollectionScanner/TagLib :)
> So actually atm, the Code doesn't make much difference, but in my opinion this is a cleaner (and more future proof) Solution to display the Filetype then just displaying the Extension of the File
> 
> 
> Thanks,
> 
> Stefan
> 
>

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


More information about the Amarok-devel mailing list