> 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. > > Thomas Lübking wrote: > 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?
> Didn't ffmpeg use to have some -gpl compile flag? Copyright licensing isn't the problem, software patents are. E.g. MP3 codecs cannot be shipped in Fedora no matter how they are licensed. And in addition, a compile flag is not sufficient (e.g. a hypothetic -unpatented flag would not help either), because 1. we aren't allowed to even ship patent-encumbered source code in our source packages and 2. the library is monolithic rather than plugin-based, which makes it a PITA for third-party repositories to add additional codecs if we ship a patent-free version in Fedora. > And do you really suggest to replace ffmpeg with gstreamer for recoding? Yes! Despite the name, GStreamer can do much more than just streaming, e.g. it can pipe to a file at encoding speed (not playback speed) just fine. Several transcoding apps (mainly GNOME-based ones) use it. And it supports an FFmpeg plugin, so you can recode basically all the formats you can recode with FFmpeg. But the big difference is that FFmpeg or other patent-encumbered codecs are NOT required, e.g. you can transcode from FLAC to Ogg Vorbis using GStreamer with only packages in Fedora. The codecs are plugins, so you can add additional codecs at runtime, not just at compile time as for FFmpeg. Now that QtGStreamer has been developed and adopted by a real-world app (Kamoso), it should really be considered as an alternative to using FFmpeg directly. As long as fingerprinting requires the FFmpeg libraries directly, it will have to be disabled in Fedora (as it currently is). - Kevin ----------------------------------------------------------- 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
