[Bug 769048] qtmux: prores-related fixes

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=769048

Sebastian Dröge (slomo) <slomo at coaxion.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #336461|none                        |needs-work
             status|                            |

--- Comment #14 from Sebastian Dröge (slomo) <slomo at coaxion.net> ---
Review of attachment 336461:
 --> (https://bugzilla.gnome.org/review?bug=769048&attachment=336461)

::: gst/isomp4/atoms.c
@@ +3917,3 @@

+AtomInfo *
+build_fiel_extension_prores (const gchar * interlace_mode, gboolean tff)

This one should be merged with the one for JPEG2000. It is a standard MOV atom
that is also relevant for other codecs, and we should always write it for MOV
if we know a field order or about interlacing

The one for JPEG2000 is also wrong (it only writes one byte, and it has a
off-by-one in that byte...)


This one depends on the other bug, we should proxy this in a generic way

@@ +3935,3 @@
+      field_order = tff ? 9 : 14;
+    else if (!g_strcmp0 (interlace_mode, "mixed"))
+      field_order = tff ? 1 : 6;

This is wrong, "mixed" does not mean that. AFAIU 1 and 6 are the same as 9 and
14, just that the fields are spatially flipped (bottom field is first line). We
don't support that at all currently (ffmpeg does).

@@ +3948,3 @@
+
+AtomInfo *
+build_colr_extension (GstVideoColorimetry colorimetry)

This is a standard MP4 atom, we should probably always write it if we can.

We should proxy the colorimetry on compressed caps too, see the other bug

@@ +4003,3 @@
+
+  /* colour specification box */
+  GST_WRITE_UINT32_LE (data, FOURCC_nclc);

This should be nclx for MP4, nclc for MOV/etc

@@ +4008,3 @@
+  GST_WRITE_UINT16_BE (data + 6, transfer_function);    /* transfer function
*/
+  GST_WRITE_UINT16_BE (data + 8, matrix);
+

For MP4 we also need to write the color range at the end: 0_255 is 0x80, 16_235
is 0x00 (all but the first bit are reserved so far)

::: gst/isomp4/gstqtmux.c
@@ +4213,3 @@
     variant = gst_structure_get_string (structure, "variant");
+    colorimetry_str = gst_structure_get_string (structure, "colorimetry");
+    interlace_mode = gst_structure_get_string (structure, "interlace-mode");

We should do all this generally for all video codecs

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.