On 05/02/2019 20:08, Andreas Rheinhardt wrote: > Up until now, a fragment that got reused was zeroed twice: Once during > uninit and once during reading the next packet/extradata/buffer. The > second zeroing has now been made optional. > > This is also in preparation of actually reusing a fragment's units array. > Otherwise it would be lost during reading. > > Signed-off-by: Andreas Rheinhardt <andreas.rheinha...@googlemail.com> > --- > libavcodec/av1_metadata_bsf.c | 4 ++-- > libavcodec/av1_parser.c | 4 ++-- > libavcodec/cbs.c | 17 +++++++++++------ > libavcodec/cbs.h | 20 +++++++++++++++++--- > libavcodec/filter_units_bsf.c | 4 ++-- > libavcodec/h264_metadata_bsf.c | 4 ++-- > libavcodec/h264_redundant_pps_bsf.c | 4 ++-- > libavcodec/h265_metadata_bsf.c | 4 ++-- > libavcodec/mpeg2_metadata_bsf.c | 4 ++-- > libavcodec/trace_headers_bsf.c | 4 ++-- > libavcodec/vp9_metadata_bsf.c | 2 +- > 11 files changed, 45 insertions(+), 26 deletions(-) > > ... > diff --git a/libavcodec/cbs.c b/libavcodec/cbs.c > index b61dedb1eb..71f9fcbe32 100644 > --- a/libavcodec/cbs.c > +++ b/libavcodec/cbs.c > @@ -217,11 +217,13 @@ static int cbs_fill_fragment_data(CodedBitstreamContext > *ctx, > > int ff_cbs_read_extradata(CodedBitstreamContext *ctx, > CodedBitstreamFragment *frag, > - const AVCodecParameters *par) > + const AVCodecParameters *par, > + int reuse) > { > int err; > > - memset(frag, 0, sizeof(*frag)); > + if (!reuse) > + memset(frag, 0, sizeof(*frag)); > > err = cbs_fill_fragment_data(ctx, frag, par->extradata, > par->extradata_size); > @@ -237,11 +239,12 @@ int ff_cbs_read_extradata(CodedBitstreamContext *ctx, > > int ff_cbs_read_packet(CodedBitstreamContext *ctx, > CodedBitstreamFragment *frag, > - const AVPacket *pkt) > + const AVPacket *pkt, int reuse) > { > int err; > > - memset(frag, 0, sizeof(*frag)); > + if (!reuse) > + memset(frag, 0, sizeof(*frag)); > > if (pkt->buf) { > frag->data_ref = av_buffer_ref(pkt->buf); > @@ -266,11 +269,13 @@ int ff_cbs_read_packet(CodedBitstreamContext *ctx, > > int ff_cbs_read(CodedBitstreamContext *ctx, > CodedBitstreamFragment *frag, > - const uint8_t *data, size_t size) > + const uint8_t *data, size_t size, > + int reuse) > { > int err; > > - memset(frag, 0, sizeof(*frag)); > + if (!reuse) > + memset(frag, 0, sizeof(*frag)); > > err = cbs_fill_fragment_data(ctx, frag, data, size); > if (err < 0) > diff --git a/libavcodec/cbs.h b/libavcodec/cbs.h > index 229cb129aa..2265b5d5bd 100644 > --- a/libavcodec/cbs.h > +++ b/libavcodec/cbs.h > @@ -240,10 +240,15 @@ void ff_cbs_close(CodedBitstreamContext **ctx); > * This also updates the internal state, so will need to be called for > * codecs with extradata to read parameter sets necessary for further > * parsing even if the fragment itself is not desired. > + * > + * If reuse is not set, the fragment will be zeroed before usage; > + * otherwise, the fragment's units_allocated and units members must > + * be valid and all the other members have to be zero/NULL. > */ > int ff_cbs_read_extradata(CodedBitstreamContext *ctx, > CodedBitstreamFragment *frag, > - const AVCodecParameters *par); > + const AVCodecParameters *par, > + int reuse); > > /** > * Read the data bitstream from a packet into a fragment, then > @@ -252,10 +257,14 @@ int ff_cbs_read_extradata(CodedBitstreamContext *ctx, > * This also updates the internal state of the coded bitstream context > * with any persistent data from the fragment which may be required to > * read following fragments (e.g. parameter sets). > + * > + * If reuse is not set, the fragment will be zeroed before usage; > + * otherwise, the fragment's units_allocated and units members must > + * be valid and all the other members have to be zero/NULL. > */ > int ff_cbs_read_packet(CodedBitstreamContext *ctx, > CodedBitstreamFragment *frag, > - const AVPacket *pkt); > + const AVPacket *pkt, int reuse); > > /** > * Read a bitstream from a memory region into a fragment, then > @@ -264,10 +273,15 @@ int ff_cbs_read_packet(CodedBitstreamContext *ctx, > * This also updates the internal state of the coded bitstream context > * with any persistent data from the fragment which may be required to > * read following fragments (e.g. parameter sets). > + * > + * If reuse is not set, the fragment will be zeroed before usage; > + * otherwise, the fragment's units_allocated and units members must > + * be valid and all the other members have to be zero/NULL. > */ > int ff_cbs_read(CodedBitstreamContext *ctx, > CodedBitstreamFragment *frag, > - const uint8_t *data, size_t size); > + const uint8_t *data, size_t size, > + int reuse); > > > /**
I don't think this patch should be needed. Can we just document that your fragment must either be zeroed (so, allocated by av_*allocz() or memset() to zero) or you've called the ff_cbs_fragment_reset() (whatever the name is) function before any read call? It can even check that the user hasn't messed up by asserting that data, data_size and nb_units are all zero. Thanks, - Mark _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel