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 <mich...@niedermayer.cc
> wrote:
> On Wed, Aug 10, 2016 at 10:24:08AM -0700, Sophia Wang wrote:
> > Signed-off-by: Sophia Wang <s...@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@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
