[Asterisk-code-review] test_res_rtp: Enable FIR and REMB nominal tests. (...asterisk[16])

[Joshua Colp]

Joshua Colp has submitted this change and it was merged. ( https://gerrit.asterisk.org/c/asterisk/+/13055 )

Change subject: test_res_rtp: Enable FIR and REMB nominal tests.
......................................................................

test_res_rtp: Enable FIR and REMB nominal tests.

Now that both FIR and REMB are being sent in compound packets
these tests can be enabled.

This also extends the REMB nominal test to cover the REMB
contents itself.

Change-Id: Ibfee526ad780eefcce5dd787f53785382210024a
---
M tests/test_res_rtp.c
1 file changed, 13 insertions(+), 13 deletions(-)

Approvals:
  George Joseph: Looks good to me, but someone else must approve
  Benjamin Keith Ford: Looks good to me, but someone else must approve
  Joshua Colp: Looks good to me, approved; Approved for Submit



diff --git a/tests/test_res_rtp.c b/tests/test_res_rtp.c
index ecedb4f..08c9971 100644
--- a/tests/test_res_rtp.c
+++ b/tests/test_res_rtp.c
@@ -327,6 +327,7 @@
 		.data.ptr = &feedback,
 		.datalen = sizeof(feedback),
 	};
+	struct ast_rtp_rtcp_feedback *received_feedback;
 
 	switch (cmd) {
 	case TEST_INIT:
@@ -337,8 +338,7 @@
 			"Tests sending and receiving a REMB packet";
 		return AST_TEST_NOT_RUN;
 	case TEST_EXECUTE:
-		/* Disable for now - there's a bug! */
-		return AST_TEST_NOT_RUN;
+		break;
 	}
 
 	test_sched = ast_sched_context_create();
@@ -353,12 +353,7 @@
 
 	ast_rtp_instance_write(instance1, &frame_out);
 
-	/*
-	 * There may be some additional work that needs to be done here, depending on how
-	 * Asterisk handles the reading in of compound packets. We might get an ast_null_frame
-	 * here instead of the REMB frame. We'll need to check the frametype to distinguish
-	 * between them (AST_FRAME_NULL for ast_null_frame, AST_FRAME_RTCP for REMB).
-	 */
+	/* Verify the high level aspects of the frame */
 	frame_in = ast_rtp_instance_read(instance2, 0);
 	ast_test_validate(test, frame_in != NULL, "Did not receive a REMB frame");
 	ast_test_validate(test, frame_in->frametype == AST_FRAME_RTCP,
@@ -366,6 +361,15 @@
 	ast_test_validate(test, frame_in->subclass.integer == AST_RTP_RTCP_PSFB,
 		"REMB frame did not have the expected subclass integer");
 
+	/* Verify the actual REMB information itself */
+	received_feedback = frame_in->data.ptr;
+	ast_test_validate(test, received_feedback->fmt == AST_RTP_RTCP_FMT_REMB,
+		"REMB frame did not have the expected feedback format");
+	ast_test_validate(test, received_feedback->remb.br_exp == feedback.remb.br_exp,
+		"REMB received exponent did not match sent exponent");
+	ast_test_validate(test, received_feedback->remb.br_mantissa == feedback.remb.br_mantissa,
+		"REMB received mantissa did not match sent mantissa");
+
 	return AST_TEST_PASS;
 }
 
@@ -447,8 +451,7 @@
 			"Tests sending and receiving a FIR packet";
 		return AST_TEST_NOT_RUN;
 	case TEST_EXECUTE:
-		/* Disable for now - there's a bug! */
-		return AST_TEST_NOT_RUN;
+		break;
 	}
 
 	test_sched = ast_sched_context_create();
@@ -475,9 +478,6 @@
 	 * We only receive one frame, the FIR request. It won't have a subclass integer of
 	 * 206 (PSFB) because ast_rtcp_interpret sets it to 18 (AST_CONTROL_VIDUPDATE), so
 	 * check for that.
-	 *
-	 * NOTE - similar to REMB, there may be more that needs to be done here when the
-	 * packet is sent as a compound packet!
 	 */
 	frame_in = ast_rtp_instance_read(instance2, 0);
 	ast_test_validate(test, frame_in != NULL, "Did not receive a FIR frame");

-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/13055
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-Change-Id: Ibfee526ad780eefcce5dd787f53785382210024a
Gerrit-Change-Number: 13055
Gerrit-PatchSet: 1
Gerrit-Owner: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Benjamin Keith Ford <bford at digium.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20191016/28a6041a/attachment-0001.html>