[FFmpeg-devel] [PATCH] avutil/frame: fix av_frame_copy for unknown layouts

[Marton Balint]

On Mon, 30 Jan 2017, Paul B Mahol wrote:

> On 1/30/17, wm4 <nfxjfg at googlemail.com> wrote:
>> On Mon, 30 Jan 2017 08:47:49 +0100
>> Hendrik Leppkes <h.leppkes at gmail.com> wrote:
>>
>>> On Mon, Jan 30, 2017 at 8:40 AM, wm4 <nfxjfg at googlemail.com> wrote:
>>> > On Mon, 30 Jan 2017 01:37:02 +0100
>>> > Marton Balint <cus at passwd.hu> wrote:
>>> >
>>> >> I wonder how unknown layouts ever worked without this?
>>> >>
>>> >> Signed-off-by: Marton Balint <cus at passwd.hu>
>>> >> ---
>>> >>  libavutil/frame.c | 2 +-
>>> >>  1 file changed, 1 insertion(+), 1 deletion(-)
>>> >>
>>> >> diff --git a/libavutil/frame.c b/libavutil/frame.c
>>> >> index c2f5509..a08e0c5 100644
>>> >> --- a/libavutil/frame.c
>>> >> +++ b/libavutil/frame.c
>>> >> @@ -725,7 +725,7 @@ int av_frame_copy(AVFrame *dst, const AVFrame *src)
>>> >>
>>> >>      if (dst->width > 0 && dst->height > 0)
>>> >>          return frame_copy_video(dst, src);
>>> >> -    else if (dst->nb_samples > 0 && dst->channel_layout)
>>> >> +    else if (dst->nb_samples > 0 && dst->channels > 0)
>>> >>          return frame_copy_audio(dst, src);
>>> >>
>>> >>      return AVERROR(EINVAL);
>>> >
>>> > The original code was written with the assumption that only channel
>>> > layouts exist in the AVFrame. And the Libav API follows that. This
>>> > patch will probably break Libav API users (or those who want to stay
>>> > compatible to them) some more.
>>>
>>> The copying code would have failed anyway if channels was 0 and not
>>> copied anything, so it doesn't break anything that worked before -
>>> except that it returns an error code now properly indicating the
>>> failure.
>>
>> Hm, ok then.
>>
>>> In any case, we've decide quite a while ago to forego any illusions of
>>> API or  ABI compatibility, because its impossible to guarantee it
>>> anyway.
>>
>> We still have API compatibility. The channels issue is one of the
>> only things that work radically different from Libav. But it looks like
>> this was broken long ago and we probably don't need to bother to fix it.
>
> The issue should be fixed, patch LGTM.

Thanks everyone, pushed.

Regards,
Marton