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 #3305: Frame reorder after the frame is read. #3319

Merged
merged 2 commits into from
May 12, 2021

Conversation

akallabeth
Copy link
Contributor

This PR reverts a change that leads to screen artefacts when reading frames with multiple references for versions > 1.8.0

@akallabeth
Copy link
Contributor Author

Could someone from the devs have a look at this?
A reply to the issue or this pr would be awsome ;)

@huili2
Copy link
Collaborator

huili2 commented Oct 12, 2020

did not dig further, but the travis CI failed like following:
[ FAILED ] DecodeFile/DecoderOutputTest.CompareOutput/43,
[ FAILED ] DecodeFile/DecoderOutputTest.CompareOutput/44,
[ FAILED ] DecodeFile/DecoderOutputTest.CompareOutput/48,
[ FAILED ] DecodeFile/DecoderOutputTest.CompareOutput/49,
[ FAILED ] DecodeFile/DecoderOutputTest.CompareOutput/50,
Are original cases wrong?

@akallabeth
Copy link
Contributor Author

@huili2 All versions since 1.8.0 have an issue that frames contain artifacts if multiple reference frames are in use.
The tarvis failing tests seem to have been added later on and I did not receive any feedback on the referenced issue so far.

@huili2
Copy link
Collaborator

huili2 commented Oct 13, 2020

I verified with one case, and found you are correct. Could you please complement this PR to pass CI, i.e., modify corresponding SHA values in these tests?

@huili2
Copy link
Collaborator

huili2 commented Oct 13, 2020

@xiaotianshi2 could you please verify this PR? I checked DecodeFile/DecoderOutputTest.CompareOutput/43, and found this PR could get correct 9 frames while current master could only generate 7 frames.

@akallabeth
Copy link
Contributor Author

@huili2 @xiaotianshi2 any update on this?

@akallabeth
Copy link
Contributor Author

@huili2 ok, did rebase this and modified the unit tests to pass

@antenore
Copy link

antenore commented Mar 2, 2021

Any news about this PR?

I've just tested it and indeed solves some issues I've with WebEx and FreeRDP

@joakim-tjernlund
Copy link

This really needs to go in, there are screen artifacts otherwise. What is the delay?

@huili2 huili2 merged commit ca0e43e into cisco:master May 12, 2021
@joakim-tjernlund
Copy link

Thank you!

New release coming soon?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants