[FFmpeg-devel] [PATCH 1/2] avcodec/avpacket: Use av_copy_packet_side_data() in av_packet_copy_props()

[Michael Niedermayer]

On Wed, May 03, 2017 at 12:07:46PM +0200, wm4 wrote:
> On Wed, 3 May 2017 11:55:16 +0200
> Michael Niedermayer <michael at niedermayer.cc> wrote:
> 
> > On Wed, May 03, 2017 at 11:50:41AM +0200, Michael Niedermayer wrote:
> > > On Wed, May 03, 2017 at 11:37:35AM +0200, wm4 wrote:  
> > > > On Wed, 3 May 2017 11:29:04 +0200
> > > > Michael Niedermayer <michael at niedermayer.cc> wrote:
> > > >   
> > > > > On Wed, May 03, 2017 at 05:29:07AM +0200, wm4 wrote:  
> > > > > > On Wed,  3 May 2017 05:21:50 +0200
> > > > > > Michael Niedermayer <michael at niedermayer.cc> wrote:
> > > > > >     
> > > > > > > Fixes timeout
> > > > > > > Fixes: 1293/clusterfuzz-testcase-minimized-6054752074858496
> > > > > > > 
> > > > > > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/targets/ffmpeg
> > > > > > > Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> > > > > > > ---
> > > > > > >  libavcodec/avpacket.c | 3 +++
> > > > > > >  1 file changed, 3 insertions(+)
> > > > > > > 
> > > > > > > diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
> > > > > > > index 4bf830bb8a..ff7ee730a4 100644
> > > > > > > --- a/libavcodec/avpacket.c
> > > > > > > +++ b/libavcodec/avpacket.c
> > > > > > > @@ -557,6 +557,9 @@ FF_ENABLE_DEPRECATION_WARNINGS
> > > > > > >      dst->flags                = src->flags;
> > > > > > >      dst->stream_index         = src->stream_index;
> > > > > > >  
> > > > > > > +    if (!dst->side_data_elems);
> > > > > > > +        return av_copy_packet_side_data(dst, src);
> > > > > > > +
> > > > > > >      for (i = 0; i < src->side_data_elems; i++) {
> > > > > > >           enum AVPacketSideDataType type = src->side_data[i].type;
> > > > > > >           int size          = src->side_data[i].size;    
> > > > > > 
> > > > > > This doesn't look right...    
> > > > > 
> > > > > already fixed the ; locally
> > > > > 
> > > > > 
> > > > > [...]  
> > > > 
> > > > I didn't see that, I was referring to the fact that you call
> > > > av_copy_packet_side_data(), and only sometimes (at least by intention).
> > > > That requires at least an explanation in the commit message.  
> > > 
> > > av_packet_copy_props() would add side data to the destination packet
> > > it doesnt replace previously existing side data except in case of
> > > error.
> > > I dont know if that is intended but i didnt want to change it as that
> > > would be unrelated to this patch  
> > 
> > added
> > "av_copy_packet_side_data() is only used when it does not lead to a change in behaviour"
> > to the commit message
> 
> Sorry, that seems all kinds of hacky, especially with the "suspected"
> reason that the function is unexpectedly faster than the existing code
> with some memory analyzers. It just doesn't make sense to me.

Its not unexpected that O(N) is signifiantly slower than O(1)

The "suspect" comes only from myself not profiling the memory analyzer
to proof that the realloc() calls are what causes of the observed
significant speed difference


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The worst form of inequality is to try to make unequal things equal.
-- Aristotle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20170503/5d580e75/attachment.sig>