diff mbox series

[FFmpeg-devel] lavu/pixdesc: handle xv30be in av_[read|write]_image_line

Message ID 20221204213458.79978-1-philipl@overt.org
State Accepted
Commit 9651f873f8ce15c5d4380f49488900bbf6e6c731
Headers show
Series [FFmpeg-devel] lavu/pixdesc: handle xv30be in av_[read|write]_image_line | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Philip Langdale Dec. 4, 2022, 9:34 p.m. UTC
xv30be is an obnoxious format that I shouldn't have included in the
first place. xv30 packs 3 10bit channels into 32bits and while our
byte-oriented logic can handle Little Endian correctly, it cannot
handle Big Endian. To avoid that, I marked xv30be as a bitstream
format, but while that didn't produce FATE errors, it turns out that
the existing read/write code silently produces incorrect results, which
can be revealed via ubsan.

In all likelyhood, the correct fix here is to remove the format. As
this format is only used by Intel vaapi, it's only going to show up
in LE form, so we could just drop the BE version. But I don't want to
deal with creating a hole in the pixfmt list and all the weirdness that
comes from that. Instead, I decided to write the correct read/write
code for it.

And that code isn't too bad, as long as it's specialised for this
format, as the channels are all bit-aligned inside a 32bit word.

Signed-off-by: Philip Langdale <philipl@overt.org>
---
 libavutil/pixdesc.c | 70 ++++++++++++++++++++++++++++++++-------------
 1 file changed, 50 insertions(+), 20 deletions(-)
diff mbox series

Patch

diff --git a/libavutil/pixdesc.c b/libavutil/pixdesc.c
index ca3e204a0b..62a2ae08d9 100644
--- a/libavutil/pixdesc.c
+++ b/libavutil/pixdesc.c
@@ -46,19 +46,35 @@  void av_read_image_line2(void *dst,
     uint32_t *dst32 = dst;
 
     if (flags & AV_PIX_FMT_FLAG_BITSTREAM) {
-        int skip = x * step + comp.offset;
-        const uint8_t *p = data[plane] + y * linesize[plane] + (skip >> 3);
-        int shift = 8 - depth - (skip & 7);
+        if (depth == 10) {
+            // Assume all channels are packed into a 32bit value
+            const uint8_t *byte_p = data[plane] + y * linesize[plane];
+            const uint32_t *p = (uint32_t *)byte_p;
 
-        while (w--) {
-            int val = (*p >> shift) & mask;
-            if (read_pal_component)
-                val = data[1][4*val + c];
-            shift -= step;
-            p -= shift >> 3;
-            shift &= 7;
-            if (dst_element_size == 4) *dst32++ = val;
-            else                       *dst16++ = val;
+            while (w--) {
+                int val = AV_RB32(p);
+                val = (val >> comp.offset) & mask;
+                if (read_pal_component)
+                    val = data[1][4*val + c];
+                if (dst_element_size == 4) *dst32++ = val;
+                else                       *dst16++ = val;
+                p++;
+            }
+        } else {
+            int skip = x * step + comp.offset;
+            const uint8_t *p = data[plane] + y * linesize[plane] + (skip >> 3);
+            int shift = 8 - depth - (skip & 7);
+
+            while (w--) {
+                int val = (*p >> shift) & mask;
+                if (read_pal_component)
+                    val = data[1][4*val + c];
+                shift -= step;
+                p -= shift >> 3;
+                shift &= 7;
+                if (dst_element_size == 4) *dst32++ = val;
+                else                       *dst16++ = val;
+            }
         }
     } else {
         const uint8_t *p = data[plane] + y * linesize[plane] +
@@ -109,15 +125,29 @@  void av_write_image_line2(const void *src,
     const uint16_t *src16 = src;
 
     if (flags & AV_PIX_FMT_FLAG_BITSTREAM) {
-        int skip = x * step + comp.offset;
-        uint8_t *p = data[plane] + y * linesize[plane] + (skip >> 3);
-        int shift = 8 - depth - (skip & 7);
+        if (depth == 10) {
+            // Assume all channels are packed into a 32bit value
+            const uint8_t *byte_p = data[plane] + y * linesize[plane];
+            uint32_t *p = (uint32_t *)byte_p;
+            int offset = comp.offset;
+            uint32_t mask  = ((1ULL << depth) - 1) << offset;
 
-        while (w--) {
-            *p |= (src_element_size == 4 ? *src32++ : *src16++) << shift;
-            shift -= step;
-            p -= shift >> 3;
-            shift &= 7;
+            while (w--) {
+                uint16_t val = src_element_size == 4 ? *src32++ : *src16++;
+                AV_WB32(p, (AV_RB32(p) & ~mask) | (val << offset));
+                p++;
+            }
+        } else {
+            int skip = x * step + comp.offset;
+            uint8_t *p = data[plane] + y * linesize[plane] + (skip >> 3);
+            int shift = 8 - depth - (skip & 7);
+
+            while (w--) {
+                *p |= (src_element_size == 4 ? *src32++ : *src16++) << shift;
+                shift -= step;
+                p -= shift >> 3;
+                shift &= 7;
+            }
         }
     } else {
         int shift = comp.shift;