After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 706509 - mp4mux: add ftyp and extra atoms to the streamheaders
mp4mux: add ftyp and extra atoms to the streamheaders
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
unspecified
Other All
: Normal minor
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2013-08-21 15:34 UTC by Andoni Morales
Modified: 2018-11-03 14:49 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
mp4mux: add ftyp and extra atoms to the streamheaders (3.28 KB, patch)
2013-08-21 15:34 UTC, Andoni Morales
needs-work Details | Review
mp4mux: add ftyp and extra atoms to the streamheaders (5.95 KB, patch)
2013-08-26 15:50 UTC, Andoni Morales
needs-work Details | Review
mp4mux: add ftyp and extra atoms to the streamheaders (15.56 KB, patch)
2013-08-27 17:29 UTC, Andoni Morales
none Details | Review

Description Andoni Morales 2013-08-21 15:34:27 UTC
These atoms are missing to reconstruct the headers from the
streamheaders caps field.
Comment 1 Andoni Morales 2013-08-21 15:34:29 UTC
Created attachment 252612 [details] [review]
mp4mux: add ftyp and extra atoms to the streamheaders
Comment 2 Sebastian Dröge (slomo) 2013-08-21 16:18:58 UTC
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.
Comment 3 Andoni Morales 2013-08-26 15:50:07 UTC
Created attachment 253149 [details] [review]
mp4mux: add ftyp and extra atoms to the streamheaders
Comment 4 Andoni Morales 2013-08-26 15:52:38 UTC
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.
Comment 5 Sebastian Dröge (slomo) 2013-08-27 08:39:56 UTC
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?
Comment 6 Sebastian Dröge (slomo) 2013-08-27 08:50:39 UTC
The change of the streamheaders structure should be fine as discussed on IRC.
Comment 7 Andoni Morales 2013-08-27 17:29:19 UTC
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.
Comment 8 Andoni Morales 2013-08-27 17:34:10 UTC
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.
Comment 9 Andoni Morales 2013-11-15 09:23:54 UTC
That's the last one related to DASH that needs to be reviewed
Comment 10 GStreamer system administrator 2018-11-03 14:49:37 UTC
-- 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.