vulkan h265 /w 10-bit + fix encoder hang#565
Conversation
01c785f to
eb9603d
Compare
|
|
7a3b6d0 to
7beb4d5
Compare
7beb4d5 to
2267f2b
Compare
56cce32 to
7ee445d
Compare
7ee445d to
ebb0afa
Compare
ebb0afa to
f450d02
Compare
|
removed wip as it is working now. let's continue with the review. |
| static uint32_t find_lsb(uint32_t v) | ||
| { | ||
| for (int i = 0; i < 32; i++) | ||
| if (v & (1u << i)) | ||
| return i; | ||
|
|
||
| return UINT32_MAX; | ||
| } | ||
|
|
||
| static uint32_t find_msb(uint32_t v) | ||
| { | ||
| for (int i = 31; i >= 0; i--) | ||
| if (v & (1u << i)) | ||
| return i; | ||
|
|
||
| return UINT32_MAX; | ||
| } |
There was a problem hiding this comment.
Those look like countl_one and countr_one from https://en.cppreference.com/w/cpp/utility/bit.html
There was a problem hiding this comment.
for find_lsb we could technically use std::countr_zero(v) instead, but if v is zero then it'll give the wrong result, so we can't really have a one-liner → would need to keep the helper function either way
for find_msb it'd look something like 31 - std::countl_zero(v) but again v == 0 needs to be handled separately.
keep in mind i'm not familiar with the c++ std library. it's the first time i'm hearing about these functions. but this is the idea i'm getting from reading the reference pages. please correct me if i'm wrong.
i think this for implementation is easier to comprehend for both cases.
There was a problem hiding this comment.
If the edge cases (returning UINT32_MAX) are supposed to be used, then
uint32_t max_transform_hierarchy = (find_msb((uint32_t)encode_h265_caps.ctbSizes) + 4) - (find_lsb((uint32_t)encode_h265_caps.transformBlockSizes) + 2);is going to overflow, is this intended?
There was a problem hiding this comment.
throwing exception now
|
Needs doc and dashboard updates to allow vulkan/h265 |
| if (st_rps.used_by_curr_pic_s0_flag == 0) | ||
| { | ||
| // ref_slot is too old → encode an IDR instead | ||
| ref_slot.reset(); | ||
| poc_history.clear(); | ||
| } |
There was a problem hiding this comment.
This happens after video_cmd_buf.beginVideoCodingKHR has been called, which already uses ref_slot. I don't think this will work.
There was a problem hiding this comment.
I did wonder about this. I had no chance to test, but nothing in the spec indicated that it wouldn't work.
ref_slothere is the local forencode_info_next.- base class will still do
setReferenceSlots, but this is fine.
- base class will still do
- no
ref_slot→encode_info_nextconfigures for IDR instead of P-frames. - spec hints that reference slots are ignored in case of IDR/CRA, so they're ok to be set in this case.
| case encoder_name::X264: | ||
| case encoder_name::Vulkan: | ||
| return {video_codec::H264}; | ||
| return { | ||
| video_codec::CodecAuto, | ||
| video_codec::H264, | ||
| video_codec::H265, | ||
| }; |
There was a problem hiding this comment.
x264 falls through, vulkan should be be its own clause
Vulkan H265 encoding works properly now.
Tested Vulkan encoders (RADV):
With 8-bit h265, ffmpeg gives a warning about deprecated format. This also happens with VAAPI, and the picture in the headset looks fine.