[FFmpeg-devel] Fwd: [PATCH] avutil/frame: document data alignment restriction

[Muhammad Faiz]

On Sat, Jul 22, 2017 at 9:14 PM, Nicolas George <george at nsup.org> wrote:
> Le quartidi 4 thermidor, an CCXXV, Muhammad Faiz a écrit :
>> The behaviour is based on assumptions:
>>     - copying audio data is cheap, so keeping data alignment is cheap,
>>     - copying video data is not cheap, so keeping data alignment is not cheap,
>>       e.g. crop filter.
>>
>> Should fix Ticket6349.
>
>> Note that after fc3a03fcf9cd7eafe7342e2508e6128888efa0bb, the crash has
>> been fixed.
>
> s/fixed/worked around in the most common situations/

It fixed the testcase's crash, but didn't fix the bug.

>
>>
>> Signed-off-by: Muhammad Faiz <mfcc64 at gmail.com>
>> ---
>>  libavutil/frame.h | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/libavutil/frame.h b/libavutil/frame.h
>> index 26261d7e40..1b08eec231 100644
>> --- a/libavutil/frame.h
>> +++ b/libavutil/frame.h
>> @@ -202,6 +202,9 @@ typedef struct AVFrame {
>>       * up to 16 bytes beyond the planes, if these filters are to be used,
>>       * then 16 extra bytes must be allocated.
>>       *
>
>> +     * For audio, the data should be aligned to CPUs alignment preference.
>
> I think this is a bad idea: it is an API change, and a rather bad one on
> top of that:
>
> - The "CPUs alignment preference" is not documented.

It is documented on linesize documentation.

>
> - Requiring the application to take care of it when it could be done by
>   the library is a waste of effort.

It is easy for application:
   - always use av_frame_get_buffer
   - never modify audio data pointer.

On the contrary, handling unaligned data/linesize in the library is not easy.

>
>> +     * For video, no restriction on the alignment.
>> +     *
>>       * NOTE: Except for hwaccel formats, pointers not needed by the format
>>       * MUST be set to NULL.
>>       */
>
> Regards,
>
> --
>   Nicolas George

Thank's.