I would be happy to help review a PR that just fixes the mp3 click issue. On Sat, 2015-03-21 at 03:26 +0100, Daniel Schürmann wrote: > Hi, > > For me the mp3 seek an click issue is a very strong, one that makes > the difference between a toy and a tool. > This is much more notable in Mixxx 1.12 alpha compared to Mixxx 1.11 > because of the new syncing code using a lot more seeks. > > I share your concerns that merging the PR will introduce new issues, > but I do not share that we loose reputation by that, because > thats the essence of a beta phase. > This branch is already reviewed and tested and is used by me and Uwe > without issues. > > Would you mind to fire your collections of faulty files again the > branch. This may help to get a better view on this topic. > I wonder if this is more robust against faulty files than the current > master, because Uwe has fixed some other issues > as well. > > (By the way, this branch introduces also the floating point API, a > great benefit for lossy codecs and a highlight of the "whats new" > list. ) > > Kind regards, > > Daniel > > > > Am 21.03.2015 um 02:38 schrieb RJ Ryan: > > > Hey everyone, > > > > > > Uwe and Daniel's work is awesome and much needed. SoundSource is the > > dark corner of Mixxx that is on average where our lowest-quality > > code lives. It hasn't been touched significantly in years and it > > will be great to fix the long standing bugs associated with it. The > > flip side of not having been touched much in years is that we have 5 > > + years worth of evidence that there are no serious decoding (i.e. > > Critical priority) bugs in SoundSource code. > > > > > > I'm with Owen on this one. The benefits of merging this branch pale > > in comparison to the risks. This release will be our biggest in > > years and it will make headlines. This is not the time for an epic > > refactor to fix some -- frankly minor -- decoding glitches. It is > > not possible to get sufficient testing of this code on the diverse > > libraries that exist in the wild in the beta period for this > > release. There will surely be crash bugs as there are with any > > refactor of complicated code (touches 5+ libraries) that deals with > > corrupt and untrusted input (user audio files). Merging this now > > will both delay the release with bugs we find during the beta period > > and hurt Mixxx's reputation (at a critical time for gaining users) > > with the crash bugs that we don't find during the beta period. > > > > > > Finally, I don't believe Uwe has been operating under the impression > > that this refactor would be part of this release. The PR milestone > > is clearly marked 1.13 and IIRC we discussed it explicitly when he > > was working on it. This was why he spit off multiple pieces of it > > into smaller refactors that have merged already. > > > > > > Cheers, > > RJ > > > > > > > > > > > > On Fri, Mar 20, 2015 at 6:48 PM, Owen Williams <owilli...@mixxx.org> > > wrote: > > I simply disagree that we have to apply the whole PR to get > > one bug fix. > > If there are are other similar issues there should be bugs > > filed for > > them so we can judge their severity. There is no reason to > > rush this > > patch in. I am not saying we will never merge this PR -- > > the audio > > subsystem does need help, and I really appreciate the work. > > > > The beta will have bugs. The release will have bugs. But I > > want to > > *have* a release, and if we merge this giant patch now that > > will set the > > release back by *months*. > > > > I don't feel any responsibility to find specific bugs with > > this PR or > > "prove" where there are hot spots. The risk is too high. > > > > owen > > > > On Fri, 2015-03-20 at 22:39 +0100, Daniel Schürmann wrote: > > > I have really reviewed every single line and it took me > > quite long, (see > > > the discussion at GitHub) > > > Yes it is really big, but it has received the same > > attention as every > > > small patch got, devided by the number of lines. > > > > > > Can you identify a hotspot of risky changes in the PR? > > Maybe we can > > > exude just this. > > > > > > Th PR Fixes a lot of issues in every sound-source. We have > > just the bug > > > reports for mp3, but Uwe and me have identified > > > a lot of more similar issues in the other sound-sources. > > > It is a good idea to have them all fixed in the upcomming > > release. > > > > > > > > > Am 20.03.2015 um 22:24 schrieb Owen Williams: > > > > I'm sorry but I can't just take your word for it that > > this 7000 line > > > > patch is safe to merge. I don't even trust myself to > > review it, it's > > > > too big. If it's the day after a big release then we > > can risk the > > > > breakage, but not right now. My experience has been > > that these reworks > > > > always introduce new problems. > > > > > > > > If there's a way to just fix the mp3 issues, let's do > > that. Backporting > > > > is annoying, but it's a pretty standard procedure on a > > major project. > > > > > > > > On Fri, 2015-03-20 at 22:15 +0100, Daniel Schürmann > > wrote: > > > >> Sure it is possible, but it feels stupid to do that. > > > >> Taking effort to exclude useful changes from a patch > > that is already > > > >> reviewed and from a high quality. > > > >> The result will be un-reviewed code introduced into a > > sound source > > > >> environment from a poor quality, > > > >> without additional tests. > > > >> > > > >> > > > >> > > > >> Am 20.03.2015 um 18:30 schrieb Owen Williams: > > > >>> How hard would it be to backport the mp3 fix to the > > current > > > >>> architecture? I would much prefer that. > > > >>> > > > >>> On Fri, 2015-03-20 at 17:18 +0100, Daniel Schürmann > > wrote: > > > >>>> Here are the bug: > > > >>>> https://bugs.launchpad.net/mixxx/+bug/1407394 > > > >>>> https://bugs.launchpad.net/mixxx/+bug/1281654 > > > >>>> > > > >>>> Uwe and I made significant progress with this beast: > > > >>>> https://github.com/mixxxdj/mixxx/pull/411 > > > >>>> It was continuously improved and testes since 30 Nov > > 2014 > > > >>>> Some important QM tasks are already finished: > > > >>>> * It is reviewed by a second pair of eyes. > > > >>>> * It has Unit-tests for the most difficult parts > > > >>>> > > > >>>> What is missing: > > > >>>> a) Fire ugly files against it > > > >>>> b) test in the wild bay a broad amount of users > > > >>>> > > > >>>> a) can't take long and b) is ideal done in a beat > > phase, we will never > > > >>>> have such an attention for the next two years. > > > >>>> > > > >>>> For me this advantages have an higher value than a > > diffuse change of > > > >>>> introducing new bugs. > > > >>>> And yes new code introduces always new bugs (not a > > big problem in a > > > >>>> beta) > > > >>>> > > > >>>> > > > >>>> > > > >>>> Am 20.03.2015 um 15:49 schrieb Owen Williams: > > > >>>>> I still need a pointer to what the mp3 bug even is. > > > >>>>> > > > >>>>> On Fri, 2015-03-20 at 07:43 -0700, Gavin Swanson > > wrote: > > > >>>>>> I exclusively use mp3s with mixxx. I would imagine > > there is a large > > > >>>>>> portion of the mixxx user base that does. > > > >>>>>> I do not agree with pushing a beta with this kind > > of issue, if the > > > >>>>>> intent is not to fix it for this release. If the > > intent is to get the > > > >>>>>> beta with all its other features out, and then work > > this issue through > > > >>>>>> the beta cycle so it is fixed for release, I could > > get behind that. > > > >>>>>> At the same time if the fix is a major-rework of > > the base of mixxx > > > >>>>>> then it doesn't make sense to do it during a beta > > cycle and we should > > > >>>>>> push beta until its ready. If it's a 1000 line fix > > with 6000 lines of > > > >>>>>> unit tests (unlikely), that's a different story. > > > >>>>>> > > > >>>>>> > > > >>>>>> my ¢2 > > > >>>>>> > > > >>>>>> > > > >>>>>> -Gavin S > > > >>>>>> > > > >>>>>> Gavin S > > > >>>>>> > > > >>>>>> On Fri, Mar 20, 2015 at 7:02 AM, Owen Williams > > <owilli...@mixxx.org> > > > >>>>>> wrote: > > > >>>>>> I can't in good conscience approve of > > merging a 7000-line > > > >>>>>> delta change > > > >>>>>> to the bedrock of the Mixxx engine, even if > > it fixes an issue > > > >>>>>> with mp3 > > > >>>>>> seeking. (I also hate how mp3s skip on the > > first beat, so I > > > >>>>>> can > > > >>>>>> sympathize). I understand there are new > > tests, and I'm really > > > >>>>>> thankful > > > >>>>>> for those. But it's going to take a long > > time to go through > > > >>>>>> that patch > > > >>>>>> and then test to find all the issues with > > bad tags, weird > > > >>>>>> files and > > > >>>>>> formats, etc etc etc. I think RJ has a > > collection of horrible > > > >>>>>> mp3s that > > > >>>>>> he'll want to test. > > > >>>>>> > > > >>>>>> To me, sitting two years after the last > > release, I can't > > > >>>>>> justify the > > > >>>>>> delay that this merge will inevitably cause > > -- I think we're > > > >>>>>> ready for a > > > >>>>>> beta and can tolerate the mp3 issues we > > have. (by the way, is > > > >>>>>> there a > > > >>>>>> bug for the mp3 issue you're talking about? > > I can't find it.) > > > >>>>>> > > > >>>>>> owen > > > >>>>>> > > > >>>>>> > > > >>>>>> > > > >>>>>> On Fri, 2015-03-20 at 09:03 +0100, Daniel > > Schürmann wrote: > > > >>>>>> > The only remaining blocker for me is the > > seek offset issue > > > >>>>>> for mp3s. > > > >>>>>> > > > > >>>>>> > > > > >>>>>> > Since we have a fully reviewed solution > > in > > > >>>>>> > https://github.com/mixxxdj/mixxx/pull/411 > > of really high > > > >>>>>> code quality, > > > >>>>>> > verified by a new set of unittests, > > > >>>>>> > > > > >>>>>> > we should just merge it and release the > > beta. > > > >>>>>> > > > > >>>>>> > And yes there is a risk of introducing > > issues and it surely > > > >>>>>> will, but > > > >>>>>> > by now it fixes much more issues than it > > might introduce. > > > >>>>>> > > > > >>>>>> > > > > >>>>>> > Of cause I would be happy about any > > pending PR that can be > > > >>>>>> review and > > > >>>>>> > merged before. > > > >>>>>> > > > > >>>>>> > > > > >>>>>> > 2015-03-20 0:35 GMT+01:00 Owen Williams > > > >>>>>> <owilli...@mixxx.org>: > > > >>>>>> > So how about that release? Do we > > still have > > > >>>>>> crashing problems > > > >>>>>> > on > > > >>>>>> > windows? is anyone looking in to > > that? Any other > > > >>>>>> blockers? > > > >>>>>> > > > > >>>>>> > owen > > > >>>>>> > > > > >>>>>> > > > > >>>>>> > > > > >>>>>> > > > > ------------------------------------------------------------------------------ > > > >>>>>> > Dive into the World of Parallel > > Programming The Go > > > >>>>>> Parallel > > > >>>>>> > Website, sponsored > > > >>>>>> > by Intel and developed in > > partnership with Slashdot > > > >>>>>> Media, is > > > >>>>>> > your hub for all > > > >>>>>> > things parallel software > > development, from weekly > > > >>>>>> thought > > > >>>>>> > leadership blogs to > > > >>>>>> > news, videos, case studies, > > tutorials and more. Take > > > >>>>>> a look > > > >>>>>> > and join the > > > >>>>>> > conversation now. > > http://goparallel.sourceforge.net/ > > > >>>>>> > > > _______________________________________________ > > > >>>>>> > Get Mixxx, the #1 Free MP3 DJ > > Mixing software Today > > > >>>>>> > http://mixxx.org > > > >>>>>> > > > > >>>>>> > > > > >>>>>> > Mixxx-devel mailing list > > > >>>>>> > Mixxx-devel@lists.sourceforge.net > > > >>>>>> > > > > >>>>>> > > https://lists.sourceforge.net/lists/listinfo/mixxx-devel > > > >>>>>> > > > > >>>>>> > > > > >>>>>> > > > >>>>>> > > > >>>>>> > > > >>>>>> > > > > ------------------------------------------------------------------------------ > > > >>>>>> Dive into the World of Parallel Programming > > The Go Parallel > > > >>>>>> Website, sponsored > > > >>>>>> by Intel and developed in partnership with > > Slashdot Media, is > > > >>>>>> your hub for all > > > >>>>>> things parallel software development, from > > weekly thought > > > >>>>>> leadership blogs to > > > >>>>>> news, videos, case studies, tutorials and > > more. Take a look > > > >>>>>> and join the > > > >>>>>> conversation now. > > http://goparallel.sourceforge.net/ > > > >>>>>> > > _______________________________________________ > > > >>>>>> Get Mixxx, the #1 Free MP3 DJ Mixing > > software Today > > > >>>>>> http://mixxx.org > > > >>>>>> > > > >>>>>> > > > >>>>>> Mixxx-devel mailing list > > > >>>>>> Mixxx-devel@lists.sourceforge.net > > > >>>>>> > > https://lists.sourceforge.net/lists/listinfo/mixxx-devel > > > >>>>>> > > > >>>>>> > > > >>>>>> > > > > ------------------------------------------------------------------------------ > > > >>>>>> Dive into the World of Parallel Programming The Go > > Parallel Website, sponsored > > > >>>>>> by Intel and developed in partnership with Slashdot > > Media, is your hub for all > > > >>>>>> things parallel software development, from weekly > > thought leadership blogs to > > > >>>>>> news, videos, case studies, tutorials and more. > > Take a look and join the > > > >>>>>> conversation now. > > http://goparallel.sourceforge.net/ > > > >>>>>> _______________________________________________ > > > >>>>>> Get Mixxx, the #1 Free MP3 DJ Mixing software Today > > > >>>>>> http://mixxx.org > > > >>>>>> > > > >>>>>> > > > >>>>>> Mixxx-devel mailing list > > > >>>>>> Mixxx-devel@lists.sourceforge.net > > > >>>>>> > > https://lists.sourceforge.net/lists/listinfo/mixxx-devel > > > >>>>> > > > > ------------------------------------------------------------------------------ > > > >>>>> Dive into the World of Parallel Programming The Go > > Parallel Website, sponsored > > > >>>>> by Intel and developed in partnership with Slashdot > > Media, is your hub for all > > > >>>>> things parallel software development, from weekly > > thought leadership blogs to > > > >>>>> news, videos, case studies, tutorials and more. Take > > a look and join the > > > >>>>> conversation now. http://goparallel.sourceforge.net/ > > > >>>>> _______________________________________________ > > > >>>>> Get Mixxx, the #1 Free MP3 DJ Mixing software Today > > > >>>>> http://mixxx.org > > > >>>>> > > > >>>>> > > > >>>>> Mixxx-devel mailing list > > > >>>>> Mixxx-devel@lists.sourceforge.net > > > >>>>> > > https://lists.sourceforge.net/lists/listinfo/mixxx-devel > > > >>>> > > > > ------------------------------------------------------------------------------ > > > >>>> Dive into the World of Parallel Programming The Go > > Parallel Website, sponsored > > > >>>> by Intel and developed in partnership with Slashdot > > Media, is your hub for all > > > >>>> things parallel software development, from weekly > > thought leadership blogs to > > > >>>> news, videos, case studies, tutorials and more. Take > > a look and join the > > > >>>> conversation now. http://goparallel.sourceforge.net/ > > > >>>> _______________________________________________ > > > >>>> Get Mixxx, the #1 Free MP3 DJ Mixing software Today > > > >>>> http://mixxx.org > > > >>>> > > > >>>> > > > >>>> Mixxx-devel mailing list > > > >>>> Mixxx-devel@lists.sourceforge.net > > > >>>> > > https://lists.sourceforge.net/lists/listinfo/mixxx-devel > > > >> > > > >> > > > > > > > > > > > > > > > > > > > > > > > ------------------------------------------------------------------------------ > > Dive into the World of Parallel Programming The Go Parallel > > Website, sponsored > > by Intel and developed in partnership with Slashdot Media, > > is your hub for all > > things parallel software development, from weekly thought > > leadership blogs to > > news, videos, case studies, tutorials and more. Take a look > > and join the > > conversation now. http://goparallel.sourceforge.net/ > > _______________________________________________ > > Get Mixxx, the #1 Free MP3 DJ Mixing software Today > > http://mixxx.org > > > > > > Mixxx-devel mailing list > > Mixxx-devel@lists.sourceforge.net > > https://lists.sourceforge.net/lists/listinfo/mixxx-devel > > > > >
------------------------------------------------------------------------------ Dive into the World of Parallel Programming The Go Parallel Website, sponsored by Intel and developed in partnership with Slashdot Media, is your hub for all things parallel software development, from weekly thought leadership blogs to news, videos, case studies, tutorials and more. Take a look and join the conversation now. http://goparallel.sourceforge.net/ _______________________________________________ Get Mixxx, the #1 Free MP3 DJ Mixing software Today http://mixxx.org Mixxx-devel mailing list Mixxx-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mixxx-devel
