[FFmpeg-devel] [PATCH] avcodec/libmp3lame: properly handle unaligned frame data

[Muhammad Faiz]

On Wed, Apr 26, 2017 at 8:53 PM, wm4 <nfxjfg at googlemail.com> wrote:
> On Wed, 26 Apr 2017 20:09:58 +0700
> Muhammad Faiz <mfcc64 at gmail.com> wrote:
>
>> On Wed, Apr 26, 2017 at 5:34 PM, wm4 <nfxjfg at googlemail.com> wrote:
>> > On Wed, 26 Apr 2017 17:16:05 +0700
>> > Muhammad Faiz <mfcc64 at gmail.com> wrote:
>> >
>> >> This should fix Ticket6349
>> >>
>> >> Since 383057f8e744efeaaa3648a59bc577b25b055835, framequeue may
>> >> generate unaligned frame data.
>> >>
>> >> Signed-off-by: Muhammad Faiz <mfcc64 at gmail.com>
>> >> ---
>> >>  libavcodec/libmp3lame.c | 13 +++++++++----
>> >>  1 file changed, 9 insertions(+), 4 deletions(-)
>> >>
>> >> diff --git a/libavcodec/libmp3lame.c b/libavcodec/libmp3lame.c
>> >> index 5e26743..cd505bb 100644
>> >> --- a/libavcodec/libmp3lame.c
>> >> +++ b/libavcodec/libmp3lame.c
>> >> @@ -203,15 +203,20 @@ static int mp3lame_encode_frame(AVCodecContext *avctx, AVPacket *avpkt,
>> >>              ENCODE_BUFFER(lame_encode_buffer_int, int32_t, frame->data);
>> >>              break;
>> >>          case AV_SAMPLE_FMT_FLTP:
>> >> -            if (frame->linesize[0] < 4 * FFALIGN(frame->nb_samples, 8)) {
>> >> -                av_log(avctx, AV_LOG_ERROR, "inadequate AVFrame plane padding\n");
>> >> -                return AVERROR(EINVAL);
>> >> -            }
>> >>              for (ch = 0; ch < avctx->channels; ch++) {
>> >> +                if (frame->linesize[0] < 4 * FFALIGN(frame->nb_samples, 8) || 0x1F & (intptr_t)(frame->data[ch])) {
>> >> +                    float *src = (float *) frame->data[ch];
>> >> +                    float *dst = s->samples_flt[ch];
>> >> +                    int k;
>> >> +
>> >> +                    for (k = 0; k < frame->nb_samples; k++)
>> >> +                        dst[k] = src[k] * 32768.0f;
>> >> +                } else {
>> >>                  s->fdsp->vector_fmul_scalar(s->samples_flt[ch],
>> >>                                             (const float *)frame->data[ch],
>> >>                                             32768.0f,
>> >>                                             FFALIGN(frame->nb_samples, 8));
>> >> +                }
>> >>              }
>> >>              ENCODE_BUFFER(lame_encode_buffer_float, float, s->samples_flt);
>> >>              break;
>> >
>> > Looks like the right solution is fixing framequeue.
>> >
>> > I don't think encoders generally allow lax alignment, and this might
>> > not be the only case.
>>
>> This requirement is not documented. Also some filters may also
>> generate unaligned data, for example crop filter.
>
> No - and it's inconsistently handled. But in general, everything seems
> to prefer aligned data, and blows up or becomes slow otherwise. Keep in
> mind that all data is allocated with large alignment in the first place.

Based on looking the code, it seems that volume filter cannot handle
unaligned data, probably others too.
Having this restriction without documenting it is considered bug.

>
> The crop filter is sort of a special case too.

As consequence, video filters/codecs can handle it consistently.
Note that frame.h only states that linesize should be multiple of
16/32. It does not state about data pointer. And crop filter does not
modify linesize, and generating unaligned data pointer is legitimate.