On Tue, Jun 02, 2015 at 11:15:16AM +0800, Zhang Rui wrote: > 2015-06-02 7:26 GMT+08:00 Michael Niedermayer <michae...@gmx.at>: > > On Thu, May 21, 2015 at 12:31:21PM +0800, Zhang Rui wrote: > >> Discontinuous sample could cause corrupted image if next video frame > >> is non-key frame. > >> > >> Mostly, avio_seek() fails on I/O error for http/ftp/... stream. > >> In my opinion, retry is better than skip. > >> > >> > - /* must be done just before reading, to avoid infinite loop on > >> > sample */ > >> > - sc->current_sample++; > >> > >> This line was first introduced in 2006 by commit b72708f8f. > >> I'm not sure if it worth an option to enable the new behaviour, > >> and keep the default behaviour as before. > > > > if its specific to avio_seek() should current_sample-- be done in > > its error case instead ? > > Also to av_get_packet(). Not sure about avpriv_dv_get_packet(). > current_sample-- would be OK to me. > > > maybe with a retry count to limit retries in case the next sample > > can be read but the current always fails > > Retry count could be used up soon, during a WiFi reconnection. > If we could recover from IO error at next packet, we also could recover > at current packet with sane position and size for most protocol, e.g. > http, ftp. > > Skipping is also OK, but we need some flag to indicate that > the following packet is discontinued for decoder to flush data. > > > Iam not sure i fully understand the bug this fixes, > > how can it be reproduced ? > > Stream a normal H264 sample from http server. > and close the socket connection from server side while streaming. > ( Paused player causes idle connection in real world ). > If client fails in the avio_seek() and make sc->current_sample++, > the next call to av_read_frames() could cause a reconnection, > which makes client to read next discontinued packet.
would below work ? commit 816047eb161e804ba6312947f6bd7349cf934f80 Author: Michael Niedermayer <michae...@gmx.at> Date: Wed Jun 3 13:04:37 2015 +0200 avformat/mov: Retry same packet on IO failure to avoid loosing a packet Based on patch by: Zhang Rui <bbcal...@gmail.com> Signed-off-by: Michael Niedermayer <michae...@gmx.at> diff --git a/libavformat/mov.c b/libavformat/mov.c index 5300704..bc5743a 100644 --- a/libavformat/mov.c +++ b/libavformat/mov.c @@ -4316,6 +4316,13 @@ static AVIndexEntry *mov_find_next_sample(AVFormatContext *s, AVStream **st) return sample; } +static int should_retry(AVIOContext *pb, int error_code) { + if (error_code == AVERROR_EOF || avio_feof(pb)) + return 0; + + return 1; +} + static int mov_read_packet(AVFormatContext *s, AVPacket *pkt) { MOVContext *mov = s->priv_data; @@ -4351,14 +4358,18 @@ static int mov_read_packet(AVFormatContext *s, AVPacket *pkt) } if (st->discard != AVDISCARD_ALL) { - if (avio_seek(sc->pb, sample->pos, SEEK_SET) != sample->pos) { + int64_t ret64 = avio_seek(sc->pb, sample->pos, SEEK_SET); + if (ret64 != sample->pos) { av_log(mov->fc, AV_LOG_ERROR, "stream %d, offset 0x%"PRIx64": partial file\n", sc->ffindex, sample->pos); + sc->current_sample -= should_retry(sc->pb, ret64); return AVERROR_INVALIDDATA; } ret = av_get_packet(sc->pb, pkt, sample->size); - if (ret < 0) + if (ret < 0) { + sc->current_sample -= should_retry(sc->pb, ret); return ret; + } if (sc->has_palette) { uint8_t *pal; [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The educated differ from the uneducated as much as the living from the dead. -- Aristotle
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel