[asterisk-dev] [Code Review] Video format treated as audio when removed from the file playback scheduler

[mjordan]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/1580/
-----------------------------------------------------------

Review request for Asterisk Developers and David Vossel.


Summary
-------

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.


This addresses bug ASTERISK-18682.
    https://issues.asterisk.org/jira/browse/ASTERISK-18682


Diffs
-----

  /branches/1.8/main/file.c 344607 

Diff: https://reviewboard.asterisk.org/r/1580/diff


Testing
-------

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.


Thanks,

mjordan

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20111111/a67a9a44/attachment.htm>