On Fri, Oct 26, 2012 at 12:44:22PM +0100, Måns Rullgård wrote:
> Kostya Shishkov <[email protected]> writes:
> 
> > On Thu, Sep 20, 2012 at 12:05:11PM +0100, Måns Rullgård wrote:
> >> Benjamin Larsson <[email protected]> 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
> 
> So can someone find out for sure?  It could be that it's supposed to
> output an empty packet at the end.

No, A64 muxer creates and writes an empty packet at the end if needed itself.
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to