> I think we should carefully consider risks and chances for the cover art > support merge.
Marcos and I have taken great care that we get rid off all issues before merging this PR. So far I have test reports from Daniel and Jus, which gave us valuable input and bug reports which have been fixed by now. I have tried to add unit-tests for every bug that we discover, IMHO this is one of the best test covered PR's that we ever had. To be on the extra save side I have asked rryan if we can get release builds of this PR and wrote to the list here to have a mini public beta of that feature before it is merged. So please build the PR and test it. @rryan How are the release builds looking for the PR? > My impression is that the branch has a good overall quality and is a real > benefit for a upcoming 1.12 version. Thank you > What are the points of risks, that makes this different to other pull > requests. > Here a probably incomplete List: > > 1) Reading picture files of poor quality We can't do much about that. But it is possible to set other covers later or rescan the covers once the files have been updated with better quality pictures > 2) Extracting pictures from audio Files using tag-lib We have added unit-tests for all supported music file types. I haven't checked with which image-codec the pictures are saved in the ID3TAGs, if someone has more information about this I can add the appropriate unit-tests we well. Every cover is extracted in a seperate thread. One point of this thread is that I would appriciate it if people test the feature before we merge it and send us files where the cover extraction is not working. > 3) Displaying pictures This shouldn't be too important in this PR. We have 3 very basic ways to display the picture (library column/ widget/ fullsize) and I would like to keep it at that. I would like that the interactions with the covers (changing/resetting/...) and the backend are working reliable. It should be easy to add new cover widgets later. > 4) Writing users library To clarify this. We are saving covers in the library if the file is originally not located in the same directory as the track. The cover is then saved as the first file that does not exists yet, album.jpg/cover.jpg/mixxx-cover.jpg. If the 'mixxx-cover.jpg' already exists it will be overwritten. This is to ensure that the covers are available after a restart. One issue with that might be that these file names are not track specific. So on a complete library rebuild all tracks in that folder will have the same cover again. We also easily save covers with '*trackname-cover.jpg*', the cover-searching algorithm then detects that the cover for this specific track is different from the rest in the folder. > 5) Performance regressions Daniel has tested the branch already with his netbook and we have worked to optimize the performance for him. If we can get windows release builds for the branch I can also test the performance on the windows netbook of my girlfriend. > 6) Threading issues We are using the Qt::Concurrent framework to load covers in parralel. It is easy to use and we can optimally use multi-core CPU's. rryan said something that this might not be optimal for some reasons in the PR but hasn't specified anything so far. I myself haven't noticed any threading issues. > Will re significant reduce these risk, if we have this PR another two years > as developers compile option or it is better to have this a part of a > public beta phase? Having this as a dev option only won't reduce the risk. We still need a larger number of testers then 4-10, which we will achieve with the publib beta. > For me, it sounds that this can be part of 1.12 if we have a solution for > taglib crashes. I haven't noticed any taglib crashes with mixxx. Plus the covers are all extracted in seperate threads. > > Kind regards, > > Daniel > > > > > > > > > > > > > > > > > > > > > > > > > 2014-10-16 14:49 GMT+02:00 Owen Williams <owilli...@mixxx.org>: > > > That model has not worked well in the past. It doesn't matter how many > > warnings there are, people will see "album art!", turn it on, and then > > get angry if/when it crashes. > > > > The new model I'd like to propose is: > > * Enable an unstable-but-desirable feature (like cover art) by default > > in development and nightly builds. > > * Disable it for release builds. > > > > I think this will prevent code rot, which was the biggest problem in the > > past with optional compile-time features. Everyone doing development > > will be using the feature and seeing any bugs it has. But then the > > release is still pristine and won't be compromised. Once the feature is > > stable, we can flip the flag and allow the feature to go to release. > > > > This does require that the compile flag is robust. There have been > > problems in the past with vinyl control, which is enabled for most > > builds but disabled for App Store builds. Things work great in > > development, and then the release build turns out to be broken. Having > > the buildbot should help prevent that -- we should set up a target to > > create release builds and run the tests, but not actually make them > > available for download. > > > > Owen > > > > On Thu, 2014-10-16 at 09:53 +0300, Tuukka Pasanen wrote: > > > Hello, > > > Ok I'm totally with this but I also understand RJ concerns about > > > hitting unwanted behavior. > > > Could this be integrated and only turned on if user makes it with BIG > > > FAT WARNING about > > > it is little bit experimental and polish it to 1.13? > > > > > > Tuukka > > > > > > 2014-10-12 15:13 GMT+03:00 Max Linke <max_li...@gmx.de>: > > > > That is exactly how it works. Currently ew only support local covers. > > We have > > > > some code to also download covers from amazon but I'm not sure if that > > will make > > > > it. > > > > > > > > Here is the algorithm that decides which cover we are loading. > > > > > > > > // Search Strategy > > > > // 0. If we have just one file, we will get it. > > > > // 1. %track-file-base%.jpg in the track directory for > > %track-file-base%.mp3 > > > > // 2. %album%.jpg > > > > // 3. cover.jpg > > > > // 4. front.jpg > > > > // 5. album.jpg > > > > // 6. folder.jpg > > > > // 7. anything else found in the folder (get the smallest one) > > > > > > > > On Sun, 12 Oct 2014 11:22:17 +0300 > > > > Tuukka Pasanen <pasanen.tuu...@gmail.com> wrote: > > > > > > > >> Hello, > > > >> Does this support scenario when you have used something like Beets > > > >> (http://beets.radbox.org/) and allready have cover.jpg (or something) > > > >> in every directory that contains music? It's nice to have somekind of > > > >> downloader but there should be also support people like me who like to > > > >> organize their music in specific way and have cover art in hand.. > > > >> > > > >> Tuukka > > > >> > > > >> > > ------------------------------------------------------------------------------ > > > >> Meet PCI DSS 3.0 Compliance Requirements with EventLog Analyzer > > > >> Achieve PCI DSS 3.0 Compliant Status with Out-of-the-box PCI DSS > > Reports > > > >> Are you Audit-Ready for PCI DSS 3.0 Compliance? Download White paper > > > >> Comply to PCI DSS 3.0 Requirement 10 and 11.5 with EventLog Analyzer > > > >> http://p.sf.net/sfu/Zoho > > > >> _______________________________________________ > > > >> 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 > > > > > > > > > > ------------------------------------------------------------------------------ > > > > Meet PCI DSS 3.0 Compliance Requirements with EventLog Analyzer > > > > Achieve PCI DSS 3.0 Compliant Status with Out-of-the-box PCI DSS > > Reports > > > > Are you Audit-Ready for PCI DSS 3.0 Compliance? Download White paper > > > > Comply to PCI DSS 3.0 Requirement 10 and 11.5 with EventLog Analyzer > > > > http://p.sf.net/sfu/Zoho > > > > _______________________________________________ > > > > 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 > > > > > > > > ------------------------------------------------------------------------------ > > > Comprehensive Server Monitoring with Site24x7. > > > Monitor 10 servers for $9/Month. > > > Get alerted through email, SMS, voice calls or mobile push notifications. > > > Take corrective actions from your mobile device. > > > http://p.sf.net/sfu/Zoho > > > _______________________________________________ > > > 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 > > > > > > > > > > > > > > > > ------------------------------------------------------------------------------ > > Comprehensive Server Monitoring with Site24x7. > > Monitor 10 servers for $9/Month. > > Get alerted through email, SMS, voice calls or mobile push notifications. > > Take corrective actions from your mobile device. > > http://p.sf.net/sfu/Zoho > > _______________________________________________ > > 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 > > ------------------------------------------------------------------------------ Comprehensive Server Monitoring with Site24x7. Monitor 10 servers for $9/Month. Get alerted through email, SMS, voice calls or mobile push notifications. Take corrective actions from your mobile device. http://p.sf.net/sfu/Zoho _______________________________________________ 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
