[Asterisk-code-review] ast format: Adds an identifier for interleaved audio formats... (asterisk[master])

[Joshua Colp]

Joshua Colp has posted comments on this change. ( https://gerrit.asterisk.org/4322 )

Change subject: ast_format: Adds an identifier for interleaved audio formats to the ast_format
......................................................................


Patch Set 1: Code-Review-1

(3 comments)

https://gerrit.asterisk.org/#/c/4322/1/main/format_cap.c
File main/format_cap.c:

PS1, Line 640: 		if (ast_format_get_channel_count(framed->format) > 1) {
             : 			ast_format_set_channel_count(format, ast_format_get_channel_count(framed->format));
             : 		}
I don't think this is the right place for this. If you follow the logic down ast_format_cap_get_compatible_format calls into ast_format_joint which, if attribute data is present, then calls into the format interface to get a joint format. I think it should be the responsibility of the format interface (res_format_attr_opus) to return a format with this set on it.

In fact it uses the ast_format_clone function which is supposed to clone an existing format. That function needs to also copy the channel count value. Right now it is missing that, so any cloned format will be incorrect.


https://gerrit.asterisk.org/#/c/4322/1/res/res_format_attr_opus.c
File res/res_format_attr_opus.c:

Line 149: 		ast_format_set_channel_count(cloned, 2);
This should be enclosed in { and }


https://gerrit.asterisk.org/#/c/4322/1/res/res_pjsip_sdp_rtp.c
File res/res_pjsip_sdp_rtp.c:

Line 272: 					ast_format_set_channel_count(format, ast_format_get_channel_count(format_parsed));
'format' is immutable. You can not alter it. The format_parsed format replaces it and should have the correct channel count on it already.


-- 
To view, visit https://gerrit.asterisk.org/4322
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I359801cc5f98c35671c48dabc81a7f4ee1183d63
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Frank Haase <fra.haase at googlemail.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-HasComments: Yes