astippich added inline comments. INLINE COMMENTS
> bruns wrote in taglibwriter.cpp:70 > Taglib does not "handle this nicely". For APE and Xiph, it just accepts *any* > unknown key and uses it verbatim, while for MP4 and ASF it rejects any > unknown key. > > The setProperties() is also quite inconsistent, for APE and ASF it only > removes items which have an empty value, while for Xiph, the properties are > completely replaced. > > As soon as you add support for a property where APE and Xiph key naming > differs, or is only supported by one, you will require two functions anyway. > > Using `APE::Tag::setItem(...)` is as efficient as manipulating the key/value > in the Taglib::PropertyMap first and setting it by `setProperties(...)`. > Likewise for Xiph. > > If you want to squeeze out the last bit of efficiency, you would skip the > `setProperties(...)` completely when no property is changed by > `writeGenericProperties(....)`. This happens e.g. if you only change the > rating. > > If you want to avoid duplicate code, move the > `properties()`/`setProperties()` into `writeGenericProperties()`, that saves > 12*2 lines and adds 2. > Taglib does not "handle this nicely". For APE and Xiph, it just accepts *any* > unknown key and uses it verbatim, while for MP4 and ASF it rejects any > unknown key. And so do APE::Tag::setItem and OGG::XiphComment::addField ...? btw, also perfectly legal, APE and Xiph allow writing arbitrary tags, while the others do not. > The setProperties() is also quite inconsistent, for APE and ASF it only > removes items which have an empty value, while for Xiph, the properties are > completely replaced. Xiph explicitly allows multiple entries per key, which need to be removed when writing. > As soon as you add support for a property where APE and Xiph key naming > differs, or is only supported by one, you will require two functions anyway. TagLib automatically translates different keys from APE to "common names", e.g. DISC->DISCNUMBER etc. I would really like to hand off manual tag handling to TagLib as much as possible. The library solely responsible for reading tags usually knows better how to handle the tags than we do (with a few exceptions to the rule of course). 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
