Re: Review Request 127468: Fix for the infinite loop in case a home-burned or old audio CD is inserted
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127468/ --- (Updated April 7, 2016, 12:23 p.m.) Status -- This change has been marked as submitted. Review request for Amarok. Changes --- Submitted with commit aaff3348862a1999069feff93d9e1e4d995b7225 by Heiko Becker on behalf of Stefano Pettini to branch master. Bugs: 339190 https://bugs.kde.org/show_bug.cgi?id=339190 Repository: amarok Description --- Home-burned or old audio CDs usually don't have CDTEXT, that is what was triggering the bug. The bug was releted to poor management of track names, that are generated using the pattern "Track%1.wav". The audiocd:/ KIO protocol requires that a device is also added to the URL, and this was not done everywhere. Lack of device was triggering an unexpected/strange behaviour in some KDE function making Amarok enter an infinite loop. Diffs - src/core-impl/collections/audiocd/AudioCdCollection.h dc2cad7 src/core-impl/collections/audiocd/AudioCdCollection.cpp 3dfa7c3 Diff: https://git.reviewboard.kde.org/r/127468/diff/ Testing --- First I reproduced the bug with an original CD 100% of the times. It was triggered in case the CD didn't have any CDTEXT. Only in that case the noInfoAvailable function was invoked. After fixing it, I verified that it works as expected both with CDs with CDTEXT and CDs without it. The "Copy to collection" feature is also tested with CDs with and without CDTEXT and multiple encodings (ogg, flac, wav). Before it was not working, now it does. Thanks, Stefano Pettini
Re: Review Request 127468: Fix for the infinite loop in case a home-burned or old audio CD is inserted
> On April 1, 2016, 7:08 p.m., Heiko Becker wrote: > > Fixes the bug as advertised. Do you have commit access or should I push > > this in your name? I don't have commit rights. - Stefano --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127468/#review94192 --- On March 23, 2016, 9:32 p.m., Stefano Pettini wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127468/ > --- > > (Updated March 23, 2016, 9:32 p.m.) > > > Review request for Amarok. > > > Bugs: 339190 > https://bugs.kde.org/show_bug.cgi?id=339190 > > > Repository: amarok > > > Description > --- > > Home-burned or old audio CDs usually don't have CDTEXT, that is what was > triggering the bug. > The bug was releted to poor management of track names, that are generated > using the pattern "Track%1.wav". > The audiocd:/ KIO protocol requires that a device is also added to the URL, > and this was not done everywhere. > Lack of device was triggering an unexpected/strange behaviour in some KDE > function making Amarok enter an infinite loop. > > > Diffs > - > > src/core-impl/collections/audiocd/AudioCdCollection.h dc2cad7 > src/core-impl/collections/audiocd/AudioCdCollection.cpp 3dfa7c3 > > Diff: https://git.reviewboard.kde.org/r/127468/diff/ > > > Testing > --- > > First I reproduced the bug with an original CD 100% of the times. It was > triggered in case the CD didn't have any CDTEXT. Only in that case the > noInfoAvailable function was invoked. > After fixing it, I verified that it works as expected both with CDs with > CDTEXT and CDs without it. > > The "Copy to collection" feature is also tested with CDs with and without > CDTEXT and multiple encodings (ogg, flac, wav). Before it was not working, > now it does. > > > Thanks, > > Stefano Pettini > >
Re: Review Request 127468: Fix for the infinite loop in case a home-burned or old audio CD is inserted
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127468/#review94192 --- Ship it! Fixes the bug as advertised. Do you have commit access or should I push this in your name? - Heiko Becker On März 23, 2016, 8:32 nachm., Stefano Pettini wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127468/ > --- > > (Updated März 23, 2016, 8:32 nachm.) > > > Review request for Amarok. > > > Bugs: 339190 > https://bugs.kde.org/show_bug.cgi?id=339190 > > > Repository: amarok > > > Description > --- > > Home-burned or old audio CDs usually don't have CDTEXT, that is what was > triggering the bug. > The bug was releted to poor management of track names, that are generated > using the pattern "Track%1.wav". > The audiocd:/ KIO protocol requires that a device is also added to the URL, > and this was not done everywhere. > Lack of device was triggering an unexpected/strange behaviour in some KDE > function making Amarok enter an infinite loop. > > > Diffs > - > > src/core-impl/collections/audiocd/AudioCdCollection.h dc2cad7 > src/core-impl/collections/audiocd/AudioCdCollection.cpp 3dfa7c3 > > Diff: https://git.reviewboard.kde.org/r/127468/diff/ > > > Testing > --- > > First I reproduced the bug with an original CD 100% of the times. It was > triggered in case the CD didn't have any CDTEXT. Only in that case the > noInfoAvailable function was invoked. > After fixing it, I verified that it works as expected both with CDs with > CDTEXT and CDs without it. > > The "Copy to collection" feature is also tested with CDs with and without > CDTEXT and multiple encodings (ogg, flac, wav). Before it was not working, > now it does. > > > Thanks, > > Stefano Pettini > >
Re: Review Request 127468: Fix for the infinite loop in case a home-burned or old audio CD is inserted
> On March 23, 2016, 3:15 p.m., Myriam Schweingruber wrote: > > Ship It! > > Stefano Pettini wrote: > Please wait, let me do some more tests this evening What about reviewing and submitting this patch? - Stefano --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127468/#review93899 --- On March 23, 2016, 9:32 p.m., Stefano Pettini wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127468/ > --- > > (Updated March 23, 2016, 9:32 p.m.) > > > Review request for Amarok. > > > Bugs: 339190 > https://bugs.kde.org/show_bug.cgi?id=339190 > > > Repository: amarok > > > Description > --- > > Home-burned or old audio CDs usually don't have CDTEXT, that is what was > triggering the bug. > The bug was releted to poor management of track names, that are generated > using the pattern "Track%1.wav". > The audiocd:/ KIO protocol requires that a device is also added to the URL, > and this was not done everywhere. > Lack of device was triggering an unexpected/strange behaviour in some KDE > function making Amarok enter an infinite loop. > > > Diffs > - > > src/core-impl/collections/audiocd/AudioCdCollection.h dc2cad7 > src/core-impl/collections/audiocd/AudioCdCollection.cpp 3dfa7c3 > > Diff: https://git.reviewboard.kde.org/r/127468/diff/ > > > Testing > --- > > First I reproduced the bug with an original CD 100% of the times. It was > triggered in case the CD didn't have any CDTEXT. Only in that case the > noInfoAvailable function was invoked. > After fixing it, I verified that it works as expected both with CDs with > CDTEXT and CDs without it. > > The "Copy to collection" feature is also tested with CDs with and without > CDTEXT and multiple encodings (ogg, flac, wav). Before it was not working, > now it does. > > > Thanks, > > Stefano Pettini > >
Re: Review Request 127468: Fix for the infinite loop in case a home-burned or old audio CD is inserted
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127468/ --- (Updated March 23, 2016, 9:32 p.m.) Review request for Amarok. Bugs: 339190 https://bugs.kde.org/show_bug.cgi?id=339190 Repository: amarok Description --- Home-burned or old audio CDs usually don't have CDTEXT, that is what was triggering the bug. The bug was releted to poor management of track names, that are generated using the pattern "Track%1.wav". The audiocd:/ KIO protocol requires that a device is also added to the URL, and this was not done everywhere. Lack of device was triggering an unexpected/strange behaviour in some KDE function making Amarok enter an infinite loop. Diffs - src/core-impl/collections/audiocd/AudioCdCollection.h dc2cad7 src/core-impl/collections/audiocd/AudioCdCollection.cpp 3dfa7c3 Diff: https://git.reviewboard.kde.org/r/127468/diff/ Testing (updated) --- First I reproduced the bug with an original CD 100% of the times. It was triggered in case the CD didn't have any CDTEXT. Only in that case the noInfoAvailable function was invoked. After fixing it, I verified that it works as expected both with CDs with CDTEXT and CDs without it. The "Copy to collection" feature is also tested with CDs with and without CDTEXT and multiple encodings (ogg, flac, wav). Before it was not working, now it does. Thanks, Stefano Pettini
Re: Review Request 127468: Fix for the infinite loop in case a home-burned or old audio CD is inserted
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127468/ --- (Updated March 23, 2016, 9:27 p.m.) Review request for Amarok. Changes --- This improves the previous patch fixing the "copy to collection" feature and by using prettier names for tracks Bugs: 339190 https://bugs.kde.org/show_bug.cgi?id=339190 Repository: amarok Description --- Home-burned or old audio CDs usually don't have CDTEXT, that is what was triggering the bug. The bug was releted to poor management of track names, that are generated using the pattern "Track%1.wav". The audiocd:/ KIO protocol requires that a device is also added to the URL, and this was not done everywhere. Lack of device was triggering an unexpected/strange behaviour in some KDE function making Amarok enter an infinite loop. Diffs (updated) - src/core-impl/collections/audiocd/AudioCdCollection.h dc2cad7 src/core-impl/collections/audiocd/AudioCdCollection.cpp 3dfa7c3 Diff: https://git.reviewboard.kde.org/r/127468/diff/ Testing --- First I reproduced the bug with an original CD 100% of the times. It was triggered in case the CD didn't have any CDTEXT. Only in that case the noInfoAvailable function was invoked. After fixing it, I verified that it works as expected both with CDs with CDTEXT and CDs without it. Thanks, Stefano Pettini
Re: Review Request 127468: Fix for the infinite loop in case a home-burned or old audio CD is inserted
> On Mar. 23, 2016, 3:15 p.m., Myriam Schweingruber wrote: > > Ship It! Please wait, let me do some more tests this evening - Stefano --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127468/#review93899 --- On Mar. 22, 2016, 10:59 p.m., Stefano Pettini wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127468/ > --- > > (Updated Mar. 22, 2016, 10:59 p.m.) > > > Review request for Amarok. > > > Bugs: 339190 > https://bugs.kde.org/show_bug.cgi?id=339190 > > > Repository: amarok > > > Description > --- > > Home-burned or old audio CDs usually don't have CDTEXT, that is what was > triggering the bug. > The bug was releted to poor management of track names, that are generated > using the pattern "Track%1.wav". > The audiocd:/ KIO protocol requires that a device is also added to the URL, > and this was not done everywhere. > Lack of device was triggering an unexpected/strange behaviour in some KDE > function making Amarok enter an infinite loop. > > > Diffs > - > > src/core-impl/collections/audiocd/AudioCdCollection.h dc2cad7 > src/core-impl/collections/audiocd/AudioCdCollection.cpp 3dfa7c3 > > Diff: https://git.reviewboard.kde.org/r/127468/diff/ > > > Testing > --- > > First I reproduced the bug with an original CD 100% of the times. It was > triggered in case the CD didn't have any CDTEXT. Only in that case the > noInfoAvailable function was invoked. > After fixing it, I verified that it works as expected both with CDs with > CDTEXT and CDs without it. > > > Thanks, > > Stefano Pettini > >
Re: Review Request 127468: Fix for the infinite loop in case a home-burned or old audio CD is inserted
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127468/#review93899 --- Ship it! Ship It! - Myriam Schweingruber On March 22, 2016, 10:59 p.m., Stefano Pettini wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127468/ > --- > > (Updated March 22, 2016, 10:59 p.m.) > > > Review request for Amarok. > > > Bugs: 339190 > https://bugs.kde.org/show_bug.cgi?id=339190 > > > Repository: amarok > > > Description > --- > > Home-burned or old audio CDs usually don't have CDTEXT, that is what was > triggering the bug. > The bug was releted to poor management of track names, that are generated > using the pattern "Track%1.wav". > The audiocd:/ KIO protocol requires that a device is also added to the URL, > and this was not done everywhere. > Lack of device was triggering an unexpected/strange behaviour in some KDE > function making Amarok enter an infinite loop. > > > Diffs > - > > src/core-impl/collections/audiocd/AudioCdCollection.h dc2cad7 > src/core-impl/collections/audiocd/AudioCdCollection.cpp 3dfa7c3 > > Diff: https://git.reviewboard.kde.org/r/127468/diff/ > > > Testing > --- > > First I reproduced the bug with an original CD 100% of the times. It was > triggered in case the CD didn't have any CDTEXT. Only in that case the > noInfoAvailable function was invoked. > After fixing it, I verified that it works as expected both with CDs with > CDTEXT and CDs without it. > > > Thanks, > > Stefano Pettini > >
