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

Fixed bug: vertical FEC/CTL packet was missing when both H/V were to be done #965

Merged
merged 1 commit into from Nov 20, 2019

Conversation

ethouris
Copy link
Collaborator

The problem was that the criteria for whether the vertical FEC control packet should be emitted were based on a comparison against the earliest top FEC group base sequence, which was written in the characteristic data of the horizontal source group. As horizontal was checked first, after it was decided that the FEC control packet is required, it updated the base, as required for a case when an FEC control packet was shipped. This base, however, was also used for vertical groups to check, which column the given sequence number belongs to. Unfortunately, after updating the base for the horizontal groups, the effective group offset was negative (in the past), in which case the check for a necessity for a vertical FEC control packet was avoided, in result the FEC control packet wasn't shipped at all.

The solution was to invert the order of checks: now first the vertical groups are checked with the horizontal base not yet updated so that this is always shipped when needed. Next time the function is called, it will find out that vertical packet is no longer needed (already shipped) and therefore will check now for horizontal packet and effectively update the base.

You can see in the logs that it still happens that a check for necessity for FEC control packet due to having the column offset in the past - but never when the vertical group is full, which means that vertical control packet would be still avoided this time anyway by this very reason.

@ethouris ethouris added [core] Area: Changes in SRT library core Priority: High Status: Review Needed Type: Bug Indicates an unexpected problem or unintended behavior labels Nov 19, 2019
@ethouris ethouris added this to Needs review in Development via automation Nov 19, 2019
@ethouris ethouris added this to the v1.4.1 milestone Nov 19, 2019
Development automation moved this from Needs review to Reviewer approved Nov 20, 2019
@rndi rndi merged commit 5683c0a into Haivision:master Nov 20, 2019
Development automation moved this from Reviewer approved to Done Nov 20, 2019
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: High Type: Bug Indicates an unexpected problem or unintended behavior
Projects
No open projects
Development
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants