Started 2.0 branch

rengels ralf.engels at nokia.com
Mon Jun 29 17:31:45 CEST 2009


...
>
> You don't need it -- my suggestion was that people that have the
> technical know-how (i.e. not necessarily in terms of code but also the
> relevant tag specs) vet the patches and leave comments for Scott
> detailing whether they think the patches are good/valid, whether they
> apply/compile cleanly, and if they've been run through any tests with
> files.
>
> Scott can still commit the patches and close the bugs -- he just doesn't
> have to do all the busywork himself.
>
> --Jeff

Hi,
here my review comments for the bugs in bugs.kde.org

168382
Patch looks fine but instead of 
 if (pos + 1 >= data.size())
I would rather test against
 if (d->mimeType.isNull())
Both ways will work


171957 
Patch is quite long and fixes multiple issues.
Applies without problems but produces three warnings regarding comparison 
between signed and unsigned expressiona.
Also replaces 
-#include <tstringlist.h>
+#include "tstringlist.h"
which I thinks is not OK.

From the functional side it looks fine and as it fixes a corruption issue in Ogg 
this is definitely worth taking.
(probably also fixes 164726)
(includes changes from 167786, 175781, 176373 and 176159)
(Marc, I am so sorry that you invested that much time and are still waiting)


107659
Patch adds padding handling in vorbis files which sounds very sensible.
Patch seems to be OK but does not follow coding style guide.
Still worth taking if some comments are added.
(also fixes 162931)


172556
Even though the patch looks stupid the explanation in the bug report is very 
good. Maybe a short comment should be added to the changed lines so that the 
next hacker doesn't optimize the change away again.


130945
Patch adds in principle handling for mpeg version 2.5 with it's audio rates 
which fixes a bug with detection of mp3 lengths.
Changes seem to be fine, there are even some comments.
On the down side it's not according to coding style guide (the original code 
also isn't) and version 2.5 is not official according to Google.
I still would take this patch.


85134
Long patch and long discussion about replay gain.
I remember that I also submitted something regarding this issue. 
The bug is from 2004 and still open.
However it will break binary compatibility.
I propose to contact the author again when we started the 2.0 branch.

181251
patch was not reviewed as it's inside a bz2.
Sounds simple though.


That seem to be all the errors with patches.
Some errors don't have patches attached but should be investigated.
Some wishes seem to be simple to handle.

What happens now?

BR,
Ralf



More information about the taglib-devel mailing list