On Thu, Sep 20, 2012 at 12:05:11PM +0100, Måns Rullgård wrote:
> Benjamin Larsson <benja...@southpole.se> writes:
> 
> > libavcodec/a64multienc.c: In function ‘a64multi_encode_frame’:
> > libavcodec/a64multienc.c:342:17: warning: ‘buf’ may be used uninitialized 
> > in this function [-Wuninitialized]
> >
> > The compiler has trouble to figure out that the if block
> > if (c->mc_frame_counter == c->mc_lifetime) {
> > never should be entered with both values being 0. So add
> > an explicit check on one of the parameters.
> > ---
> >  libavcodec/a64multienc.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/libavcodec/a64multienc.c b/libavcodec/a64multienc.c
> > index c9d3b3c..9d3dca5 100644
> > --- a/libavcodec/a64multienc.c
> > +++ b/libavcodec/a64multienc.c
> > @@ -304,7 +304,7 @@ static int a64multi_encode_frame(AVCodecContext *avctx, 
> > AVPacket *pkt,
> >      }
> >
> >      /* lifetime reached so now convert X frames at once */
> > -    if (c->mc_frame_counter == c->mc_lifetime) {
> > +    if ((c->mc_frame_counter == c->mc_lifetime) && c->mc_lifetime) {
> >          req_size = 0;
> >          /* any frames to encode? */
> >          if (c->mc_lifetime) {
> > -- 
> 
> If this is correct, simpler would be to remove the if (c->mc_lifetime)
> condition on the last line above.  However, I do not think this is the
> case, at least it is not equivalent to the current code.  If the !pict
> condition is true, mc_lifetime and mc_frame_counter can both be set to
> zero (in a rather convoluted way).  This happens if encode() is called
> twice with a null pict parameter, which is normal to flush the delayed
> frames at the end of an encoding.  On the second call with null input,
> the encoder will return a zero-sized packet.  On the third, it finally
> reports end.  This seems somewhat strange, so it may well be an error.
> 
> Assuming the final zero-length packet is incorrect, there is a simpler
> solution to it all (untested):
> 
> diff --git a/libavcodec/a64multienc.c b/libavcodec/a64multienc.c
> index c9d3b3c..9de5458 100644
> --- a/libavcodec/a64multienc.c
> +++ b/libavcodec/a64multienc.c
> @@ -279,14 +279,10 @@ static int a64multi_encode_frame(AVCodecContext *avctx, 
> AVPacket *pkt,
>  
>      /* no data, means end encoding asap */
>      if (!pict) {
> +        /* limit lifetime to remaining frames */
> +        c->mc_lifetime = c->mc_frame_counter;
>          /* all done, end encoding */
>          if (!c->mc_lifetime) return 0;
> -        /* no more frames in queue, prepare to flush remaining frames */
> -        if (!c->mc_frame_counter) {
> -            c->mc_lifetime = 0;
> -        }
> -        /* still frames in queue so limit lifetime to remaining frames */
> -        else c->mc_lifetime = c->mc_frame_counter;
>      /* still new data available */
>      } else {
>          /* fill up mc_meta_charset with data until lifetime exceeds */
> @@ -306,8 +302,6 @@ static int a64multi_encode_frame(AVCodecContext *avctx, 
> AVPacket *pkt,
>      /* lifetime reached so now convert X frames at once */
>      if (c->mc_frame_counter == c->mc_lifetime) {
>          req_size = 0;
> -        /* any frames to encode? */
> -        if (c->mc_lifetime) {
>              req_size = charset_size + c->mc_lifetime*(screen_size + 
> colram_size);
>              if ((ret = ff_alloc_packet(pkt, req_size)) < 0) {
>                  av_log(avctx, AV_LOG_ERROR, "Error getting output packet of 
> size %d.\n", req_size);
> @@ -328,7 +322,6 @@ static int a64multi_encode_frame(AVCodecContext *avctx, 
> AVPacket *pkt,
>              /* advance pointers */
>              buf      += charset_size;
>              charset  += charset_size;
> -        }
>  
>          /* write x frames to buf */
>          for (frame = 0; frame < c->mc_lifetime; frame++) {
> 
> -- 

probably OK
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to