Andreas Rheinhardt: > Kieran Kunhya: >> On Fri, 16 Aug 2019 at 06:08, Andreas Rheinhardt < >> andreas.rheinha...@gmail.com> wrote: >> >>> Kieran Kunhya: >>>> On Fri, 16 Aug 2019 at 04:20, Andreas Rheinhardt < >>>> andreas.rheinha...@gmail.com> wrote: >>>> >>>>> Up until now, the H.264 parser has allocated a new buffer for every >>>>> frame in order to store the unescaped RBSP in case the part of the RBSP >>>>> that will be unescaped contains 0x03 escape bytes. This is expensive >>>>> and this commit changes this: The buffer is now kept and reused. >>>>> >>>>> For an AVC file with an average framesize of 15304 B (without in-band >>>>> parameter sets etc.) and 132044 frames (131072 of which ended up in the >>>>> results) this reduced the average time used by the H.264 parser from >>>>> 87708 decicycles (excluding about 1100-1200 skips in each iteration) >>>>> to 16963 decicycles (excluding about 10-15 skips in each iteration). >>>>> The test has been iterated 10 times. >>>>> >>>> >>>> If I understand correctly, this patch means if you have a large frame (or >>>> large NAL) you are stuck with the large allocation of memory until the >>>> decoder is closed. >>>> Not sure if you have read the discussion here >>>> https://patchwork.ffmpeg.org/patch/5631/ >>>> >>>> Kieran >>>> >>> I am aware of said discussion; and also of your solution [1] to it. It >>> actually does exactly the same as I propose for the parser: It keeps a >>> single buffer that does not shrink. Given that this is used for the >>> decoders (and for cbs_h2645), I didn't deem this to be problematic. >>> (There is clearly no quadratic memory usage and what Derek warns about >>> (with huge NALs at specific positions) is a no issue for both.) >>> >>> - Andreas >>> >>> [1]: https://ffmpeg.org/pipermail/ffmpeg-devel/2017-November/219100.html >> >> >> My solution frees the buffer when it's done. With yours it stays around as >> a large buffer essentially forever. >> >> Kieran >> > Your solution frees the buffer in the parser when it's done, but the > buffers for the decoders are kept (and only grow) during decoding. So > this commit merely does for the parser what is already done for the > deocder. > Anyway, it would be easy to add a check to the parser to free the > buffer if it is considered excessively large. I don't know how easy it > would be to add such precautions to the decoder, though. I am also not > sure what should be considered excessively large? 5 MB? 10 MB? Setting > it so high that even the highest profiles can't hit it is impossible, > because for certain profiles ((Progressive) High 10, High 4:2:2, ...) > no limit is imposed at all (apart from that, such a limit would surely > be too high). > > - Andreas > Ping. What is other's opinion on this matter? Notice that the current behaviour is suboptimal even if it is decided that the buffer should not be kept: It allocates 1/16 more than needed (in addition to AV_INPUT_BUFFER_PADDING_SIZE) that is guaranteed not to be used; and it uses mallocz for the allocation.
- Andreas _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".