GNOME Bugzilla – Bug 760779
qtdemux: fix framerate calculation for fragmented format in push mode
Last modified: 2016-04-21 09:55:41 UTC
In case of push mode, qtdemux expose streams after got moov box. We can not guarantee that moov box has sample data such as sample duration and the number of sample in stbl box in fragmented format case. If moov has no sample data, this patch will not expose streams until get the first moof. This patch depends on https://bugzilla.gnome.org/show_bug.cgi?id=760505
Created attachment 319252 [details] [review] fix framerate calculation for fragmented format in push mode
This with Bug #760774 is solution for bug #760505
Created attachment 319407 [details] [review] fix framerate calculation for fragmented format in push mode patchset#2
Review of attachment 319407 [details] [review]: ::: gst/isomp4/qtdemux.c @@ +5834,3 @@ + if (!demux->got_moov) + qtdemux_expose_streams (demux); + else { Please add some {} around the if-case @@ +5844,3 @@ + demux->pending_configure = FALSE; + } else + demux->pending_configure = TRUE; ... and the else here @@ +5909,3 @@ qtdemux_expose_streams (demux); + } else if (demux->pending_configure) { + if (!demux->exposed) Note that the code above seems very similar (for MSS). Maybe this should all be generalized a bit so that we always ever expose streams for fragmented MP4 after the first moof is parsed. That can probably simplify the code everywhere a bit instead of adding yet another code path @@ +5911,3 @@ + if (!demux->exposed) + qtdemux_expose_streams (demux); + else { and here
Created attachment 319749 [details] [review] expose streams with first moof for fragmented format
{} are add to if-case There is big difference between non-MSS fragmented format with MSS. That is, as far as I know, MSS did not get moov but only get moof with media_caps. So, it is possible that streams are exposed with the first moof. However, just for non-MSS fragmented format case, following structure can be possible. [moov + mdat + moof + mdat....] For that case, if qtdemux always expose streams after parsing the first moof, some delay is inevitable, due to mdat between moov and the first moof. I think that is the reason why qtdemux do not wait the first moof (in current implementation). So, this patch check availability of sample data in moov (using got_samples variable). If moov have sample data, this patch will expose streams without waiting moof (identical to current qtdemux implementation). + for (n = 0; n < demux->n_streams; n++) { + QtDemuxStream *stream = demux->streams[n]; + got_samples |= stream->stbl_index >= 0 ? TRUE : FALSE; + } + if (!demux->fragmented || got_samples) { However, [moov + moof + mdat...] case, the first moof is triggering condition for expose stream by this patch. In summary, I think it is desirable to always expose streams after parsing the first moof, because [moov + mdat + moof + mdat....] case.
Makes sense but I still wonder if it wouldn't be possible to merge the two code paths a bit more. Basically you're doing almost the same as for MSS, just that you would also configure the streams directly if there was mdat after moov without a moof first.
Review of attachment 319749 [details] [review]: Otherwise looks good ::: gst/isomp4/qtdemux.c @@ +5829,3 @@ + for (n = 0; n < demux->n_streams; n++) { + QtDemuxStream *stream = demux->streams[n]; + got_samples |= stream->stbl_index >= 0 ? TRUE : FALSE; You can make that "got_samples |= stream->stbl_index >= 0;" btw.
Created attachment 319922 [details] [review] expose streams with first moof for fragmented format (patch set #2)
- "got_samples |= stream->stbl_index >= 0;" is fixed - I unified proposed patch with MSS. Please examine this patch
commit d8bb6687ea251570c331038279a43d448167d6ad Author: Seungha Yang <sh.yang@lge.com> Date: Thu Jan 28 21:36:54 2016 +0900 qtdemux: expose streams with first moof for fragmented format In case of push mode, qtdemux expose streams after got moov box. We can not guarantee that a moov box has sample data such as sample duration and the number of sample in stbl box for fragmented format case. So, if a moov has no sample data, streams will not be exposed until get the first moof. https://bugzilla.gnome.org/show_bug.cgi?id=760779
This was reverted, see https://bugzilla.gnome.org/show_bug.cgi?id=764733