From df9033dcb9d2b5ee8ef521b75abc32cda6498827 Mon Sep 17 00:00:00 2001 From: iphydf Date: Sun, 12 Aug 2018 21:53:59 +0000 Subject: [PATCH] Fix coding style in rtp module. * Named callback types only. * No anonymous enums or structs. * `++i` instead of `i++`. * Don't use enums to specify integer constants. Enums should be enumerations. All values of an enum type should be listed[1]. [1] I don't know what to do about bit masks yet, but given that enums by C standard can only go up to 32767 portably and 2^31 in reality, they are probably not useful for 64 bit bit masks. --- toxav/rtp.c | 63 +++++++++++++++++++++++++---------------------------- toxav/rtp.h | 15 +++++++------ 2 files changed, 38 insertions(+), 40 deletions(-) diff --git a/toxav/rtp.c b/toxav/rtp.c index ca4a4a7b..61297937 100644 --- a/toxav/rtp.c +++ b/toxav/rtp.c @@ -35,13 +35,11 @@ #include "../toxcore/mono_time.h" #include "../toxcore/util.h" -enum { - /** - * The number of milliseconds we want to keep a keyframe in the buffer for, - * even though there are no free slots for incoming frames. - */ - VIDEO_KEEP_KEYFRAME_IN_BUFFER_FOR_MS = 15, -}; +/** + * The number of milliseconds we want to keep a keyframe in the buffer for, + * even though there are no free slots for incoming frames. + */ +#define VIDEO_KEEP_KEYFRAME_IN_BUFFER_FOR_MS 15 // allocate_len is NOT including header! static struct RTPMessage *new_message(const struct RTPHeader *header, size_t allocate_len, const uint8_t *data, @@ -60,16 +58,15 @@ static struct RTPMessage *new_message(const struct RTPHeader *header, size_t all return msg; } -enum { - /** - * Instruct the caller to clear slot 0. - */ - GET_SLOT_RESULT_DROP_OLDEST_SLOT = -1, - /** - * Instruct the caller to drop the incoming packet. - */ - GET_SLOT_RESULT_DROP_INCOMING = -2, -}; +/** + * Instruct the caller to clear slot 0. + */ +#define GET_SLOT_RESULT_DROP_OLDEST_SLOT (-1) + +/** + * Instruct the caller to drop the incoming packet. + */ +#define GET_SLOT_RESULT_DROP_INCOMING (-2) /** * Find the next free slot in work_buffer for the incoming data packet. @@ -89,7 +86,7 @@ static int8_t get_slot(const Logger *log, struct RTPWorkBufferList *wkbl, bool i if (is_multipart) { // This RTP message is part of a multipart frame, so we try to find an // existing slot with the previous parts of the frame in it. - for (uint8_t i = 0; i < wkbl->next_free_entry; i++) { + for (uint8_t i = 0; i < wkbl->next_free_entry; ++i) { const struct RTPWorkBuffer *slot = &wkbl->work_buffer[i]; if ((slot->buf->header.sequnum == header->sequnum) && (slot->buf->header.timestamp == header->timestamp)) { @@ -228,14 +225,14 @@ static struct RTPMessage *process_frame(const Logger *log, struct RTPWorkBufferL if (slot_id != wkbl->next_free_entry - 1) { // The slot is not the last slot, so we created a gap. We move all the // entries after it one step up. - for (uint8_t i = slot_id; i < wkbl->next_free_entry - 1; i++) { + for (uint8_t i = slot_id; i < wkbl->next_free_entry - 1; ++i) { // Move entry (i+1) into entry (i). wkbl->work_buffer[i] = wkbl->work_buffer[i + 1]; } } // We now have a free entry at the end of the array. - wkbl->next_free_entry--; + --wkbl->next_free_entry; // Clear the newly freed entry. const struct RTPWorkBuffer empty = {0}; @@ -290,7 +287,7 @@ static bool fill_data_into_slot(const Logger *log, struct RTPWorkBufferList *wkb slot->received_len = 0; assert(wkbl->next_free_entry < USED_RTP_WORKBUFFER_COUNT); - wkbl->next_free_entry++; + ++wkbl->next_free_entry; } // We already checked this when we received the packet, but we rely on it @@ -317,7 +314,7 @@ static bool fill_data_into_slot(const Logger *log, struct RTPWorkBufferList *wkb static void update_bwc_values(const Logger *log, RTPSession *session, const struct RTPMessage *msg) { if (session->first_packets_counter < DISMISS_FIRST_LOST_VIDEO_PACKET_COUNT) { - session->first_packets_counter++; + ++session->first_packets_counter; } else { uint32_t data_length_full = msg->header.data_length_full; // without header uint32_t received_length_full = msg->header.received_length_full; // without header @@ -586,12 +583,14 @@ NEW_MULTIPARTED: size_t rtp_header_pack(uint8_t *const rdata, const struct RTPHeader *header) { uint8_t *p = rdata; - *p++ = (header->ve & 3) << 6 - | (header->pe & 1) << 5 - | (header->xe & 1) << 4 - | (header->cc & 0xf); - *p++ = (header->ma & 1) << 7 - | (header->pt & 0x7f); + *p = (header->ve & 3) << 6 + | (header->pe & 1) << 5 + | (header->xe & 1) << 4 + | (header->cc & 0xf); + ++p; + *p = (header->ma & 1) << 7 + | (header->pt & 0x7f); + ++p; p += net_pack_u16(p, header->sequnum); p += net_pack_u32(p, header->timestamp); @@ -601,7 +600,7 @@ size_t rtp_header_pack(uint8_t *const rdata, const struct RTPHeader *header) p += net_pack_u32(p, header->data_length_full); p += net_pack_u32(p, header->received_length_full); - for (size_t i = 0; i < RTP_PADDING_FIELDS; i++) { + for (size_t i = 0; i < RTP_PADDING_FIELDS; ++i) { p += net_pack_u32(p, 0); } @@ -640,10 +639,8 @@ size_t rtp_header_unpack(const uint8_t *data, struct RTPHeader *header) return p - data; } - RTPSession *rtp_new(int payload_type, Messenger *m, uint32_t friendnumber, - BWController *bwc, void *cs, - int (*mcb)(void *, struct RTPMessage *)) + BWController *bwc, void *cs, rtp_m_cb *mcb) { assert(mcb != nullptr); assert(cs != nullptr); @@ -851,6 +848,6 @@ int rtp_send_data(RTPSession *session, const uint8_t *data, uint32_t length, } } - session->sequnum ++; + ++session->sequnum; return 0; } diff --git a/toxav/rtp.h b/toxav/rtp.h index e1cf7950..225f4c03 100644 --- a/toxav/rtp.h +++ b/toxav/rtp.h @@ -45,16 +45,16 @@ extern "C" { /** * Payload type identifier. Also used as rtp callback prefix. */ -enum { +typedef enum RTP_Type { RTP_TYPE_AUDIO = 192, RTP_TYPE_VIDEO = 193, -}; +} RTP_Type; /** * A bit mask (up to 64 bits) specifying features of the current frame affecting * the behaviour of the decoder. */ -enum RTPFlags { +typedef enum RTPFlags { /** * Support frames larger than 64KiB. The full 32 bit length and offset are * set in \ref RTPHeader::data_length_full and \ref RTPHeader::offset_full. @@ -64,7 +64,7 @@ enum RTPFlags { * Whether the packet is part of a key frame. */ RTP_KEY_FRAME = 1 << 1, -}; +} RTPFlags; struct RTPHeader { @@ -159,6 +159,8 @@ struct RTPWorkBufferList { #define DISMISS_FIRST_LOST_VIDEO_PACKET_COUNT 10 +typedef int rtp_m_cb(void *cs, struct RTPMessage *msg); + /** * RTP control session. */ @@ -175,7 +177,7 @@ typedef struct RTPSession { uint32_t friend_number; BWController *bwc; void *cs; - int (*mcb)(void *, struct RTPMessage *msg); + rtp_m_cb *mcb; } RTPSession; @@ -198,8 +200,7 @@ size_t rtp_header_pack(uint8_t *rdata, const struct RTPHeader *header); size_t rtp_header_unpack(const uint8_t *data, struct RTPHeader *header); RTPSession *rtp_new(int payload_type, Messenger *m, uint32_t friendnumber, - BWController *bwc, void *cs, - int (*mcb)(void *, struct RTPMessage *)); + BWController *bwc, void *cs, rtp_m_cb *mcb); void rtp_kill(RTPSession *session); int rtp_allow_receiving(RTPSession *session); int rtp_stop_receiving(RTPSession *session);