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.

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

Adding file format network/rtpdump #283

Merged
merged 4 commits into from
May 10, 2020
Merged

Conversation

julienblitte
Copy link
Contributor

Popular file format for RTP tools to replay streams (telephony, audio, video) from network capture.
http://www.cs.cmu.edu/~./libra-demo/rtptools-1.17/rtptools.html

Copy link
Member

@generalmimon generalmimon left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution, would you read a few minor suggestions I write below?

network/rtpdump.ksy Outdated Show resolved Hide resolved
network/rtpdump.ksy Outdated Show resolved Hide resolved
network/rtpdump.ksy Outdated Show resolved Hide resolved
network/rtpdump.ksy Outdated Show resolved Hide resolved
network/rtpdump.ksy Outdated Show resolved Hide resolved
network/rtpdump.ksy Outdated Show resolved Hide resolved
julienblitte and others added 2 commits May 9, 2020 14:19
remove version, fix comments, naming variable rule

Co-authored-by: Petr Pučil <47499687+generalmimon@users.noreply.github.com>
meta:
id: rtmpdump
title: Rtpdump (rtptools)
file-extension: rtmpdump
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
file-extension: rtmpdump
file-extension: rtp

According to the page http://www.cs.cmu.edu/~./libra-demo/rtptools-1.17/rtptools.html you link, "the extension .rtp is suggested for files generated in rtpdump -F dump format", not .rtmpdump.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks.

Before answering this, I see typos I made:

  • The id should be rtpdump not rtmpdump;
  • Same, the file-extension I wanted originally was rtpdump.

Now, regarding the extension, most of modern tools in the industries (commercial tools) use the .rtpdump extension as well as the modern open source tool Wireshark.

See an example of export on Wireshark, the most used tool today to generate such files:
image

Using only rtp as extension would look a bit conservative as it sticks to the command line rtptools recommendation but differs from people's custom today.

I commit a proposed fix to add the two extensions (original rtp and rtpdump widely in use today). I hope it is fine for your side.

* Fix id typo
* Fix extension typo
* Add recommended extension .rtp
@GreyCat GreyCat merged commit fdeb099 into kaitai-io:master May 10, 2020
@GreyCat
Copy link
Member

GreyCat commented May 10, 2020

Thanks for great work, @julienblitte and @generalmimon for reviewing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants