GNOME Bugzilla – Bug 769389
aiffparse: Add bitrate and container format tags
Last modified: 2016-08-02 12:22:11 UTC
AIFF is a container format for audio data. Inform the application that the data comes from an AIFF container by adding a CONTAINER_FORMAT tag. Also, add bitrate tags. The birate is essentially bytes_per_second * 8, since the data is uncompressed. This helps downstream elements which use the bitrate to estimate network buffer sizes.
Created attachment 332485 [details] [review] Additional bitrate and container tags
Review of attachment 332485 [details] [review]: ::: gst/aiff/aiffparse.c @@ +1009,3 @@ + + /* Since this is uncompressed PCM data, the nominal + *, actual, minimum, maximuzm bitrate are the same */ Typo here though. Should make sure that whenever compressed support is added (aiff can do that, right?), we change this part here...
Review of attachment 332485 [details] [review]: Actually ::: gst/aiff/aiffparse.c @@ +1001,3 @@ + gst_tag_list_add (aiff->tags, GST_TAG_MERGE_REPLACE, + GST_TAG_CONTAINER_FORMAT, "Audio Interchange File Format (AIFF)", + NULL); Use gst_pb_utils_add_codec_description_to_tag_list() here. That ensures consistent naming
(In reply to Sebastian Dröge (slomo) from comment #2) > Review of attachment 332485 [details] [review] [review]: > > ::: gst/aiff/aiffparse.c > @@ +1009,3 @@ > + > + /* Since this is uncompressed PCM data, the nominal > + *, actual, minimum, maximuzm bitrate are the same */ > > Typo here though. > > Should make sure that whenever compressed support is added (aiff can do > that, right?), we change this part here... Yes, AIFF-C supports compression. However, aiffparse only supports uncompressed PCM data. From gst_aiff_parse_parse_comm(): /* We only support the 'trivial' uncompressed AIFC, but it can be * either big or little endian */ Adding support for alaw/mulaw/adpcm and others would perhaps be better kept in a separate commit. Nevertheless, I'll modify the bitrate comment above to reflect the AIFF-C situation.
Created attachment 332508 [details] [review] Additional bitrate and container tags, v2
Review of attachment 332508 [details] [review]: ::: gst/aiff/aiffparse.c @@ +1004,3 @@ + gst_pb_utils_add_codec_description_to_tag_list (aiff->tags, + GST_TAG_CONTAINER_FORMAT, desccaps); + gst_caps_unref (desccaps); No need to create new caps here, just get the caps from the sinkpad template :) gst_pad_get_pad_template_caps() on the sinkpad
Created attachment 332543 [details] [review] Additional bitrate and container tags, v3
commit 9c596d20fe03896eb4817d11c35cfb45c83eaa94 Author: Carlos Rafael Giani <dv@pseudoterminal.org> Date: Tue Aug 2 13:48:43 2016 +0200 aiffparse: Add tags for container format and bitrate The bitrate is added to help downstream elements (like uridecodebin) figure out a proper network buffer size https://bugzilla.gnome.org/show_bug.cgi?id=769389