-
Notifications
You must be signed in to change notification settings - Fork 286
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor: Make ToxAV independent of toxcore internals. #2651
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2651 +/- ##
==========================================
- Coverage 73.08% 73.04% -0.04%
==========================================
Files 149 149
Lines 30531 30479 -52
==========================================
- Hits 22313 22264 -49
+ Misses 8218 8215 -3 ☔ View full report in Codecov by Sentry. |
e59814d
to
8a4092b
Compare
This is ready for review. |
8a4092b
to
55a8294
Compare
e0896da
to
756ee54
Compare
eb8a05a
to
5b737a8
Compare
5b737a8
to
59d3feb
Compare
664ee5f
to
560cd96
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
toxav_old.c
uses the Messenger
logger, instead the ToxAV
logger.
edit: I see that they are still more tightly coupled to messenger and conferences. Maybe we just remove them for v0.3 ?
free(session); | ||
return nullptr; | ||
} | ||
|
||
// First entry is free. | ||
session->work_buffer_list->next_free_entry = 0; | ||
|
||
session->ssrc = payload_type == RTP_TYPE_VIDEO ? 0 : random_u32(m->rng); | ||
session->ssrc = payload_type == RTP_TYPE_VIDEO ? 0 : rtp_random_u32(); // Zoff: what is this?? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
according to https://www.rfc-editor.org/rfc/rfc3550 , this is used to disambiguate multiple sessions. However we don't use this (yet?).
rc = TOXAV_ERR_NEW_MALLOC; | ||
goto RETURN; | ||
} | ||
|
||
av->log = tox->m->log; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
meh
dac5f88
to
d4978a3
Compare
|
||
if (length - 1 != sizeof(struct BWCMessage)) { | ||
return -1; | ||
LOGGER_ERROR(log, "Actual data size and length argument do not match"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sizeof(struct xxx)
is an unreliable number that's implementation defined. We want 2 u32 instead.
Also, the log reads kinda wrong.
@@ -5,19 +5,24 @@ | |||
#ifndef C_TOXCORE_TOXAV_BWCONTROLLER_H | |||
#define C_TOXCORE_TOXAV_BWCONTROLLER_H | |||
|
|||
#include "../toxcore/Messenger.h" | |||
#include <stdint.h> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
standard headers always after your own headers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just realized that this problem plagues the whole of toxcore <.<
typedef void m_msi_packet_cb(Messenger *m, uint32_t friend_number, const uint8_t *data, uint16_t length, | ||
void *user_data); | ||
typedef int m_lossy_rtp_packet_cb(Messenger *m, uint32_t friend_number, const uint8_t *data, uint16_t length, void *object); | ||
typedef int m_lossy_rtp_packet_cb(Messenger *m, uint32_t friend_number, const uint8_t *data, uint16_t len, void *object); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
len? -> length
d4978a3
to
3297b62
Compare
This change is