diff mbox

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

Message ID 20190210044452.16820-1-agrecascino123@gmail.com
State New
Headers show

Commit Message

no pls Feb. 10, 2019, 4:44 a.m. UTC 
From: mptcultist  <agrecascino123@gmail.com>

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(-)

Comments

Michael Niedermayer Feb. 11, 2019, 9:55 a.m. UTC | #1 
On Sat, Feb 09, 2019 at 11:44:52PM -0500, agrecascino123@gmail.com wrote:
> From: mptcultist  <agrecascino123@gmail.com>
> 
> 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

[...]
diff mbox

Patch

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)