Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

implement a wav play into pcap #98

Open
wants to merge 10 commits into
base: develop
Choose a base branch
from
Open

implement a wav play into pcap #98

wants to merge 10 commits into from

Conversation

jinserk
Copy link

@jinserk jinserk commented Sep 22, 2017

Please consider this code updated as:

  • capable to insert wav file into pcap using a-law or u-law encoding
  • slightely updated the DTMF packet generation

bklang
bklang requested changes Oct 6, 2017
View reviewed changes
Copy link
Member

@bklang bklang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is very cool! Thank you for the contribution, this feature is something I wanted for a long time, but never got around to figuring out. Kudos!

I've left some comments that will need to be addressed prior to merging this. Let me know if you need any more suggestions or feedback.

CHANGELOG.md
* Change: Call limits (`number_of_calls`, `concurrent_max` and `calls_per_second`) no longer have default values for simplicity of UAS scenarios. The value of `to_user` now defaults to the SIPp default of `s`.
* Feature: Support for setting rate scaling independently of reporting frequency via the new `calls_per_second_interval` option. See also https://github.com/SIPp/sipp/pull/107 and https://github.com/SIPp/sipp/pull/126.
* Change: Call limits (`number_of_calls`, `concurrent_max` and `call_rate`) no longer have default values for simplicity of UAS scenarios. The value of `to_user` now defaults to the SIPp default of `s`.
* Feature: Support for setting rate scaling independently of reporting frequency via the new `call_rate_interval` option. See also https://github.com/SIPp/sipp/pull/107 and https://github.com/SIPp/sipp/pull/126.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you changing old Changelog entries? It looks like you are changing calls_per_second to call_rate to match the other variables as part of this PR. That's fine, but the change note belongs in the current release.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry for late reply. I changed the command for my own need, but didn't rollback when make PR. sorry for that, and will change it soon. Thanks for review!

README.markdown
- need spandsp library for encoidng a-law and u-law
* Debug the DTMF packet generation (end of event)
- reduce the duration to 200 milliseconds
- change obsolete rfc2833 to rfc4733
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To merge this into the main branch, would you move these notes into appropriate areas of the README?

lib/sippy_cup/g711.rb
module SippyCup
module G711
extend FFI::Library
ffi_lib 'spandsp'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This new dependency on spandsp needs a couple of things:

  1. Document how and where a person can get spandsp, if they don't already have it
  2. Ensure sippy_cup continues to work the way it did before if spandsp is not available

lib/sippy_cup/g711.rb
iptr = FFI::MemoryPointer.new(:int16, samples.size)
optr = FFI::MemoryPointer.new(:uint8, samples.size)

#puts samples.join(' ')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dead code should be removed

@nikolas-n
Copy link

Hello @jinserk
It has been a while, but was wondering if you can help with the following. I've updated ffi to the 1.15.5 version and getting this:

Compiling media to /sippy/scenaria/template.pcap...Traceback (most recent call last):
	15: from /usr/local/bin/sippy_cup:23:in `<main>'
	14: from /usr/local/bin/sippy_cup:23:in `load'
	13: from /Library/Ruby/Gems/2.6.0/gems/sippy_cup-0.7.2/bin/sippy_cup:64:in `<top (required)>'
	12: from /Library/Ruby/Gems/2.6.0/gems/sippy_cup-0.7.2/lib/sippy_cup/scenario.rb:940:in `compile!'
	11: from /Library/Ruby/Gems/2.6.0/gems/sippy_cup-0.7.2/lib/sippy_cup/scenario.rb:1041:in `compile_media'
	10: from /Library/Ruby/Gems/2.6.0/gems/sippy_cup-0.7.2/lib/sippy_cup/media.rb:44:in `compile!'
	 9: from /Library/Ruby/Gems/2.6.0/gems/sippy_cup-0.7.2/lib/sippy_cup/media.rb:44:in `each'
	 8: from /Library/Ruby/Gems/2.6.0/gems/sippy_cup-0.7.2/lib/sippy_cup/media.rb:95:in `block in compile!'
	 7: from /Library/Ruby/Gems/2.6.0/gems/sippy_cup-0.7.2/lib/sippy_cup/media.rb:95:in `times'
	 6: from /Library/Ruby/Gems/2.6.0/gems/sippy_cup-0.7.2/lib/sippy_cup/media.rb:109:in `block (2 levels) in compile!'
	 5: from /Library/Ruby/Gems/2.6.0/gems/sippy_cup-0.7.2/lib/sippy_cup/g711.rb:28:in `encode'
	 4: from /Library/Ruby/Gems/2.6.0/gems/ffi-1.15.5/lib/ffi/pointer.rb:134:in `write_array_of_type'
	 3: from /Library/Ruby/Gems/2.6.0/gems/ffi-1.15.5/lib/ffi/pointer.rb:134:in `each_with_index'
	 2: from /Library/Ruby/Gems/2.6.0/gems/ffi-1.15.5/lib/ffi/pointer.rb:134:in `each'
	 1: from /Library/Ruby/Gems/2.6.0/gems/ffi-1.15.5/lib/ffi/pointer.rb:136:in `block in write_array_of_type'
/Library/Ruby/Gems/2.6.0/gems/ffi-1.15.5/lib/ffi/pointer.rb:136:in `write_int16': wrong number of arguments (given 2, expected 1) (ArgumentError)

Do you happen to know how this could be fixed? Or what I could do to make it work? Maybe downgrade?

Sorry for asking this here, but I couldn't add an issue to your fork.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bklang bklang bklang requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jinserk @nikolas-n @bklang