Patch fails on asterisk 13.6.0

traud / asterisk-amr

Asterisk 13 transcoding module: AMR-WB

The Unlicense

32 stars 19 forks source link

Patch fails on asterisk 13.6.0 #3

Closed mtryfoss closed 8 years ago

mtryfoss commented 8 years ago

Great work so far!

However, this part failed:

--- main/rtp_engine.c (Asterisk 13.5.0) +++ main/rtp_engine.c (working copy) @@ -666 +666,5 @@

new_type->format = ao2_bump(static_RTP_PT[payload].format);
new_type->format = static_RTP_PT[payload].format;
if (new_type->format) {
new_type->format = ast_format_parse_sdp_fmtp(new_type->format, "");
}
new_type->format = ao2_bump(new_type->format);

traud commented 8 years ago

Yes, thank for opening an issue for this. That code part changed with Asterisk 13.6.0. However actually, this part of the patch (hunk) is not required for AMR, because AMR is a dynamic payload and has a=rtpmap always. Consequently, it is safe to ignore this issue until I create a new version of that patch.

mtryfoss commented 8 years ago

Great! We're going to test this patch against an Ericsson MSC/AXE running AMR(-NB). Later when the switch gets license WB I can test that too. Will let you know about any issues.

mtryfoss commented 8 years ago

One more thing regarding the patch. I don't know if asterisk-opus is required for this patch. I suppose not. This patch seems to have been done against one with opus applied.

Mine is already has opus so it worked well, but I think it will probably fail against a standard asterisk.

For example: @@ -2188,2 +2190,4 @@ add_static_payload(107, ast_format_opus, 0);

add_static_payload(108, ast_format_amr, 0);
add_static_payload(109, ast_format_amrwb, 0);

traud commented 8 years ago

Thanks for the note. However, Asterisk 13 includes support for pass-through Opus already. I think that might have confused you. Although I use the Opus transcoding module, that patch was tested with a vanilla Asterisk 13 as well.

traud commented 8 years ago

Thank you again for reporting! While investigating, I was able to find a memory leak. I am changing that and reduce the patch to the part required by AMR. Then, everything is compatible with Asterisk 13.6.0 again. By the way, the initial patch is now part of ASTERISK-25537. Again, the first part patch was not required for AMR, because AMR uses a dynamic RTP payload ID.