module-rtp: Move incoming RTP packet checks to rtp_stream_receive_packet()

This fixes code duplication, since the checks are the same regardless
of payload type. hlen is also calculated the same across payload types,
so it is moved as well. The impl->receive_rtp() function pointer then
solely addresses the actual payload processing.

src/modules/module-rtp/audio.c

@@ -324,40 +324,21 @@ static void rtp_audio_process_playback(void *data)
 }
 
 static int rtp_audio_receive(struct impl *impl, uint8_t *buffer, ssize_t len,
-				uint64_t current_time)
+				ssize_t hlen, uint64_t current_time)
 {
 	struct rtp_header *hdr;
-	ssize_t hlen, plen;
+	ssize_t plen;
 	uint16_t seq;
 	uint32_t timestamp, samples, write, expected_write;
 	uint32_t stride = impl->stride;
 	int32_t filled;
 
-	if (len < 12)
-		goto short_packet;
-
 	hdr = (struct rtp_header*)buffer;
-	if (hdr->v != 2)
-		goto invalid_version;
-
-	hlen = 12 + hdr->cc * 4;
-	if (hdr->x) {
-		if (hlen + 4 > len)
-			goto invalid_len;
-		hlen += 4 + ntohs(*(uint16_t *)(buffer + hlen + 2)) * 4;
-	}
-	if (hlen > len)
-		goto invalid_len;
-
-	if (impl->have_ssrc && impl->ssrc != hdr->ssrc)
-		goto unexpected_ssrc;
-	impl->ssrc = hdr->ssrc;
-	impl->have_ssrc = !impl->ignore_ssrc;
 
 	seq = ntohs(hdr->sequence_number);
 	if (impl->have_seq && impl->seq != seq) {
 		pw_log_info("unexpected seq (%d != %d) SSRC:%u",
-				seq, impl->seq, hdr->ssrc);
+				seq, impl->seq, impl->ssrc);
 		/* No need to resynchronize here. If packets arrive out of
 		 * order, then they are still written in order into the ring
 		 * buffer, since they are written according to where the
@@ -468,25 +449,6 @@ static int rtp_audio_receive(struct impl *impl, uint8_t *buffer, ssize_t len,
 	}
 
 	return 0;
-
-short_packet:
-	pw_log_warn("short packet received");
-	return -EINVAL;
-invalid_version:
-	pw_log_warn("invalid RTP version");
-	spa_debug_log_mem(pw_log_get(), SPA_LOG_LEVEL_INFO, 0, buffer, len);
-	return -EPROTO;
-invalid_len:
-	pw_log_warn("invalid RTP length");
-	return -EINVAL;
-unexpected_ssrc:
-	if (!impl->fixed_ssrc) {
-		/* We didn't have a configured SSRC, and there's more than one SSRC on
-		 * this address/port pair */
-		pw_log_warn("unexpected SSRC (expected %u != %u)", impl->ssrc,
-			hdr->ssrc);
-	}
-	return -EINVAL;
 }
 
 static void set_timer(struct impl *impl, uint64_t time, uint64_t itime)

src/modules/module-rtp/midi.c

@@ -310,39 +310,18 @@ static int rtp_midi_receive_midi(struct impl *impl, uint8_t *packet, uint32_t ti
 }
 
 static int rtp_midi_receive(struct impl *impl, uint8_t *buffer, ssize_t len,
-			uint64_t current_time)
+			ssize_t hlen, uint64_t current_time)
 {
 	struct rtp_header *hdr;
-	ssize_t hlen;
 	uint16_t seq;
 	uint32_t timestamp;
 
-	SPA_STATIC_ASSERT(sizeof(struct rtp_header) == 12);
-	if (len < 12)
-		goto short_packet;
-
 	hdr = (struct rtp_header*)buffer;
-	if (hdr->v != 2)
-		goto invalid_version;
-
-	hlen = 12 + hdr->cc * 4;
-	if (hdr->x) {
-		if (hlen + 4 >= len)
-			goto invalid_len;
-		hlen += 4 + ntohs(*(uint16_t *)(buffer + hlen + 2)) * 4;
-	}
-	if (hlen >= len)
-		goto invalid_len;
-
-	if (impl->have_ssrc && impl->ssrc != hdr->ssrc)
-		goto unexpected_ssrc;
-	impl->ssrc = hdr->ssrc;
-	impl->have_ssrc = !impl->ignore_ssrc;
 
 	seq = ntohs(hdr->sequence_number);
 	if (impl->have_seq && impl->seq != seq) {
 		pw_log_info("unexpected seq (%d != %d) SSRC:%u",
-				seq, impl->seq, hdr->ssrc);
+				seq, impl->seq, impl->ssrc);
 		impl->have_sync = false;
 	}
 	impl->seq = seq + 1;
@@ -353,25 +332,6 @@ static int rtp_midi_receive(struct impl *impl, uint8_t *buffer, ssize_t len,
 	impl->receiving = true;
 
 	return rtp_midi_receive_midi(impl, buffer, timestamp, seq, hlen, len);
-
-short_packet:
-	pw_log_warn("short packet received");
-	return -EINVAL;
-invalid_version:
-	pw_log_warn("invalid RTP version");
-	spa_debug_log_mem(pw_log_get(), SPA_LOG_LEVEL_INFO, 0, buffer, len);
-	return -EPROTO;
-invalid_len:
-	pw_log_warn("invalid RTP length");
-	return -EINVAL;
-unexpected_ssrc:
-	if (!impl->fixed_ssrc) {
-		/* We didn't have a configured SSRC, and there's more than one SSRC on
-		* this address/port pair */
-		pw_log_warn("unexpected SSRC (expected %u != %u)",
-			impl->ssrc, hdr->ssrc);
-	}
-	return -EINVAL;
 }
 
 static int write_event(uint8_t *p, uint32_t buffer_size, uint32_t delta, const uint8_t *ev, uint32_t size)

src/modules/module-rtp/opus.c

@@ -100,10 +100,10 @@ static void rtp_opus_process_playback(void *data)
 }
 
 static int rtp_opus_receive(struct impl *impl, uint8_t *buffer, ssize_t len,
-			uint64_t current_time)
+			ssize_t hlen, uint64_t current_time)
 {
 	struct rtp_header *hdr;
-	ssize_t hlen, plen;
+	ssize_t plen;
 	uint16_t seq;
 	uint32_t timestamp, samples, write, expected_write;
 	uint32_t stride = impl->stride;
@@ -111,31 +111,12 @@ static int rtp_opus_receive(struct impl *impl, uint8_t *buffer, ssize_t len,
 	int32_t filled;
 	int res;
 
-	if (len < 12)
-		goto short_packet;
-
 	hdr = (struct rtp_header*)buffer;
-	if (hdr->v != 2)
-		goto invalid_version;
-
-	hlen = 12 + hdr->cc * 4;
-	if (hdr->x) {
-		if (hlen + 4 > len)
-			goto invalid_len;
-		hlen += 4 + ntohs(*(uint16_t *)(buffer + hlen + 2)) * 4;
-	}
-	if (hlen > len)
-		goto invalid_len;
-
-	if (impl->have_ssrc && impl->ssrc != hdr->ssrc)
-		goto unexpected_ssrc;
-	impl->ssrc = hdr->ssrc;
-	impl->have_ssrc = !impl->ignore_ssrc;
 
 	seq = ntohs(hdr->sequence_number);
 	if (impl->have_seq && impl->seq != seq) {
 		pw_log_info("unexpected seq (%d != %d) SSRC:%u",
-				seq, impl->seq, hdr->ssrc);
+				seq, impl->seq, impl->ssrc);
 		impl->have_sync = false;
 	}
 	impl->seq = seq + 1;
@@ -196,25 +177,6 @@ static int rtp_opus_receive(struct impl *impl, uint8_t *buffer, ssize_t len,
 		spa_ringbuffer_write_update(&impl->ring, write);
 	}
 	return 0;
-
-short_packet:
-	pw_log_warn("short packet received");
-	return -EINVAL;
-invalid_version:
-	pw_log_warn("invalid RTP version");
-	spa_debug_log_mem(pw_log_get(), SPA_LOG_LEVEL_INFO, 0, buffer, len);
-	return -EPROTO;
-invalid_len:
-	pw_log_warn("invalid RTP length");
-	return -EINVAL;
-unexpected_ssrc:
-	if (!impl->fixed_ssrc) {
-		/* We didn't have a configured SSRC, and there's more than one SSRC on
-		* this address/port pair */
-		pw_log_warn("unexpected SSRC (expected %u != %u)",
-			impl->ssrc, hdr->ssrc);
-	}
-	return -EINVAL;
 }
 
 static void rtp_opus_flush_packets(struct impl *impl)

src/modules/module-rtp/stream.c

@@ -170,7 +170,7 @@ struct impl {
 	uint8_t timer_running;
 
 	int (*receive_rtp)(struct impl *impl, uint8_t *buffer, ssize_t len,
-			uint64_t current_time);
+			ssize_t hlen, uint64_t current_time);
 	/* Used for resetting the ring buffer before the stream starts, to prevent
 	 * reading from uninitialized memory. This can otherwise happen in direct
 	 * timestamp mode when the read index is set to an uninitialized location.
@@ -1080,7 +1080,51 @@ int rtp_stream_receive_packet(struct rtp_stream *s, uint8_t *buffer, size_t len,
 				uint64_t current_time)
 {
 	struct impl *impl = (struct impl*)s;
-	return impl->receive_rtp(impl, buffer, len, current_time);
+	struct rtp_header *hdr;
+	ssize_t hlen;
+
+	SPA_STATIC_ASSERT(sizeof(struct rtp_header) == 12);
+	if (len < 12)
+		goto short_packet;
+
+	hdr = (struct rtp_header*)buffer;
+	if (hdr->v != 2)
+		goto invalid_version;
+
+	hlen = 12 + hdr->cc * 4;
+	if (hdr->x) {
+		if (hlen + 4 > (ssize_t)len)
+			goto invalid_len;
+		hlen += 4 + ntohs(*(uint16_t *)(buffer + hlen + 2)) * 4;
+	}
+	if (hlen > (ssize_t)len)
+		goto invalid_len;
+
+	if (impl->have_ssrc && impl->ssrc != hdr->ssrc)
+		goto unexpected_ssrc;
+	impl->ssrc = hdr->ssrc;
+	impl->have_ssrc = !impl->ignore_ssrc;
+
+	return impl->receive_rtp(impl, buffer, len, hlen, current_time);
+
+short_packet:
+	pw_log_warn("short packet received");
+	return -EINVAL;
+invalid_version:
+	pw_log_warn("invalid RTP version");
+	spa_debug_log_mem(pw_log_get(), SPA_LOG_LEVEL_INFO, 0, buffer, len);
+	return -EPROTO;
+invalid_len:
+	pw_log_warn("invalid RTP length");
+	return -EINVAL;
+unexpected_ssrc:
+	if (!impl->fixed_ssrc) {
+		/* We didn't have a configured SSRC, and there's more than one SSRC on
+		 * this address/port pair */
+		pw_log_warn("unexpected SSRC (expected %u != %u)", impl->ssrc,
+			hdr->ssrc);
+	}
+	return -EINVAL;
 }
 int rtp_stream_resend_packets(struct rtp_stream *s, uint16_t seq, uint16_t num)
 {