| Message ID | 1474578189-135377-1-git-send-email-skw@google.com |
|---|---|
| State | Superseded |
| Headers | show |
Hi, On 22/09/2016 23:03, Sophia Wang wrote: > Signed-off-by: Sophia Wang <skw@google.com> > --- > libavformat/matroskadec.c | 13 ++++++++----- > 1 file changed, 8 insertions(+), 5 deletions(-) > > diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c > index 77b8a5d..936690d 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; doesn't this generate a warning, returning an int64 from a function supposed to return an int? > + } > > 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; > } > @@ -3317,13 +3319,14 @@ static int matroska_parse_cluster(MatroskaDemuxContext *matroska) > static int matroska_read_packet(AVFormatContext *s, AVPacket *pkt) > { > MatroskaDemuxContext *matroska = s->priv_data; > + int ret = 0; > > while (matroska_deliver_packet(matroska, pkt)) { > int64_t pos = avio_tell(matroska->ctx->pb); > if (matroska->done) > - return AVERROR_EOF; > + return (ret < 0) ? ret : AVERROR_EOF; > if (matroska_parse_cluster(matroska) < 0) > - matroska_resync(matroska, pos); > + ret = matroska_resync(matroska, pos); > } > > return 0; You might want to return ret instead of 0 here.
On Fri, Sep 23, 2016 at 1:40 AM, Benoit Fouet <benoit.fouet@free.fr> wrote: > Hi, > > > On 22/09/2016 23:03, Sophia Wang wrote: > >> Signed-off-by: Sophia Wang <skw@google.com> >> --- >> libavformat/matroskadec.c | 13 ++++++++----- >> 1 file changed, 8 insertions(+), 5 deletions(-) >> >> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c >> index 77b8a5d..936690d 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; >> > > doesn't this generate a warning, returning an int64 from a function > supposed to return an int? avio_seek returns an int64, so at some point an int64 is going to be converted to an int. I don't believe it will make a difference where, especially since the values that will actually be returned (error codes) should fit in an int. > > + } >> 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; >> } >> @@ -3317,13 +3319,14 @@ static int matroska_parse_cluster(MatroskaDemuxContext >> *matroska) >> static int matroska_read_packet(AVFormatContext *s, AVPacket *pkt) >> { >> MatroskaDemuxContext *matroska = s->priv_data; >> + int ret = 0; >> while (matroska_deliver_packet(matroska, pkt)) { >> int64_t pos = avio_tell(matroska->ctx->pb); >> if (matroska->done) >> - return AVERROR_EOF; >> + return (ret < 0) ? ret : AVERROR_EOF; >> if (matroska_parse_cluster(matroska) < 0) >> - matroska_resync(matroska, pos); >> + ret = matroska_resync(matroska, pos); >> } >> return 0; >> > > You might want to return ret instead of 0 here. Done. > > > -- > Ben > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c index 77b8a5d..936690d 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; } @@ -3317,13 +3319,14 @@ static int matroska_parse_cluster(MatroskaDemuxContext *matroska) static int matroska_read_packet(AVFormatContext *s, AVPacket *pkt) { MatroskaDemuxContext *matroska = s->priv_data; + int ret = 0; while (matroska_deliver_packet(matroska, pkt)) { int64_t pos = avio_tell(matroska->ctx->pb); if (matroska->done) - return AVERROR_EOF; + return (ret < 0) ? ret : AVERROR_EOF; if (matroska_parse_cluster(matroska) < 0) - matroska_resync(matroska, pos); + ret = matroska_resync(matroska, pos); } return 0;
Signed-off-by: Sophia Wang <skw@google.com> --- libavformat/matroskadec.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)