GNOME Bugzilla – Bug 352605
[PLUGIN-MOVE] move wavpack to -good
Last modified: 2007-06-12 19:26:41 UTC
Hi, please move the wavpack plugin from -bad to -good. All the requirements from docs/random/moving-plugins should be fulfilled, only a sponsor and reviewer is needed (and it would be nice to have the patch to bug #344472 committed before). Wavpack can go to -good as it is to Wavpack's author's and my best knowledge not patent-encumbered. Below is what the author of Wavpack told me regarding patents some months ago. Bye ------------- This is a little more difficult question! You are right to say that just about everything is patented, and so it's difficult to to much without breaking some obscure, over-broad patent that could never be enforced. I can say that I have intentionally avoided everything that I know to be patented, like arithmetic coding or LWZ (although I just noticed that this patent has expired). The techniques I use are simple variations on linear prediction and Rice coding, which are certainly considered patent-free. I obviously cannot guarantee that some patent will not someday be discovered that WavPack violates, but I can say that no suggestion has ever been made to me that WavPack violates any patent (which interestingly is not true of FLAC's technique, but I haven't heard anything of this lately). Is that good enough? Thanks, David
Additionally the following paragraph was added to www.wavpack.com (but doesn't contain much more information than the stuff above): WavPack employs only well known, public domain techniques (i.e., linear prediction with LMS adaptation, Elias and Golomb codes) in its implementation. Methods and algorithms that have ever been patented (e.g., arithmetic coding, LZW compression) are specifically avoided. This ensures that WavPack encoders and decoders will remain open and royalty-free.
code looks very clean. one minor cosmetic change for all: Code from gstwavpackdec.c::gst_wavpack_dec_init() and gst_wavpack_dec_change_state() can be factored out into gst_wavpack_dec_reset(). Same for encoder and parse. In encoder: * in gst_wavpack_enc_base_init make ElementDetails const * in gst_wavpack_enc_sink_set_caps() move error handing to the end * can you add enums for enc_mode, enc_correction_mode, joint_stereo_mode then you can use them instread on 0,1,2,3,... in gst_wavpack_enc_mode_get_type(), gst_wavpack_enc_correction_mode_get_type(), gst_wavpack_enc_joint_stereo_mode_get_type() and in e.g. gst_wavpack_enc_set_wp_config() * wouldn't it make sense to add a gboolean return to gst_wavpack_enc_rewrite_first_block() ? <superminornictpick> in the doc sections you write 'wavpackdec', but 'Wavpackenc' and 'Wavpackparse'. What about 'WavPackDec' or 'WavpackDec' (and likewise for the others). </superminornictpick>
one more thing, you can add docbook-xml to the docblobs, what about an ulink to the wavpack.com site?
Created attachment 76157 [details] [review] wavpack.diff patch that does all this except letting gst_wavpack_enc_rewrite_first_block return a boolean because it wouldn't make much sense as discussed on IRC ;)
Thanks, committed: 2006-11-15 Tim-Philipp Müller <tim at centricular dot net> Patch by: Sebastian Dröge <slomo@circular-chaos.org> * ext/wavpack/gstwavpackdec.c: (gst_wavpack_dec_reset), (gst_wavpack_dec_init), (gst_wavpack_dec_change_state): * ext/wavpack/gstwavpackenc.c: (gst_wavpack_enc_base_init), (gst_wavpack_enc_class_init), (gst_wavpack_enc_reset), (gst_wavpack_enc_init), (gst_wavpack_enc_set_wp_config), (gst_wavpack_enc_change_state): * ext/wavpack/gstwavpackparse.c: Some small clean-ups: use enums instead of hard-coded numbers, const-ify element details, re-factor some code into _reset() functions (#352605).
assigning to myself to see the docs actually get built and move it
> assigning to myself to see the docs actually get built and move it Not sure if it matters, just pointing it out, but maybe it's better to wait with the move until after the next -good release? Thing is that we need to release the upcoming core/base/good at the same time (Andy's new pull_range stuff required changes in id3demux and apedemux); and since we should probably also release good/bad at the same time when we move stuff, it might be better to do core/base/good first, and then do a good/bad with the moved plugins afterwards, IMHO.
Sounds like a sane plan, Tim. Would be a bit hard to release all modules at once for the one making the release ;)
Ok, as core/base can now be released without a new good what about moving this now for the next good release already? :) Any objections?
Can this please be moved before the next -good release? There's nothing blocking the move
Agreed, this one is ready to move IMHO as well.
+1 from my side too. @thomas: you got to move it, move it - let's move it!
Some notes from looking over wavpackenc: - the examples do not work. Tim tells me this is because wavpackenc was changed to only accept 32 bit. An audioconvert should be added to all examples. I would also appreciate a note explaining why that audioconvert is needed. - Why is the bitrate property a double and not an int like for all other encoders ? - Some of these properties are described vaguely: - correction-mode - I assume it creates a file. Which file will it create ? How does it decide the name ? What is it for ? - extra-processing - is this some secret option that just uses more CPU without any purpose ? Does it have lots of empty while loops ? -
Since this was the first of the three wavpack elements I looked at, I stopped here. It is possible the other two have similar problems with the examples or properties.
Should be fine now with the commit below... 2007-05-21 Sebastian Dröge <slomo@circular-chaos.org> * ext/wavpack/gstwavpackenc.c: (gst_wavpack_enc_class_init), (gst_wavpack_enc_init), (gst_wavpack_enc_set_wp_config), (gst_wavpack_enc_set_property), (gst_wavpack_enc_get_property): * ext/wavpack/gstwavpackenc.h: Fixup docs, make the bitrate property an int as it should be and allow to set the different extra processing modes instead of only allowing none and the default one.
Making this a 0.10.5 blocker
done.