Re: [FFmpeg-devel] [PATCH] mov.c: read fragment start dts from fragmented mp4

2014-10-11 Thread Mika Raento
Resubmission follows Carl's advice on braces. On 11 October 2014 16:49, Carl Eugen Hoyos wrote: > Mika Raento iki.fi> writes: > >> Somehow I'd gotten the impression that the opposite >> was preferred, and indeed mov.c tends not to have >> those braces. Has the convention changed? > > I don't thi

Re: [FFmpeg-devel] [PATCH] mov.c: read fragment start dts from fragmented mp4

2014-10-11 Thread Carl Eugen Hoyos
Mika Raento iki.fi> writes: > Somehow I'd gotten the impression that the opposite > was preferred, and indeed mov.c tends not to have > those braces. Has the convention changed? I don't think so, tools/patcheck should tell you more. Carl Eugen ___

Re: [FFmpeg-devel] [PATCH] mov.c: read fragment start dts from fragmented mp4

2014-10-11 Thread Mika Raento
On 11 October 2014 16:41, Carl Eugen Hoyos wrote: > Mika Raento iki.fi> writes: > >> +if (ret < 0) >> +av_log(c->fc, AV_LOG_ERROR, >> + "failed to seek back after looking for mfra\n"); >> +else >> +ret = 0; > > If you resubmit please consider making this: > i

Re: [FFmpeg-devel] [PATCH] mov.c: read fragment start dts from fragmented mp4

2014-10-11 Thread Carl Eugen Hoyos
Mika Raento iki.fi> writes: > +if (ret < 0) > +av_log(c->fc, AV_LOG_ERROR, > + "failed to seek back after looking for mfra\n"); > +else > +ret = 0; If you resubmit please consider making this: if (ret < 0) { avl_log(); } else { ret = 0; } This makes fut

Re: [FFmpeg-devel] [PATCH] mov.c: read fragment start dts from fragmented mp4

2014-10-11 Thread Mika Raento
Hm. After fixing the obvious problems with matching moofs with tfra entries I noticed that the timestamps aren't right even after that. AFAICT, ffmpeg's fragmentation code puts dts in the tfra time. In movenc.c: mov->tracks[i].frag_start += mov->tracks[i].start_dts +

Re: [FFmpeg-devel] [PATCH] mov.c: read fragment start dts from fragmented mp4

2014-10-11 Thread Mika Raento
Interesting. The input.mov created with ./ffmpeg -i matrixbench_mpeg2.mpg -t 1 -frag_duration 200k input.mov does not have tfra entries for all the moofs for the audio track. The single tfra it has is not for the first moof, but the last one. I'll improve the matching of the moofs and tfra entrie

Re: [FFmpeg-devel] [PATCH] mov.c: read fragment start dts from fragmented mp4

2014-10-11 Thread Michael Niedermayer
On Sat, Oct 11, 2014 at 07:25:52AM +0300, Mika Raento wrote: > If present, an MFRA box and its TFRAs are read for fragment start times. > > Without this change, timestamps for discontinuous fragmented mp4 are > wrong, and cause audio/video desync and are not usable for generating > HLS. > --- > l

[FFmpeg-devel] [PATCH] mov.c: read fragment start dts from fragmented mp4

2014-10-10 Thread Mika Raento
If present, an MFRA box and its TFRAs are read for fragment start times. Without this change, timestamps for discontinuous fragmented mp4 are wrong, and cause audio/video desync and are not usable for generating HLS. --- libavformat/isom.h | 15 ++ libavformat/mov.c | 146 ++

Re: [FFmpeg-devel] [PATCH] mov.c: read fragment start dts from fragmented mp4

2014-10-10 Thread Mika Raento
On 10 October 2014 23:08, Mika Raento wrote: > Firstly, thank you for the detailed explanation. > > Secondly, how should we proceed? > > I am not confident I'm able to implement that correctly, especially > with no test coverage. > > My current implementation improves discontinuous fragmented mp4s

Re: [FFmpeg-devel] [PATCH] mov.c: read fragment start dts from fragmented mp4

2014-10-10 Thread Mika Raento
Firstly, thank you for the detailed explanation. Secondly, how should we proceed? I am not confident I'm able to implement that correctly, especially with no test coverage. My current implementation improves discontinuous fragmented mp4s significantly (from unusable to close-to-perfect) while sl

Re: [FFmpeg-devel] [PATCH] mov.c: read fragment start dts from fragmented mp4

2014-10-10 Thread Yusuke Nakamura
2014-10-10 13:38 GMT+09:00 Mika Raento : > On 9 October 2014 23:37, Yusuke Nakamura > wrote: > > 2014-10-10 4:49 GMT+09:00 Michael Niedermayer : > > > >> On Thu, Oct 09, 2014 at 09:44:43PM +0200, Michael Niedermayer wrote: > >> > On Thu, Oct 09, 2014 at 06:57:59PM +0300, Mika Raento wrote: > >> >

[FFmpeg-devel] [PATCH] mov.c: read fragment start dts from fragmented mp4

2014-10-10 Thread Mika Raento
If present, an MFRA box and its TFRAs are read for fragment start times. Without this change, timestamps for discontinuous fragmented mp4 are wrong, and cause audio/video desync and are not usable for generating HLS. --- libavformat/isom.h | 15 ++ libavformat/mov.c | 140 ++

Re: [FFmpeg-devel] [PATCH] mov.c: read fragment start dts from fragmented mp4

2014-10-09 Thread Mika Raento
Ah. My approach to matching truns to fragment times was way too naive. Rewritten to look up the time by sequence number and to handle multiple truns inside a single traf. Resubmitting. Mika On 9 October 2014 22:44, Michael Niedermayer wrote: > On Thu, Oct 09, 2014 at 06:57:59PM +0300, Mika R

Re: [FFmpeg-devel] [PATCH] mov.c: read fragment start dts from fragmented mp4

2014-10-09 Thread Mika Raento
On 9 October 2014 23:37, Yusuke Nakamura wrote: > 2014-10-10 4:49 GMT+09:00 Michael Niedermayer : > >> On Thu, Oct 09, 2014 at 09:44:43PM +0200, Michael Niedermayer wrote: >> > On Thu, Oct 09, 2014 at 06:57:59PM +0300, Mika Raento wrote: >> > > If present, an MFRA box and its TFRAs are read for fr

Re: [FFmpeg-devel] [PATCH] mov.c: read fragment start dts from fragmented mp4

2014-10-09 Thread Mika Raento
Right. Will definitely take a look. I wouldn't mind help turning that into a fate testcase. I've been struggling with writing fate tests. At least for somebody like me, having more coverage in the fate tests would help enormously, as I definitely do not have the required expertise to know what may

Re: [FFmpeg-devel] [PATCH] mov.c: read fragment start dts from fragmented mp4

2014-10-09 Thread Yusuke Nakamura
2014-10-10 4:49 GMT+09:00 Michael Niedermayer : > On Thu, Oct 09, 2014 at 09:44:43PM +0200, Michael Niedermayer wrote: > > On Thu, Oct 09, 2014 at 06:57:59PM +0300, Mika Raento wrote: > > > If present, an MFRA box and its TFRAs are read for fragment start > times. > > > > > > Without this change,

Re: [FFmpeg-devel] [PATCH] mov.c: read fragment start dts from fragmented mp4

2014-10-09 Thread Michael Niedermayer
On Thu, Oct 09, 2014 at 09:44:43PM +0200, Michael Niedermayer wrote: > On Thu, Oct 09, 2014 at 06:57:59PM +0300, Mika Raento wrote: > > If present, an MFRA box and its TFRAs are read for fragment start times. > > > > Without this change, timestamps for discontinuous fragmented mp4 are > > wrong, a

Re: [FFmpeg-devel] [PATCH] mov.c: read fragment start dts from fragmented mp4

2014-10-09 Thread Michael Niedermayer
On Thu, Oct 09, 2014 at 06:57:59PM +0300, Mika Raento wrote: > If present, an MFRA box and its TFRAs are read for fragment start times. > > Without this change, timestamps for discontinuous fragmented mp4 are > wrong, and cause audio/video desync and are not usable for generating > HLS. > --- > l

[FFmpeg-devel] [PATCH] mov.c: read fragment start dts from fragmented mp4

2014-10-09 Thread Mika Raento
If present, an MFRA box and its TFRAs are read for fragment start times. Without this change, timestamps for discontinuous fragmented mp4 are wrong, and cause audio/video desync and are not usable for generating HLS. --- libavformat/isom.h | 15 ++ libavformat/mov.c | 140 ++

Re: [FFmpeg-devel] [PATCH] mov.c: read fragment start dts from fragmented mp4

2014-10-09 Thread Mika Raento
On 9 October 2014 18:30, Michael Niedermayer wrote: > On Thu, Oct 09, 2014 at 08:52:29AM +0300, Mika Raento wrote: >> On 8 October 2014 16:03, Michael Niedermayer wrote: > [...] >> >> +if (avio_rb32(f) != mfra_size) { >> >> +av_log(s, AV_LOG_DEBUG, "doesn't look like mfra (size)\n");

Re: [FFmpeg-devel] [PATCH] mov.c: read fragment start dts from fragmented mp4

2014-10-09 Thread Michael Niedermayer
On Thu, Oct 09, 2014 at 08:52:29AM +0300, Mika Raento wrote: > On 8 October 2014 16:03, Michael Niedermayer wrote: [...] > >> +if (avio_rb32(f) != mfra_size) { > >> +av_log(s, AV_LOG_DEBUG, "doesn't look like mfra (size)\n"); > >> +return -1; > >> +} > >> +if (avio_rb32

Re: [FFmpeg-devel] [PATCH] mov.c: read fragment start dts from fragmented mp4

2014-10-09 Thread Mika Raento
fate passes with these changes, no new tests added. - mika On 9 October 2014 08:53, Mika Raento wrote: > If present, an MFRA box and its TFRAs are read for fragment start times. > > Without this change, timestamps for discontinuous fragmented mp4 are > wrong, and cause audio/video desync and are

[FFmpeg-devel] [PATCH] mov.c: read fragment start dts from fragmented mp4

2014-10-08 Thread Mika Raento
If present, an MFRA box and its TFRAs are read for fragment start times. Without this change, timestamps for discontinuous fragmented mp4 are wrong, and cause audio/video desync and are not usable for generating HLS. --- libavformat/isom.h | 14 ++ libavformat/mov.c | 131 ++

Re: [FFmpeg-devel] [PATCH] mov.c: read fragment start dts from fragmented mp4

2014-10-08 Thread Mika Raento
On 8 October 2014 16:15, Michael Niedermayer wrote: > On Wed, Oct 08, 2014 at 03:03:50PM +0200, Michael Niedermayer wrote: > [...] >> > @@ -3565,6 +3678,9 @@ static int mov_read_header(AVFormatContext *s) >> > else >> > atom.size = INT64_MAX; >> > >> > +if (pb->seekable) { >> > +

Re: [FFmpeg-devel] [PATCH] mov.c: read fragment start dts from fragmented mp4

2014-10-08 Thread Mika Raento
On 8 October 2014 16:03, Michael Niedermayer wrote: > Hi > > On Wed, Oct 08, 2014 at 03:05:07PM +0300, Mika Raento wrote: >> If present, an MFRA box and its TFRAs are read for fragment start times. >> >> Without this change, timestamps for discontinuous fragmented mp4 are >> wrong, and cause audio

Re: [FFmpeg-devel] [PATCH] mov.c: read fragment start dts from fragmented mp4

2014-10-08 Thread Michael Niedermayer
On Wed, Oct 08, 2014 at 03:03:50PM +0200, Michael Niedermayer wrote: [...] > > @@ -3565,6 +3678,9 @@ static int mov_read_header(AVFormatContext *s) > > else > > atom.size = INT64_MAX; > > > > +if (pb->seekable) { > > +mov_read_mfra(s); > > +} > > mov_read_mfra() req

Re: [FFmpeg-devel] [PATCH] mov.c: read fragment start dts from fragmented mp4

2014-10-08 Thread Mika Raento
Thanks a lot for the detailed comments, will address and resubmit. - mika On 8 October 2014 16:03, Michael Niedermayer wrote: > Hi > > On Wed, Oct 08, 2014 at 03:05:07PM +0300, Mika Raento wrote: >> If present, an MFRA box and its TFRAs are read for fragment start times. >> >> Without this change

Re: [FFmpeg-devel] [PATCH] mov.c: read fragment start dts from fragmented mp4

2014-10-08 Thread Michael Niedermayer
Hi On Wed, Oct 08, 2014 at 03:05:07PM +0300, Mika Raento wrote: > If present, an MFRA box and its TFRAs are read for fragment start times. > > Without this change, timestamps for discontinuous fragmented mp4 are > wrong, and cause audio/video desync and are not usable for generating > HLS. > ---

[FFmpeg-devel] [PATCH] mov.c: read fragment start dts from fragmented mp4

2014-10-08 Thread Mika Raento
If present, an MFRA box and its TFRAs are read for fragment start times. Without this change, timestamps for discontinuous fragmented mp4 are wrong, and cause audio/video desync and are not usable for generating HLS. --- libavformat/isom.h | 14 +++ libavformat/mov.c | 116 +