Andreas Rheinhardt: > The allocated buffer of an AVFrame need not be height * linesize > big. In case there is padding between lines, the last line need > not have this padding. Pathological examples exist with height == 1 > (where linesize is not really meaningful); non-pathological examples > are produced by the separatefields filter, which simply offsets > the pointers and doubles linesize, so that the frame created from > the bottom field has only height - 1 lines with padding (containing > the top field) and one line without padding. > > The mpegvideo encoders sometimes copy the input frames for padding/ > alignment reasons and when doing so, there is a fast path in which > everything is copied with one memcpy() of height * linesize. And > this is wrong as explained above. Use av_image_copy_plane() instead > of our ad-hoc code for copying. > > Fixes ticket #10754 (discovered by Zeng Yunxiang). > > (The above discussion presupposes linesize > 0 and would need some > adaptation for negative linesizes; the code removed seems even more > buggy for negative linesizes.) > > Signed-off-by: Andreas Rheinhardt <andreas.rheinha...@outlook.com> > --- > 1. One could also use av_image_copy(). > 2. The "direct" (non-copy) path only ensures a height mod 16, yet > the non-direct mode sometimes pads to 32 (for non-progressive mpeg2). > Seems fishy. > > libavcodec/mpegvideo_enc.c | 19 ++++--------------- > 1 file changed, 4 insertions(+), 15 deletions(-) > > diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c > index 6d2d417454..fac9bb7ae7 100644 > --- a/libavcodec/mpegvideo_enc.c > +++ b/libavcodec/mpegvideo_enc.c > @@ -36,6 +36,7 @@ > #include <stdint.h> > > #include "libavutil/emms.h" > +#include "libavutil/imgutils.h" > #include "libavutil/internal.h" > #include "libavutil/intmath.h" > #include "libavutil/mathematics.h" > @@ -1201,28 +1202,16 @@ static int load_input_picture(MpegEncContext *s, > const AVFrame *pic_arg) > int w = s->width >> h_shift; > int h = s->height >> v_shift; > const uint8_t *src = pic_arg->data[i]; > - uint8_t *dst = pic->f->data[i]; > + uint8_t *dst = pic->f->data[i] + !s->avctx->rc_buffer_size * > INPLACE_OFFSET; > int vpad = 16; > > + av_image_copy_plane(dst, dst_stride, src, src_stride, w, h); > + > if ( s->codec_id == AV_CODEC_ID_MPEG2VIDEO > && !s->progressive_sequence > && FFALIGN(s->height, 32) - s->height > 16) > vpad = 32; > > - if (!s->avctx->rc_buffer_size) > - dst += INPLACE_OFFSET; > - > - if (src_stride == dst_stride) > - memcpy(dst, src, src_stride * h); > - else { > - int h2 = h; > - uint8_t *dst2 = dst; > - while (h2--) { > - memcpy(dst2, src, w); > - dst2 += dst_stride; > - src += src_stride; > - } > - } > if ((s->width & 15) || (s->height & (vpad-1))) { > s->mpvencdsp.draw_edges(dst, dst_stride, > w, h,
Will apply this patch tomorrow unless there are objections. - Andreas _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".