[FFmpeg-devel] [PATCH 5/8] ffmdec: break infinite resync loop
Hi,
some broken files can lead to an endless resync loop, which is avoided
by attached patch.
Best regards,
Andreas
>From 5682a0cafbaf9339352f3147ef7c494dea47 Mon Sep 17 00:00:00 2001
From: Andreas Cadhalpun
Date: Sun, 8 Mar 2015 23:29:42 +0100
Subject: [PATCH 5/8] ffmdec: break infinite resync loop
If resyncing leads to the same position as previously, it will again
lead to a resync attempt, resulting in an infinite loop.
Signed-off-by: Andreas Cadhalpun
---
libavformat/ffmdec.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/libavformat/ffmdec.c b/libavformat/ffmdec.c
index 35e4c03..9cbd20c 100644
--- a/libavformat/ffmdec.c
+++ b/libavformat/ffmdec.c
@@ -82,6 +82,7 @@ static int ffm_read_data(AVFormatContext *s,
FFMContext *ffm = s->priv_data;
AVIOContext *pb = s->pb;
int len, fill_size, size1, frame_offset, id;
+int64_t last_pos = -1;
size1 = size;
while (size > 0) {
@@ -101,9 +102,16 @@ static int ffm_read_data(AVFormatContext *s,
avio_seek(pb, tell, SEEK_SET);
}
id = avio_rb16(pb); /* PACKET_ID */
-if (id != PACKET_ID)
+if (id != PACKET_ID) {
if (ffm_resync(s, id) < 0)
return -1;
+if (avio_tell(pb) == last_pos) {
+av_log(s, AV_LOG_ERROR,
+ "breaking resync loop at pos %"PRIx64"\n", last_pos);
+return AVERROR_INVALIDDATA;
+}
+last_pos = avio_tell(pb);
+}
fill_size = avio_rb16(pb);
ffm->dts = avio_rb64(pb);
frame_offset = avio_rb16(pb);
--
2.1.4
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 5/8] ffmdec: break infinite resync loop
On Mon, Mar 09, 2015 at 12:04:13AM +0100, Andreas Cadhalpun wrote: > Hi, > > some broken files can lead to an endless resync loop, which is > avoided by attached patch. > > Best regards, > Andreas > ffmdec.c | 10 +- > 1 file changed, 9 insertions(+), 1 deletion(-) > 048852d2d9b0c25157015a4befd76323fc4b2cc6 > 0005-ffmdec-break-infinite-resync-loop.patch > From 5682a0cafbaf9339352f3147ef7c494dea47 Mon Sep 17 00:00:00 2001 > From: Andreas Cadhalpun > Date: Sun, 8 Mar 2015 23:29:42 +0100 > Subject: [PATCH 5/8] ffmdec: break infinite resync loop > > If resyncing leads to the same position as previously, it will again > lead to a resync attempt, resulting in an infinite loop. iam not sure this is sufficient and loops over more than 1 resync point arent possible maybe its better to never allow resync to start before or at the previous resync point [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB If a bugfix only changes things apparently unrelated to the bug with no further explanation, that is a good sign that the bugfix is wrong. signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 5/8] ffmdec: break infinite resync loop
On 09.03.2015 03:13, Michael Niedermayer wrote:
On Mon, Mar 09, 2015 at 12:04:13AM +0100, Andreas Cadhalpun wrote:
Hi,
some broken files can lead to an endless resync loop, which is
avoided by attached patch.
Best regards,
Andreas
ffmdec.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
048852d2d9b0c25157015a4befd76323fc4b2cc6
0005-ffmdec-break-infinite-resync-loop.patch
From 5682a0cafbaf9339352f3147ef7c494dea47 Mon Sep 17 00:00:00 2001
From: Andreas Cadhalpun
Date: Sun, 8 Mar 2015 23:29:42 +0100
Subject: [PATCH 5/8] ffmdec: break infinite resync loop
If resyncing leads to the same position as previously, it will again
lead to a resync attempt, resulting in an infinite loop.
iam not sure this is sufficient and loops over more than 1 resync
point arent possible
maybe its better to never allow resync to start before
or at the previous resync point
I don't think this is possible, but it shouldn't hurt to change the
check to '<='. This forces the syncpoints to always increase.
Best regards,
Andreas
>From aa2d0cc852fe51734d9c9fd318dc94ec4af3970f Mon Sep 17 00:00:00 2001
From: Andreas Cadhalpun
Date: Sun, 8 Mar 2015 23:29:42 +0100
Subject: [PATCH 5/8] ffmdec: break infinite resync loop
If resyncing leads to the same position as previously, it will again
lead to a resync attempt, resulting in an infinite loop.
Signed-off-by: Andreas Cadhalpun
---
libavformat/ffmdec.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/libavformat/ffmdec.c b/libavformat/ffmdec.c
index ee34e73..c54d1d6 100644
--- a/libavformat/ffmdec.c
+++ b/libavformat/ffmdec.c
@@ -82,6 +82,7 @@ static int ffm_read_data(AVFormatContext *s,
FFMContext *ffm = s->priv_data;
AVIOContext *pb = s->pb;
int len, fill_size, size1, frame_offset, id;
+int64_t last_pos = -1;
size1 = size;
while (size > 0) {
@@ -101,9 +102,16 @@ static int ffm_read_data(AVFormatContext *s,
avio_seek(pb, tell, SEEK_SET);
}
id = avio_rb16(pb); /* PACKET_ID */
-if (id != PACKET_ID)
+if (id != PACKET_ID) {
if (ffm_resync(s, id) < 0)
return -1;
+if (avio_tell(pb) <= last_pos) {
+av_log(s, AV_LOG_ERROR,
+ "breaking resync loop at pos %"PRIx64"\n", last_pos);
+return AVERROR_INVALIDDATA;
+}
+last_pos = avio_tell(pb);
+}
fill_size = avio_rb16(pb);
ffm->dts = avio_rb64(pb);
frame_offset = avio_rb16(pb);
--
2.1.4
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 5/8] ffmdec: break infinite resync loop
On Mon, Mar 09, 2015 at 12:17:38PM +0100, Andreas Cadhalpun wrote: > On 09.03.2015 03:13, Michael Niedermayer wrote: > >On Mon, Mar 09, 2015 at 12:04:13AM +0100, Andreas Cadhalpun wrote: > >>Hi, > >> > >>some broken files can lead to an endless resync loop, which is > >>avoided by attached patch. > >> > >>Best regards, > >>Andreas > > > >> ffmdec.c | 10 +- > >> 1 file changed, 9 insertions(+), 1 deletion(-) > >>048852d2d9b0c25157015a4befd76323fc4b2cc6 > >>0005-ffmdec-break-infinite-resync-loop.patch > >> From 5682a0cafbaf9339352f3147ef7c494dea47 Mon Sep 17 00:00:00 2001 > >>From: Andreas Cadhalpun > >>Date: Sun, 8 Mar 2015 23:29:42 +0100 > >>Subject: [PATCH 5/8] ffmdec: break infinite resync loop > >> > >>If resyncing leads to the same position as previously, it will again > >>lead to a resync attempt, resulting in an infinite loop. > > > >iam not sure this is sufficient and loops over more than 1 resync > >point arent possible > >maybe its better to never allow resync to start before > >or at the previous resync point > > I don't think this is possible, but it shouldn't hurt to change the > check to '<='. This forces the syncpoints to always increase. i was thinking more about limiting the backward seek before resync to the last resync position +1 if there was a previous resync so that resync which moves forward could not end before. This would avoid the failure and allow the demuxer to continue, or at least thats the idea [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Freedom in capitalist society always remains about the same as it was in ancient Greek republics: Freedom for slave owners. -- Vladimir Lenin signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 5/8] ffmdec: break infinite resync loop
On 09.03.2015 13:59, Michael Niedermayer wrote:
i was thinking more about limiting the backward seek before resync
to the last resync position +1 if there was a previous resync
so that resync which moves forward could not end before. This would
avoid the failure and allow the demuxer to continue, or at least thats
the idea
I see, new patch attached.
The +1 is not necessary, because it always reads the PACKET_ID field
before resyncing.
Best regards,
Andreas
>From 6bcd1b4a906e2da39f6361db19826a337ac5a745 Mon Sep 17 00:00:00 2001
From: Andreas Cadhalpun
Date: Mon, 9 Mar 2015 14:59:44 +0100
Subject: [PATCH] ffmdec: limit the backward seek to the last resync position
If resyncing leads to the same position as previously, it will again
lead to a resync attempt, resulting in an infinite loop.
Thus don't seek back beyond the last syncpoint.
Signed-off-by: Andreas Cadhalpun
---
libavformat/ffmdec.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/libavformat/ffmdec.c b/libavformat/ffmdec.c
index fc43668..ec91b6f 100644
--- a/libavformat/ffmdec.c
+++ b/libavformat/ffmdec.c
@@ -79,6 +79,7 @@ static int ffm_read_data(AVFormatContext *s,
FFMContext *ffm = s->priv_data;
AVIOContext *pb = s->pb;
int len, fill_size, size1, frame_offset, id;
+int64_t last_pos = -1;
size1 = size;
while (size > 0) {
@@ -98,9 +99,11 @@ static int ffm_read_data(AVFormatContext *s,
avio_seek(pb, tell, SEEK_SET);
}
id = avio_rb16(pb); /* PACKET_ID */
-if (id != PACKET_ID)
+if (id != PACKET_ID) {
if (ffm_resync(s, id) < 0)
return -1;
+last_pos = avio_tell(pb);
+}
fill_size = avio_rb16(pb);
ffm->dts = avio_rb64(pb);
frame_offset = avio_rb16(pb);
@@ -114,7 +117,9 @@ static int ffm_read_data(AVFormatContext *s,
if (!frame_offset) {
/* This packet has no frame headers in it */
if (avio_tell(pb) >= ffm->packet_size * 3LL) {
-avio_seek(pb, -ffm->packet_size * 2LL, SEEK_CUR);
+int64_t seekback = FFMIN(ffm->packet_size * 2LL, avio_tell(pb) - last_pos);
+seekback = FFMAX(seekback, 0);
+avio_seek(pb, -seekback, SEEK_CUR);
goto retry_read;
}
/* This is bad, we cannot find a valid frame header */
--
2.1.4
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 5/8] ffmdec: break infinite resync loop
On Mon, Mar 09, 2015 at 03:14:50PM +0100, Andreas Cadhalpun wrote: > On 09.03.2015 13:59, Michael Niedermayer wrote: > >i was thinking more about limiting the backward seek before resync > >to the last resync position +1 if there was a previous resync > >so that resync which moves forward could not end before. This would > >avoid the failure and allow the demuxer to continue, or at least thats > >the idea > > I see, new patch attached. > The +1 is not necessary, because it always reads the PACKET_ID field > before resyncing. > > Best regards, > Andreas > > ffmdec.c |9 +++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > 2f242fcf689ae8307a818b8a219d08af7c27921e > 0001-ffmdec-limit-the-backward-seek-to-the-last-resync-po.patch > From 6bcd1b4a906e2da39f6361db19826a337ac5a745 Mon Sep 17 00:00:00 2001 > From: Andreas Cadhalpun > Date: Mon, 9 Mar 2015 14:59:44 +0100 > Subject: [PATCH] ffmdec: limit the backward seek to the last resync position > > If resyncing leads to the same position as previously, it will again > lead to a resync attempt, resulting in an infinite loop. > > Thus don't seek back beyond the last syncpoint. applied thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Asymptotically faster algorithms should always be preferred if you have asymptotical amounts of data signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
