After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 339838 - [audioconvert] support floats with non-native endianness
[audioconvert] support floats with non-native endianness
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
git master
Other Linux
: Normal enhancement
: 0.10.13
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks: 424527
 
 
Reported: 2006-04-26 17:18 UTC by Tim-Philipp Müller
Modified: 2007-03-30 11:02 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Add float endianness conversion to audioconvert. (9.52 KB, patch)
2006-10-07 19:32 UTC, René Stadler
none Details | Review
Remove endianness conversion hack from auparse. (2.96 KB, patch)
2006-10-07 19:34 UTC, René Stadler
committed Details | Review
Add float endianness conversion to audioconvert (2) (25.41 KB, patch)
2007-03-14 18:11 UTC, René Stadler
none Details | Review
audioconvert-float.diff (25.81 KB, patch)
2007-03-29 15:25 UTC, Sebastian Dröge (slomo)
committed Details | Review

Description Tim-Philipp Müller 2006-04-26 17:18:31 UTC
Audioconvert currently doesn't handle audio/x-raw-float with an endianness other than the native one.

At least the input side should have support for both little and big endianness IMHO.

Sample file (big endian content, try to play back on little endian system):
http://www-mmsp.ece.mcgill.ca/Documents/AudioFormats/AU/Samples/AFsp/M1F1-float32-AFsp.au

Currently this works only because auparse does the endianness swapping itself, which is more of a hack.
Comment 1 René Stadler 2006-10-07 19:32:57 UTC
Created attachment 74242 [details] [review]
Add float endianness conversion to audioconvert.

This patch adds the described capability (for input side only).  Supporting foreign endianness conversion for float on the output side in a sane way needs introduction of float as intermediary format.  This restriction will surely be lifted at some point in the future (probably as part of solving bug #339837).

I have one uncertainity about the changes I made (I think I don't know enough about caps negotiation yet):  The static caps in the pad templates have been changed to reflect the new capability on the sink side only, but the transform_caps and fixate_caps functions never make use of the "direction" parameter.  Is there special handling needed somewhere?  With the patch, the make_lossless_changes function unconditionally sets the endianness field to both values.

Also includes an addition to the test suite.  A patch to remove the swapping hack from auparse follows.
Comment 2 René Stadler 2006-10-07 19:34:47 UTC
Created attachment 74243 [details] [review]
Remove endianness conversion hack from auparse.
Comment 3 Stefan Sauer (gstreamer, gtkdoc dev) 2007-03-01 08:01:15 UTC
Audioconvert now has float as an intermediate format.
Comment 4 René Stadler 2007-03-14 18:11:52 UTC
Created attachment 84589 [details] [review]
Add float endianness conversion to audioconvert (2)

Updated patch, which delivers the full thing.
Comment 5 Sebastian Dröge (slomo) 2007-03-28 21:32:23 UTC
I'll take a look at this patch in the next days and port it to the current audioconvert in CVS... I'll probably need this before adding float support to wavparse/wavenc :)

At a first glance the patch looks good btw...
Comment 6 Sebastian Dröge (slomo) 2007-03-29 14:31:52 UTC
Hm, wouldn't this be an ABI change? audio/x-raw-float must have an endianess field now but it didn't even had one before so every caps that have audio/x-raw-float without endianess field would break unless I missed something.

We should, until 0.11, take float caps without endianess as floats in host byte order... I'll change that for now.
Comment 7 Tim-Philipp Müller 2007-03-29 14:54:07 UTC
> Hm, wouldn't this be an ABI change? audio/x-raw-float must have an endianess
> field now but it didn't even had one before so every caps that have
> audio/x-raw-float without endianess field would break unless I missed
> something.

I think endianness was always a required field (see appendix in plugin writer's guide, for example), so it shouldn't be a problem.


> We should, until 0.11, take float caps without endianess as floats in host byte
> order... I'll change that for now.

If you want to be nice (or know of any actually existing broken audio/x-raw-float producers), otherwise I'm not sure it's really needed.
 

Comment 8 Sebastian Dröge (slomo) 2007-03-29 15:25:28 UTC
Created attachment 85516 [details] [review]
audioconvert-float.diff

Oh, ignore my last comment...

Anyway, I moved your LE<->BE conversion methods into the floatcast library, they're doing the same anyway and your's should be faster.

If nobody has any objections I will commit the attached patch later, IMHO it's good...
Comment 9 Sebastian Dröge (slomo) 2007-03-29 18:42:38 UTC
Ok, committed the patch:

2007-03-29  Sebastian Dröge  <slomo@circular-chaos.org>

	Patch by: René Stadler <mail at renestadler dot de>
	with some minor changes

	* gst-libs/gst/floatcast/floatcast.h:
	Use more efficient float endianness conversion functions that don't
	involve 2 function calls per value.
	* gst/audioconvert/audioconvert.c: (audio_convert_get_func_index),
	(check_default), (audio_convert_prepare_context):
	* gst/audioconvert/gstaudioconvert.c:
	(gst_audio_convert_parse_caps), (make_lossless_changes):
	Support non-native endianness floats as input and output.
	Fixes #339838.
	* tests/check/elements/audioconvert.c: (verify_convert),
	(GST_START_TEST):
	Add unit tests for the non-native endianness float conversions.
Comment 10 Sebastian Dröge (slomo) 2007-03-29 18:51:29 UTC
And committed the auparse patch...

2007-03-29  Sebastian Dröge  <slomo@circular-chaos.org>

        * configure.ac:
        Require gst-plugins-base CVS for audioconvert with non-native
        float support and width/depth fix in libgstriff.

        Patch by: René Stadler <mail at renestadler dot de>

        * gst/auparse/gstauparse.c: (gst_au_parse_reset),
        (gst_au_parse_parse_header), (gst_au_parse_chain):
        * gst/auparse/gstauparse.h:
        Don't swap the floats ourself if they're not in native endianness.
        Instead let audioconvert handle this. Fixes #339838.
Comment 11 Jan Schmidt 2007-03-30 11:02:29 UTC
The auparse patch needs to come back out, or be reworked so that compilation depends on the version of gst-plugins-base available - plugins-good is the next module I want to release, so it can't depend on CVS plugins-base for now.