Created attachment 252612 [details] [review]
mp4mux: add ftyp and extra atoms to the streamheaders

Review of attachment 252612 [details] [review]:

This would mean that the caps would be set three times on the srcpad. That's not ideal, it should wait until the final caps are known and then set it all together.

Created attachment 253149 [details] [review]
mp4mux: add ftyp and extra atoms to the streamheaders

In  this new patch we keep a copy of ftyp, moov and extra atoms buffers and create the streamheaders buffer array once all are know to set the streamheader caps. They are kept in different buffers because the moov one might change.

Review of attachment 253149 [details] [review]:

Now the only problem I see here is that it breaks ABI. Some code might assume that it gets a single buffer as qtmux streamheaders... now it would get an array. Not sure how relevant that is.

::: gst/isomp4/gstqtmux.c
@@ +1619,3 @@
+    gst_value_array_append_value (&array, &value);
+    g_value_unset (&value);
+  }

Shouldn't all three always be available when this is called? Maybe add an assertion for that?

@@ +2006,3 @@
     /* no need to seek back */
+    ret = gst_qt_mux_send_moov (qtmux, NULL, FALSE);
+    gst_qt_mux_set_header_on_caps (qtmux);

First set caps, then push buffers

@@ +2094,3 @@
     return ret;
 
+  gst_qt_mux_set_header_on_caps (qtmux);

Why is this required here too?

The change of the streamheaders structure should be fine as discussed on IRC.

Created attachment 253282 [details] [review]
mp4mux: add ftyp and extra atoms to the streamheaders

ftyp and extra atoms are needed in the caps' streamheader
field to reconstruct a valid header from the caps, useful
for fragmented streaming, where the moov is created at the
very beginning with only the useful initialization data.

For fragmented mp4 streams, the header buffers are queued until
the header is completed and they are pushed after setting the
streamheader in caps and before starting to push moof's.
It doesn't change the behaviour with regular or fast_start mp4 files,
where the final moov is only known when finishing the file.

Another attempt :)
This time the header atoms are queued and pushed when we have the whole header and its set in the caps for fragmented mp4.
For regular mp4 files, the moov is not known until the we stop the file. For fast_start files we set the streamheader and than push the whole header to the new file. For regular ones we push the rest of the header (moov + extra_atoms) at the end of the final and update the caps. The behaviour doesn't change with regular mp4 files, except that now the streamheader contains the whole header too.

That's the last one related to DASH that needs to be reviewed

-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/issues/91.