On Wed, Apr 26, 2017 at 10:05:40PM +0200, wm4 wrote:
> On Thu, 27 Apr 2017 01:41:04 +0700
> Muhammad Faiz <mfc...@gmail.com> wrote:
>
> > On Wed, Apr 26, 2017 at 8:53 PM, wm4 <nfx...@googlemail.com> wrote:
> > > On Wed, 26 Apr 2017 20:09:58 +0700
> > > Muhammad Faiz <mfc...@gmail.com> wrote:
> > >
> > >> On Wed, Apr 26, 2017 at 5:34 PM, wm4 <nfx...@googlemail.com> wrote:
> > >> > On Wed, 26 Apr 2017 17:16:05 +0700
> > >> > Muhammad Faiz <mfc...@gmail.com> wrote:
> > >> >
> > >> >> This should fix Ticket6349
> > >> >>
> > >> >> Since 383057f8e744efeaaa3648a59bc577b25b055835, framequeue may
> > >> >> generate unaligned frame data.
> > >> >>
> > >> >> Signed-off-by: Muhammad Faiz <mfc...@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.
>
> Well yes, these things are not entirely documented. As a user I'd
> actually expect unaligned things to work, even if there's a performance
> impact.
>
> But still:
> - framequeue should probably not trigger low-performance code paths
> intentionally
> - it's hard to fix all the cases where unaligned things cause crashes
>
> AVPacket actually documented an alignment and padding requirement. I
> suppose AVFrame is less-well documented, because it has many use-cases
> associated (encoding, filtering, as direct rendering buffer for
> decoding, etc.).
>
> Where should we go from here? Personally I'd claim framequeue should be
> changed to produce aligned output.> For encoders and filter input, we > should probably add a flag whether unaligned input is supported, and if > not, have the generic code before it copy the frame to make it aligned. I agree in fact i added such a flag in 2011 (4d34b6c1a1254850e39a36f08f4d2730092a54db) within the API of that time to avfilter [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Rewriting code that is poorly written but fully understood is good. Rewriting code that one doesnt understand is a sign that one is less smart then the original author, trying to rewrite it will not make it better.
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
