[FFmpeg-devel] [PATCH] lavc/decode: do not use a context variable for function-local data

Anton Khirnov anton at khirnov.net
Sat May 23 11:57:53 EEST 2020


Quoting James Almer (2020-05-22 19:32:07)
> On 5/22/2020 1:56 PM, Anton Khirnov wrote:
> > ---
> >  libavcodec/decode.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/libavcodec/decode.c b/libavcodec/decode.c
> > index f3327d74af..dbecdf3f46 100644
> > --- a/libavcodec/decode.c
> > +++ b/libavcodec/decode.c
> > @@ -586,6 +586,7 @@ static int decode_receive_frame_internal(AVCodecContext *avctx, AVFrame *frame)
> >  int attribute_align_arg avcodec_send_packet(AVCodecContext *avctx, const AVPacket *avpkt)
> >  {
> >      AVCodecInternal *avci = avctx->internal;
> > +    AVPacket buffer_pkt = { NULL };
> >      int ret;
> >  
> >      if (!avcodec_is_open(avctx) || !av_codec_is_decoder(avctx->codec))
> > @@ -597,16 +598,15 @@ int attribute_align_arg avcodec_send_packet(AVCodecContext *avctx, const AVPacke
> >      if (avpkt && !avpkt->size && avpkt->data)
> >          return AVERROR(EINVAL);
> >  
> > -    av_packet_unref(avci->buffer_pkt);
> >      if (avpkt && (avpkt->data || avpkt->side_data_elems)) {
> > -        ret = av_packet_ref(avci->buffer_pkt, avpkt);
> > +        ret = av_packet_ref(&buffer_pkt, avpkt);
> >          if (ret < 0)
> >              return ret;
> >      }
> >  
> > -    ret = av_bsf_send_packet(avci->bsf, avci->buffer_pkt);
> > +    ret = av_bsf_send_packet(avci->bsf, &buffer_pkt);
> >      if (ret < 0) {
> > -        av_packet_unref(avci->buffer_pkt);
> > +        av_packet_unref(&buffer_pkt);
> >          return ret;
> >      }
> 
> What's the gain here? You're not removing the context variable since
> it's used in the encode framework as well, and you're introducing a
> stack AVPacket (that, while harmless, is not even properly initialized).

The gain is clarity/readability. A context variable suggests it is used
to some longer-term state. This change makes it clear that the packets
is only used within this function. Even the name is rather confusing,
since the packet does not get buffered there.

-- 
Anton Khirnov


More information about the ffmpeg-devel mailing list