Summary:	DAHLIN-00007: Allow modification of ZT_CHUNKSIZE
Reporter:	Ivan Ukhov (uvsoft)	Labels:
Date Opened:	2007-07-20 02:37:18	Date Closed:	2009-06-15 16:25:54
Priority:	Major	Regression?	No
Status:	Closed/Complete	Components:	NewFeature
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) zaptel-1.2.19-timing_v2.patch ( 1) zaptel-1.2.19-timing_v3.patch ( 2) zaptel-1.2.19-timing_v4.patch ( 3) zaptel-1.2.19-timing.patch ( 4) zaptel-1.4.5.1_chunksize_v2.patch ( 5) zaptel-1.4.5.1_chunksize.patch
Description:	I've made a patch that makes ZT_CHUNKSIZE variable, thus someone can change it to whatever he wants. It's very useful, e.g. I'm working on a driver for some device and the device requires ZT_CHUNKSIZE to be much more, it's hard for the device to operate with chunks of 8 samples, but 80 will do. The problem is that there are some pieces of the code, where ZT_CHUNKSIZE is implied to be 8, and if I just change ZT_CHUNKSIZE to 80, all the timeouts will become crazy, the patch tries to fix this issue.
Comments:	By: Tzafrir Cohen (tzafrir) 2007-07-21 17:21:40 The patch changes the default chunk size, and hence cannot be accepted as-is. Also: do you have a patch vs. zaptel trunk or zaptel 1.4? Also: #define ZT_CHUNKTIME (ZT_CHUNKSIZE / 8) /* 8kHz */ You still have a magic number 8 there. That 8 is actually the single sample size, right? By: Ivan Ukhov (uvsoft) 2007-07-22 02:31:40 The patch shows the idea. I don't offer someone just apply it and '$ make', it won't do, because wct4xxp requires ZT_CHUNKSIZE to be 8: wct4xxp/base.c: 1859: #if (ZT_CHUNKSIZE != 8) 1860: #error Sorry, nextgen does not support chunksize != 8 1861: #endif I've uploaded a new version of the patch (v4) that doesn't change ZT_CHUNKSIZE and fixes Makefile and Makefile.kernel26 to exclude wct4xxp if ZT_CHUNKSIZE differs from 8. About magic 8, actually there's no magic here at all, it's just a way to move from samples to ms, therefore if someone wants to change ZT_CHUNKSIZE and knows sample rate of the device he intends communicating with, there won't be any problem to set ZT_CHUNTIME properly. In most cases you don't need to care about ZT_CHUNKTIME, because most likely you have 8kHz (8 samples = 1 ms). As for the 1.2 trunk and 1.4, I don't have such patches, but if someone is interested in it, there won't be any problem to make them. Moreover I do think that the patch can be applied for the 1.2 trunk. By: Tzafrir Cohen (tzafrir) 2007-07-23 06:18:53 I don't really understand what you try to change here. ZT_CHUNKSIZE (8 bitess) is currently the ammount of data sent on each chanel in each tick. If I understand your patch correctly, you intend to send 80 bytes per ms? By: Ivan Ukhov (uvsoft) 2007-07-23 13:07:26 Yep, I wanna use 80 as ZT_CHUNKSIZE. How knows may be someone wants too, and this patch will help him. By: Tzafrir Cohen (tzafrir) 2007-08-24 02:43:30 Just to clarify, that I don't really see how this patch should work. Do you combine 10 channels in one transfer (and hence send a chunk of 10 samples)? Or is whatever you pass here data rather than audio, and thus standard calculations don't matter? By: Tzafrir Cohen (tzafrir) 2008-01-23 02:08:49.000-0600 Any news? By: Ivan Ukhov (uvsoft) 2008-01-26 09:14:39.000-0600 Here is a patch for 1.4.5.1 with some additional bug fixes. By: Tzafrir Cohen (tzafrir) 2008-01-27 04:23:35.000-0600 Again, this looks like something that shouldn't really work. And you have not answered my previous question (comment 81063), let alone provide a patch vs. up-to-date Zaptel. For startes, I'd really like to understand what you're trying to do. By: Jason Parker (jparker) 2008-04-22 17:00:25 I think this makes sense to me.  It isn't that it would be getting data from 10 channels at once, it's that it would be getting it every 10ms (rather than 1ms). If I'm understanding it correctly, ZT_CHUNKTIME would be the number of samples that exist in one chunk. By: Kevin P. Fleming (kpfleming) 2008-04-28 15:12:50 I think it is fine to have the Zaptel core able to tolerate a chunk size that is not 8 samples; many drivers will not work properly in that configuration, but if someone has a driver that does, then Zaptel should be able to support it. The most recent patch on this bug seems to be headed in that direction and if someone wants to bring this up to date against Zaptel branch 1.4 and test it with the chunk size set to 8 (to verify no regressions) I'll be happy to review it and move it along. By: Ivan Ukhov (uvsoft) 2008-05-09 01:15:20 I'm glad to see that someone is interested in it) That patch does help me. By: Leif Madsen (lmadsen) 2008-12-04 15:34:48.000-0600 How close is this issue to being able to be moved into Zaptel/DAHDI? I believe this will need to be updated to DAHDI trunk. Is anyone still interested in updating this and then allowing Mr. Fleming to review it? By: Shaun Ruffell (sruffell) 2009-06-15 16:25:54 This issue has lingered and I'm going to suspend it for the time being.  While I see how this could be beneficial for hardware that cannot deal with 1ms samples.  However I still think it would be *possible* for a device driver to split a chunk of audio into the 1ms samples when calling dahdi_receive and dahdi_transmit. This, IMHO, would be simpler than updating all the other card drivers to work with a DAHDI_CHUNKSIZE that is not 8 or fail appropriately. I do realize that there are potential performance improvements to be had from increasing the chunk size used to process audio (less function call overhead, better cache locality, etc..).  But if this patch is going to be merged for performance, there needs to be some work to make it easy to profile DAHDI and get objective numbers before. But this issue can be reopened if someone wants to make it against the trunk of DAHDI and there are appropriate error messages generated for all the boards that are not updated, and selectable by flipping a flag in include/dahdi/dahdi_config.h, then this could be reopened.