[FFmpeg-devel] [PATCH 4/6] lavc: Add coded bitstream read/write support for VP9

[Mark Thompson]

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 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.

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