[FFmpeg-devel] SOC Qualifiction tasks

[Uoti Urpala]

On Mon, 2008-03-17 at 01:20 +0100, Michael Niedermayer wrote:
> On Mon, Mar 17, 2008 at 01:05:26AM +0200, Uoti Urpala wrote:
> > When I mentioned the AvCodecContext field I intended that as an internal
> > libavcodec implementation detail to move the hack inside libavcodec and
> > present a sane API outside. Meaning you'd have an API like
> > 
> > avcodec_decode_video(AvCodecContext *avctx, ..., union {void *p; double
> > d; int64_t i64;} user_data);
> 
> This is totally insane. Do you really suggest we break the API&ABI because
> you dont like it?

I do not mean that this would have to replace the current function.

>  And then add this union nonsense?

An union makes sense for the same reason it does in the epoll interface.
Making an user data field an union helps applications at no real cost to
the library. It must be possible to store a void * for general data; but
being able to store timestamps directly as double or int64_t when
they're all that's needed allows simpler code that doesn't deal with
memory allocations.

> Why in hell should this opaque user data be treated differently from every
> other field in AVCodecContext?!

The way it's treated is what is needed to make the functionality work. I
didn't claim this would be the cleanest possible implementation, but the
one that would be easiest to implement.

> No, we have a system that works, use of AVPackets as input might be cleaner
> the other suggestions are a mess at least as big as what we have.
> And the whole is pure bikeshed anyway.

The current system only works if the user application implements hacks
to work around its flaws. That it is possible to implement such hacks
and the work required for them is not huge does not make the issue "pure
bikeshed" IMO.

What exactly the function interface is and whether the parameters are
separate or collected in an AVPacket shouldn't cause more than minor
syntactic differences in client code (assuming a sane implementation of
each). The need to implement buffer callbacks to identify outputs even
if you want nothing to do with buffer management has a bigger impact on
client code and the changes are trickier to get right.


> > > > > We have coded_picture_number and display_picture_number in AVFrame, maybe
> > > > > they could be usefull here?
> > > > 
> > > > coded_picture_number doesn't tell anything about reordering with
> > > > LOW_DELAY, and the only references to display_picture_number seem to be
> > > > in encoding code.
> > > 
> > > Could be but theres nothing stoping a decoder from setting it.
> > 
> > There's nothing stopping it, but the mere existence of the field in
> > AVFrame isn't really any closer to a solution than having nothing at
> > all.
> 
> What exactly are you arguing about? We do NOT have support for frame level
> threading with LOW_DELAY. Any solution should use the existing API.

I'm not sure what your reply is about either... There was nothing
specific to threading in what I said about reordering information with
LOW_DELAY.

You said that coded_picture_number and display_picture_number "could be
useful". coded_picture_number is not useful and display_picture_number
is just an unused field in a struct during decoding (so it's not really
meaningfully part of the current decoding API either). So IMO the extent
to which they "could be useful" is quite limited.


I'm not sure whether giving an output picture number at decoding time
would be a good way to export ordering information or not. The most
obvious problem is whether you can always know it accurately when
decoding the frame, and how sanely could error cases be handled (at
least you can lose frames from the expected output order after giving a
number to a frame after them).