Applied to trunk in r2397, good catch Tobias! Thanks, Albert
On Mon, May 17, 2010 at 2:25 PM, Tobias Rafreider <[email protected]> wrote: > Hi all, > > attached you find a patch that makes shoutcast almost stable along with > lame. > Albert and I were on the wrong way, threading issues are not the problem -- > in my opinion. > The patch is against current revision r2395 > > The big fault was in encodermp3.cpp where the field m_bufferInSize hasn't > been initalized to 0. > At runtime the Integer field was inialized by a large random value on my > system > such that the following pointer fields were never initialized in the > methods 'bufferOutGrow()' and 'bufferInGrow()' (Valgrind showed invalid > writes) > =====SUFFICIENT MEMORY HAS NEVER BEEN ALLOCATED FOR======= > * m_bufferOut = (unsigned char *)realloc(m_bufferOut, size); > * m_bufferIn[0] = (float *)realloc(m_bufferIn[0], size * sizeof(float)); > * m_bufferIn[1] = (float *)realloc(m_bufferIn[1], size * sizeof(float)); > ============================================================ > Initalizing m_bufferInSize has solved many segfaults when using and even > closing shoutcast along with LAME. Now it should be possible to connect to > shoutcast. Disabling the connection by closing Mixxx or using the > preferences should not segfault anymore. > > It would be awesome if some other guys could test it. Any comments are > welcome! > > > Thanks, > > Tobias > > > Am 16.05.2010 23:15, schrieb Albert Santoni: >> >> Hi Tobias, >> >> On Sat, 2010-05-15 at 20:36 +0200, Tobias Rafreider wrote: >> >>> >>> Hi all, >>> >>> I have spend a lot time this weekend on debugging shoutcast. I could >>> eliminate a potential segfault when using OGG streams. This, however, >>> was only a tiny step towards a stable feature. >>> >>> ==================CHANGE LINE 175 in ENCODERVORBIS.CPP to============= >>> >>> if (metaDataHasChanged()&& m_pMetaData != NULL) >>> updateMetaData(m_pMetaData); >>> >>> ================= END================================================ >>> >> >> Thanks for the patch, I've committed this safety check to trunk. >> >> >>> >>> There are at least two other segfaults. I observed that lame_close(..) >>> causes quite often a segfault, although lame was initialized properly. I >>> have no glue why this happens -- threading issues???? In otherwords, >>> Mixxx crashes if you close an established connection. >>> >> >> Yeah, probably threading issues. We'll need to make sure the relevant >> code is checked for thread safety (encodervorbis.cpp, encodermp3.cpp, >> engineshoutcast.cpp, libshout, lame). >> >> >>> >>> In many cases a connection can be established, however, loading a track >>> may lead to segfaults as well, especially when using lame. Again, no >>> glue why. I sometimes disabled method calls to metaDataHasChanged and >>> updateMetaData to exclude NULLs from m_pConfig->getValueString().data() >>> >>> Since I assume threading issues behind the problems, I need more input >>> how EngineSideChain works. For instance, is it possible that >>> >>> if (prefEnabled != shoutcastEnabled) { >>> if (prefEnabled) { >>> shoutcast = new EngineShoutcast(m_pConfig); >>> } else { >>> delete shoutcast; >>> shoutcast = NULL; >>> } >>> } >>> >>> in EgineSieChain.cpp causes some of the problems due to missing locks? >>> >>> >> >> I've looked over EngineSidechain and aside from the one fix I made >> earlier today (shoutcast shutdown problem), I don't see any threading >> problems. The only part of that class that needs locks is the buffer >> swapping and buffer handling code. >> >> Thanks, >> Albert >> >> >> >>> >>> Thanks for your attention! >>> >>> Tobias >>> >>> >>> >>> Am 08.05.2010 13:59, schrieb Sean M. Pappalardo - D.J. Pegasus: >>> >>>> >>>> Hello everyone. >>>> >>>> Since it seems Shoutcast/Icecast support is close to being ready for >>>> testing and has been sitting for months in that state, and we have LOTS >>>> of requests for it, I want to see it done for 1.8.x. >>>> >>>> To that end, I looked at the TODO, >>>> http://mixxx.org/wiki/doku.php/1.8.0_to_do_list#shoutcast and focused on >>>> the crashing issue since that seems to be what's holding this up. So I >>>> went into #icecast and a very nice ph3-der-loewe (who is CCed here) took >>>> a look at the engineshoutcast.cpp code in trunk and gave me some >>>> pointers: >>>> >>>> <ph3-der-loewe> m_pConfig->getValueString().data() can return >>>> NULL? >>>> <Pegasus_RPG> Apparently >>>> http://doc.trolltech.com/4.6/qstring.html#data >>>> and http://doc.trolltech.com/4.6/qbytearray.html#data >>>> <ph3-der-loewe> it if returns NULL you should skip the >>>> corresponding >>>> shout_set_*() >>>> <ph3-der-loewe> in case of username it is important that you >>>> default to >>>> 'source' >>>> <ph3-der-loewe> maybe add some printf()s or what ever debug >>>> interface >>>> you use within Mixxx and have a look if there are some NULLs and if the >>>> username is set up correctly. >>>> >>>> Since I know nothing about Shoutcast and have never used it, I'm >>>> thinking Madjester or someone else familiar with testing it should try >>>> these changes and see if they fix the crashing issue. (Or tell me how I >>>> can test it.) >>>> >>>> Thanks for your attention and cooperation. This is one more step toward >>>> commercial feature parity for us, and it looks like we're so close! >>>> >>>> Sincerely, >>>> Sean M. Pappalardo >>>> "D.J. Pegasus" >>>> Mixxx Developer - Controller Specialist >>>> >>>> >>>> ------------------------------------------------------------------------------ >>>> >>>> _______________________________________________ >>>> Mixxx-devel mailing list >>>> [email protected] >>>> https://lists.sourceforge.net/lists/listinfo/mixxx-devel >>>> >>> >>> >>> ------------------------------------------------------------------------------ >>> >>> _______________________________________________ >>> Mixxx-devel mailing list >>> [email protected] >>> https://lists.sourceforge.net/lists/listinfo/mixxx-devel >>> >> >> > > ------------------------------------------------------------------------------ > > > _______________________________________________ > Mixxx-devel mailing list > [email protected] > https://lists.sourceforge.net/lists/listinfo/mixxx-devel > > ------------------------------------------------------------------------------ _______________________________________________ Mixxx-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/mixxx-devel
