<table><tr><td style="">bruns added inline comments.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D18601">View Revision</a></tr></table><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D18601#inline-109940">View Inline</a><span style="color: #4b4d51; font-weight: bold;">astippich</span> wrote in <span style="color: #4b4d51; font-weight: bold;">taglibwriter.cpp:70</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><p style="padding: 0; margin: 8px;">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.</p></blockquote>

<p style="padding: 0; margin: 8px;">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.</p>

<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><p style="padding: 0; margin: 8px;">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.</p></blockquote>

<p style="padding: 0; margin: 8px;">Xiph explicitly allows multiple entries per key, which need to be removed when writing.</p>

<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><p style="padding: 0; margin: 8px;">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.</p></blockquote>

<p style="padding: 0; margin: 8px;">TagLib automatically translates different keys from APE to "common names", e.g. DISC->DISCNUMBER etc.</p>

<p style="padding: 0; margin: 8px;">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).</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">By using the type specific function you signal you are aware of the differences between the two, and supply the appropriate data.</p>

<p style="padding: 0; margin: 8px;">RATING is <strong>not handled</strong> by the properties interface, it just works by coincidence, not by design.</p>

<p style="padding: 0; margin: 8px;">In case Taglib properly handles a tag, I am not against using it, as said several times. This is the case for DISC, but not for RATING.</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R286 KFileMetaData</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D18601">https://phabricator.kde.org/D18601</a></div></div><br /><div><strong>To: </strong>astippich, bruns, mgallien, broulik, cfeck<br /><strong>Cc: </strong>kde-frameworks-devel, Baloo, gennad, domson, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams<br /></div>