astippich added inline comments.

INLINE COMMENTS

> bruns wrote in taglibwriter.cpp:70
> I think for **reading** it as generic as possible is fine, but for writing 
> being a little bit more explicit does not hurt.
> 
> My proposal for Ape and Ogg is to split the writing for these to two 
> different functions. Yes, it **is** possible to use the propertymap for both, 
> as both use the same scale and the same tag name, but the implementations on 
> the taglib side are completely different.
> 
> Writing the "rating" for XiphComment (Ogg) and  Ape in distinct functions 
> (see my comment in D18604 <https://phabricator.kde.org/D18604>) hardly adds 
> any code, but gets the Ape and Ogg code paths in line with the other file 
> formats. You don't write the ASF/MP4 rating using the property interface 
> although it would be possible, and IMHO thats the right thing to do,  also 
> for Ape and Ogg.
> 
> As soon Ape and Ogg are split, you no longer rely on the PropertyMap for the 
> rating, and you won't have to use `properties()`/`setProperties()` twice.

No, I cannot use the the PropertyMap for ASF/MP4, those atoms/attributes are 
unsupported in the PropertyMap and need to be handled separately. I would have 
done so if it is possible.
I really do not like to write unnecessary, duplicated code when TagLib handles 
this nicely for us. I've only chosen the rating property to be implemented 
first, but there are more to come. The code will be 100% the same for APE and 
OGG with the PropertyMap, and also more efficient as we query the PropertyMap 
anyway.

REPOSITORY
  R286 KFileMetaData

REVISION DETAIL
  https://phabricator.kde.org/D18601

To: astippich, bruns, mgallien, broulik, cfeck
Cc: kde-frameworks-devel, #baloo, gennad, domson, ashaposhnikov, michaelh, 
astippich, spoorun, ngraham, bruns, abrahams

Reply via email to