Message ID | 20190424185021.2060-1-jamrial@gmail.com |
---|---|
State | New |
Headers | show |
Hello, James Almer: > cbs_mpeg2_free_slice() calls av_buffer_unref() on extra_information_ref, > meaning allocating with av_malloc() was not the intention. > > Signed-off-by: James Almer <jamrial@gmail.com> > --- > Couldn't find any mpeg2 sample containing these fields, so it's untested. > The leak is obvious regardless of that. >> libavcodec/cbs_mpeg2_syntax_template.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/libavcodec/cbs_mpeg2_syntax_template.c b/libavcodec/cbs_mpeg2_syntax_template.c > index 88cf453b17..672ff66141 100644 > --- a/libavcodec/cbs_mpeg2_syntax_template.c > +++ b/libavcodec/cbs_mpeg2_syntax_template.c > @@ -361,10 +361,11 @@ static int FUNC(slice_header)(CodedBitstreamContext *ctx, RWContext *rw, > current->extra_information_length = k; > if (k > 0) { > *rw = start; > - current->extra_information = > - av_malloc(current->extra_information_length); > - if (!current->extra_information) > + current->extra_information_ref = > + av_buffer_alloc(current->extra_information_length); > + if (!current->extra_information_ref) > return AVERROR(ENOMEM); > + current->extra_information = current->extra_information_ref->data; > for (k = 0; k < current->extra_information_length; k++) { > xui(1, extra_bit_slice, bit, 0); > xui(8, extra_information_slice[k], > a) The reason you can't find any samples is probably that they don't exist. The newest version of the standard that I have (Amendment 4 of ITU-T H.262|ISO/IEC 13818-2 [1] -- this is a prepublished document and should be taken with a grain of salt) contains this about this field: "Reserved. A decoder conforming to this Specification that encounters extra_information_slice in a bitstream shall ignore it (i.e. remove from the bitstream and discard). A bitstream conforming to this Specification shall not contain this syntax element." The version from 02/2000 contains the same. b) Shouldn't this buffer be padded? See 41ed2c38. c) The parsing process of the picture header seems wrong, too: The specs from 02/2000 contain an extra_information_picture construct just like extra_information_slice (the description of the semantics of these fields are analogouos); and the aformentioned version from 2012 now contains the possibility for content_description_data at this place which can also be arbitrarily large (there can be arbitrarily many content_description_data blocks, but each one can't be arbitrarily large). Should we implement the 02/2000 version (way simpler than the newer one), then this means that it might make sense to do this allocation stuff via a macro like in cbs_h2645. d) I have already said that this pre-published version should be taken with a grain of salt: The syntax in this version doesn't contain an extra_information_picture element any more; but the semantics still do. So it would be helpful if someone has the version currently in force. - Andreas [1]: https://www.itu.int/rec/T-REC-H.262-201202-T!Amd4/en
diff --git a/libavcodec/cbs_mpeg2_syntax_template.c b/libavcodec/cbs_mpeg2_syntax_template.c index 88cf453b17..672ff66141 100644 --- a/libavcodec/cbs_mpeg2_syntax_template.c +++ b/libavcodec/cbs_mpeg2_syntax_template.c @@ -361,10 +361,11 @@ static int FUNC(slice_header)(CodedBitstreamContext *ctx, RWContext *rw, current->extra_information_length = k; if (k > 0) { *rw = start; - current->extra_information = - av_malloc(current->extra_information_length); - if (!current->extra_information) + current->extra_information_ref = + av_buffer_alloc(current->extra_information_length); + if (!current->extra_information_ref) return AVERROR(ENOMEM); + current->extra_information = current->extra_information_ref->data; for (k = 0; k < current->extra_information_length; k++) { xui(1, extra_bit_slice, bit, 0); xui(8, extra_information_slice[k],
cbs_mpeg2_free_slice() calls av_buffer_unref() on extra_information_ref, meaning allocating with av_malloc() was not the intention. Signed-off-by: James Almer <jamrial@gmail.com> --- Couldn't find any mpeg2 sample containing these fields, so it's untested. The leak is obvious regardless of that. libavcodec/cbs_mpeg2_syntax_template.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)