From 6225ad5c19751ed55555ac9ffb738c2e4acc1f92 Mon Sep 17 00:00:00 2001 From: Niklas Haas Date: Sat, 25 May 2024 18:05:04 +0200 Subject: [PATCH] avcodec/dovi_rpu{enc,dec}: fix ms_weight handling The code as written was wrong. In the spec, these fields are treated merely as plain integers in the range 0 to 4095. The only difference between L2 and L8 is that L2.ms_weight also accepts an additional value of -1, hence the extra sign bit. While it's likely that these are still shifted integers in disguise, since all real-world samples seem to use a value of 2048 here, the offset used in the code was wrong. In addition, because the l8.ms_weight struct member is unsigned, these wrong shifting semantics ended up overflowing the field, leading to undefined behavior when transcoding. Fortunately, the damage was relatively contained in practice, because it just corrupts the coding of this field, which is ignored in practice in all implementations I have seen. --- libavcodec/dovi_rpudec.c | 4 ++-- libavcodec/dovi_rpuenc.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/libavcodec/dovi_rpudec.c b/libavcodec/dovi_rpudec.c index 7c7eda9d09..a477dbd4e3 100644 --- a/libavcodec/dovi_rpudec.c +++ b/libavcodec/dovi_rpudec.c @@ -142,7 +142,7 @@ static void parse_ext_v1(DOVIContext *s, GetBitContext *gb, AVDOVIDmData *dm) dm->l2.trim_power = get_bits(gb, 12); dm->l2.trim_chroma_weight = get_bits(gb, 12); dm->l2.trim_saturation_gain = get_bits(gb, 12); - dm->l2.ms_weight = get_bits(gb, 13) - 8192; + dm->l2.ms_weight = get_sbits(gb, 13); break; case 4: dm->l4.anchor_pq = get_bits(gb, 12); @@ -197,7 +197,7 @@ static void parse_ext_v2(DOVIContext *s, GetBitContext *gb, AVDOVIDmData *dm, dm->l8.trim_power = get_bits(gb, 12); dm->l8.trim_chroma_weight = get_bits(gb, 12); dm->l8.trim_saturation_gain = get_bits(gb, 12); - dm->l8.ms_weight = get_bits(gb, 12) - 8192; + dm->l8.ms_weight = get_bits(gb, 12); if (ext_block_length < 12) break; dm->l8.target_mid_contrast = get_bits(gb, 12); diff --git a/libavcodec/dovi_rpuenc.c b/libavcodec/dovi_rpuenc.c index 3c3e0f84c0..dacb8b54e7 100644 --- a/libavcodec/dovi_rpuenc.c +++ b/libavcodec/dovi_rpuenc.c @@ -276,7 +276,7 @@ static void generate_ext_v1(PutBitContext *pb, const AVDOVIDmData *dm) put_bits(pb, 12, dm->l2.trim_power); put_bits(pb, 12, dm->l2.trim_chroma_weight); put_bits(pb, 12, dm->l2.trim_saturation_gain); - put_bits(pb, 13, dm->l2.ms_weight + 8192); + put_sbits(pb, 13, dm->l2.ms_weight); break; case 4: put_bits(pb, 12, dm->l4.anchor_pq); @@ -374,7 +374,7 @@ static void generate_ext_v2(PutBitContext *pb, const AVDOVIDmData *dm) put_bits(pb, 12, dm->l8.trim_power); put_bits(pb, 12, dm->l8.trim_chroma_weight); put_bits(pb, 12, dm->l8.trim_saturation_gain); - put_bits(pb, 12, dm->l8.ms_weight + 8192); + put_bits(pb, 12, dm->l8.ms_weight); if (ext_block_length < 12) break; put_bits(pb, 12, dm->l8.target_mid_contrast);