[FFmpeg-devel] [PATCH 1/4] avformat/flvenc: Forward errors from allocating keyframe index

[Andreas Rheinhardt]

On Fri, Oct 25, 2019 at 11:12 PM Michael Niedermayer <michael at niedermayer.cc>
wrote:

> On Fri, Oct 25, 2019 at 10:52:19AM +0200, Andreas Rheinhardt wrote:
> > While the function adding a new element to the keyframe index checked
> > the allocation, the caller didn't check the return value. This has been
> > changed. To do so, the return value has been changed to an ordinary ret
> > instead of pb->error. This doesn't pose a problem, as write_packet() in
> > mux.c already checks for write errors (since 9ad1e0c1).
> >
> > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
> > ---
> >  libavformat/flvenc.c | 16 +++++++++-------
> >  1 file changed, 9 insertions(+), 7 deletions(-)
> >
> > diff --git a/libavformat/flvenc.c b/libavformat/flvenc.c
> > index fb1dede7ae..e327545dcf 100644
> > --- a/libavformat/flvenc.c
> > +++ b/libavformat/flvenc.c
> > @@ -887,7 +887,7 @@ static int flv_write_packet(AVFormatContext *s,
> AVPacket *pkt)
> >      unsigned ts;
> >      int size = pkt->size;
> >      uint8_t *data = NULL;
> > -    int flags = -1, flags_size, ret;
> > +    int flags = -1, flags_size, ret = 0;
> >      int64_t cur_offset = avio_tell(pb);
> >
> >      if (par->codec_type == AVMEDIA_TYPE_AUDIO && !pkt->size) {
> > @@ -1055,15 +1055,17 @@ static int flv_write_packet(AVFormatContext *s,
> AVPacket *pkt)
> >              case AVMEDIA_TYPE_VIDEO:
> >                  flv->videosize += (avio_tell(pb) - cur_offset);
> >                  flv->lasttimestamp = flv->acurframeindex /
> flv->framerate;
> > +                flv->acurframeindex++;
> >                  if (pkt->flags & AV_PKT_FLAG_KEY) {
> > -                    double ts = flv->acurframeindex / flv->framerate;
> > +                    double ts = flv->lasttimestamp;
> >                      int64_t pos = cur_offset;
> >
> > -                    flv->lastkeyframetimestamp = flv->acurframeindex /
> flv->framerate;
> > +                    flv->lastkeyframetimestamp = ts;
> >                      flv->lastkeyframelocation = pos;
> > -                    flv_append_keyframe_info(s, flv, ts, pos);
> > +                    ret = flv_append_keyframe_info(s, flv, ts, pos);
> > +                    if (ret < 0)
> > +                        goto fail;
> >                  }
> > -                flv->acurframeindex++;
> >                  break;
>
> Some of these changes look like unrelated simplifications
> that makes it a hair harder to see the actual change
>

The earlier code incremented flv->acurframeindex even when the index
allocation failed.
I wanted to keep it that way and the easiest way to do so is to move the
code incrementing
it before flv_append_keyframe_info(). But this would change the values of
ts and
flv->lastkeyframetimestamp, so one has to change how they are derived, too.

- Andreas