Skip to content
Snippets Groups Projects
  • George Joseph's avatar
    88da59ef
    jitterbuffer: Correct signed/unsigned mismatch causing assert · 88da59ef
    George Joseph authored
    If the system time has stepped 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.
    
    So...
    
     * hook_event_cb()'s "now" was renamed to "relative_frame_start" and
       changed to an int64_t.
     * hook_event_cb() now checks for a negative value right after
       retrieving both the current and framedata timestamps and just
       returns the frame if the difference is negative.
     * fixed_jb_get()'s local define of ASSERT() was changed to call
       ast_assert() instead of the system assert().
    
    ASTERISK-29480
    Reported by: Dan Cropp
    
    Change-Id: Ic469dec73c2edc3ba134cda6721a999a9714f3c9
    88da59ef
    History
    jitterbuffer: Correct signed/unsigned mismatch causing assert
    George Joseph authored
    If the system time has stepped 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.
    
    So...
    
     * hook_event_cb()'s "now" was renamed to "relative_frame_start" and
       changed to an int64_t.
     * hook_event_cb() now checks for a negative value right after
       retrieving both the current and framedata timestamps and just
       returns the frame if the difference is negative.
     * fixed_jb_get()'s local define of ASSERT() was changed to call
       ast_assert() instead of the system assert().
    
    ASTERISK-29480
    Reported by: Dan Cropp
    
    Change-Id: Ic469dec73c2edc3ba134cda6721a999a9714f3c9