[FFmpeg-devel] [PATCH] pkt_pts reordering
Mon Jan 10 17:45:06 CET 2011
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 :)
More information about the ffmpeg-devel