[FFmpeg-devel] [PATCH] pkt_pts reordering

[Uoti Urpala]

On Mon, 2011-01-10 at 17:01 +0100, Vladimir Pantelic wrote:
> Uoti Urpala wrote:
> > On Mon, 2011-01-10 at 15:26 +0100, Vladimir Pantelic wrote:
> >>  yes, a void* value would be better indeed now
> >>
> >>  The use case is to allow the user to carry frame private data from
> >>  a pkt to decoded frame
> >
> > The current int64_t type for the opaque field is poorly chosen; at least
> > it should be an union of int64_t/double/void *. Storing timestamps as
> > doubles is one good use case which is not "non timestamp" but where
> > libavcodec trying to do anything with the value interpreted as int64_t
> > would be wrong. Another could be an application wanting to just create a
> > map of the input->output reordering - you could use "pts" increasing by
> > one for each packet for that, but wouldn't want libavcodec to do any
> > interpretation based on that value.
> >
> > "void *" uses suffer from the problem that if you point to allocated
> > memory you can't easily know when it could be freed in cases where the
> > pointer will never be returned from libavcodec (there's no explicit
> > "it's now known this value will never have any corresponding output
> > frame").
> 
> The only thing needed is a way for the libav* users to attach
> arbitrary and opaque data when decoding and get that back
> again reordered correctly. libav* does not need to "interpret"
> this data in any way, so void* is fine IMHO.

As I tried to explain above, "attaching arbitrary data" through a "void
*" type wouldn't be that easy. The general pattern would be to allocate
memory for the data, then place the pointer to the allocated memory in
the "void *" field. But here a straightforward implementation of this
pattern would lead to a memory leak, since some of the pointers may
never be returned by libavcodec.

Just going from "int64_t" to "void *" would actually make things worse:
in practice you at least use the int64_t field as a storage for any type
fitting in 8 bytes (though an explicit union of common types like
int64_t, double and void * would of course be better), but "void *"
would be smaller on 32-bit platforms and thus you'd really _have to_
allocate external memory to hold the actual value in more cases,
triggering the above problem.