[FFmpeg-devel] [RFC][PATCH] avformat/flvdec: avoid reseting eof_reached to 0 silently
wm4
nfxjfg at googlemail.com
Fri Apr 10 12:24:52 CEST 2015
On Wed, 8 Apr 2015 23:28:30 +0800
Zhang Rui <bbcallen at gmail.com> wrote:
> avio_feof() and avio_seek() could reset eof_reached to 0,
> after some silent IO error in avio_r*(). And the reconnection
> caused by seek makes demuxer to continue read at wrong position.
> ---
> libavformat/flvdec.c | 66 ++++++++++++++++++++++++++++++++--------------------
> 1 file changed, 41 insertions(+), 25 deletions(-)
>
> diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c
> index b625c78..ae8e2fe 100644
> --- a/libavformat/flvdec.c
> +++ b/libavformat/flvdec.c
> @@ -85,6 +85,22 @@ static int live_flv_probe(AVProbeData *p)
> return probe(p, 1);
> }
>
> +static int64_t safe_io_seek(AVIOContext *s, int64_t offset, int whence)
> +{
> + if (s->eof_reached)
> + return AVERROR_EOF;
> +
> + return avio_seek(s, offset, whence);
> +}
> +
> +static int64_t safe_io_skip(AVIOContext *s, int64_t offset)
> +{
> + if (s->eof_reached)
> + return AVERROR_EOF;
> +
> + return avio_skip(s, offset);
> +}
> +
> static AVStream *create_stream(AVFormatContext *s, int codec_type)
> {
> AVStream *st = avformat_new_stream(s, NULL);
> @@ -265,7 +281,7 @@ static int flv_set_video_codec(AVFormatContext *s, AVStream *vstream,
> if (vcodec->extradata)
> vcodec->extradata[0] = avio_r8(s->pb);
> else
> - avio_skip(s->pb, 1);
> + safe_io_skip(s->pb, 1);
> }
> return 1; // 1 byte body size adjustment for flv_read_packet()
> case FLV_CODECID_H264:
> @@ -287,7 +303,7 @@ static int amf_get_string(AVIOContext *ioc, char *buffer, int buffsize)
> {
> int length = avio_rb16(ioc);
> if (length >= buffsize) {
> - avio_skip(ioc, length);
> + safe_io_skip(ioc, length);
> return -1;
> }
>
> @@ -378,7 +394,7 @@ invalid:
> finish:
> av_freep(×);
> av_freep(&filepositions);
> - avio_seek(ioc, initial_pos, SEEK_SET);
> + safe_io_seek(ioc, initial_pos, SEEK_SET);
> return ret;
> }
>
> @@ -433,7 +449,7 @@ static int amf_parse_object(AVFormatContext *s, AVStream *astream,
> case AMF_DATA_TYPE_UNSUPPORTED:
> break; // these take up no additional space
> case AMF_DATA_TYPE_MIXEDARRAY:
> - avio_skip(ioc, 4); // skip 32-bit max array index
> + safe_io_skip(ioc, 4); // skip 32-bit max array index
> while (avio_tell(ioc) < max_pos - 2 &&
> amf_get_string(ioc, str_val, sizeof(str_val)) > 0)
> // this is the only case in which we would want a nested
> @@ -458,7 +474,7 @@ static int amf_parse_object(AVFormatContext *s, AVStream *astream,
> }
> break;
> case AMF_DATA_TYPE_DATE:
> - avio_skip(ioc, 8 + 2); // timestamp (double) and UTC offset (int16)
> + safe_io_skip(ioc, 8 + 2); // timestamp (double) and UTC offset (int16)
> break;
> default: // unsupported type, we couldn't skip
> av_log(s, AV_LOG_ERROR, "unsupported amf type %d\n", amf_type);
> @@ -603,7 +619,7 @@ static int flv_read_header(AVFormatContext *s)
> {
> int offset, flags;
>
> - avio_skip(s->pb, 4);
> + safe_io_skip(s->pb, 4);
> flags = avio_r8(s->pb);
>
> s->ctx_flags |= AVFMTCTX_NOHEADER;
> @@ -618,8 +634,8 @@ static int flv_read_header(AVFormatContext *s)
> // create that stream if it's encountered.
>
> offset = avio_rb32(s->pb);
> - avio_seek(s->pb, offset, SEEK_SET);
> - avio_skip(s->pb, 4);
> + safe_io_seek(s->pb, offset, SEEK_SET);
> + safe_io_skip(s->pb, 4);
>
> s->start_time = 0;
>
> @@ -678,13 +694,13 @@ static int amf_skip_tag(AVIOContext *pb, AMFDataType type)
>
> switch (type) {
> case AMF_DATA_TYPE_NUMBER:
> - avio_skip(pb, 8);
> + safe_io_skip(pb, 8);
> break;
> case AMF_DATA_TYPE_BOOL:
> - avio_skip(pb, 1);
> + safe_io_skip(pb, 1);
> break;
> case AMF_DATA_TYPE_STRING:
> - avio_skip(pb, avio_rb16(pb));
> + safe_io_skip(pb, avio_rb16(pb));
> break;
> case AMF_DATA_TYPE_ARRAY:
> parse_name = 0;
> @@ -695,10 +711,10 @@ static int amf_skip_tag(AVIOContext *pb, AMFDataType type)
> if (parse_name) {
> int size = avio_rb16(pb);
> if (!size) {
> - avio_skip(pb, 1);
> + safe_io_skip(pb, 1);
> break;
> }
> - avio_skip(pb, size);
> + safe_io_skip(pb, size);
> }
> if ((ret = amf_skip_tag(pb, avio_r8(pb))) < 0)
> return ret;
> @@ -727,7 +743,7 @@ static int flv_data_packet(AVFormatContext *s, AVPacket *pkt,
> case AMF_DATA_TYPE_ARRAY:
> array = 1;
> case AMF_DATA_TYPE_MIXEDARRAY:
> - avio_seek(pb, 4, SEEK_CUR);
> + safe_io_seek(pb, 4, SEEK_CUR);
> case AMF_DATA_TYPE_OBJECT:
> break;
> default:
> @@ -775,7 +791,7 @@ static int flv_data_packet(AVFormatContext *s, AVPacket *pkt,
> pkt->flags |= AV_PKT_FLAG_KEY;
>
> skip:
> - avio_seek(s->pb, next + 4, SEEK_SET);
> + safe_io_seek(s->pb, next + 4, SEEK_SET);
>
> return ret;
> }
> @@ -792,16 +808,16 @@ static int flv_read_packet(AVFormatContext *s, AVPacket *pkt)
> AVStream *st = NULL;
>
> /* pkt size is repeated at end. skip it */
> - for (;; avio_skip(s->pb, 4)) {
> + for (;; safe_io_skip(s->pb, 4)) {
> pos = avio_tell(s->pb);
> type = (avio_r8(s->pb) & 0x1F);
> size = avio_rb24(s->pb);
> dts = avio_rb24(s->pb);
> dts |= avio_r8(s->pb) << 24;
> av_dlog(s, "type:%d, size:%d, dts:%"PRId64" pos:%"PRId64"\n", type, size, dts, avio_tell(s->pb));
> - if (avio_feof(s->pb))
> + if (s->pb->eof_reached || avio_feof(s->pb))
> return AVERROR_EOF;
> - avio_skip(s->pb, 3); /* stream id, always 0 */
> + safe_io_skip(s->pb, 3); /* stream id, always 0 */
> flags = 0;
>
> if (flv->validate_next < flv->validate_count) {
> @@ -849,14 +865,14 @@ static int flv_read_packet(AVFormatContext *s, AVPacket *pkt)
> } else if (type == TYPE_ONCAPTION) {
> return flv_data_packet(s, pkt, dts, next);
> }
> - avio_seek(s->pb, meta_pos, SEEK_SET);
> + safe_io_seek(s->pb, meta_pos, SEEK_SET);
> }
> } else {
> av_log(s, AV_LOG_DEBUG,
> "Skipping flv packet: type %d, size %d, flags %d.\n",
> type, size, flags);
> skip:
> - avio_seek(s->pb, next, SEEK_SET);
> + safe_io_seek(s->pb, next, SEEK_SET);
> continue;
> }
>
> @@ -898,7 +914,7 @@ skip:
> ||(st->discard >= AVDISCARD_BIDIR && ((flags & FLV_VIDEO_FRAMETYPE_MASK) == FLV_FRAME_DISP_INTER && (stream_type == FLV_STREAM_TYPE_VIDEO)))
> || st->discard >= AVDISCARD_ALL
> ) {
> - avio_seek(s->pb, next, SEEK_SET);
> + safe_io_seek(s->pb, next, SEEK_SET);
> continue;
> }
> break;
> @@ -913,11 +929,11 @@ skip:
> // previous FLV tag. Use the timestamp of its payload as duration.
> int64_t fsize = avio_size(s->pb);
> retry_duration:
> - avio_seek(s->pb, fsize - 4, SEEK_SET);
> + safe_io_seek(s->pb, fsize - 4, SEEK_SET);
> size = avio_rb32(s->pb);
> // Seek to the start of the last FLV tag at position (fsize - 4 - size)
> // but skip the byte indicating the type.
> - avio_seek(s->pb, fsize - 3 - size, SEEK_SET);
> + safe_io_seek(s->pb, fsize - 3 - size, SEEK_SET);
> if (size == avio_rb24(s->pb) + 11) {
> uint32_t ts = avio_rb24(s->pb);
> ts |= avio_r8(s->pb) << 24;
> @@ -929,7 +945,7 @@ retry_duration:
> }
> }
>
> - avio_seek(s->pb, pos, SEEK_SET);
> + safe_io_seek(s->pb, pos, SEEK_SET);
> flv->searched_for_end = 1;
> }
>
> @@ -1063,7 +1079,7 @@ retry_duration:
> pkt->flags |= AV_PKT_FLAG_KEY;
>
> leave:
> - avio_skip(s->pb, 4);
> + safe_io_skip(s->pb, 4);
> return ret;
> }
>
IMO the approach might be ok, but the naming of the functions
definitely needs work. Functions which are not static need either a ff_
prefix (if they're private), or a av prefix if they're public. Also,
there's nothing "safe" about these functions, even if they facilitate
safe behavior. They merely return with an error when EOF was reached.
Also, why doesn't avio_skip() return an error if the skip count is not
0 and the stream has reached EOF?
More information about the ffmpeg-devel
mailing list