Fix T94237: Glitch when copying unaligned ffmpeg buffer

Using a negative linesize to flip an image vertically is supported in
ffmpeg but not for every function.

This method treats frames that need and those that do not need alignment
the same. An RGBA frame buffer with alignment that ffmpeg decides is
optimal for the CPU and build options is allocated by ffmpeg.
The `sws_scale` does the colorspace transformation into this RGBA frame
buffer without flipping. Now the image is upside down and aligned.
The combined unaligning and vertical flipping is then done by
`av_image_copy_to_buffer` which seems to handle negative linesize
correctly.

Reviewed By: ISS

Differential Revision: https://developer.blender.org/D13908

1 files changed, 28 insertions, 116 deletions

diff --git a/source/blender/imbuf/intern/anim_movie.c b/source/blender/imbuf/intern/anim_movie.c
index 6a05b681c88..17e82fc79c9 100644
--- a/source/blender/imbuf/intern/anim_movie.c
+++ b/source/blender/imbuf/intern/anim_movie.c
@@ -504,11 +504,6 @@ static ImBuf *avi_fetchibuf(struct anim *anim, int position)
 
 #ifdef WITH_FFMPEG
 
-BLI_INLINE bool need_aligned_ffmpeg_buffer(struct anim *anim)
-{
-  return (anim->x & 31) != 0;
-}
-
 static int startffmpeg(struct anim *anim)
 {
   int i, video_stream_index;
@@ -707,23 +702,20 @@ static int startffmpeg(struct anim *anim)
   anim->pFrameComplete = false;
   anim->pFrameDeinterlaced = av_frame_alloc();
   anim->pFrameRGB = av_frame_alloc();
+  anim->pFrameRGB->format = AV_PIX_FMT_RGBA;
+  anim->pFrameRGB->width = anim->x;
+  anim->pFrameRGB->height = anim->y;
 
-  if (need_aligned_ffmpeg_buffer(anim)) {
-    anim->pFrameRGB->format = AV_PIX_FMT_RGBA;
-    anim->pFrameRGB->width = anim->x;
-    anim->pFrameRGB->height = anim->y;
-
-    if (av_frame_get_buffer(anim->pFrameRGB, 32) < 0) {
-      fprintf(stderr, "Could not allocate frame data.\n");
-      avcodec_free_context(&anim->pCodecCtx);
-      avformat_close_input(&anim->pFormatCtx);
-      av_packet_free(&anim->cur_packet);
-      av_frame_free(&anim->pFrameRGB);
-      av_frame_free(&anim->pFrameDeinterlaced);
-      av_frame_free(&anim->pFrame);
-      anim->pCodecCtx = NULL;
-      return -1;
-    }
+  if (av_frame_get_buffer(anim->pFrameRGB, 0) < 0) {
+    fprintf(stderr, "Could not allocate frame data.\n");
+    avcodec_free_context(&anim->pCodecCtx);
+    avformat_close_input(&anim->pFormatCtx);
+    av_packet_free(&anim->cur_packet);
+    av_frame_free(&anim->pFrameRGB);
+    av_frame_free(&anim->pFrameDeinterlaced);
+    av_frame_free(&anim->pFrame);
+    anim->pCodecCtx = NULL;
+    return -1;
   }
 
   if (av_image_get_buffer_size(AV_PIX_FMT_RGBA, anim->x, anim->y, 1) != anim->x * anim->y * 4) {
@@ -851,92 +843,26 @@ static void ffmpeg_postprocess(struct anim *anim)
     }
   }
 
-  if (!need_aligned_ffmpeg_buffer(anim)) {
-    av_image_fill_arrays(anim->pFrameRGB->data,
-                         anim->pFrameRGB->linesize,
-                         (unsigned char *)ibuf->rect,
-                         AV_PIX_FMT_RGBA,
-                         anim->x,
-                         anim->y,
-                         1);
-  }
-
-#  if defined(__x86_64__) || defined(_M_X64)
-  /* Scale and flip image over Y axis in one go, using negative strides.
-   * This doesn't work with ARM/PowerPC though and may be misusing the API.
-   * Limit it x86_64 where it appears to work.
-   * http://trac.ffmpeg.org/ticket/9060 */
-  int *dstStride = anim->pFrameRGB->linesize;
-  uint8_t **dst = anim->pFrameRGB->data;
-  const int dstStride2[4] = {-dstStride[0], 0, 0, 0};
-  uint8_t *dst2[4] = {dst[0] + (anim->y - 1) * dstStride[0], 0, 0, 0};
-
   sws_scale(anim->img_convert_ctx,
             (const uint8_t *const *)input->data,
             input->linesize,
             0,
             anim->y,
-            dst2,
-            dstStride2);
-#  else
-  /* Scale with swscale. */
-  int *dstStride = anim->pFrameRGB->linesize;
-  uint8_t **dst = anim->pFrameRGB->data;
-  const int dstStride2[4] = {dstStride[0], 0, 0, 0};
-  uint8_t *dst2[4] = {dst[0], 0, 0, 0};
-  int x, y, h, w;
-  unsigned char *bottom;
-  unsigned char *top;
-
-  sws_scale(anim->img_convert_ctx,
-            (const uint8_t *const *)input->data,
-            input->linesize,
-            0,
-            anim->y,
-            dst2,
-            dstStride2);
-
-  /* Flip destination image buffer over Y axis. */
-  bottom = (unsigned char *)dst[0];
-  top = bottom + anim->x * (anim->y - 1) * 4;
-
-  h = (anim->y + 1) / 2;
-  w = anim->x;
-
-  for (y = 0; y < h; y++) {
-    unsigned char tmp[4];
-    unsigned int *tmp_l = (unsigned int *)tmp;
-
-    for (x = 0; x < w; x++) {
-      tmp[0] = bottom[0];
-      tmp[1] = bottom[1];
-      tmp[2] = bottom[2];
-      tmp[3] = bottom[3];
-
-      bottom[0] = top[0];
-      bottom[1] = top[1];
-      bottom[2] = top[2];
-      bottom[3] = top[3];
-
-      *(unsigned int *)top = *tmp_l;
-
-      bottom += 4;
-      top += 4;
-    }
-    top -= 8 * w;
-  }
-#  endif
-
-  if (need_aligned_ffmpeg_buffer(anim)) {
-    uint8_t *buf_src = anim->pFrameRGB->data[0];
-    uint8_t *buf_dst = (uint8_t *)ibuf->rect;
-    for (int y = 0; y < anim->y; y++) {
-      memcpy(buf_dst, buf_src, anim->x * 4);
-      buf_dst += anim->x * 4;
-      buf_src += anim->pFrameRGB->linesize[0];
-    }
-  }
-
+            anim->pFrameRGB->data,
+            anim->pFrameRGB->linesize);
+
+  /* Copy the valid bytes from the aligned buffer vertically flipped into ImBuf */
+  int aligned_stride = anim->pFrameRGB->linesize[0];
+  const uint8_t *const src[4] = {
+      anim->pFrameRGB->data[0] + (anim->y - 1) * aligned_stride, 0, 0, 0};
+  /* NOTE: Negative linesize is used to copy and flip image at once with function
+   * `av_image_copy_to_buffer`. This could cause issues in future and image may need to be flipped
+   * explicitly. */
+  const int src_linesize[4] = {-anim->pFrameRGB->linesize[0], 0, 0, 0};
+  int dst_size = av_image_get_buffer_size(
+      anim->pFrameRGB->format, anim->pFrameRGB->width, anim->pFrameRGB->height, 1);
+  av_image_copy_to_buffer(
+      (uint8_t *)ibuf->rect, dst_size, src, src_linesize, AV_PIX_FMT_RGBA, anim->x, anim->y, 1);
   if (filter_y) {
     IMB_filtery(ibuf);
   }
@@ -1482,20 +1408,6 @@ static void free_anim_ffmpeg(struct anim *anim)
     av_packet_free(&anim->cur_packet);
 
     av_frame_free(&anim->pFrame);
-
-    if (!need_aligned_ffmpeg_buffer(anim)) {
-      /* If there's no need for own aligned buffer it means that FFmpeg's
-       * frame shares the same buffer as temporary ImBuf. In this case we
-       * should not free the buffer when freeing the FFmpeg buffer.
-       */
-      av_image_fill_arrays(anim->pFrameRGB->data,
-                           anim->pFrameRGB->linesize,
-                           NULL,
-                           AV_PIX_FMT_RGBA,
-                           anim->x,
-                           anim->y,
-                           1);
-    }
     av_frame_free(&anim->pFrameRGB);
     av_frame_free(&anim->pFrameDeinterlaced);