[FFmpeg-devel] [PATCH] Check av_dup_packet() return code

Michael Niedermayer michael at niedermayer.cc
Mon Feb 8 13:49:41 CET 2016


On Mon, Feb 08, 2016 at 12:46:11PM +0100, wm4 wrote:
> On Mon, 8 Feb 2016 00:33:15 +0100
> Michael Niedermayer <michael at niedermayer.cc> wrote:
> 
> > On Wed, Feb 03, 2016 at 11:27:00PM +0100, Andreas Cadhalpun wrote:
> > > On 03.02.2016 19:05, Michael Niedermayer wrote:  
> > > > Fixes: CID1338320
> > > > 
> > > > Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> > > > ---
> > > >  libavcodec/frame_thread_encoder.c |    4 +++-
> > > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/libavcodec/frame_thread_encoder.c b/libavcodec/frame_thread_encoder.c
> > > > index 04c9a0e..27ae356 100644
> > > > --- a/libavcodec/frame_thread_encoder.c
> > > > +++ b/libavcodec/frame_thread_encoder.c
> > > > @@ -89,7 +89,9 @@ static void * attribute_align_arg worker(void *v){
> > > >          pthread_mutex_unlock(&c->buffer_mutex);
> > > >          av_frame_free(&frame);
> > > >          if(got_packet) {
> > > > -            av_dup_packet(pkt);
> > > > +            int ret2 = av_dup_packet(pkt);
> > > > +            if (ret >= 0 && ret2 < 0)
> > > > +                ret = ret2;
> > > >          } else {
> > > >              pkt->data = NULL;
> > > >              pkt->size = 0;
> > > >   
> > > 
> > > Checking the return code this way is probably OK, but maybe you can also replace the
> > > deprecated av_dup_packet with av_packet_ref while at it?  
> > 
> > what exactly do you suggest ?
> > 
> > av_packet_ref() creates a reference, av_dup_packet() turns "reusable"
> > packets into ref counted ones
> > av_dup_packet() does nothing if the packet is already a ref counted
> > one.
> > 
> > i can not see a efficient and clean way to replace av_dup_packet()
> > by av_packet_ref()
> > 
> > if its ref counted already nothing needs to be done, if its not
> > ref counted i would have to litterally reimplement av_dup_packet()
> > and i dont even have a testcase for that, all cases seem to return
> > ref counted packets
> > 
> > i can remove av_dup_packet and put an assert there assuming that
> > all packets will always be already ref counted (this might be true
> > currently, i dont know, i also dont know if we always want this to
> > be true in the future)
> > 
> > I dont really want to always run av_packet_ref() over it just to
> > unref it in the next line, which probably would work too
> > 
> > [...]
> 
> It's deprecated?

av_dup_packet() has been marked as deprecated, yes, the function or a
equivalent is needed though for efficicent support of non ref counted
packets.
Because what it does is exactly that it ensures that the packet
after the call is ref counted

iam happy with any solution the community likes

av_dup_packet() can be undeprecated or aded back under a different name

support for non reference counted packets can be removed (with the
obvious consequence that its then not supported anymore)

a redundant reference can always be added (this feels silly though as
it performs operations which take cpu time& thread sync when they are
entirely unneeded)

something else, there likely is some other option ...


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

Those who would give up essential Liberty, to purchase a little
temporary Safety, deserve neither Liberty nor Safety -- Benjamin Franklin
-------------- 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/20160208/9eeed431/attachment.sig>


More information about the ffmpeg-devel mailing list