On 5/1/2018 9:53 PM, Mark Thompson wrote: > On 01/05/18 18:33, James Almer wrote: >> On 4/30/2018 8:26 PM, Mark Thompson wrote: >>> --- >>> Main change since last time is including the array subscripts. Constants >>> are also cleaned up a bit, but stay in the cbs header (vp9.h could probably >>> be taken over for this purpose, but currently it's an unnamespaced header >>> used by the decoder so I haven't touched it). >>> >>> >>> configure | 2 + >>> doc/bitstream_filters.texi | 2 +- >>> libavcodec/Makefile | 1 + >>> libavcodec/cbs.c | 6 + >>> libavcodec/cbs.h | 1 + >>> libavcodec/cbs_internal.h | 1 + >>> libavcodec/cbs_vp9.c | 679 >>> +++++++++++++++++++++++++++++++++++ >>> libavcodec/cbs_vp9.h | 201 +++++++++++ >>> libavcodec/cbs_vp9_syntax_template.c | 390 ++++++++++++++++++++ >>> 9 files changed, 1282 insertions(+), 1 deletion(-) >>> create mode 100644 libavcodec/cbs_vp9.c >>> create mode 100644 libavcodec/cbs_vp9.h >>> create mode 100644 libavcodec/cbs_vp9_syntax_template.c >> >> LGTM. No apparent data copy on any of the read methods which is very >> promising for the AV1 implementation. >> Only CodedBitstreamType->write_unit() still seems a tad sub-optimal with >> the temp write_buffer, especially in this module where unlike >> mpeg2/h2645 you memcpy the data twice. > > It's copied twice in MPEG-2/H.26[45] as well - once from the write buffer to > the unit data buffers, then a second time to combine them into the fragment > data buffer (which also adds the emulation prevention in the H.26[45] case, > so not really a direct copy).
It's probably impossible to avoid the memcpys when assembling a fragment out of each unit data for most codecs, so we should focus on optimizing write_unit(). > > It's not very easy to do better - a possible way would be to allow the write > buffer to contain multiple units and make the reference point at the write > buffer itself, then we could avoid the first copy to a per-unit buffer. That > requires somewhat more care in writing, though, because some special cases > become nastier to handle (re-copying previous units on overflow; allocating a > new write buffer if unit data needs to live beyond the reassembly). > > In theory that can also elide the second copy in VP9 (because the frames are > just concatenated in the write buffer, and the superframe header placed at > the end), but in practice that isn't necessarily a benefit because you end up > with even more allocation if the following component ever needs to hold more > than one output packet at once. One option in write_unit() could be making the unit take ownership of the buffer as filled by the codec specific bitstream functions (what's currently priv->write_buffer) instead of allocating a new one within ff_cbs_alloc_unit_data and then copying data to it. This would make priv->write_buffer something local to write_unit(), allocated anew once per call, and then wrapped as the unit's buffer. This could be done with a new ff_cbs_wrap_unit_data() function or similar. > > So, I'm not intending to do anything with this for now, but if you want to > look at it (or have any other ideas you think I should pursue) then please do. > > Thanks, > > - Mark > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
