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

RTPStream/PCAP: Support variables for filename #521

Merged
merged 1 commit into from
Oct 25, 2021
Merged

RTPStream/PCAP: Support variables for filename #521

merged 1 commit into from
Oct 25, 2021
+18 −22

Conversation

orgads
Copy link
Contributor

@orgads orgads commented Apr 12, 2021

Allow using [fieldX] and all types of variables on playback.

@orgads
Copy link
Contributor Author

orgads commented Apr 12, 2021

Please review carefully. I probably missed something.

Works for me™.

@orgads
Copy link
Contributor Author

orgads commented May 23, 2021

ping

@orgads
Copy link
Contributor Author

orgads commented Jul 12, 2021

ping?

@wdoekes
Copy link
Member

wdoekes commented Oct 25, 2021

I'm pretty sure this breaks the original usage. Seeing the -2 to strip CRLFs which should not be in there.

@wdoekes wdoekes added this to the not-doing-now milestone Oct 25, 2021
@orgads
Copy link
Contributor Author

orgads commented Oct 25, 2021

Why shouldn't it be there? I copied it from https://github.com/SIPp/sipp/blob/master/src/call.cpp#L5844 and many more similar usages.

And why do you expect it to break the normal usage?

@orgads
RTPStream/PCAP: Support variables for filename
cc4d64a 
Allow using [fieldX] and all types of variables on playback.
@orgads
Copy link
Contributor Author

orgads commented Oct 25, 2021

Rebased

@wdoekes
Copy link
Member

wdoekes commented Oct 25, 2021

Urgh. I stand corrected.

createSendingMessage() should be called something like renderTemplate and that -2 should be WITHOUT_CRLF. Although I don't quite see how it does that either. Looks to me like that -2 is totally irrelevant apart from E_Message_Branch and E_Message_Index usage.

Ok, I guess then it works. I thought it was actually outputting lines from a CSV and trimming 2 characters... silly me.

(Additionally I wonder if all those static char's aren't causing threading issues. But that's irrelevant for this case.)

@wdoekes
Copy link
Member

wdoekes commented Oct 25, 2021

Apparently that was already altered in 29e648e.

Nice stuff... 🙄

@wdoekes wdoekes merged commit d8899fc into SIPp:master Oct 25, 2021
@orgads orgads deleted the play-variables branch October 25, 2021 13:33
@orgads orgads mentioned this pull request Nov 17, 2022
Open
rkday added a commit that referenced this pull request Nov 16, 2023
@rkday
Remove fprintf causing log spam
defe68f 
Since #521, prepare_pkts may be called
on every call, not just on startup. This means that the log it prints to
stderr is very visible and may cause performance problems. This commit
removes it.
@rkday rkday mentioned this pull request Nov 16, 2023
Merged
rkday added a commit that referenced this pull request Nov 16, 2023
@rkday
Remove fprintf causing log spam (#669)

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
e3b7748 
Since #521, prepare_pkts may be called
on every call, not just on startup. This means that the log it prints to
stderr is very visible and may cause performance problems. This commit
removes it.
This was referenced Mar 19, 2024
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
not-doing-now
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@orgads @wdoekes