[Home]

Summary:ASTERISK-08074: [patch] Recording synchronization fails due to bad number of samples correlation in ast_read / ast_write
Reporter:Guillermo Winkler (guillecabeza)Labels:
Date Opened:2006-11-06 19:59:54.000-0600Date Closed:2007-01-23 19:01:28.000-0600
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Core/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) channel.c.patch
( 1) channel.c.patch124
Description:When recording a bridged call leg synchronization fails due to jumping bytes being miscalculated in ast_read/ast_write.

The total byte/sample difference might be greater than 1 frame samples, in that scenario the total sample difference should be advanced on disk, but it's not.

When comparing leg samples 'jump + f->samples' is advanced on disk, but 'chan->insmpl += jump + 4 * f->samples;' is added to the number of samples on the memory structure, which makes no sense.

This situation gets worsened when:
ast_log(LOG_DEBUG, "Servicing channel %s twice in a row?\n", last->name);

This happens on the bridge(which happens a lot).



****** ADDITIONAL INFORMATION ******

This code:
if (chan->monitor && chan->monitor->read_stream ) {
      int jump = chan->outsmpl - chan->insmpl - 4 * f->samples;
      if (jump >= 0) {
      if (ast_seekstream(chan->monitor->read_stream, jump + f->samples, SEEK_FORCECUR) == -1)
   ast_log(LOG_WARNING, "Failed to perform seek in monitoring read stream, synchronization between the files may be broken\n");
chan->insmpl += jump + 4 * f->samples;
} else
     chan->insmpl+= f->samples;

Should be:

int jump = chan->outsmpl - chan->insmpl - 4 * f->samples;
if (jump >= 0) {
      jump = chan->outsmpl - chan->insmpl; //You wanna adjust by this difference
      if (ast_seekstream(chan->monitor->read_stream, jump, SEEK_FORCECUR) == -1)
   ast_log(LOG_WARNING, "Failed to perform seek in monitoring read stream, synchronization between the files may be broken\n");
chan->insmpl += jump + f->samples; //add the bytes jumped plus the bytes in the current frame

-----------------
The same thing applies for ast_write
Comments:By: jmls (jmls) 2006-11-07 02:11:11.000-0600

guillecabeza, thanks for the report. As you've supplied a code change / patch would you please confirm that either you have a disclaimer on file, or that you have sent a disclaimer to digium ? Thanks.

For future reference, please would you send any code change as an attachment ? It makes it a lot easier for the bug marshals to deal with issues like code review etc.
By: Guillermo Winkler (guillecabeza) 2006-11-07 08:24:00.000-0600

Disclaimer sent.

Regards
By: wes (whoiswes) 2006-11-07 09:45:39.000-0600

Interesting that this bug just popped up - I have been having issues with recorded call sync on one of my phone servers.  I have already backported this to 1.2.4 and will give it a shot, see if it helps.

Thanks!
By: Guillermo Winkler (guillecabeza) 2006-11-13 21:01:05.000-0600

so?
By: wes (whoiswes) 2006-11-14 07:59:13.000-0600

initial testing on my test server indicates no issues, but i have yet to put this on our production box.
By: wes (whoiswes) 2006-11-15 08:45:59.000-0600

I've created two patch files - the first is for trunk, the second is for 1.2.4, which is what I've tested on.  again, this hasn't been used in a production environment yet, but my testing indicates the patch works fine.

I'm not a programmer, so I cannot follow the code as well as I'd like - there is a IF statement involving the variable MONITOR_CONSTANT_DELAY - this patch only modifies the code if that variable is defined.  The else part of that statement also includes a sampling jump statement, so that may need to be patched as well - I honestly do not know (eek, sorry!).

In any case, hopefully these patch files help a little - sorry I cannot do more.
By: Serge Vecher (serge-v) 2006-11-20 09:21:17.000-0600

whoiswes: please also report your disclaimer status.
By: wes (whoiswes) 2006-11-20 09:50:40.000-0600

I have a disclaimer on file, sorry for not mentioning that earlier.  I'm not the one who wrote the original code, just made it into the patch file, so legally I'm not even sure a disclaimer would be required, but I have one on file anyway.
By: Serge Vecher (serge-v) 2006-11-21 08:54:03.000-0600

switching to 1.2 as it needs to be fixed there first.
By: Guillermo Winkler (guillecabeza) 2006-12-13 16:47:06.000-0600

Is there any information missing from this bug or anything the administrators need to move on?

Regards
By: Serge Vecher (serge-v) 2006-12-14 08:30:42.000-0600

guillecabeza: have you tested whoiswes's patch? Does it apply to the latest 1.2 release?
By: Russell Bryant (russell) 2007-01-23 19:01:27.000-0600

This patch has been applied to 1.2, 1.4, and the trunk in revisions 51843, 51848, and 51850.  Thanks!!