> On Oct. 4, 2010, 1:30 p.m., Daniel Dewald wrote: > > > > Daniel Dewald wrote: > This is far from being complete. It something of a beginning. I just > basically removes the ffmpeg stuff but puts nothing into place to replace it. > A slot for the audioDataReady signal is created but not implemented anywhere > let alone being connected somehow to some audioDataOutput. This has to go a > long way before it could work. Also remember that the audioDataOutput was > originally invented for visualizing stuff.. so the data wont come in at > once.. you'd have to build some kind of buffer (as I did) before this could > work. otherwise musicbrainz will either hang or starve. > > Kevin Kofler wrote: > > I just basically removes the ffmpeg stuff but puts nothing into place > to replace it. > > Uh yes, it puts a Phonon::AudioDataOutput in place. > > > A slot for the audioDataReady signal is created but not implemented > anywhere > > line 160: > void MusicDNSAudioDecoder::handleFrame(const > QMap<Phonon::AudioDataOutput::Channel, QVector<qint16> > &data) > > > let alone being connected somehow to some audioDataOutput. > > lines 130-131: > connect(&dataout, SIGNAL(dataReady(const > QMap<Phonon::AudioDataOutput::Channel, QVector<qint16> >&)), > this, SLOT(handleFrame(const > QMap<Phonon::AudioDataOutput::Channel, QVector<qint16> >&))); > > > you'd have to build some kind of buffer (as I did) before this could > work. > > The DecodedAudioData class in the original FFmpeg-based code is already > such a buffer. FFmpeg doesn't deliver all the data at once either, the code > used a loop. Due to how Phonon::AudioDataOutput works, I replaced the loop > with a QThread event loop. > > Daniel Dewald wrote: > > line 160: > > void MusicDNSAudioDecoder::handleFrame(const > QMap<Phonon::AudioDataOutput::Channel, QVector<qint16> > &data) > > Sorry overlooked that because of strange coloring (still dont get this > reviewboard stuff). Looking through that stuff now things look better ;o). > > > lines 130-131: > > connect(&dataout, SIGNAL(dataReady(const > QMap<Phonon::AudioDataOutput::Channel, QVector<qint16> >&)), > > this, SLOT(handleFrame(const > QMap<Phonon::AudioDataOutput::Channel, QVector<qint16> >&))); > > I also overlooked that because of the color ;o). > > > The DecodedAudioData class in the original FFmpeg-based code is already > such a buffer. FFmpeg doesn't deliver all the data at once either, the code > used a loop. Due to how Phonon::AudioDataOutput works, I replaced the >loop > with a QThread event loop. > > I've no clue about ffmpeg of how its internal stuff works. May work or > may not.. I'd say it won't because I know phonon. If it does I'd be > surprised. Some other comments: > > - Dont try to get the samplerate via dataout.sampleRate(). I know the > docu says it should work but most times and with most backends it wont. I had > / have the same problem. > - With most (if not all) backends you wont get ANY data at all (for > example VLC or gstreamer). With other it will crash (xine). Your theoretical > approach might work (haven't checked all of it) but as for my experience it > wont because of phonon and the backends. > - As Leo mentioned before phonon wont give you the data any faster as if > you'd play the song in real.. so this will be FUCKING slow ;o). > > So my conclusion: > > Good approach (I dont know if it could / would work ) and maybe even a > smarter solution then what I implemented. But as for now this will have to > sit and wait for the phonon stuff to work out I guess. I didn't try any of > this though. This is just my brain speaking. > > Mark Kretschmann wrote: > I can't really see this working well. Also, we depend on FFmpeg anyway > (for Teo's Transcoding). And FFmpeg is proven and well maintained. > > IMHO we should reject this one, sorry. Thanks for the effort, though. > > > Kevin Kofler wrote: > Well, the problem we have in Fedora is that we cannot ship FFmpeg in > Fedora and thus we cannot link Amarok to it. > > The fact that it is "proven and well maintained" does not help us in any > way, because the people who are maintaining it do not care about patent > issues. FFmpeg is monolithic and so it is not possible to split it into a > patent-free part to ship in Fedora and to link other software against and a > patent-encumbered part which can be put into third-party add-on repositories, > as we have done for e.g. xine-lib. (Even better would be for upstream to do > this split for us, as GStreamer does it.) > > Kevin Kofler wrote: > PS: Looks like I forgot to publish the above reply, which was written > back in October. > > I now think we should look into using QtGStreamer, it'd avoid both the > legal issues with FFmpeg and the reliability and design issues with > Phonon::AudioDataOutput.
Didn't ffmpeg use to have some -gpl compile flag? Has it been dropped meanwhile? (not built it myself since ages ;-) And do you really suggest to replace ffmpeg with gstreamer for recoding? - Thomas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/100021/#review69 ----------------------------------------------------------- On Oct. 3, 2010, 11:07 p.m., Kevin Kofler wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/100021/ > ----------------------------------------------------------- > > (Updated Oct. 3, 2010, 11:07 p.m.) > > > Review request for Amarok. > > > Summary > ------- > > This patch removes FFmpeg dependencies from the new MusicBrainz > fingerprinting by using Phonon::AudioDataOutput instead of FFmpeg directly. > > WARNING: This version of the patch is completely untested and may not even > build. > In addition, Phonon::AudioDataOutput may not be ready for production use yet > (crashes, decoding speed issues). > You have been warned. > > > Diffs > ----- > > CMakeLists.txt baacef5 > src/musicbrainz/MusicDNSAudioDecoder.h 97976f6 > src/musicbrainz/MusicDNSAudioDecoder.cpp eef459a > > Diff: http://git.reviewboard.kde.org/r/100021/diff > > > Testing > ------- > > None yet. > > > Thanks, > > Kevin > >
_______________________________________________ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
