Summary: | ASTERISK-03749: [patch] OSS and ALSA channel drivers use wrong endianness. | ||
Reporter: | David Woodhouse (dwmw2) | Labels: | |
Date Opened: | 2005-03-23 06:40:33.000-0600 | Date Closed: | 2008-01-15 15:29:43.000-0600 |
Priority: | Minor | Regression? | No |
Status: | Closed/Complete | Components: | Core/General |
Versions: | Frequency of Occurrence | ||
Related Issues: | |||
Environment: | Attachments: | ( 0) endian.patch.txt ( 1) endiancvs.patch.txt | |
Description: | As far as I can tell, we are assuming that AST_FORMAT_SLINEAR is native-endian. We have no way of dealing with non-native-endian slinear samples; not even a conversion routine? However, chan_oss and chan_alsa are both explicitly requesting little-endian samples from the sound hardware. That sounds fairly unhealthy on my Fedora/PPC box. This fixes the immediate problem; I'm not sure if other similar problems are lurking. For example, I don't immediately see how we handle the endianness of incoming audio/L16 in rtp.c -- shouldn't that be in network order and hence on i386 shouldn't we be swapping it rather than just treating it as AST_FORMAT_SLINEAR? AFAICT ast_rtp_write() isn't capable of _sending_ AST_FORMAT_LINEAR frames. I wonder if we should remove AST_FORMAT_SLINEAR and replace it with both AST_FORMAT_SLINEAR_BE and AST_FORMAT_SLINEAR_LE ? | ||
Comments: | By: Mark Spencer (markster) 2005-03-23 14:39:45.000-0600 This patch does not appear to apply to CVS... By: David Woodhouse (dwmw2) 2005-03-23 15:45:57.000-0600 Sorry; this one should work. By: Mark Spencer (markster) 2005-03-23 19:01:47.000-0600 oh, disclaimer status? By: David Woodhouse (dwmw2) 2005-03-23 19:06:48.000-0600 Disclaimer faxed 2005-03-22 at 14:00 UTC. By: Mark Spencer (markster) 2005-03-23 19:43:46.000-0600 Added to CVS, thanks! By: David Woodhouse (dwmw2) 2005-03-23 22:13:42.000-0600 Sorry, could do with an answer on the ancillary question(s). Fixing these two drivers to match the assumption that everything is always native-endian is all very well, since the hardware can actually _do_ that. But now I'm looking at chan_bluetooth, where the hardware _can't_ do that; the data are always little-endian. My hack for now is to byteswap data on the way to/from the device, but that's not how format conversion should work; we may have _other_ devices which produce or require little-endian samples, and we'd end up swapping twice for no real reason. I think we really ought to switch to having two separate formats for SLINEAR; either 'native' and 'non-native', or 'little-endian' and 'big-endian'. The former might be a little easier to implement but the latter would probably be cleaner. I can provide a patch which does this, and provide conversion routines, if you agree that it's required. By: Mark Spencer (markster) 2005-03-23 23:03:25.000-0600 Yes, sorry, rtp, if enabled for sending/receiving linear *should* do it in network byte order. if you want to submit a patch it will be welcome. We don't need routines though, we can already use the assume-optimized ntohs and htons functions. By: Russell Bryant (russell) 2005-03-31 22:17:24.000-0600 fixed in 1.0 By: Digium Subversion (svnbot) 2008-01-15 15:28:25.000-0600 Repository: asterisk Revision: 5241 U trunk/channels/chan_alsa.c U trunk/channels/chan_oss.c ------------------------------------------------------------------------ r5241 | markster | 2008-01-15 15:28:25 -0600 (Tue, 15 Jan 2008) | 2 lines fix endianness of OSS/Alsa (bug ASTERISK-3749) ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=5241 By: Digium Subversion (svnbot) 2008-01-15 15:29:43.000-0600 Repository: asterisk Revision: 5333 U branches/v1-0/channels/chan_alsa.c U branches/v1-0/channels/chan_oss.c ------------------------------------------------------------------------ r5333 | russell | 2008-01-15 15:29:43 -0600 (Tue, 15 Jan 2008) | 2 lines fix endianness (bug ASTERISK-3749) ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=5333 |