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

Small improvements of CSndLossList::insert #1148

Merged

Conversation

maxsharabayko
Copy link
Collaborator

  1. Added several unit tests.
  2. Minor refactoring of the CSndLossList::insert(..).
  3. Improved CSndLossList::insert(..) for the serial element.

Example of the improvement. Three insert operations:

insert(1, 2);
insert(4, 4);
insert(3, 3);

The state of the list after every insert operation was (format pos:[seqstart, seqend]):

0:[1, 2]
0:[1, 2], 3:[4]
0:[1, 2], 2:[3], 3:[4]

After this PR:

0:[1, 2], 
0:[1, 2], 3:[4] 
0:[1, 4] 

Taken from TEST_F(CSndLossListTest, InsertCoalesce)

@maxsharabayko maxsharabayko added Type: Maintenance Work required to maintain or clean up the code [core] Area: Changes in SRT library core labels Feb 23, 2020
@maxsharabayko maxsharabayko added this to the v1.5.0 milestone Feb 23, 2020
@maxsharabayko maxsharabayko added this to Backlog in Development via automation Feb 23, 2020
int pos = m_iHead;
while (pos != -1)
{
::cout << pos << ":[" << m_caSeq[pos].seqstart;
Copy link
Collaborator

Choose a reason for hiding this comment

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

It would be better to use LOG* facility to display it. Even if this is only left for future debugging.

Of course, you'd need std::ostringstream for intermediate collection, as a single log instruction cannot print a fragment of a line.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Can LOGF not end the line with \n like Verb() allows to do?

Copy link
Collaborator

@ethouris ethouris Feb 25, 2020

Choose a reason for hiding this comment

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

Unfortunately not. One log instruction always ends with an end of line. Partial-line logging would have to be added support.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Then would be simpler to leave this type of output for the sake of code simplicity.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Verb() has a special VerbNoEOL tag to leave without EOL. But Verb() doesn't have a log header, so there's a little problem with this. For LOG* we'd need something like a temporary log cumulation variable, which will be then reused when doing a final log with EOL. But I don't think we could this way do any better than by using std::ostringstream.

Development automation moved this from Backlog to Reviewer Approved Feb 25, 2020
@maxsharabayko maxsharabayko merged commit 93d7601 into Haivision:master Feb 26, 2020
Development automation moved this from Reviewer Approved to Done Feb 26, 2020
@maxsharabayko maxsharabayko deleted the develop/snd-loss-list-insert branch February 26, 2020 08:40
@mbakholdina mbakholdina modified the milestones: v1.5.0, v1.4.2 Oct 14, 2020
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 Type: Maintenance Work required to maintain or clean up the code
Projects
No open projects
Development
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants