gst/mxf/: Add mxf_metadata_generic_sound_essence_descriptor_set_caps() to set rate and channels and use this for all ...

Original commit message from CVS:
* gst/mxf/mxfaes-bwf.c: (mxf_bwf_create_caps),
(mxf_aes3_create_caps):
* gst/mxf/mxfalaw.c: (mxf_alaw_create_caps):
* gst/mxf/mxfd10.c: (mxf_d10_create_caps):
* gst/mxf/mxfdemux.c:
* gst/mxf/mxfmetadata.c: (mxf_metadata_source_package_resolve),
(mxf_metadata_generic_picture_essence_descriptor_set_caps),
(mxf_metadata_generic_sound_essence_descriptor_set_caps):
* gst/mxf/mxfmetadata.h:
* gst/mxf/mxfmpeg.c: (mxf_mpeg_es_create_caps):
Add mxf_metadata_generic_sound_essence_descriptor_set_caps() to
set rate and channels and use this for all sound essence.
Give some debug output when setting picture essence caps with
invalid descriptor values.
Fix height calculation from the frame layout a bit more and
add a TODO to check if it's really correct now or if it needs
more fixing (especially, does the framerate need adjustments?).

8 files changed, 115 insertions, 69 deletions

diff --git a/ChangeLog b/ChangeLog
index 6983a81d..b7ded873 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,25 @@
+2008-12-19  Sebastian Dröge  <sebastian.droege@collabora.co.uk>
+
+	* gst/mxf/mxfaes-bwf.c: (mxf_bwf_create_caps),
+	(mxf_aes3_create_caps):
+	* gst/mxf/mxfalaw.c: (mxf_alaw_create_caps):
+	* gst/mxf/mxfd10.c: (mxf_d10_create_caps):
+	* gst/mxf/mxfdemux.c:
+	* gst/mxf/mxfmetadata.c: (mxf_metadata_source_package_resolve),
+	(mxf_metadata_generic_picture_essence_descriptor_set_caps),
+	(mxf_metadata_generic_sound_essence_descriptor_set_caps):
+	* gst/mxf/mxfmetadata.h:
+	* gst/mxf/mxfmpeg.c: (mxf_mpeg_es_create_caps):
+	Add mxf_metadata_generic_sound_essence_descriptor_set_caps() to
+	set rate and channels and use this for all sound essence.
+
+	Give some debug output when setting picture essence caps with
+	invalid descriptor values.
+
+	Fix height calculation from the frame layout a bit more and
+	add a TODO to check if it's really correct now or if it needs
+	more fixing (especially, does the framerate need adjustments?).
+
 2008-12-18  Sebastian Dröge  <sebastian.droege@collabora.co.uk>
 
 	* gst/mxf/Makefile.am:
diff --git a/gst/mxf/mxfaes-bwf.c b/gst/mxf/mxfaes-bwf.c
index b684c193..9acdf673 100644
--- a/gst/mxf/mxfaes-bwf.c
+++ b/gst/mxf/mxfaes-bwf.c
@@ -664,14 +664,13 @@ mxf_bwf_create_caps (MXFMetadataTimelineTrack * track,
           descriptor->channel_count) / 8;
 
     ret = gst_caps_new_simple ("audio/x-raw-int",
-        "rate", G_TYPE_INT,
-        (gint) (((gdouble) descriptor->audio_sampling_rate.n) /
-            ((gdouble) descriptor->audio_sampling_rate.d) + 0.5), "channels",
-        G_TYPE_INT, descriptor->channel_count, "signed", G_TYPE_BOOLEAN,
+        "signed", G_TYPE_BOOLEAN,
         (block_align != 1), "endianness", G_TYPE_INT, G_LITTLE_ENDIAN, "depth",
         G_TYPE_INT, (block_align / descriptor->channel_count) * 8, "width",
         G_TYPE_INT, (block_align / descriptor->channel_count) * 8, NULL);
 
+    mxf_metadata_generic_sound_essence_descriptor_set_caps (descriptor, ret);
+
     codec_name =
         g_strdup_printf ("Uncompressed %u-bit little endian integer PCM audio",
         (block_align / descriptor->channel_count) * 8);
@@ -695,14 +694,13 @@ mxf_bwf_create_caps (MXFMetadataTimelineTrack * track,
           descriptor->channel_count) / 8;
 
     ret = gst_caps_new_simple ("audio/x-raw-int",
-        "rate", G_TYPE_INT,
-        (gint) (((gdouble) descriptor->audio_sampling_rate.n) /
-            ((gdouble) descriptor->audio_sampling_rate.d) + 0.5), "channels",
-        G_TYPE_INT, descriptor->channel_count, "signed", G_TYPE_BOOLEAN,
+        "signed", G_TYPE_BOOLEAN,
         (block_align != 1), "endianness", G_TYPE_INT, G_BIG_ENDIAN, "depth",
         G_TYPE_INT, (block_align / descriptor->channel_count) * 8, "width",
         G_TYPE_INT, (block_align / descriptor->channel_count) * 8, NULL);
 
+    mxf_metadata_generic_sound_essence_descriptor_set_caps (descriptor, ret);
+
     codec_name =
         g_strdup_printf ("Uncompressed %u-bit big endian integer PCM audio",
         (block_align / descriptor->channel_count) * 8);
@@ -715,10 +713,9 @@ mxf_bwf_create_caps (MXFMetadataTimelineTrack * track,
       GST_ERROR ("Invalid descriptor");
       return NULL;
     }
-    ret = gst_caps_new_simple ("audio/x-alaw", "rate", G_TYPE_INT,
-        (gint) (((gdouble) descriptor->audio_sampling_rate.n) /
-            ((gdouble) descriptor->audio_sampling_rate.d) + 0.5),
-        "channels", G_TYPE_INT, descriptor->channel_count);
+    ret = gst_caps_new_simple ("audio/x-alaw", NULL);
+    mxf_metadata_generic_sound_essence_descriptor_set_caps (descriptor, ret);
+
     codec_name = g_strdup ("A-law encoded audio");
   } else {
     GST_ERROR ("Unsupported sound essence compression: %s",
@@ -773,14 +770,13 @@ mxf_aes3_create_caps (MXFMetadataTimelineTrack * track,
         descriptor->channel_count) / 8;
 
   ret = gst_caps_new_simple ("audio/x-raw-int",
-      "rate", G_TYPE_INT,
-      (gint) (((gdouble) descriptor->audio_sampling_rate.n) /
-          ((gdouble) descriptor->audio_sampling_rate.d) + 0.5), "channels",
-      G_TYPE_INT, descriptor->channel_count, "signed", G_TYPE_BOOLEAN,
+      "signed", G_TYPE_BOOLEAN,
       (block_align != 1), "endianness", G_TYPE_INT, G_LITTLE_ENDIAN, "depth",
       G_TYPE_INT, (block_align / descriptor->channel_count) * 8, "width",
       G_TYPE_INT, (block_align / descriptor->channel_count) * 8, NULL);
 
+  mxf_metadata_generic_sound_essence_descriptor_set_caps (descriptor, ret);
+
   codec_name =
       g_strdup_printf ("Uncompressed %u-bit AES3 audio",
       (block_align / descriptor->channel_count) * 8);
@@ -816,24 +812,24 @@ mxf_aes_bwf_create_caps (MXFMetadataTimelineTrack * track, GstTagList ** tags,
     if (!track->parent.descriptor[i])
       continue;
 
-    if (MXF_IS_METADATA_GENERIC_SOUND_ESSENCE_DESCRIPTOR (track->parent.
-            descriptor[i])
+    if (MXF_IS_METADATA_GENERIC_SOUND_ESSENCE_DESCRIPTOR (track->
+            parent.descriptor[i])
         && (track->parent.descriptor[i]->essence_container.u[14] == 0x01
             || track->parent.descriptor[i]->essence_container.u[14] == 0x02
             || track->parent.descriptor[i]->essence_container.u[14] == 0x08)) {
-      s = (MXFMetadataGenericSoundEssenceDescriptor *) track->parent.
-          descriptor[i];
+      s = (MXFMetadataGenericSoundEssenceDescriptor *) track->
+          parent.descriptor[i];
       bwf = TRUE;
       break;
     } else
-        if (MXF_IS_METADATA_GENERIC_SOUND_ESSENCE_DESCRIPTOR (track->parent.
-            descriptor[i])
+        if (MXF_IS_METADATA_GENERIC_SOUND_ESSENCE_DESCRIPTOR (track->
+            parent.descriptor[i])
         && (track->parent.descriptor[i]->essence_container.u[14] == 0x03
             || track->parent.descriptor[i]->essence_container.u[14] == 0x04
             || track->parent.descriptor[i]->essence_container.u[14] == 0x09)) {
 
-      s = (MXFMetadataGenericSoundEssenceDescriptor *) track->parent.
-          descriptor[i];
+      s = (MXFMetadataGenericSoundEssenceDescriptor *) track->
+          parent.descriptor[i];
       bwf = FALSE;
       break;
     }
diff --git a/gst/mxf/mxfalaw.c b/gst/mxf/mxfalaw.c
index 437f82d9..1cea993a 100644
--- a/gst/mxf/mxfalaw.c
+++ b/gst/mxf/mxfalaw.c
@@ -100,10 +100,10 @@ mxf_alaw_create_caps (MXFMetadataTimelineTrack * track, GstTagList ** tags,
     if (!track->parent.descriptor[i])
       continue;
 
-    if (MXF_IS_METADATA_GENERIC_SOUND_ESSENCE_DESCRIPTOR (track->parent.
-            descriptor[i])) {
-      s = (MXFMetadataGenericSoundEssenceDescriptor *) track->parent.
-          descriptor[i];
+    if (MXF_IS_METADATA_GENERIC_SOUND_ESSENCE_DESCRIPTOR (track->
+            parent.descriptor[i])) {
+      s = (MXFMetadataGenericSoundEssenceDescriptor *) track->
+          parent.descriptor[i];
       break;
     }
   }
@@ -118,10 +118,8 @@ mxf_alaw_create_caps (MXFMetadataTimelineTrack * track, GstTagList ** tags,
   if (s && s->audio_sampling_rate.n != 0 && s->audio_sampling_rate.d != 0 &&
       s->channel_count != 0) {
 
-    caps = gst_caps_new_simple ("audio/x-alaw", "rate", G_TYPE_INT,
-        (gint) (((gdouble) s->audio_sampling_rate.n) /
-            ((gdouble) s->audio_sampling_rate.d) + 0.5),
-        "channels", G_TYPE_INT, s->channel_count, NULL);
+    caps = gst_caps_new_simple ("audio/x-alaw", NULL);
+    mxf_metadata_generic_sound_essence_descriptor_set_caps (s, caps);
 
     /* TODO: Handle channel layout somehow?
      * Or is alaw limited to two channels? */
diff --git a/gst/mxf/mxfd10.c b/gst/mxf/mxfd10.c
index aaa4477b..4a63b50a 100644
--- a/gst/mxf/mxfd10.c
+++ b/gst/mxf/mxfd10.c
@@ -175,15 +175,15 @@ mxf_d10_create_caps (MXFMetadataTimelineTrack * track, GstTagList ** tags,
     if (!track->parent.descriptor[i])
       continue;
 
-    if (MXF_IS_METADATA_GENERIC_PICTURE_ESSENCE_DESCRIPTOR (track->
-            parent.descriptor[i])) {
-      p = (MXFMetadataGenericPictureEssenceDescriptor *) track->parent.
-          descriptor[i];
+    if (MXF_IS_METADATA_GENERIC_PICTURE_ESSENCE_DESCRIPTOR (track->parent.
+            descriptor[i])) {
+      p = (MXFMetadataGenericPictureEssenceDescriptor *) track->
+          parent.descriptor[i];
       break;
-    } else if (MXF_IS_METADATA_GENERIC_SOUND_ESSENCE_DESCRIPTOR (track->
-            parent.descriptor[i])) {
-      s = (MXFMetadataGenericSoundEssenceDescriptor *) track->parent.
-          descriptor[i];
+    } else if (MXF_IS_METADATA_GENERIC_SOUND_ESSENCE_DESCRIPTOR (track->parent.
+            descriptor[i])) {
+      s = (MXFMetadataGenericSoundEssenceDescriptor *) track->
+          parent.descriptor[i];
       break;
     }
   }
@@ -214,14 +214,13 @@ mxf_d10_create_caps (MXFMetadataTimelineTrack * track, GstTagList ** tags,
     /* FIXME: set channel layout */
 
     caps = gst_caps_new_simple ("audio/x-raw-int",
-        "rate", G_TYPE_INT,
-        (gint) (((gdouble) s->audio_sampling_rate.n) /
-            ((gdouble) s->audio_sampling_rate.d) + 0.5), "channels",
-        G_TYPE_INT, s->channel_count, "signed", G_TYPE_BOOLEAN,
+        "signed", G_TYPE_BOOLEAN,
         (s->quantization_bits != 8), "endianness", G_TYPE_INT, G_LITTLE_ENDIAN,
         "depth", G_TYPE_INT, s->quantization_bits, "width", G_TYPE_INT,
         s->quantization_bits, NULL);
 
+    mxf_metadata_generic_sound_essence_descriptor_set_caps (s, caps);
+
     *handler = mxf_d10_sound_handle_essence_element;
 
     data = g_new0 (MXFD10AudioMappingData, 1);
diff --git a/gst/mxf/mxfdemux.c b/gst/mxf/mxfdemux.c
index 17ec3a18..88048bf0 100644
--- a/gst/mxf/mxfdemux.c
+++ b/gst/mxf/mxfdemux.c
@@ -54,6 +54,7 @@
  *   - Multichannel audio needs channel layouts, define them (SMPTE S320M?).
  *   - Correctly handle the different rectangles and aspect-ratio for video
  *   - Add support for non-standard MXF used by Avid (bug #561922).
+ *   - Fix frame layout stuff, i.e. interlaced/progressive
  *
  *   - Implement SMPTE D11 essence and the digital cinema/MXF specs
  *
@@ -554,11 +555,11 @@ gst_mxf_demux_choose_package (GstMXFDemux * demux)
 
   for (i = 0; i < demux->preface->content_storage->n_packages; i++) {
     if (demux->preface->content_storage->packages[i] &&
-        MXF_IS_METADATA_MATERIAL_PACKAGE (demux->preface->content_storage->
-            packages[i])) {
+        MXF_IS_METADATA_MATERIAL_PACKAGE (demux->preface->
+            content_storage->packages[i])) {
       ret =
-          MXF_METADATA_GENERIC_PACKAGE (demux->preface->content_storage->
-          packages[i]);
+          MXF_METADATA_GENERIC_PACKAGE (demux->preface->
+          content_storage->packages[i]);
       break;
     }
   }
diff --git a/gst/mxf/mxfmetadata.c b/gst/mxf/mxfmetadata.c
index 9a5a153f..88280d84 100644
--- a/gst/mxf/mxfmetadata.c
+++ b/gst/mxf/mxfmetadata.c
@@ -1301,6 +1301,10 @@ mxf_metadata_source_package_resolve (MXFMetadataBase * m,
     }
   }
 
+  /* TODO: Check if there is a EssenceContainerData for this source package
+   * and store this in the source package instance. Without
+   * EssenceContainerData this package must be external */
+
   return ret;
 }
 
@@ -2628,8 +2632,12 @@ void mxf_metadata_generic_picture_essence_descriptor_set_caps
   g_return_if_fail (MXF_IS_METADATA_GENERIC_PICTURE_ESSENCE_DESCRIPTOR (self));
   g_return_if_fail (GST_IS_CAPS (caps));
 
-  gst_caps_set_simple (caps, "framerate", GST_TYPE_FRACTION, f->sample_rate.n,
-      f->sample_rate.d, NULL);
+  if (f->sample_rate.d == 0) {
+    GST_ERROR ("Invalid framerate");
+  } else {
+    gst_caps_set_simple (caps, "framerate", GST_TYPE_FRACTION, f->sample_rate.n,
+        f->sample_rate.d, NULL);
+  }
 
   width = self->stored_width;
   height = self->stored_height;
@@ -2640,17 +2648,22 @@ void mxf_metadata_generic_picture_essence_descriptor_set_caps
    *
    * See SMPTE 377M E2.2 and E1.2
    */
-  if (self->frame_layout != 0)
+  if (self->frame_layout == 1 || self->frame_layout == 2
+      || self->frame_layout == 4)
     height *= 2;
 
-  if (width == 0 || height == 0)
+  if (width == 0 || height == 0) {
+    GST_ERROR ("Invalid width/height");
     return;
+  }
 
   gst_caps_set_simple (caps, "width", G_TYPE_INT, width, "height", G_TYPE_INT,
       height, NULL);
 
-  if (self->aspect_ratio.n == 0 || self->aspect_ratio.d == 0)
+  if (self->aspect_ratio.n == 0 || self->aspect_ratio.d == 0) {
+    GST_ERROR ("Invalid aspect ratio");
     return;
+  }
 
   par_n = height * self->aspect_ratio.n;
   par_d = width * self->aspect_ratio.d;
@@ -2761,6 +2774,29 @@ static void
       mxf_metadata_generic_sound_essence_descriptor_handle_tag;
 }
 
+void mxf_metadata_generic_sound_essence_descriptor_set_caps
+    (MXFMetadataGenericSoundEssenceDescriptor * self, GstCaps * caps)
+{
+  g_return_if_fail (MXF_IS_METADATA_GENERIC_SOUND_ESSENCE_DESCRIPTOR (self));
+  g_return_if_fail (GST_IS_CAPS (caps));
+
+  if (self->audio_sampling_rate.n == 0 || self->audio_sampling_rate.d == 0) {
+    GST_ERROR ("Invalid audio sampling rate");
+  } else {
+    gst_caps_set_simple (caps,
+        "rate", G_TYPE_INT,
+        (gint) ((((gdouble) self->audio_sampling_rate.n) /
+                ((gdouble) self->audio_sampling_rate.d)) + 0.5), NULL);
+  }
+
+  if (self->channel_count == 0) {
+    GST_ERROR ("Invalid number of channels (0)");
+  } else {
+    gst_caps_set_simple (caps, "channels", G_TYPE_INT, self->channel_count,
+        NULL);
+  }
+}
+
 G_DEFINE_TYPE (MXFMetadataCDCIPictureEssenceDescriptor,
     mxf_metadata_cdci_picture_essence_descriptor,
     MXF_TYPE_METADATA_GENERIC_PICTURE_ESSENCE_DESCRIPTOR);
diff --git a/gst/mxf/mxfmetadata.h b/gst/mxf/mxfmetadata.h
index 8b91a40c..34512572 100644
--- a/gst/mxf/mxfmetadata.h
+++ b/gst/mxf/mxfmetadata.h
@@ -699,5 +699,6 @@ void mxf_metadata_init_types (void);
 MXFMetadataTrackType mxf_metadata_track_identifier_parse (const MXFUL * track_identifier);
 
 void mxf_metadata_generic_picture_essence_descriptor_set_caps (MXFMetadataGenericPictureEssenceDescriptor * self, GstCaps * caps);
+void mxf_metadata_generic_sound_essence_descriptor_set_caps (MXFMetadataGenericSoundEssenceDescriptor * self, GstCaps * caps);
 
 #endif /* __MXF_METADATA_H__ */
diff --git a/gst/mxf/mxfmpeg.c b/gst/mxf/mxfmpeg.c
index 9a9b6851..c8ac23ca 100644
--- a/gst/mxf/mxfmpeg.c
+++ b/gst/mxf/mxfmpeg.c
@@ -435,14 +435,7 @@ mxf_mpeg_es_create_caps (MXFMetadataTimelineTrack * track, GstTagList ** tags,
     }
 
     if (caps) {
-      if (s->audio_sampling_rate.n != 0 && s->audio_sampling_rate.d != 0)
-        gst_caps_set_simple (caps, "rate", G_TYPE_INT,
-            (gint) (((gdouble) s->audio_sampling_rate.n) /
-                ((gdouble) s->audio_sampling_rate.d) + 0.5), NULL);
-      if (s->channel_count != 0)
-        gst_caps_set_simple (caps, "channels", G_TYPE_INT, s->channel_count,
-            NULL);
-
+      mxf_metadata_generic_sound_essence_descriptor_set_caps (s, caps);
       *handler = mxf_mpeg_audio_handle_essence_element;
     }
   }
@@ -479,17 +472,17 @@ mxf_mpeg_create_caps (MXFMetadataTimelineTrack * track, GstTagList ** tags,
     if (!track->parent.descriptor[i])
       continue;
 
-    if (MXF_IS_METADATA_GENERIC_PICTURE_ESSENCE_DESCRIPTOR (track->
-            parent.descriptor[i])) {
+    if (MXF_IS_METADATA_GENERIC_PICTURE_ESSENCE_DESCRIPTOR (track->parent.
+            descriptor[i])) {
       f = track->parent.descriptor[i];
-      p = (MXFMetadataGenericPictureEssenceDescriptor *) track->parent.
-          descriptor[i];
+      p = (MXFMetadataGenericPictureEssenceDescriptor *) track->
+          parent.descriptor[i];
       break;
-    } else if (MXF_IS_METADATA_GENERIC_SOUND_ESSENCE_DESCRIPTOR (track->
-            parent.descriptor[i])) {
+    } else if (MXF_IS_METADATA_GENERIC_SOUND_ESSENCE_DESCRIPTOR (track->parent.
+            descriptor[i])) {
       f = track->parent.descriptor[i];
-      s = (MXFMetadataGenericSoundEssenceDescriptor *) track->parent.
-          descriptor[i];
+      s = (MXFMetadataGenericSoundEssenceDescriptor *) track->
+          parent.descriptor[i];
       break;
     }
   }