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 749109 - codec-utils: add a gst_codec_utils_aac_get_sample_rate
codec-utils: add a gst_codec_utils_aac_get_sample_rate
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
1.4.5
Other Linux
: Normal enhancement
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-05-08 13:25 UTC by Jimmy Ohn
Modified: 2016-03-24 13:04 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
codec-utils: add a gst_codec_utils_aac_get_sample_rate (3.36 KB, patch)
2015-05-08 13:45 UTC, Jimmy Ohn
none Details | Review
codec-utils: add a gst_codec_utils_aac_get_sample_rate (3.44 KB, patch)
2015-05-27 18:57 UTC, Jimmy Ohn
none Details | Review
codec-utils: add a gst_codec_utils_aac_get_sample_rate (3.47 KB, patch)
2015-05-28 14:06 UTC, Jimmy Ohn
rejected Details | Review

Description Jimmy Ohn 2015-05-08 13:25:01 UTC
To remove the FIXME in the qtdemux, I add this function.
Comment 1 Jimmy Ohn 2015-05-08 13:45:32 UTC
Created attachment 303078 [details] [review]
codec-utils: add a gst_codec_utils_aac_get_sample_rate
Comment 2 Tim-Philipp Müller 2015-05-26 17:43:21 UTC
Comment on attachment 303078 [details] [review]
codec-utils: add a gst_codec_utils_aac_get_sample_rate

I'm not sure yet if we really need this new function, but some review comments nevertheless:


>+ * Returns the sample rate of the given AAC stream.
>+ *
>+ * Returns: The sample rate if rate_index is valid, 0 otherwise.
>+ */

Copy'n'paste from the index-based function. Should also have a Since: 1.6 marker at the end.


>+guint
>+gst_codec_utils_aac_get_sample_rate (const guint8 * audio_config)

Should take a size/length argument like the other functions (and check it).

>+  rate = gst_codec_utils_aac_get_sample_rate_from_index (rate_index);
>+  if (rate > 0)
>+    return rate;
>+  else
>+    return 0;
>+}

Might just as well just write

  return gst_codec_utils_aac_get_sample_rate_from_index (..)

then :)
Comment 3 Jimmy Ohn 2015-05-27 18:56:34 UTC
(In reply to Tim-Philipp Müller from comment #2)
> Comment on attachment 303078 [details] [review] [review]
> codec-utils: add a gst_codec_utils_aac_get_sample_rate
> 
> I'm not sure yet if we really need this new function, but some review
> comments nevertheless:
> 
> 
> >+ * Returns the sample rate of the given AAC stream.
> >+ *
> >+ * Returns: The sample rate if rate_index is valid, 0 otherwise.
> >+ */
> 
> Copy'n'paste from the index-based function. Should also have a Since: 1.6
> marker at the end.
> 
> 
> >+guint
> >+gst_codec_utils_aac_get_sample_rate (const guint8 * audio_config)
> 
> Should take a size/length argument like the other functions (and check it).
> 
> >+  rate = gst_codec_utils_aac_get_sample_rate_from_index (rate_index);
> >+  if (rate > 0)
> >+    return rate;
> >+  else
> >+    return 0;
> >+}
> 
> Might just as well just write
> 
>   return gst_codec_utils_aac_get_sample_rate_from_index (..)
> 
> then :)

It's kind of you to help me. It's clear:)
Comment 4 Jimmy Ohn 2015-05-27 18:57:07 UTC
Created attachment 304100 [details] [review]
codec-utils: add a gst_codec_utils_aac_get_sample_rate
Comment 5 Jimmy Ohn 2015-05-27 22:57:43 UTC
:( I forgot the since 1.6 marker I'll update new patch
Comment 6 Jimmy Ohn 2015-05-28 14:06:14 UTC
Created attachment 304165 [details] [review]
codec-utils: add a gst_codec_utils_aac_get_sample_rate
Comment 7 Sebastian Dröge (slomo) 2016-03-21 08:53:57 UTC
Review of attachment 304165 [details] [review]:

Please merge this with the patch that gets the channel count.

::: gst-libs/gst/pbutils/codec-utils.c
@@ +118,3 @@
+ * Returns: The sample rate if sr_idx is valid, 0 otherwise.
+ *
+ * Since 1.6

We're at 1.10 now
Comment 8 Jimmy Ohn 2016-03-22 02:43:32 UTC
I've merge patches at below link.
https://bugzilla.gnome.org/show_bug.cgi?id=749110