Review Board 1.7.16


Video format treated as audio when removed from the file playback scheduler

Review Request #1580 - Created Nov. 11, 2011 and submitted

Matt Jordan
1.8
ASTERISK-18682
Reviewers
asterisk-dev
dvossel
Asterisk
When audio formats were changed to 64 bits, audio formats bit masks were no longer contiguous.  Previously, a comparison operator could be used with AST_FORMAT_AUDIO_MASK to check if a format was audio - however, since AUDIO_FORMAT_MASK now includes audio formats that have a bit mask value greater than some video formats, this no longer works and a bitwise AND operator must be used.  The check to delete a scheduled audio / video read callback in file.c still used a comparison check.  As a result, regardless of the format, the stream was treated as audio and would not be descheduled properly.  If a video stream was stopped before it reached the end of the file, the read callback would not be removed from the scheduler, and the next time the scheduler was called (which could be on a subsequent audio file), the video callback would be called on an already disposed of stream.

This patch fixes that by properly checking for the audio / video masks.  It also consolidates the closing of the audio / video streams into a single private function to reduce code duplication between ast_closestream and the filestream_destructor.

I also searched through the rest of the codebase for any other improper checking of a format against AUDIO_FORMAT_MASK - while comparison operators are used against AST_FORMAT_AUDIO_MASK in a few other places (such as in while loops), in those locations, the format is still explicitly checked as to whether or not it is audio using a bitwise AND operator.
Tested using a video softphone and h.264.  Tested listening back a recorded video message and using DTMF tones to interrupt the playback multiple times in various locations.  Tested also listening to the recorded video message using audio only.  In all cases the playback works correctly with this patch.
Ship it!
Posted (Nov. 11, 2011, 8:36 a.m.)
Yep, that should do it!

https://reviewboard.asterisk.org/ runs on a server provided by Digium, Inc. and uses bandwidth donated to the open source Asterisk community by API Digital Communications in Huntsville, AL USA.
Please report problems with this site to asteriskteam@digium.com.