Currently, ff_read_packet() sometimes forwards the return value of
AVInputFormat.read_packet() (which should be zero on success, but isn't
for all demuxers) and sometimes it overwrites this with zero.
Furthermore, it uses two variables, one for the read_packet return value
and one for other errors, which is a bit confusing; it is also
unnecessary given that the documentation explicitly states that
ff_read_packet() never returns positive values. Returning a positive
value would lead to leaks with some callers (namely asfrtp_parse_packet
and estimate_timings_from_pts). So always return zero in case of
success.
(This behaviour stems from a time before av_read_packet sanitized
the return value of read_packet at all: It was added in commit
626004690c23c981f67228ea325dde3f35193988 and was unnecessary since
88b00723906f68b7563214c30333e48888dddf78.)
Signed-off-by: Andreas Rheinhardt <andreas.rheinha...@gmail.com>
---
asfrtp_parse_packet contains this:
for (;;) {
int i;
res = ff_read_packet(rt->asf_ctx, pkt);
rt->asf_pb_pos = avio_tell(pb);
if (res != 0)
break;
for (i = 0; i < s->nb_streams; i++) {
if (s->streams[i]->id ==
rt->asf_ctx->streams[pkt->stream_index]->id) {
pkt->stream_index = i;
return 1; // FIXME: return 0 if last packet
}
}
av_packet_unref(pkt);
}
return res == 1 ? -1 : res;
It's the very last line that bothers me: ff_read_packet and
av_read_packet (its predecessor that was used when this code has
originally been added) are not allowed to return anything > 0 and
they didn't for the asf demuxer. Does someone see a hidden intent
in this or was it just wrong from the beginning?
libavformat/utils.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/libavformat/utils.c b/libavformat/utils.c
index 8573117694..a3de4af9a1 100644
--- a/libavformat/utils.c
+++ b/libavformat/utils.c
@@ -804,7 +804,7 @@ static int update_wrap_reference(AVFormatContext *s,
AVStream *st, int stream_in
int ff_read_packet(AVFormatContext *s, AVPacket *pkt)
{
- int ret, i, err;
+ int err, i;
AVStream *st;
pkt->data = NULL;
@@ -828,17 +828,17 @@ int ff_read_packet(AVFormatContext *s, AVPacket *pkt)
}
}
- ret = s->iformat->read_packet(s, pkt);
- if (ret < 0) {
+ err = s->iformat->read_packet(s, pkt);
+ if (err < 0) {
av_packet_unref(pkt);
/* Some demuxers return FFERROR_REDO when they consume
data and discard it (ignored streams, junk, extradata).
We must re-call the demuxer to get the real packet. */
- if (ret == FFERROR_REDO)
+ if (err == FFERROR_REDO)
continue;
- if (!pktl || ret == AVERROR(EAGAIN))
- return ret;
+ if (!pktl || err == AVERROR(EAGAIN))
+ return err;
for (i = 0; i < s->nb_streams; i++) {
st = s->streams[i];
if (st->probe_packets || st->internal->request_probe > 0)
@@ -892,7 +892,7 @@ int ff_read_packet(AVFormatContext *s, AVPacket *pkt)
pkt->dts = pkt->pts = av_rescale_q(av_gettime(), AV_TIME_BASE_Q,
st->time_base);
if (!pktl && st->internal->request_probe <= 0)
- return ret;
+ return 0;
err = avpriv_packet_list_put(&s->internal->raw_packet_buffer,
&s->internal->raw_packet_buffer_end,
--
2.27.0
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
