[FFmpeg-devel] About avpicture_alloc (deprecate?)

[Reimar Döffinger]

On Sun, May 02, 2010 at 07:31:44PM +0200, Cyril Russo wrote:
> 
> Le 02/05/2010 17:57, Reimar D?ffinger a ?crit :
> >
> >I think you are confusing it with avcodec_alloc_frame.
> >Which is basically the reason I'd like to change it since I get
> >the suspicion a lot of people confuse the two.
> >Given the signature of
> >int avpicture_alloc(AVPicture *picture, enum PixelFormat pix_fmt, int width, int height)
> >it doesn't work at all for that purpose.
> >
> As soon as you use swscale, you need it.
> avcodec_alloc_frame allocate the frame structure but not the "data"
> buffer, while avpicture_alloc does.
> 
> So unless you want people starting to allocate data[0] et al, by
> themselves (and doing bad thing with alignment since it's very hard
> to find out the required stride for decoder, see issue1909)

The get_buffer functions are for allocating the data, and their
documentation also explains how to get proper values _if_ you want
to implement them yourselves (which is a good idea if performance is
important).

> That is:
>     AVFrame frame;
>     avcodec_decode_video2(context, &frame, .... // Might crash, it's
> confusing since the doc might say that it allocates frame

What do you mean by "doc might say"?
The documentation says
"sizeof(AVFrame) must not be used outside libav*."
I can understand that this maybe is easily overlooked, and maybe we
need a "common but often overlooked design considerations" that
mention this and padding requirements and some other things, but it
is documented in principle.
Btw. it only ever breaks when compiling against a different libavcodec
version than the version that is actually used in the end.

> But this leaks:
>     AVFrame frame;
>     avpicture_alloc((AVPicture*)&frame, pix_fmt, width, height);
>     avcodec_decode_video2(context, &frame, .... // Leaks

Well, the cast there might be a hint something's not right with this.
Worse, it still has exactly the same issues as the previous code,
it just leaks memory in addition.
Actually that is the only different that the avpicture_alloc makes (though
by chance it might "fix" things by changing the stack layout).

> For me, the option would probably to render picture & frame not
> binary compatible, so casting from any other is not possible. That
> way, when you deal with a AVFrame, you call x_frame functions, and
> when you deal with picture, you call x_picture functions.

I fail to see the point of it. Particularly since I doubt whether many
if any of the imgconvert.c functions should still be there (non-deprecated).