[FFmpeg-devel] MPEG user data handling (read & write sample patches attached)

[Daniel Kristjansson]

On Mon, 2010-07-05 at 02:14 +0200, Michael Niedermayer wrote:
> please add a -p when generating patches
ok

> anyway, your patches look odd
> [...]
> > @@ -962,6 +966,17 @@
> >                  pic->reference = 3;
> >          }
> >  
> > +        av_freep(&pic->user_data_buf);
> > +        pic->user_data_len = 0;
> > +        if (s->user_data_len)
> > +        {
> > +            pic->user_data_buf = av_malloc(s->user_data_len);
> > +            memcpy(pic->user_data_buf, s->user_data_buf,
> > +                   s->user_data_len);
> > +            pic->user_data_len = s->user_data_len;
> > +            s->user_data_len = 0;
> > +        }
> > +
> 
> thats odd
> why dont you store the user data in the current AVFrame but rather store
> it in the main context?

This is a bit of a leftover. When I was working with an earlier version
of the CEA-708 standard it was unclear on what do to in the presensce
of B-Frames, so a complicated reordering was necessary to make sure all
the user_data ended up in I-frames. Version D of that standard clarified
the point so I removed the reordering, but forgot that I could now move
the data out of the context. I'll fix this in the next version.

<snip>
> av_fast_realloc()
ok

> [...]
> > Index: libavcodec/mpegvideo_enc.c
> > ===================================================================
> > --- libavcodec/mpegvideo_enc.c	(revision 23934)
> > +++ libavcodec/mpegvideo_enc.c	(working copy)
> > @@ -201,6 +201,13 @@
> >              }
> >          }
> >      }
> > +
> > +    if (src->user_data_size && dst->type == FF_BUFFER_TYPE_INTERNAL)
> > +    {
> > +        dst->user_data = av_realloc(dst->user_data, src->user_data_size);
> > +        dst->user_data_size = src->user_data_size;
> > +        memcpy(dst->user_data, src->user_data, src->user_data_size);
> > +    }
> >  }
> 
> with that what has been allocated by the user must be freed by lavc
> thats different from other tables like MVs are handled

But in this case the data has been allocated by lavc on a frame copy.
I thought it was not safe to use pointers to the user data from the
original frame since it might have been returned to the user and they
might have free the data.

I have to say I'm not all that clear on this point. It looks like for
the internal copies of frames lavc keeps a buffer allocated for video
frame while the private context is in use and sets
FF_BUFFER_TYPE_INTERNAL as the frame type while externally managed
frames have FF_BUFFER_TYPE_SHARED set. Maybe the copies don't even
need a copy of the user data if they have already been encoded and
written to disk, as presumably these are I and P frames which we need
a copy of solely for generating P and B frames from.

-- Daniel