[FFmpeg-devel] [PATCH 2/3] lavc: add a framework to fix alignment problems.

[Nicolas George]

A lot of codecs require aligned frame data, but do not document it.
It can result in crashes with perfectly valid uses of the API.

Design rationale:

- requiring frame data to be always aligned would be wasteful;

- alignment requirements will evolve
  (the 16->32 bump is still recent);

- requiring applications to worry about alignment is not convenient.

For now, the default alignment is fixed at 32.

Fix: trac ticket #6349
Signed-off-by: Nicolas George <george at nsup.org>
---
 libavcodec/avcodec.h | 10 +++++++++
 libavcodec/encode.c  | 57 +++++++++++++++++++++++++++++++++++++++++++++++++---
 libavcodec/utils.c   |  2 ++
 3 files changed, 66 insertions(+), 3 deletions(-)


Without the 1<<.


diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index 00f9c82afc..1e45cf28b2 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -3682,6 +3682,16 @@ typedef struct AVCodecContext {
      * (with the display dimensions being determined by the crop_* fields).
      */
     int apply_cropping;
+
+    /**
+     * Minimum alignment of frame data required by the codec.
+     * All frame data pointers must have the alignment lower bits cleared,
+     * i.e. be a multiple of alignment.
+     * - encoding: set by the encoder and used by the framework
+     * - decoding: unused
+     */
+    unsigned alignment;
+
 } AVCodecContext;
 
 AVRational av_codec_get_pkt_timebase         (const AVCodecContext *avctx);
diff --git a/libavcodec/encode.c b/libavcodec/encode.c
index 525ee1f5d6..11d5b2f7c7 100644
--- a/libavcodec/encode.c
+++ b/libavcodec/encode.c
@@ -115,6 +115,40 @@ fail:
     return ret;
 }
 
+static int frame_realign(AVCodecContext *avctx, const AVFrame **frame, AVFrame **realigned)
+{
+    AVFrame *tmp;
+    int ret;
+
+    if (!*frame || av_frame_check_align(*frame, avctx->alignment))
+        return 0;
+
+    tmp = av_frame_alloc();
+    if (!tmp)
+        return AVERROR(ENOMEM);
+    tmp->format         = (*frame)->format;
+    tmp->width          = (*frame)->width;
+    tmp->height         = (*frame)->height;
+    tmp->channels       = (*frame)->channels;
+    tmp->channel_layout = (*frame)->channel_layout;
+    tmp->nb_samples     = (*frame)->nb_samples;
+    ret = av_frame_get_buffer(tmp, avctx->alignment);
+    if (ret < 0)
+        goto fail;
+    ret = av_frame_copy(tmp, *frame);
+    if (ret < 0)
+        return ret;
+    ret = av_frame_copy_props(tmp, *frame);
+    if (ret < 0)
+        return ret;
+    *frame = *realigned = tmp;
+    return 0;
+
+fail:
+    av_frame_free(&tmp);
+    return ret;
+}
+
 int attribute_align_arg avcodec_encode_audio2(AVCodecContext *avctx,
                                               AVPacket *avpkt,
                                               const AVFrame *frame,
@@ -122,6 +156,7 @@ int attribute_align_arg avcodec_encode_audio2(AVCodecContext *avctx,
 {
     AVFrame *extended_frame = NULL;
     AVFrame *padded_frame = NULL;
+    AVFrame *realigned_frame = NULL;
     int ret;
     AVPacket user_pkt = *avpkt;
     int needs_realloc = !user_pkt.data;
@@ -195,6 +230,9 @@ int attribute_align_arg avcodec_encode_audio2(AVCodecContext *avctx,
 
     av_assert0(avctx->codec->encode2);
 
+    ret = frame_realign(avctx, &frame, &realigned_frame);
+    if (ret < 0)
+        goto end;
     ret = avctx->codec->encode2(avctx, avpkt, frame, got_packet_ptr);
     if (!ret) {
         if (*got_packet_ptr) {
@@ -252,6 +290,7 @@ int attribute_align_arg avcodec_encode_audio2(AVCodecContext *avctx,
 
 end:
     av_frame_free(&padded_frame);
+    av_frame_free(&realigned_frame);
     av_free(extended_frame);
 
 #if FF_API_AUDIOENC_DELAY
@@ -269,6 +308,7 @@ int attribute_align_arg avcodec_encode_video2(AVCodecContext *avctx,
     int ret;
     AVPacket user_pkt = *avpkt;
     int needs_realloc = !user_pkt.data;
+    AVFrame *realigned_frame = NULL;
 
     *got_packet_ptr = 0;
 
@@ -301,7 +341,9 @@ int attribute_align_arg avcodec_encode_video2(AVCodecContext *avctx,
 
     av_assert0(avctx->codec->encode2);
 
-    ret = avctx->codec->encode2(avctx, avpkt, frame, got_packet_ptr);
+    ret = frame_realign(avctx, &frame, &realigned_frame);
+    if (!ret)
+        ret = avctx->codec->encode2(avctx, avpkt, frame, got_packet_ptr);
     av_assert0(ret <= 0);
 
     emms_c();
@@ -340,6 +382,7 @@ int attribute_align_arg avcodec_encode_video2(AVCodecContext *avctx,
         avctx->frame_number++;
     }
 
+    av_frame_free(&realigned_frame);
     if (ret < 0 || !*got_packet_ptr)
         av_packet_unref(avpkt);
 
@@ -406,8 +449,16 @@ int attribute_align_arg avcodec_send_frame(AVCodecContext *avctx, const AVFrame
             return 0;
     }
 
-    if (avctx->codec->send_frame)
-        return avctx->codec->send_frame(avctx, frame);
+    if (avctx->codec->send_frame) {
+        AVFrame *realigned_frame = NULL;
+        int ret;
+        ret = frame_realign(avctx, &frame, &realigned_frame);
+        if (ret < 0)
+            return ret;
+        ret = avctx->codec->send_frame(avctx, frame);
+        av_frame_free(&realigned_frame);
+        return ret;
+    }
 
     // Emulation via old API. Do it here instead of avcodec_receive_packet, because:
     // 1. if the AVFrame is not refcounted, the copying will be much more
diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index 1336e921c9..cddeb70268 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -1003,6 +1003,8 @@ FF_ENABLE_DEPRECATION_WARNINGS
             }
             avctx->sw_pix_fmt = frames_ctx->sw_format;
         }
+
+        avctx->alignment = 32;
     }
 
     avctx->pts_correction_num_faulty_pts =
-- 
2.11.0