[2/2] mpegvideo_enc: Don't use direct mode for unaligned input

diff mbox

From: Michael Niedermayer <michaelni@gmx.at>

CC: libav-stable@libav.org
---
 libavcodec/internal.h      | 6 ++++++
 libavcodec/mpegvideo_enc.c | 4 ++++
 libavcodec/utils.c         | 6 ------
 3 files changed, 10 insertions(+), 6 deletions(-)

Quoting Martin Storsjö (2015-01-20 15:09:08)
> From: Michael Niedermayer <michaelni@gmx.at>
> 
> CC: libav-stable@libav.org
> ---
>  libavcodec/internal.h      | 6 ++++++
>  libavcodec/mpegvideo_enc.c | 4 ++++
>  libavcodec/utils.c         | 6 ------
>  3 files changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/libavcodec/internal.h b/libavcodec/internal.h
> index a68d613..2b50830 100644
> --- a/libavcodec/internal.h
> +++ b/libavcodec/internal.h
> @@ -37,6 +37,12 @@
>  
>  #define FF_SIGNBIT(x) (x >> CHAR_BIT * sizeof(x) - 1)
>  
> +#if HAVE_SIMD_ALIGN_16
> +#   define STRIDE_ALIGN 16
> +#else
> +#   define STRIDE_ALIGN 8
> +#endif
> +
>  typedef struct FramePool {
>      /**
>       * Pools for each data plane. For audio all the planes have the same size,
> diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c
> index be6fb08..baeba70 100644
> --- a/libavcodec/mpegvideo_enc.c
> +++ b/libavcodec/mpegvideo_enc.c
> @@ -992,6 +992,10 @@ static int load_input_picture(MpegEncContext *s, const AVFrame *pic_arg)
>              direct = 0;
>          if ((s->width & 15) || (s->height & 15))
>              direct = 0;
> +        if (((intptr_t)(pic_arg->data[0])) & (STRIDE_ALIGN - 1))
> +            direct = 0;
> +        if (s->linesize & (STRIDE_ALIGN - 1))
> +            direct = 0;
>  
>          av_dlog(s->avctx, "%d %d %td %td\n", pic_arg->linesize[0],
>                  pic_arg->linesize[1], s->linesize, s->uvlinesize);
> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> index 27f1039..4c362f7 100644
> --- a/libavcodec/utils.c
> +++ b/libavcodec/utils.c
> @@ -177,12 +177,6 @@ int ff_side_data_update_matrix_encoding(AVFrame *frame,
>      return 0;
>  }
>  
> -#if HAVE_SIMD_ALIGN_16
> -#   define STRIDE_ALIGN 16
> -#else
> -#   define STRIDE_ALIGN 8
> -#endif
> -
>  void avcodec_align_dimensions2(AVCodecContext *s, int *width, int *height,
>                                 int linesize_align[AV_NUM_DATA_POINTERS])
>  {
> -- 
> 1.9.3 (Apple Git-50)
> 

It does not seem to me that this is the right way to handle this.

The same problem is likely to exist for many other encoders, both audio
and video. The simplest solution is probably to check alignment in
generic code in utils.c.

The patch also checks only data[0], which is certainly not enough.

On Fri, 23 Jan 2015, Anton Khirnov wrote:

> Quoting Martin Storsjö (2015-01-20 15:09:08)
>> From: Michael Niedermayer <michaelni@gmx.at>
>> 
>> CC: libav-stable@libav.org
>> ---
>>  libavcodec/internal.h      | 6 ++++++
>>  libavcodec/mpegvideo_enc.c | 4 ++++
>>  libavcodec/utils.c         | 6 ------
>>  3 files changed, 10 insertions(+), 6 deletions(-)
>> 
>> diff --git a/libavcodec/internal.h b/libavcodec/internal.h
>> index a68d613..2b50830 100644
>> --- a/libavcodec/internal.h
>> +++ b/libavcodec/internal.h
>> @@ -37,6 +37,12 @@
>>
>>  #define FF_SIGNBIT(x) (x >> CHAR_BIT * sizeof(x) - 1)
>> 
>> +#if HAVE_SIMD_ALIGN_16
>> +#   define STRIDE_ALIGN 16
>> +#else
>> +#   define STRIDE_ALIGN 8
>> +#endif
>> +
>>  typedef struct FramePool {
>>      /**
>>       * Pools for each data plane. For audio all the planes have the same size,
>> diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c
>> index be6fb08..baeba70 100644
>> --- a/libavcodec/mpegvideo_enc.c
>> +++ b/libavcodec/mpegvideo_enc.c
>> @@ -992,6 +992,10 @@ static int load_input_picture(MpegEncContext *s, const AVFrame *pic_arg)
>>              direct = 0;
>>          if ((s->width & 15) || (s->height & 15))
>>              direct = 0;
>> +        if (((intptr_t)(pic_arg->data[0])) & (STRIDE_ALIGN - 1))
>> +            direct = 0;
>> +        if (s->linesize & (STRIDE_ALIGN - 1))
>> +            direct = 0;
>>
>>          av_dlog(s->avctx, "%d %d %td %td\n", pic_arg->linesize[0],
>>                  pic_arg->linesize[1], s->linesize, s->uvlinesize);
>> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
>> index 27f1039..4c362f7 100644
>> --- a/libavcodec/utils.c
>> +++ b/libavcodec/utils.c
>> @@ -177,12 +177,6 @@ int ff_side_data_update_matrix_encoding(AVFrame *frame,
>>      return 0;
>>  }
>> 
>> -#if HAVE_SIMD_ALIGN_16
>> -#   define STRIDE_ALIGN 16
>> -#else
>> -#   define STRIDE_ALIGN 8
>> -#endif
>> -
>>  void avcodec_align_dimensions2(AVCodecContext *s, int *width, int *height,
>>                                 int linesize_align[AV_NUM_DATA_POINTERS])
>>  {
>> -- 
>> 1.9.3 (Apple Git-50)
>> 
>
> It does not seem to me that this is the right way to handle this.
>
> The same problem is likely to exist for many other encoders, both audio
> and video. The simplest solution is probably to check alignment in
> generic code in utils.c.
>
> The patch also checks only data[0], which is certainly not enough.

Ok, good points. I'll defer this one then, I was mostly interested in the 
other one, and I just picked this one along with it since it seemed 
similar.

// Martin