search

kornelski / cavif-rs

AVIF image creator in pure Rust
https://lib.rs/cavif
BSD 3-Clause "New" or "Revised" License
570 stars 27 forks source link

Set chroma_sample_position only for YUV 4:2:0 #41

Closed wantehchang closed 2 years ago

wantehchang commented 2 years ago

The chroma_sample_position syntax element is only used for YUV 4:2:0 in the AV1 bitstream. So set chroma_sample_position to ChromaSamplePosition::Unknown for all other YUV formats, including monochrome (YUV 4:0:0).

Note: For still images, the most common chroma sample position in practice is the "center" position. Unfortunately the AV1 specification does not have a value for the "center" chroma sample position. See https://github.com/AOMediaCodec/av1-avif/issues/88 and https://github.com/AOMediaCodec/av1-spec/pull/308. This pull request preserves the ChromaSamplePosition::Colocated value that is currently used, but it is likely to be incorrect when YUV 4:2:0 is supported.

wantehchang commented 2 years ago

@kornelski Please review. Thanks!

kornelski commented 2 years ago

Thanks.

I'm not planning to support chroma subsampling of color channels ever. If I remember correctly, this chroma setting was only a workaround for lack of monochrome encoding.

wantehchang commented 2 years ago

I see. I guess older versions of rav1e could not encode monochrome (YUV 4:0:0) images. The current version of rav1e encodes alpha images as monochrome correctly.

The current cavif-rs code uses only ChromaSampling::Cs444 and ChromaSampling::Cs400. If it will stay this way, we can simply set chroma_sample_position to ChromaSamplePosition::Unknown unconditionally. Would you like me to do that?

kornelski commented 2 years ago

Yes, please