[FFmpeg-devel] [PATCH] pkt_pts reordering

[Vladimir Pantelic]

Uoti Urpala wrote:
> 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.

so lets just keep the int64_t value we have today :)