h26forge / h26forge

Domain-specific infrastructure for analyzing, generating, and manipulating syntactically correct but semantically spec-non-compliant video files.
MIT License
292 stars 17 forks source link

Panic with out-of-range config #3

Closed natashenka closed 1 year ago

natashenka commented 1 year ago

The following run of h26forge causes a crash:

./h26forge generate --small --seed 9328824254276034406 -o o.h264 --ignore-ipcm --ignore-edge-intra-pred --config ~/extreme.json

extreme json contains some ranges that are larger than the specification accepts, but they should be possible to generate (i.e. ue(v) lengths that are supposed to be less than 255)

extreme.json (JSON attachments not allowed, so you'll need to change the extension)

wrv commented 1 year ago

Thanks for this report!

The issue seems to be from the way PPSes are associated to either SPSes or SubsetSPSes when encoding. The generated video has an SPS and SubsetSPS with the same seq_parameter_set_id and while generating the PPS it uses SubsetSPS parameters, but for encoding the PPS uses the SPS. This is causing Slice encoding to panic.

I think the way to fix this would be to add a bool to PicParameterSet struct to determine if it's associated with an SPS or SubsetSPS. Adding a #[serde(default)] to this bool should also help ensure synthesis mode can work with any existing videos.

Running the debug build on h26forge with these arguments causes a different crash from the calculation of max_frame_num: https://github.com/h26forge/h26forge/blob/main/src/common/data_structures.rs#L443 . This variable is relevant if h26forge were to serve as the basis of complete H.264 decoder, but I think it's safe to remove for now.

wrv commented 1 year ago

Fixed with the following commits

natashenka commented 1 year ago

Thanks so much!

natashenka commented 1 year ago

Sorry, I'm now getting crashes with the following:

./h26forge generate --small --seed 10784273232790499014 -o o.h264 --ignore-ipcm --ignore-edge-intra-pred --config ~/extreme.json ./h26forge generate --small --seed 7064665097279482406 -o o.h264 --ignore-ipcm --ignore-edge-intra-pred --config ~/extreme.json

I'm not 100% sure that they are related, let me know if I should file separate bugs

wrv commented 1 year ago

Oh no! Looking into it now.

wrv commented 1 year ago

Fixed with the most recent patch.

natashenka commented 1 year ago

Thanks! That improved the stability of my distributed fuzzing a lot. I'm seeing one more crash:

./h26forge generate --small --seed 10779984210671442896 --ignore-ipcm --ignore-edge-intra-pred -o o.h264

Thanks for bearing with me here. Running so many test cases is 'fuzzing the fuzzer' so to speak

wrv commented 1 year ago

I appreciate the reports!

I hope this most recent commit completely addresses this issue, but if not please continue to let me know.

natashenka commented 1 year ago

I've done 100 million runs with no crashes now. 🎉 and thank you!