Re: [FFmpeg-devel] [PATCH] avformat/mov: fix sidx loading issues in fragmented files.

[Michael Niedermayer]

On Sat, Feb 09, 2019 at 11:44:52PM -0500, agrecascino...@gmail.com wrote:
> From: mptcultist  
> 
> fixed issue where if sidx was after another sidx and happened to point to the 
> same media, it wouldn't be read.
> this is done by counting the sidx atoms before they're read. for #7572
> ---
>  libavformat/isom.h |  2 ++
>  libavformat/mov.c  | 45 +++--
>  2 files changed, 45 insertions(+), 2 deletions(-)

this looks not ok as it effectivly doubles the number of passes over the whole 
file
also there is no need for this, doing a single pass either you are done or you
are not so its possible to know if all have been read.
If the existing check for the last sidx is wrong then it should be fixed.

also this patch only fixes the issue if seeking for a 2nd pass is supported

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Dictatorship: All citizens are under surveillance, all their steps and
actions recorded, for the politicians to enforce control.
Democracy: All politicians are under surveillance, all their steps and
actions recorded, for the citizens to enforce control.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] avformat/mov: fix sidx loading issues in fragmented files.

[agrecascino123]

From: mptcultist  

fixed issue where if sidx was after another sidx and happened to point to the 
same media, it wouldn't be read.
this is done by counting the sidx atoms before they're read. for #7572
---
 libavformat/isom.h |  2 ++
 libavformat/mov.c  | 45 +++--
 2 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/libavformat/isom.h b/libavformat/isom.h
index 69452cae8e..cc3067d2c0 100644
--- a/libavformat/isom.h
+++ b/libavformat/isom.h
@@ -288,6 +288,8 @@ typedef struct MOVContext {
 int decryption_key_len;
 int enable_drefs;
 int32_t movie_display_matrix[3][3]; ///< display matrix from mvhd
+int sidx_count;
+int cur_sidx;
 } MOVContext;
 
 int ff_mp4_read_descr_len(AVIOContext *pb);
diff --git a/libavformat/mov.c b/libavformat/mov.c
index cc91beb7f9..a59c893274 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -4961,6 +4961,8 @@ static int mov_read_sidx(MOVContext *c, AVIOContext *pb, 
MOVAtom atom)
 MOVStreamContext *sc, *ref_sc = NULL;
 AVRational timescale;
 
+c->cur_sidx++;
+
 version = avio_r8(pb);
 if (version > 1) {
 avpriv_request_sample(c->fc, "sidx version %u", version);
@@ -5027,7 +5029,8 @@ static int mov_read_sidx(MOVContext *c, AVIOContext *pb, 
MOVAtom atom)
 
 sc->has_sidx = 1;
 
-if (offset == avio_size(pb)) {
+if ((!(pb->seekable & AVIO_SEEKABLE_NORMAL) && (offset == avio_size(pb))) 
||
+ ((pb->seekable & AVIO_SEEKABLE_NORMAL) && (c->sidx_count == 
c->cur_sidx))  ) {
 // Find first entry in fragment index that came from an sidx.
 // This will pretty much always be the first entry.
 for (i = 0; i < c->frag_index.nb_items; i++) {
@@ -6779,6 +6782,43 @@ static const MOVParseTableEntry 
mov_default_parse_table[] = {
 { 0, NULL }
 };
 
+static int mov_count_sidx(MOVContext *c, AVIOContext *pb, MOVAtom atom)
+{
+int64_t total_size = 0;
+MOVAtom a;
+int si = 0;
+
+if (atom.size < 0)
+atom.size = INT64_MAX;
+while (total_size <= atom.size - 8 && !avio_feof(pb)) {
+a.size = atom.size;
+a.type=0;
+if (atom.size >= 8) {
+a.size = avio_rb32(pb);
+a.type = avio_rl32(pb);
+if (a.type == MKTAG('s','i','d','x')) {
+si++;
+}
+total_size += 8;
+if (a.size == 1 && total_size + 8 <= atom.size) { /* 64 bit 
extended size */
+a.size = avio_rb64(pb) - 8;
+total_size += 8;
+}
+}
+if (a.size == 0) {
+a.size = atom.size - total_size + 8;
+}
+a.size -= 8;
+if (a.size < 0)
+break;
+a.size = FFMIN(a.size, atom.size - total_size);
+avio_skip(pb, a.size);
+total_size += a.size;
+}
+avio_seek(pb, 0, SEEK_SET);
+return si;
+}
+
 static int mov_read_default(MOVContext *c, AVIOContext *pb, MOVAtom atom)
 {
 int64_t total_size = 0;
@@ -7401,7 +7441,8 @@ static int mov_read_header(AVFormatContext *s)
 atom.size = avio_size(pb);
 else
 atom.size = INT64_MAX;
-
+if (pb->seekable & AVIO_SEEKABLE_NORMAL)
+mov->sidx_count = mov_count_sidx(mov, pb, atom);
 /* check MOV header */
 do {
 if (mov->moov_retry)
-- 
2.20.1

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel