Don't update sps if they are only repeated #372
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is an attempt to improve the mitigations from strukturag#365 and strukturag#366 and picks up an idea I described at strukturag#345: > One way would be just to look at the pointers of the SPS (fast and easy, but > may reject more than required), or investigate if the SPS used for the image > generations are "compatible". This changes do exactly this: It (very conservativly) checks if the old and new sps have identical information -- except the reference picture set, which I believe is supposed to be updated by new sps'). If they are basically identical, the old sps will be used instead of the new one, (of course, reference image set is updated from the new one) I'm using standalone operator== and helper functions to avoid changing ABI of the library; if an ABI bump would be done, of course this should go to the respective classes.
|
The CVEs have been fixed in another way, but I leave this open as it would be nice to have a check like this that can detect when there was an illegal SPS switch in the input stream. |
|
thanks @farindk for your work on all of those issue. Do plan to cut a new release soon? Asking, because Debian is entering freeze in around two weeks and it would be nice to have your official version in the next stable… |
|
@coldtobi Yes, I've just released v1.0.10. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is an attempt to improve the mitigations from #365 and #366.
The mitigations in the PR assumes that sps with different pointers are incompatible; however, when new sps are encountered, always a new sps object is created, leading to an over-rejection of sps, as they might just have been repeated with compatible information. I guess this over-rejection might cause that images cannot be decoded, which would otherwise be decode-able.
It picks up the idea from #345 (comment):
This changes do exactly this: It (very conservativly -- as in "use new one if in doubt") checks if the old and new sps have identical information -- except the reference picture set, which I believe is supposed to be updated by new sps'). If they are basically identical, the old sps will be used instead of the new one, (of course, reference image set is updated from the new one)
I'm using standalone operator== and helper functions to avoid changing ABI of the library; if an ABI bump would be done, of course this should go to the respective classes.
I've tested the patch with several videos, they still play fine.
@farindk I'd really appreciate to receive your feedback; the reason is that I want to fix the many open CVE's for the package as it is currently in Debian and other distributions…
--
Cheers,
tobi