Skip to content
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

[core] Minor code cleanup in processData() #749

Merged
merged 1 commit into from
Jul 15, 2019

Conversation

maxsharabayko
Copy link
Collaborator

No description provided.

@maxsharabayko maxsharabayko added Priority: Low [core] Area: Changes in SRT library core labels Jul 4, 2019
@maxsharabayko maxsharabayko added this to the v.1.3.3 milestone Jul 4, 2019
srtcore/core.cpp Outdated

bool excessive = false;
string exc_type = "EXPECTED";
if ((offset < 0))
#ifdef ENABLE_HEAVY_LOGGING
Copy link
Collaborator

@ethouris ethouris Jul 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kinda overspoken. In one of the works I created somewhere a type named DebugString, which turned into a nothing and ignoring all operations if compiling without heavy logging. In this particular case maybe it would be easier something like this:

#if ENABLE_HEAVY_LOGGING
#define IF_HEAVY_LOGGING(instr) instr
#else 
#define IF_HEAVY_LOGGING(instr) (void)0
#endif 

This will work, as long as the instruction passed inside contains no commas.

Note also that HLOGC macro resolves to nothing if ENABLE_HEAVY_LOGGING is not defined, so you can safely use nonexistent symbols inside this macro call, as long as they exist under this condition.

int32_t seqlo = CSeqNo::incseq(m_iRcvCurrSeqNo);
int32_t seqhi = CSeqNo::decseq(packet.m_iSeqNo);
// If loss found, insert them to the receiver loss list
// TODO: Can unlock rcvloss after m_pRcvLossList->insert(...)?
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

m_FreshLoss probably should be also protected. Otherwise it can be unlocked.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would leave it as a TODO, because this PR should not introduce changes, only minor code cleanup. Added you note to the comment.

@maxsharabayko maxsharabayko modified the milestones: v.1.3.3, v.1.3.4 Jul 5, 2019
@rndi rndi merged commit 9e34fe5 into Haivision:master Jul 15, 2019
@maxsharabayko maxsharabayko deleted the develop/processData branch July 19, 2019 13:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[core] Area: Changes in SRT library core Priority: Low
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants