[FFmpeg-devel] [PATCH] avcodec/internal: improve min_size documentation for ff_alloc_packet2()

Michael Niedermayer michael at niedermayer.cc
Tue Aug 4 15:01:10 CEST 2015


On Tue, Aug 04, 2015 at 02:06:03PM +0200, wm4 wrote:
> On Tue, 4 Aug 2015 01:46:24 +0200
> Michael Niedermayer <michael at niedermayer.cc> wrote:
> 
> > On Tue, Aug 04, 2015 at 12:57:38AM +0200, wm4 wrote:
> > > On Tue, 4 Aug 2015 00:51:17 +0200
> > > Michael Niedermayer <michael at niedermayer.cc> wrote:
> > > 
> > > > On Tue, Aug 04, 2015 at 12:12:24AM +0200, wm4 wrote:
> > > 
> > > > > > As well as benchmark results in some commits like
> > > > > > 4302a92835313000d4bf8030baae32c2b130d8a1
> > > > > 
> > > > >     avcodec/pcm: Better min_size for ff_alloc_packet2()
> > > > >     
> > > > >     33318 -> 30601 decicycles
> > > > > 
> > > > > That doesn't look convincing at all. So you save ~3000 decicycles for
> > > > > a task that isn't CPU bound, but which might blocks entire milliseconds
> > > > > on I/O.
> > > > 
> > > > There was a bug report of a overall (aka time ffmpeg) speedloss
> > > > in the % values with high res rawvideo
> > > 
> > > Was it fixed?
> > 
> > for rawideo and for several others too yes
> 
> I see no proof though. The numbers you posted so far are not really
> convincing. They're micro-benchmarks too, and might not even matter in
> real world work loads.

try some high res rawvideo like 4k
it should be very noticable IIRC

and there was a real bug report about
this so it did occur in a real world use case


> 
> > iam pretty sure there are other encoders that suffer from speedloss
> > due to this too though
> 
> What is "this"?
> 
> > 
> > > 
> > > > > > i do intend to fine tune this for more codecs but i spend more time
> > > > > > in such disussions than working on the code and having people tell me
> > > > > > to do it differently each time i take a step kind of slows it down
> > > > > > and turns it into a mess
> > > > > 
> > > > > We also must take care of not turning the codebase into a mess.
> > > > 
> > > > I want to fix the mess
> > > 
> > > I don't mind, but...
> > > 
> > > > and i would suggest that we first test if any / which encoders benefit
> > > > from the static buffer.
> > > > It where quite a few long ago but it seems from a few quick tests that
> > > > this is not so anymore.
> > > > Can someone confirm ?
> > > > if it has become useless then I could drop quite a bit of code
> > > 
> > > Do you have to do your experiments on the live codebase? If you just
> > > want to try something, you could do that in a branch. If it gives good
> > > results, merge, if not, it's a failed experiment.
> > 
> > noone did "experiments on the live codebase"
> 
> Then what's that "FIXME" in the heuristic, and the new API parameter
> that by now uses only 2 possible values, which make this "heuristic"
> completely pointless?

> What bugs me is that you insist on this (IMHO)

i wrote very clearly that i picked this option after waiting for 20
days and asking people what they prefer. And noone cared or suggested
anything that people liked
I have not the slightest preferrance for min_size over other solutions.


[...]

> Now I don't claim I can do better (actually I do, so feel free to call
> me an idiot), but here are my own (untested) conclusions.

you are certainly not an idiot. You are a very talented developer
and you have good ideas about designs.
But discussing with you withiout the discussion becoming a battlefield
of trolls and strawman is something i clearly fail at.


[...]
> I suppose the min_size parameter is supposed to select between these
> cases dynamically and use the "best" method. I suggest doing the
> following instead:
> 
> - have a buffer pool in the codec context
> - if a packet is allocated with a size larger then the current pool
>   size, realloc the buffer pool to that larger size
> - likewise, if the requested packet is much smaller than the current
>   pool size, you could either resize the pool, or just allocate the
>   packet normally (apply clever heuristic 1 here)
> - the encoder gets a refcounted packet from the buffer pool
> - after encoding, check how much memory is "wasted"
> - if too much is wasted (apply clever heuristic 2 here), create a new
>   copy (this could even use another buffer pool which might help if the
>   encoded packet sizes are usually similar, but maybe it's not worth
>   the trouble)

this is a good idea but more complex than what i have time to
implement in the near future.


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Breaking DRM is a little like attempting to break through a door even
though the window is wide open and the only thing in the house is a bunch
of things you dont want and which you would get tomorrow for free anyway
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20150804/084b7413/attachment.sig>


More information about the ffmpeg-devel mailing list