qtmux: Fix writing of the 'fiel' extension atom

This was also wrong for JPEG2000. Also write it for all MOV files and
JPEG2000, not only for ProRes.

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

3 files changed, 44 insertions, 45 deletions

diff --git a/gst/isomp4/atoms.c b/gst/isomp4/atoms.c
index 948bfb651..1846d9b4d 100644
--- a/gst/isomp4/atoms.c
+++ b/gst/isomp4/atoms.c
@@ -3919,7 +3919,7 @@ build_pasp_extension (gint par_width, gint par_height)
 }
 
 AtomInfo *
-build_fiel_extension_prores (const gchar * interlace_mode, gboolean tff)
+build_fiel_extension (GstVideoInterlaceMode mode, GstVideoFieldOrder order)
 {
   AtomData *atom_data = atom_data_new (FOURCC_fiel);
   guint8 *data;
@@ -3929,17 +3929,15 @@ build_fiel_extension_prores (const gchar * interlace_mode, gboolean tff)
   atom_data_alloc_mem (atom_data, 2);
   data = atom_data->data;
 
-  if (!g_strcmp0 (interlace_mode, "progressive")) {
+  if (mode == GST_VIDEO_INTERLACE_MODE_PROGRESSIVE) {
     interlace = 1;
     field_order = 0;
-  } else {
+  } else if (mode == GST_VIDEO_INTERLACE_MODE_INTERLEAVED) {
     interlace = 2;
-    if (!g_strcmp0 (interlace_mode, "interleaved"))
-      field_order = tff ? 9 : 14;
-    else if (!g_strcmp0 (interlace_mode, "mixed"))
-      field_order = tff ? 1 : 6;
-    else
-      g_assert_not_reached ();
+    field_order = order == GST_VIDEO_FIELD_ORDER_TOP_FIELD_FIRST ? 9 : 14;
+  } else {
+    interlace = 0;
+    field_order = 0;
   }
 
   GST_WRITE_UINT8 (data, interlace);
@@ -4881,22 +4879,6 @@ build_mov_alac_extension (const GstBuffer * codec_data)
 }
 
 AtomInfo *
-build_fiel_extension (gint fields)
-{
-  AtomData *atom_data;
-  guint8 f = fields;
-
-  if (fields == 1) {
-    return NULL;
-  }
-
-  atom_data = atom_data_new_from_data (FOURCC_fiel, &f, 1);
-
-  return build_atom_info_wrapper ((Atom *) atom_data, atom_data_copy_data,
-      atom_data_free);
-}
-
-AtomInfo *
 build_jp2x_extension (const GstBuffer * prefix)
 {
   AtomData *atom_data;
diff --git a/gst/isomp4/atoms.h b/gst/isomp4/atoms.h
index ba829e2a2..d2e354a2f 100644
--- a/gst/isomp4/atoms.h
+++ b/gst/isomp4/atoms.h
@@ -1043,8 +1043,7 @@ AtomInfo *   build_jp2h_extension        (gint width, gint height, const gchar *
                                           const GValue * cdef_array);
 
 AtomInfo *   build_jp2x_extension        (const GstBuffer * prefix);
-AtomInfo *   build_fiel_extension        (gint fields);
-AtomInfo *   build_fiel_extension_prores (const gchar * interlace_mode, gboolean tff);
+AtomInfo *   build_fiel_extension        (GstVideoInterlaceMode mode, GstVideoFieldOrder order);
 AtomInfo *   build_colr_extension        (GstVideoColorimetry colorimetry);
 AtomInfo *   build_ac3_extension         (guint8 fscod, guint8 bsid,
                                           guint8 bsmod, guint8 acmod,
diff --git a/gst/isomp4/gstqtmux.c b/gst/isomp4/gstqtmux.c
index 42e9533a6..2005621e9 100644
--- a/gst/isomp4/gstqtmux.c
+++ b/gst/isomp4/gstqtmux.c
@@ -4196,14 +4196,12 @@ gst_qt_mux_video_sink_set_caps (GstQTPad * qtpad, GstCaps * caps)
     const GValue *cmap_array;
     const GValue *cdef_array;
     gint ncomp = 0;
-    gint fields = 1;
 
     if (strcmp (mimetype, "image/x-jpc") == 0) {
       qtpad->prepare_buf_func = gst_qt_mux_prepare_jpc_buffer;
     }
 
     gst_structure_get_int (structure, "num-components", &ncomp);
-    gst_structure_get_int (structure, "fields", &fields);
     cmap_array = gst_structure_get_value (structure, "component-map");
     cdef_array = gst_structure_get_value (structure, "channel-definitions");
 
@@ -4218,10 +4216,6 @@ gst_qt_mux_video_sink_set_caps (GstQTPad * qtpad, GstCaps * caps)
                 cdef_array)) != NULL) {
       ext_atom_list = g_list_append (ext_atom_list, ext_atom);
 
-      ext_atom = build_fiel_extension (fields);
-      if (ext_atom)
-        ext_atom_list = g_list_append (ext_atom_list, ext_atom);
-
       ext_atom = build_jp2x_extension (codec_data);
       if (ext_atom)
         ext_atom_list = g_list_append (ext_atom_list, ext_atom);
@@ -4247,13 +4241,10 @@ gst_qt_mux_video_sink_set_caps (GstQTPad * qtpad, GstCaps * caps)
   } else if (strcmp (mimetype, "video/x-prores") == 0) {
     const gchar *variant;
     const gchar *colorimetry_str;
-    const gchar *interlace_mode;
-    gboolean tff = TRUE;
     GstVideoColorimetry colorimetry;
 
     variant = gst_structure_get_string (structure, "variant");
     colorimetry_str = gst_structure_get_string (structure, "colorimetry");
-    interlace_mode = gst_structure_get_string (structure, "interlace-mode");
     if (!variant || !g_strcmp0 (variant, "standard"))
       entry.fourcc = FOURCC_apcn;
     else if (!g_strcmp0 (variant, "lt"))
@@ -4262,11 +4253,6 @@ gst_qt_mux_video_sink_set_caps (GstQTPad * qtpad, GstCaps * caps)
       entry.fourcc = FOURCC_apch;
     else if (!g_strcmp0 (variant, "proxy"))
       entry.fourcc = FOURCC_apco;
-    if (!g_strcmp0 (interlace_mode, "interleaved") ||
-        !g_strcmp0 (interlace_mode, "mixed")) {
-      /* Assume top-fields-first if unspecified */
-      gst_structure_get_boolean (structure, "top-field-first", &tff);
-    }
 
     if (gst_video_colorimetry_from_string (&colorimetry, colorimetry_str)) {
       ext_atom = build_colr_extension (colorimetry);
@@ -4274,10 +4260,6 @@ gst_qt_mux_video_sink_set_caps (GstQTPad * qtpad, GstCaps * caps)
         ext_atom_list = g_list_append (ext_atom_list, ext_atom);
     }
 
-    ext_atom = build_fiel_extension_prores (interlace_mode, tff);
-    if (ext_atom)
-      ext_atom_list = g_list_append (ext_atom_list, ext_atom);
-
     if (colorimetry_str == NULL) {
       /* TODO: Maybe implement better heuristics */
       GST_WARNING_OBJECT (qtmux,
@@ -4290,6 +4272,42 @@ gst_qt_mux_video_sink_set_caps (GstQTPad * qtpad, GstCaps * caps)
   if (!entry.fourcc)
     goto refuse_caps;
 
+  if (qtmux_klass->format == GST_QT_MUX_FORMAT_QT
+      || strcmp (mimetype, "image/x-j2c") == 0
+      || strcmp (mimetype, "image/x-jpc") == 0) {
+    const gchar *s;
+    GstVideoInterlaceMode interlace_mode;
+    GstVideoFieldOrder field_order;
+    gint fields = -1;
+
+    if (strcmp (mimetype, "image/x-j2c") == 0 ||
+        strcmp (mimetype, "image/x-jpc") == 0) {
+
+      fields = 1;
+      gst_structure_get_int (structure, "fields", &fields);
+    }
+
+    s = gst_structure_get_string (structure, "interlace-mode");
+    if (s)
+      interlace_mode = gst_video_interlace_mode_from_string (s);
+    else
+      interlace_mode =
+          (fields <=
+          1) ? GST_VIDEO_INTERLACE_MODE_PROGRESSIVE :
+          GST_VIDEO_INTERLACE_MODE_MIXED;
+
+    field_order = GST_VIDEO_FIELD_ORDER_UNKNOWN;
+    if (interlace_mode == GST_VIDEO_INTERLACE_MODE_INTERLEAVED) {
+      s = gst_structure_get_string (structure, "field-order");
+      if (s)
+        field_order = gst_video_field_order_from_string (s);
+    }
+
+    ext_atom = build_fiel_extension (interlace_mode, field_order);
+    if (ext_atom)
+      ext_atom_list = g_list_append (ext_atom_list, ext_atom);
+  }
+
   /* ok, set the pad info accordingly */
   qtpad->fourcc = entry.fourcc;
   qtpad->sync = sync;