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

pcaputil: Support parsing decoder options from FMTP input #4205

Merged
merged 1 commit into from
Dec 12, 2024

Conversation

orgads
Copy link
Contributor

@orgads orgads commented Dec 9, 2024

pcaputil: Support parsing decoder options from FMTP input

Some codecs need specific flags that are given in the SDP media description. For example, AMR has octet-align=1.

Instead of adding distinct flags for each codec, allow pcaputil to accept the fmtp content and parse the attributes from there.

Example:

pcaputil \
  --dst-port=52422 \
  --codec=AMR/8000 \
  --codec-fmtp='mode-set=0,1,2,3,4,5,6,7;octet-align=1' \
  amr.pcap amr.wav

@sauwming
Copy link
Member

It's rather strange to pass the entire SDP as a parameter.
Why not just pass the codec specific parameters only?
e.g. --codec-fmtp='octet-align=1' --codec-fmtp='mode-set=0,1,2,3,4,5,6,7'

@orgads
Copy link
Contributor Author

orgads commented Dec 10, 2024

How do I parse it and set the params in dec_fmtp?

@sauwming
Copy link
Member

https://github.com/pjsip/pjproject/blob/master/pjmedia/src/pjmedia/stream_common.c#L110

You can make it simpler by requiring the parameter to have no whitespace, so:

  • find '='
  • if '=' exists {anything before '=' is fmtp name, anything after '=' is fmtp val } else {everything is fmtp val}

@orgads
Copy link
Contributor Author

orgads commented Dec 10, 2024

https://github.com/pjsip/pjproject/blob/master/pjmedia/src/pjmedia/stream_common.c#L110

You can make it simpler by requiring the parameter to have no whitespace, so:

  • find '='
  • if '=' exists {anything before '=' is fmtp name, anything after '=' is fmtp val } else {everything is fmtp val}

So do you want me to factor this out? What about passing this to the codec? Should I also factor out parts of pjmedia_stream_info_parse_fmtp?

@sauwming
Copy link
Member

What I mean is to add new parameter --codec-fmtp and provide very simple parsing in pcaputil to put it in dec_fmtp.

@orgads orgads force-pushed the pcaputil-sdp-media branch 2 times, most recently from 9ff932e to ac0cefa Compare December 11, 2024 10:55
@orgads orgads changed the title pcaputil: Support parsing decoder options from SDP media pcaputil: Support parsing decoder options from FMTP input Dec 11, 2024
@orgads orgads force-pushed the pcaputil-sdp-media branch from ac0cefa to 4c99091 Compare December 11, 2024 12:12
@orgads
Copy link
Contributor Author

orgads commented Dec 11, 2024

What I mean is to add new parameter --codec-fmtp and provide very simple parsing in pcaputil to put it in dec_fmtp.

Done, with a small refactoring.

@orgads
Copy link
Contributor Author

orgads commented Dec 11, 2024

The failed test looks unrelated to my changes. @sauwming please review.

pjsip-apps/src/samples/pcaputil.c Show resolved Hide resolved
Some codecs need specific flags that are given in the SDP media
description. For example, AMR has octet-align=1.

Instead of adding distinct flags for each codec, allow pcaputil
to accept the fmtp content and parse the attributes from there.

Example:
pcaputil \
  --dst-port=52422 \
  --codec=AMR/8000 \
  --codec-fmtp='mode-set=0,1,2,3,4,5,6,7;octet-align=1' \
  amr.pcap amr.wav
@orgads orgads force-pushed the pcaputil-sdp-media branch from 4c99091 to d20063d Compare December 12, 2024 06:00
@sauwming sauwming merged commit 484c6b6 into pjsip:master Dec 12, 2024
36 checks passed
@orgads orgads deleted the pcaputil-sdp-media branch December 12, 2024 07:40
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.

4 participants