diff mbox

[FFmpeg-devel] avformat/mov: fix sidx size being doubled in offset.

Message ID 6992F3BE-908B-4753-ADAA-57B72D5F833F@gmail.com
State Superseded
Headers show

Commit Message

no pls Jan. 30, 2019, 6:44 p.m. UTC
From: mptcultist <agrecascino123@gmail.com>

fixes an issue where if the video size was very specific, ffmpeg would hang from not filling the sidx_pts for all streams, due to not reading the last sidx lump. for #7572

---
libavformat/mov.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Carl Eugen Hoyos Jan. 30, 2019, 8:18 p.m. UTC | #1
2019-01-30 19:44 GMT+01:00, no pls <agrecascino123@gmail.com>:
> From: mptcultist <agrecascino123@gmail.com>
>
> fixes an issue where if the video size was very specific,
> ffmpeg would hang from not filling the sidx_pts for all
> streams, due to not reading the last sidx lump. for #7572

Please wait for a review from John who has promised to look at
this issue "on Wednesday".

Carl Eugen
John Stebbins Feb. 1, 2019, 7:12 p.m. UTC | #2
On 1/30/19 1:18 PM, Carl Eugen Hoyos wrote:
> 2019-01-30 19:44 GMT+01:00, no pls <agrecascino123@gmail.com>:
>> From: mptcultist <agrecascino123@gmail.com>
>>
>> fixes an issue where if the video size was very specific,
>> ffmpeg would hang from not filling the sidx_pts for all
>> streams, due to not reading the last sidx lump. for #7572
> Please wait for a review from John who has promised to look at
> this issue "on Wednesday".
>
> Carl Eugen


Sorry I haven't had time to devote to this yet.

This patch is incorrect however.  offsets are relative to the anchor 
point which is the *end* of the sidx. See ISO 14496-12 section 8.16.3.

"In the file containing the Segment Index box, the anchor point for a 
Segment Index box is the first byte after that box."
no pls Feb. 1, 2019, 7:49 p.m. UTC | #3
ah, i couldn’t find a copy of the standard, i’ll look it over and figure out what’s actually wrong, as the bug still remains.

Sent from my iPhone

> On Feb 1, 2019, at 2:12 PM, John Stebbins <jstebbins@jetheaddev.com> wrote:
> 
>> On 1/30/19 1:18 PM, Carl Eugen Hoyos wrote:
>> 2019-01-30 19:44 GMT+01:00, no pls <agrecascino123@gmail.com>:
>>> From: mptcultist <agrecascino123@gmail.com>
>>> 
>>> fixes an issue where if the video size was very specific,
>>> ffmpeg would hang from not filling the sidx_pts for all
>>> streams, due to not reading the last sidx lump. for #7572
>> Please wait for a review from John who has promised to look at
>> this issue "on Wednesday".
>> 
>> Carl Eugen
> 
> 
> Sorry I haven't had time to devote to this yet.
> 
> This patch is incorrect however.  offsets are relative to the anchor point which is the *end* of the sidx. See ISO 14496-12 section 8.16.3.
> 
> "In the file containing the Segment Index box, the anchor point for a Segment Index box is the first byte after that box."
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Jan Ekström Feb. 1, 2019, 10:25 p.m. UTC | #4
On Fri, Feb 1, 2019 at 9:54 PM mptcultist <agrecascino123@gmail.com> wrote:
>
> ah, i couldn’t find a copy of the standard, i’ll look it over and figure out what’s actually wrong, as the bug still remains.
>

ISOBMFF (14496-12) is available for free at
https://standards.iso.org/ittf/PubliclyAvailableStandards/index.html .

Best regards,
Jan
no pls Feb. 2, 2019, 3:38 a.m. UTC | #5
From skimming the documents, FF's behavior appears to be correct. However,
in both of the sidx atoms of the problematic file, they point to the same
media and as such both point to the end of the file, meaning either will
complete the fragment index incorrectly.
I'm not sure what's to be done.

On Fri, Feb 1, 2019 at 5:30 PM Jan Ekström <jeebjp@gmail.com> wrote:

> On Fri, Feb 1, 2019 at 9:54 PM mptcultist <agrecascino123@gmail.com>
> wrote:
> >
> > ah, i couldn’t find a copy of the standard, i’ll look it over and figure
> out what’s actually wrong, as the bug still remains.
> >
>
> ISOBMFF (14496-12) is available for free at
> https://standards.iso.org/ittf/PubliclyAvailableStandards/index.html .
>
> Best regards,
> Jan
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
diff mbox

Patch

diff --git a/libavformat/mov.c b/libavformat/mov.c
index 9b9739f788..c222582886 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -4933,7 +4933,7 @@  static int mov_read_trun(MOVContext *c, AVIOContext *pb, MOVAtom atom)

static int mov_read_sidx(MOVContext *c, AVIOContext *pb, MOVAtom atom)
{
-    int64_t offset = avio_tell(pb) + atom.size, pts, timestamp;
+    int64_t offset = avio_tell(pb), pts, timestamp;
    uint8_t version;
    unsigned i, j, track_id, item_count;
    AVStream *st = NULL;