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 728646 - siren: fix sample list rate
siren: fix sample list rate
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal normal
: 1.5.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
: 736958 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2014-04-21 10:53 UTC by Vincent Penquerc'h
Modified: 2014-10-30 15:22 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
fix sample rate list (1.00 KB, patch)
2014-04-21 10:53 UTC, Vincent Penquerc'h
committed Details | Review

Description Vincent Penquerc'h 2014-04-21 10:53:55 UTC
Created attachment 274801 [details] [review]
fix sample rate list

Would need someone who knows the format to double check.

Spec found there:

https://www.itu.int/rec/dologin_pub.asp?lang=e&id=T-REC-G.722.1-200505-I!!SOFT-ZST-E&type=items

but not helpful about this. I might have the wrong spec though, since it appears there are a handful of different codecs named G.722 (one or more of which appears to be siren).
Comment 1 Olivier Crête 2014-04-22 17:05:57 UTC
We would need to compare with output from Microsoft's encoder. But your patch makes sense to me.
Comment 2 Hehehdh 2014-04-30 09:39:18 UTC
Review of attachment 274801 [details] [review]:

From: Vincent Penquerc'h <vincent.penquerch@collabora.co.uk>
Date: Mon, 21 Apr 2014 11:48:22 +0100
Subject: [PATCH] siren: fix sample rate list
 
It was using a 24000/24000/48000, but I think it meant to use
24000/32000/48000. Not 100% sure...
 
https://en.wikipedia.org/wiki/G.722.1 has the list of supported
bitrates. It's not clear whether the "flag" code maps to this,
however.
 
Coverity 206072
---
 gst/siren/common.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Comment 3 Felix Schwarz 2014-09-19 13:31:58 UTC
Looks good to me too (though I don't have any in-depth knowledge): 
- Wikipedia (https://en.wikipedia.org/wiki/Siren_%28codec%29#Editions) mentions 16, 24 and 32 kbit.

Looking at the code that might imply that flag=1 is siren 7, while flag=2 is siren 14.

number_of_regions=14 and number_of_coefs=320 for flag=1 matches the G.722 standard as far as I can see (http://www.itu.int/rec/dologin_pub.asp?lang=e&id=T-REC-G.722.1-200505-I!!SOFT-ZST-E&type=items).

G.722.1 Annex C uses also number_of_coefs=640 which indicates that hypothesis might be correct. If so I suggest you also update the comment in common.c ("Looks like the flag means what kind of encoding is used").
Comment 4 Sanjay NM 2014-09-22 09:30:04 UTC
*** Bug 736958 has been marked as a duplicate of this bug. ***
Comment 5 Vincent Penquerc'h 2014-10-30 15:22:34 UTC
This seems almost 100% certain to be correct now, so I pushed it.

commit a5350f2d0ce0a59fb21afaaef5e519f2a3b93aa0
Author: Vincent Penquerc'h <vincent.penquerch@collabora.co.uk>
Date:   Mon Apr 21 11:48:22 2014 +0100

    siren: fix sample rate list
    
    It was using a 24000/24000/48000, but I think it meant to use
    24000/32000/48000. Not 100% sure...
    
    https://en.wikipedia.org/wiki/G.722.1 has the list of supported
    bitrates. It's not clear whether the "flag" code maps to this,
    however.
    
    Coverity 206072