| Summary: | ASTERISK-29480: fixedjitterbuffer contains an un-wrappered assert that triggers on a negative time slew | ||
| Reporter: | Dan Cropp (daninmadison) | Labels: | |
| Date Opened: | 2021-06-17 07:23:18 | Date Closed: | 2021-06-24 08:18:21 |
| Priority: | Minor | Regression? | |
| Status: | Closed/Complete | Components: | Core/Jitterbuffer |
| Versions: | 16.18.0 18.4.0 | Frequency of Occurrence | |
| Related Issues: | |||
| Environment: | Attachments: | ||
| Description: | If the system time slews backwards because of a time adjustment between the time a frame is timestamped and the time we check the timestamps in abstract_jb:hook_event_cb(), we get a negative interval, but we don't check for that there. abstract_jb:hook_event_cb() then calls fixedjitterbuffer:fixed_jb_get() (via abstract_jb:jb_get_fixed) and the first thing that does is assert(interval >= 0).
There are several issues with this... * abstract_jb:hook_event_cb() saves the interval in a variable named "now" which is confusing in itself. :) * "now" is defined as an unsigned int which converts the negative value returned from ast_tvdiff_ms() to a large positive value. * fixed_jb_get()'s parameter is defined as a signed int so the interval gets converted back to a negative value. * fixed_jb_get()'s assert is NOT an ast_assert but a direct define that points to the system assert() so it triggers even in production mode. We should... * Redefine hook_event_cb()'s "now" variable as an int64_t which matches the return from ast_tvdiff_ms(). * Check for negative difference right after the call to ast_tvdiff_ms() and just pass the frame through if it is. * Fix any asserts in fixedjitterbuffer to call the ast_assert wrapper. | ||
| Comments: | By: Friendly Automation (friendly-automation) 2021-06-24 08:18:22.551-0500 Change 16111 merged by Friendly Automation: jitterbuffer: Correct signed/unsigned mismatch causing assert [https://gerrit.asterisk.org/c/asterisk/+/16111|https://gerrit.asterisk.org/c/asterisk/+/16111] By: Friendly Automation (friendly-automation) 2021-06-24 08:19:52.205-0500 Change 16073 merged by Friendly Automation: jitterbuffer: Correct signed/unsigned mismatch causing assert [https://gerrit.asterisk.org/c/asterisk/+/16073|https://gerrit.asterisk.org/c/asterisk/+/16073] By: Friendly Automation (friendly-automation) 2021-06-24 08:21:37.875-0500 Change 16110 merged by George Joseph: jitterbuffer: Correct signed/unsigned mismatch causing assert [https://gerrit.asterisk.org/c/asterisk/+/16110|https://gerrit.asterisk.org/c/asterisk/+/16110] | ||