Re: [FFmpeg-devel] [PATCH] lavf/mov: atom box parsing return eof cause play fail
On Thu, Dec 07, 2017 at 05:47:46PM +0800, tiejun.peng wrote: > fix eof lead to play fail. > > Signed-off-by: tiejun.peng> --- > libavformat/mov.c | 47 --- > 1 file changed, 36 insertions(+), 11 deletions(-) please split the addition of warning messages from the change to EOF behavior did you check that every EOF return case is safe to continue as if no error occured ? That change has quite wide effects possibly unless i misunderstand. [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Republics decline into democracies and democracies degenerate into despotisms. -- Aristotle signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] lavf/mov: atom box parsing return eof cause play fail
fix eof lead to play fail. Signed-off-by: tiejun.peng--- libavformat/mov.c | 47 --- 1 file changed, 36 insertions(+), 11 deletions(-) diff --git a/libavformat/mov.c b/libavformat/mov.c index c901859..6c3567f 100644 --- a/libavformat/mov.c +++ b/libavformat/mov.c @@ -1991,8 +1991,10 @@ static int mov_read_stco(MOVContext *c, AVIOContext *pb, MOVAtom atom) sc->chunk_count = i; -if (pb->eof_reached) +if (pb->eof_reached) { +av_log(c->fc, AV_LOG_WARNING, "reached eof, corrupted STCO atom\n"); return AVERROR_EOF; +} return 0; } @@ -2522,8 +2524,10 @@ int ff_mov_read_stsd_entries(MOVContext *c, AVIOContext *pb, int entries) sc->stsd_count++; } -if (pb->eof_reached) +if (pb->eof_reached) { +av_log(c->fc, AV_LOG_WARNING, "reached eof, corrupted STSD atom\n"); return AVERROR_EOF; +} return 0; } @@ -2624,8 +2628,10 @@ static int mov_read_stsc(MOVContext *c, AVIOContext *pb, MOVAtom atom) sc->stsc_count = i; -if (pb->eof_reached) +if (pb->eof_reached) { +av_log(c->fc, AV_LOG_WARNING, "reached eof, corrupted STSC atom\n"); return AVERROR_EOF; +} return 0; } @@ -2676,8 +2682,10 @@ static int mov_read_stps(MOVContext *c, AVIOContext *pb, MOVAtom atom) sc->stps_count = i; -if (pb->eof_reached) +if (pb->eof_reached) { +av_log(c->fc, AV_LOG_WARNING, "reached eof, corrupted STPS atom\n"); return AVERROR_EOF; +} return 0; } @@ -2723,8 +2731,10 @@ static int mov_read_stss(MOVContext *c, AVIOContext *pb, MOVAtom atom) sc->keyframe_count = i; -if (pb->eof_reached) +if (pb->eof_reached) { +av_log(c->fc, AV_LOG_WARNING, "reached eof, corrupted STSS atom\n"); return AVERROR_EOF; +} return 0; } @@ -2808,8 +2818,10 @@ static int mov_read_stsz(MOVContext *c, AVIOContext *pb, MOVAtom atom) av_free(buf); -if (pb->eof_reached) +if (pb->eof_reached) { +av_log(c->fc, AV_LOG_WARNING, "reached eof, corrupted STSZ atom\n"); return AVERROR_EOF; +} return 0; } @@ -2870,8 +2882,10 @@ static int mov_read_stts(MOVContext *c, AVIOContext *pb, MOVAtom atom) sc->duration_for_fps += duration; sc->nb_frames_for_fps += total_sample_count; -if (pb->eof_reached) +if (pb->eof_reached) { +av_log(c->fc, AV_LOG_WARNING, "reached eof, corrupted STTS atom\n"); return AVERROR_EOF; +} st->nb_frames= total_sample_count; if (duration) @@ -2948,8 +2962,10 @@ static int mov_read_ctts(MOVContext *c, AVIOContext *pb, MOVAtom atom) sc->ctts_count = ctts_count; -if (pb->eof_reached) +if (pb->eof_reached) { +av_log(c->fc, AV_LOG_WARNING, "reached eof, corrupted CTTS atom\n"); return AVERROR_EOF; +} av_log(c->fc, AV_LOG_TRACE, "dts shift %d\n", sc->dts_shift); @@ -2995,7 +3011,12 @@ static int mov_read_sbgp(MOVContext *c, AVIOContext *pb, MOVAtom atom) sc->rap_group_count = i; -return pb->eof_reached ? AVERROR_EOF : 0; +if (pb->eof_reached) { +av_log(c->fc, AV_LOG_WARNING, "reached eof, corrupted SBGP atom\n"); +return AVERROR_EOF; +} + +return 0; } /** @@ -4720,8 +4741,10 @@ static int mov_read_trun(MOVContext *c, AVIOContext *pb, MOVAtom atom) fix_frag_index_entries(>frag_index, next_frag_index, frag->track_id, entries); -if (pb->eof_reached) +if (pb->eof_reached) { +av_log(c->fc, AV_LOG_WARNING, "reached eof, corrupted TRUN atom\n"); return AVERROR_EOF; +} frag->implicit_offset = offset; @@ -6609,7 +6632,9 @@ static int mov_read_header(AVFormatContext *s) do { if (mov->moov_retry) avio_seek(pb, 0, SEEK_SET); -if ((err = mov_read_default(mov, pb, atom)) < 0) { +/* EOF don't mean the file to play fail*/ +err = mov_read_default(mov, pb, atom); +if (err < 0 && err != AVERROR_EOF) { av_log(s, AV_LOG_ERROR, "error reading header\n"); mov_read_close(s); return err; -- 2.7.4 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavf/mov: atom box parsing return eof cause play fail
2017-12-06 9:25 GMT+01:00 tiejun.peng: > /* check MOV header */ > do { > -if (mov->moov_retry) > -avio_seek(pb, 0, SEEK_SET); > -if ((err = mov_read_default(mov, pb, atom)) < 0) { > -av_log(s, AV_LOG_ERROR, "error reading header\n"); > -mov_read_close(s); > -return err; > -} > +if (mov->moov_retry) > +avio_seek(pb, 0, SEEK_SET); > +/* EOF don't mean the file to play fail*/ > +err = mov_read_default(mov, pb, atom); > +if (err < 0 && err != AVERROR_EOF) { > +av_log(s, AV_LOG_ERROR, "error reading header\n"); > +mov_read_close(s); > +return err; > +} Most of this change should be a separate (purely cosmetic) patch to make reviewing the change (now and later) easier. Feel free to only send your functional change (without the re-indentation) now, the indentation can still be fixed. Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] lavf/mov: atom box parsing return eof cause play fail
fix eof lead to play fail. Signed-off-by: tiejun.peng--- libavformat/mov.c | 59 +++ 1 file changed, 42 insertions(+), 17 deletions(-) diff --git a/libavformat/mov.c b/libavformat/mov.c index c901859..870fdd6 100644 --- a/libavformat/mov.c +++ b/libavformat/mov.c @@ -1991,8 +1991,10 @@ static int mov_read_stco(MOVContext *c, AVIOContext *pb, MOVAtom atom) sc->chunk_count = i; -if (pb->eof_reached) +if (pb->eof_reached) { +av_log(c->fc, AV_LOG_WARNING, "reached eof, corrupted STCO atom\n"); return AVERROR_EOF; +} return 0; } @@ -2522,8 +2524,10 @@ int ff_mov_read_stsd_entries(MOVContext *c, AVIOContext *pb, int entries) sc->stsd_count++; } -if (pb->eof_reached) +if (pb->eof_reached) { +av_log(c->fc, AV_LOG_WARNING, "reached eof, corrupted STSD atom\n"); return AVERROR_EOF; +} return 0; } @@ -2624,8 +2628,10 @@ static int mov_read_stsc(MOVContext *c, AVIOContext *pb, MOVAtom atom) sc->stsc_count = i; -if (pb->eof_reached) +if (pb->eof_reached) { +av_log(c->fc, AV_LOG_WARNING, "reached eof, corrupted STSC atom\n"); return AVERROR_EOF; +} return 0; } @@ -2676,8 +2682,10 @@ static int mov_read_stps(MOVContext *c, AVIOContext *pb, MOVAtom atom) sc->stps_count = i; -if (pb->eof_reached) +if (pb->eof_reached) { +av_log(c->fc, AV_LOG_WARNING, "reached eof, corrupted STPS atom\n"); return AVERROR_EOF; +} return 0; } @@ -2723,8 +2731,10 @@ static int mov_read_stss(MOVContext *c, AVIOContext *pb, MOVAtom atom) sc->keyframe_count = i; -if (pb->eof_reached) +if (pb->eof_reached) { +av_log(c->fc, AV_LOG_WARNING, "reached eof, corrupted STSS atom\n"); return AVERROR_EOF; +} return 0; } @@ -2808,8 +2818,10 @@ static int mov_read_stsz(MOVContext *c, AVIOContext *pb, MOVAtom atom) av_free(buf); -if (pb->eof_reached) +if (pb->eof_reached) { +av_log(c->fc, AV_LOG_WARNING, "reached eof, corrupted STSZ atom\n"); return AVERROR_EOF; +} return 0; } @@ -2870,8 +2882,10 @@ static int mov_read_stts(MOVContext *c, AVIOContext *pb, MOVAtom atom) sc->duration_for_fps += duration; sc->nb_frames_for_fps += total_sample_count; -if (pb->eof_reached) +if (pb->eof_reached) { +av_log(c->fc, AV_LOG_WARNING, "reached eof, corrupted STTS atom\n"); return AVERROR_EOF; +} st->nb_frames= total_sample_count; if (duration) @@ -2948,8 +2962,10 @@ static int mov_read_ctts(MOVContext *c, AVIOContext *pb, MOVAtom atom) sc->ctts_count = ctts_count; -if (pb->eof_reached) +if (pb->eof_reached) { +av_log(c->fc, AV_LOG_WARNING, "reached eof, corrupted CTTS atom\n"); return AVERROR_EOF; +} av_log(c->fc, AV_LOG_TRACE, "dts shift %d\n", sc->dts_shift); @@ -2995,7 +3011,12 @@ static int mov_read_sbgp(MOVContext *c, AVIOContext *pb, MOVAtom atom) sc->rap_group_count = i; -return pb->eof_reached ? AVERROR_EOF : 0; +if (pb->eof_reached) { +av_log(c->fc, AV_LOG_WARNING, "reached eof, corrupted SBGP atom\n"); +return AVERROR_EOF; +} + +return 0; } /** @@ -4720,8 +4741,10 @@ static int mov_read_trun(MOVContext *c, AVIOContext *pb, MOVAtom atom) fix_frag_index_entries(>frag_index, next_frag_index, frag->track_id, entries); -if (pb->eof_reached) +if (pb->eof_reached) { +av_log(c->fc, AV_LOG_WARNING, "reached eof, corrupted TRUN atom\n"); return AVERROR_EOF; +} frag->implicit_offset = offset; @@ -6607,13 +6630,15 @@ static int mov_read_header(AVFormatContext *s) /* check MOV header */ do { -if (mov->moov_retry) -avio_seek(pb, 0, SEEK_SET); -if ((err = mov_read_default(mov, pb, atom)) < 0) { -av_log(s, AV_LOG_ERROR, "error reading header\n"); -mov_read_close(s); -return err; -} +if (mov->moov_retry) +avio_seek(pb, 0, SEEK_SET); +/* EOF don't mean the file to play fail*/ +err = mov_read_default(mov, pb, atom); +if (err < 0 && err != AVERROR_EOF) { +av_log(s, AV_LOG_ERROR, "error reading header\n"); +mov_read_close(s); +return err; +} } while ((pb->seekable & AVIO_SEEKABLE_NORMAL) && !mov->found_moov && !mov->moov_retry++); if (!mov->found_moov) { av_log(s, AV_LOG_ERROR, "moov atom not found\n"); -- 2.7.4 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavf/mov: atom box parsing return eof cause play fail
On Tue, Dec 05, 2017 at 11:34:12AM +0800, tiejun.peng wrote: > 1.add warning info about corrupted atom box parsing. > 2.atom box parsing return eof cause mov_read_close called in mov_read_header > and user have no chance to play the file. i think someone should cleanup the english, it sounds a bit odd [...] > @@ -6609,7 +6632,8 @@ static int mov_read_header(AVFormatContext *s) > do { > if (mov->moov_retry) > avio_seek(pb, 0, SEEK_SET); > -if ((err = mov_read_default(mov, pb, atom)) < 0) { > +/* EOF don't mean the file to play fail*/ > +if ((err = mov_read_default(mov, pb, atom) && err != AVERROR_EOF) < 0) { this does look wrong [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Those who are too smart to engage in politics are punished by being governed by those who are dumber. -- Plato signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] lavf/mov: atom box parsing return eof cause play fail
1.add warning info about corrupted atom box parsing. 2.atom box parsing return eof cause mov_read_close called in mov_read_header and user have no chance to play the file. Signed-off-by: tiejun.peng--- libavformat/mov.c | 46 +++--- 1 file changed, 35 insertions(+), 11 deletions(-) diff --git a/libavformat/mov.c b/libavformat/mov.c index c901859..ef8b357 100644 --- a/libavformat/mov.c +++ b/libavformat/mov.c @@ -1991,8 +1991,10 @@ static int mov_read_stco(MOVContext *c, AVIOContext *pb, MOVAtom atom) sc->chunk_count = i; -if (pb->eof_reached) +if (pb->eof_reached) { +av_log(c->fc, AV_LOG_WARNING, "reached eof, corrupted STCO atom\n"); return AVERROR_EOF; +} return 0; } @@ -2522,8 +2524,10 @@ int ff_mov_read_stsd_entries(MOVContext *c, AVIOContext *pb, int entries) sc->stsd_count++; } -if (pb->eof_reached) +if (pb->eof_reached) { +av_log(c->fc, AV_LOG_WARNING, "reached eof, corrupted STSD atom\n"); return AVERROR_EOF; +} return 0; } @@ -2624,8 +2628,10 @@ static int mov_read_stsc(MOVContext *c, AVIOContext *pb, MOVAtom atom) sc->stsc_count = i; -if (pb->eof_reached) +if (pb->eof_reached) { +av_log(c->fc, AV_LOG_WARNING, "reached eof, corrupted STSC atom\n"); return AVERROR_EOF; +} return 0; } @@ -2676,8 +2682,10 @@ static int mov_read_stps(MOVContext *c, AVIOContext *pb, MOVAtom atom) sc->stps_count = i; -if (pb->eof_reached) +if (pb->eof_reached) { +av_log(c->fc, AV_LOG_WARNING, "reached eof, corrupted STPS atom\n"); return AVERROR_EOF; +} return 0; } @@ -2723,8 +2731,10 @@ static int mov_read_stss(MOVContext *c, AVIOContext *pb, MOVAtom atom) sc->keyframe_count = i; -if (pb->eof_reached) +if (pb->eof_reached) { +av_log(c->fc, AV_LOG_WARNING, "reached eof, corrupted STSS atom\n"); return AVERROR_EOF; +} return 0; } @@ -2808,8 +2818,10 @@ static int mov_read_stsz(MOVContext *c, AVIOContext *pb, MOVAtom atom) av_free(buf); -if (pb->eof_reached) +if (pb->eof_reached) { +av_log(c->fc, AV_LOG_WARNING, "reached eof, corrupted STSZ atom\n"); return AVERROR_EOF; +} return 0; } @@ -2870,8 +2882,10 @@ static int mov_read_stts(MOVContext *c, AVIOContext *pb, MOVAtom atom) sc->duration_for_fps += duration; sc->nb_frames_for_fps += total_sample_count; -if (pb->eof_reached) +if (pb->eof_reached) { +av_log(c->fc, AV_LOG_WARNING, "reached eof, corrupted STTS atom\n"); return AVERROR_EOF; +} st->nb_frames= total_sample_count; if (duration) @@ -2948,8 +2962,10 @@ static int mov_read_ctts(MOVContext *c, AVIOContext *pb, MOVAtom atom) sc->ctts_count = ctts_count; -if (pb->eof_reached) +if (pb->eof_reached) { +av_log(c->fc, AV_LOG_WARNING, "reached eof, corrupted CTTS atom\n"); return AVERROR_EOF; +} av_log(c->fc, AV_LOG_TRACE, "dts shift %d\n", sc->dts_shift); @@ -2995,7 +3011,12 @@ static int mov_read_sbgp(MOVContext *c, AVIOContext *pb, MOVAtom atom) sc->rap_group_count = i; -return pb->eof_reached ? AVERROR_EOF : 0; +if (pb->eof_reached) { +av_log(c->fc, AV_LOG_WARNING, "reached eof, corrupted SBGP atom\n"); +return AVERROR_EOF; +} + +return 0; } /** @@ -4720,8 +4741,10 @@ static int mov_read_trun(MOVContext *c, AVIOContext *pb, MOVAtom atom) fix_frag_index_entries(>frag_index, next_frag_index, frag->track_id, entries); -if (pb->eof_reached) +if (pb->eof_reached) { +av_log(c->fc, AV_LOG_WARNING, "reached eof, corrupted TRUN atom\n"); return AVERROR_EOF; +} frag->implicit_offset = offset; @@ -6609,7 +6632,8 @@ static int mov_read_header(AVFormatContext *s) do { if (mov->moov_retry) avio_seek(pb, 0, SEEK_SET); -if ((err = mov_read_default(mov, pb, atom)) < 0) { +/* EOF don't mean the file to play fail*/ +if ((err = mov_read_default(mov, pb, atom) && err != AVERROR_EOF) < 0) { av_log(s, AV_LOG_ERROR, "error reading header\n"); mov_read_close(s); return err; -- 2.7.4 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel