GNOME Bugzilla – Bug 660364
opus: misc cleanup/fixes
Last modified: 2011-10-03 16:03:33 UTC
Created attachment 197668 [details] [review] opus: make it build against current, and remove cruft
Created attachment 197669 [details] [review] opusdec: opus supports a select set of sampling rates
Created attachment 197670 [details] [review] opusenc: use the same frame size setup as the opus test code
Created attachment 197671 [details] [review] opus: properly setup caps and init state from caps
Created attachment 197672 [details] [review] opusenc: moan if we get an unexpected amount of data
Created attachment 197673 [details] [review] opusdec: fix decoding A simple ... opusenc ! opusdec ... pipeline now works.
Review of attachment 197668 [details] [review]: ::: ext/opus/gstopusdec.c @@ -611,3 @@ - dec->frame_size = dec->header.frame_size; -#else - opus_mode_info (dec->mode, OPUS_GET_FRAME_SIZE, &dec->frame_size); You don't set dec->frame_size anywhere, is this correct? ::: ext/opus/gstopusenc.c @@ -693,3 @@ - enc->frame_size = enc->header.frame_size; -#else - opus_mode_info (enc->mode, OPUS_GET_FRAME_SIZE, &enc->frame_size); Same here about the frame_size and additionally the header creation
> You don't set dec->frame_size anywhere, is this correct? No, it's set via: if (!gst_structure_get_int (s, "frame-size", &dec->frame_size)) { GST_WARNING_OBJECT (dec, "Frame size not included in caps"); } > Same here about the frame_size and additionally the header creation This was already set in existing code, gst_opus_enc_sink_setcaps. As for any header, I'm not too sure what to do with that. There is a header when embedded in Ogg (ID header, and comments), but apparently not in other uses. The code calling the header stuff was all commented out anyway. I guess I'll come back to that when I get some time and fix up what needs fixing up for Ogg framing, which will include that header stuff (not sure if the ogg mapping is fully defined yet though).
commit 9f9d52c6cb863e9e1339f795b63d57c9f28b1057 Author: Vincent Penquerc'h <vincent.penquerch@collabora.co.uk> Date: Wed Sep 28 14:57:02 2011 +0100 opusdec: fix decoding A simple ... opusenc ! opusdec ... pipeline now works. https://bugzilla.gnome.org/show_bug.cgi?id=660364 commit 532e90a34d479306bf366008da738b33f84caa69 Author: Vincent Penquerc'h <vincent.penquerch@collabora.co.uk> Date: Wed Sep 28 14:56:18 2011 +0100 opusenc: moan if we get an unexpected amount of data https://bugzilla.gnome.org/show_bug.cgi?id=660364 commit 934144c352a5edd054e4ebdf5113630ccf963be9 Author: Vincent Penquerc'h <vincent.penquerch@collabora.co.uk> Date: Wed Sep 28 14:22:02 2011 +0100 opus: properly setup caps and init state from caps https://bugzilla.gnome.org/show_bug.cgi?id=660364 commit 85de20b8a14834059b5a60f33ba4757ed55f1ed6 Author: Vincent Penquerc'h <vincent.penquerch@collabora.co.uk> Date: Wed Sep 28 13:25:21 2011 +0100 opusenc: use the same frame size setup as the opus test code https://bugzilla.gnome.org/show_bug.cgi?id=660364 commit 8caa7adb5e851e419f1c47d558c03d81351fc1b9 Author: Vincent Penquerc'h <vincent.penquerch@collabora.co.uk> Date: Wed Sep 28 13:24:52 2011 +0100 opusdec: opus supports a select set of sampling rates https://bugzilla.gnome.org/show_bug.cgi?id=660364 commit ae510870e9aa42ce22c553363a76f5b710ade828 Author: Vincent Penquerc'h <vincent.penquerch@collabora.co.uk> Date: Wed Sep 28 13:24:21 2011 +0100 opus: make it build against current, and remove cruft https://bugzilla.gnome.org/show_bug.cgi?id=660364
I gues adding a link to http://opus-codec.org/ from the doc blob would make sense and/or at least mention that it is a voice codec.