[FFmpeg-devel] [PATCH] avformat/matroskadec: retain error codes in matroska_resync() and matroska_read_packet()

Sophia Wang skw at google.com
Mon Aug 15 20:27:47 EEST 2016


Of the 3 functions that directly add to matroska->packets, 2 of them will
always return 0 afterwards, and the third (matroska_parse_rm_audio) will
only return something other than 0 in the case of AVERROR(ENOMEM) or
AVERROR(EINVAL). All 3 functions are called from matroska_parse_block,
which immediately returns the result if there is no error. If an error does
occur, then the error code may eventually be returned, or it may be
overwritten by the return value of any of the 3 packet-adding functions.

matroska_parse_block is called from matroska_parse_cluster and its helper
matroska_parse_cluster_incremental, both of which simply return the result
(though it maybe overwritten by another invocation in
matroska_parse_cluster). So in any case, if packets are added, the only
sources of failure are ENOMEM and EINVAL, and I'm not sure if it makes
sense to write code to work around those.

On Wed, Aug 10, 2016 at 4:51 PM, Michael Niedermayer <michael at niedermayer.cc
> wrote:

> On Wed, Aug 10, 2016 at 10:24:08AM -0700, Sophia Wang wrote:
> > Signed-off-by: Sophia Wang <skw at google.com>
> > ---
> >  libavformat/matroskadec.c | 15 ++++++++++-----
> >  1 file changed, 10 insertions(+), 5 deletions(-)
> >
> > diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
> > index d07a092..8c809ad 100644
> > --- a/libavformat/matroskadec.c
> > +++ b/libavformat/matroskadec.c
> > @@ -738,13 +738,16 @@ static int matroska_read_close(AVFormatContext
> *s);
> >  static int matroska_resync(MatroskaDemuxContext *matroska, int64_t
> last_pos)
> >  {
> >      AVIOContext *pb = matroska->ctx->pb;
> > +    int64_t ret;
> >      uint32_t id;
> >      matroska->current_id = 0;
> >      matroska->num_levels = 0;
> >
> >      /* seek to next position to resync from */
> > -    if (avio_seek(pb, last_pos + 1, SEEK_SET) < 0)
> > -        goto eof;
> > +    if ((ret = avio_seek(pb, last_pos + 1, SEEK_SET)) < 0) {
> > +        matroska->done = 1;
> > +        return ret;
> > +    }
> >
> >      id = avio_rb32(pb);
> >
> > @@ -760,7 +763,6 @@ static int matroska_resync(MatroskaDemuxContext
> *matroska, int64_t last_pos)
> >          id = (id << 8) | avio_r8(pb);
> >      }
> >
> > -eof:
> >      matroska->done = 1;
> >      return AVERROR_EOF;
> >  }
>
> > @@ -3322,13 +3324,16 @@ static int matroska_parse_cluster(MatroskaDemuxContext
> *matroska)
> >  static int matroska_read_packet(AVFormatContext *s, AVPacket *pkt)
> >  {
> >      MatroskaDemuxContext *matroska = s->priv_data;
> > +    int ret;
> >
> >      while (matroska_deliver_packet(matroska, pkt)) {
> >          int64_t pos = avio_tell(matroska->ctx->pb);
> >          if (matroska->done)
> >              return AVERROR_EOF;
> > -        if (matroska_parse_cluster(matroska) < 0)
> > -            matroska_resync(matroska, pos);
> > +        if (matroska_parse_cluster(matroska) < 0) {
> > +            if ((ret = matroska_resync(matroska, pos)) < 0)
> > +                return ret;
> > +        }
> >      }
>
> is it possible that matroska_parse_cluster() adds packets to
> matroska->packets and then fails?
> if so and if matroska_resync subsequently fails too
> the previous code would have returned the packet in
> matroska_deliver_packet() the new code would not i think unless
> i miss something
> (the application would likley not call matroska_read_packet() again
> after it signaled EOF, and might or might not after an error even
> though there would the still be buffered packets prior to the error
>
> [...]
>
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> No great genius has ever existed without some touch of madness. --
> Aristotle
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>


More information about the ffmpeg-devel mailing list