feat: add per-picture forced IDR API#706
Conversation
|
Thank you very much for the contribution. From a first glance it looks very good to me, but I think we need to talk about this internally. I currently just have one small remark: I think the Furthermore, you might want to add your name to the AUTHORS.md file. |
Hi! Thanks for your feedback. I opened the PR to start the discussion. I understand this is an important change API-wise and that it needs to be reviewed and approved internally.
I will do that as well. |
I had to guard the implementation with I will review everything to see how to omit them in EncoderLib and CommonLib |
|
Ah, ok. I had the impression that #ifdefs in EncoderLib and CommonLib were not necessarily needed. But if they are, that's fine. If removing the #ifdefs there complicates the implementation, just leave it as it is. |
Most of them can be removed. I have updated the PR with that fixed. |
|
Hey! I had a short look at the PR and I'd like to run some tests. Therefore I'd like to know how can I configure the encoder to insert the IDRs. I want to run tests with fixQP and rate control and check other test cases. Could you give me an example configuration? |
No special configuration is required for the encoder. Insertion of IDRs is done programmatically, by setting |
|
@K-os One thing I have doubts about is the name |
Did you test it with more frames too? When I set framesToEncode >= 24 only the first frame is encoded and he stops processing more frames. It seems like a flush is missing. I'm having a deeper look. And FYI, I added config parameters, i.e. the number and list of the intra POCs |
I didn't test with more frames. I think the issue come from the fact that when a new GOP is force-started, the previous GOP has gaps in the coding numbers. This causes the process to stall after the first frame with This is the instrumented output from encodePicture |
Enable the unstable API in the CI so that these code paths are built and tested to prevent regressions.
|
It should be fixed now. I will also expand the test suite to verify that forcing an IDR at any picture from the GOP works. |
c609e2e to
c4bbaa2
Compare
|
Thanks for your fix. Unfortunately there are still issues when I do more tests. For example:
|
|
Thanks for your findings. I'm planning to expand the test suite to catch all these errors and probably others. I'm new to this codebase and I am using this to learn about it in more depth. |
Introduce `VVENC_PIC_FLAG_FORCE_IDR` on `vvencYUVBuffer` and thread the request through preprocessing and GOP handling so a selected input frame is encoded as a forced IDR/CRA picture respecting the configured refresh type. Also add library/unit tests for the new behavior. Fixes fraunhoferhhi#311
|
I have extended the test suite to cover the following scenarios with all their permutations:
All these scenarios are now working correctly. I am now trying to fix the other issue you detected, encoding 50 frames. |
Introduce
VVENC_PIC_FLAG_FORCE_IDRonvvencYUVBuffer, and thread the request through preprocessing and GOP handling so that a selected input frame is encoded as a forced IDR/CRA picture, respecting the configured refresh type.I tried to mimic the API of other encoders that use per-picture flags to define optional encode requests.
The new API is only enabled with
VVENC_USE_UNSTABLE_API.Unit tests have been added for the new behavior.
Fixes #311