Created attachment 161399 [details] [review]
patch

A lot (if not most) vinyl rips use 24 bits. At the moment the spectrum plugin can process only 16 and 32 bit streams.

Actually, prepending `spectrum` with an `audioconvert` element works too, so may be this fix is not really needed.

It's still nice to have though, especially if it's quite easy to implement, as seems to be the case here.

Agreed and it would also be nice if random depths are supported for all sample widths. This should be quite easy to add the same way (dividing samples by the maximum value). Do you want to extend the patch to do that? Otherwise I'll do it on top of your patch.

(Also 24 bit width would be nice to have :) )

(In reply to comment #4)
> Agreed and it would also be nice if random depths are supported for all sample
> widths. This should be quite easy to add the same way (dividing samples by the
> maximum value). Do you want to extend the patch to do that? Otherwise I'll do
> it on top of your patch.

You mean {8,16,24,32} bit depths for 32 bit width and {8,16} bit depths for 16 bit widths? Or all depths between 1 and the width?

Let me know, I will adapt the patch this weekend.

> (Also 24 bit width would be nice to have :) )

Just curious, is it possible to generate such a stream using standard plugins?

(In reply to comment #5)
> (In reply to comment #4)
> > Agreed and it would also be nice if random depths are supported for all sample
> > widths. This should be quite easy to add the same way (dividing samples by the
> > maximum value). Do you want to extend the patch to do that? Otherwise I'll do
> > it on top of your patch.
> 
> You mean {8,16,24,32} bit depths for 32 bit width and {8,16} bit depths for 16
> bit widths? Or all depths between 1 and the width?

All depths between 1 and width, i.e. [1,32] for width=32, [1,24] for width=24, etc.
 
> Let me know, I will adapt the patch this weekend.

Thanks :)

> > (Also 24 bit width would be nice to have :) )
> 
> Just curious, is it possible to generate such a stream using standard plugins?

Sure: audiotestsrc ! audioconvert ! "audio/x-raw-int,width=24" ! ...

Created attachment 161759 [details] [review]
Support arbitrary bit depth

The patch adds support for depths between 2 and the width. The depth cannot be 1 bit because we are using signed ints.

I will look into 24-bit widths later and will submit it as a separate patch.

Created attachment 161762 [details] [review]
fix a typo

Created attachment 161821 [details] [review]
Support 24-bit width

This should be applied on top of the previous patch. It also fixes a compile warning introduced by the previous patch.

Review of attachment 161762 [details] [review]:

Looks good in general, thanks

::: gst/spectrum/gstspectrum.c
@@ +118,3 @@
     "audio/x-raw-int, "                                               \
     " width = (int) 32, "                                             \
+    " depth = (int) [ 2, 32 ], "                                      \

1 bit depth is supported too, not that it's very useful but still... Maybe someone uses it for morse codes ;)

Review of attachment 161821 [details] [review]:

::: gst/spectrum/gstspectrum.c
@@ +576,3 @@
+        }
+        if (value & 0x800000)
+          value = -(value ^ 0xffffff) - 1;

Use something like this here, that's used in GstByteReader too for example. Your's works too of course but that's a bit larger ;)

#if G_BYTE_ORDER == G_BIG_ENDIAN
  value = GST_UTIL_READ_UINT24_BE (data);
#else
  value = GST_UTIL_READ_UINT24_LE (data);
#endif
  if (value & 0x00800000)
    val |= 0xff000000;

Created attachment 161858 [details] [review]
Now with Morse code support

Created attachment 161859 [details] [review]
Use macros from gstutils.h

Created attachment 161908 [details] [review]
fix a typo in the second path

Thanks, I'll push these patches after next gst-plugins-good release. Please base any new patches on top of these :)

commit 2d13b15376df91327f301f3225f404d7fb75bb9a
Author: Alexander Kojevnikov <alexander@kojevnikov.com>
Date:   Tue May 25 15:16:06 2010 +1000

    spectrum: support 24-bit width
    
    Fixes #619045

commit c69dd320af2ad8d1e5e97bf1c6a25c9a2a72071c
Author: Alexander Kojevnikov <alexander@kojevnikov.com>
Date:   Mon May 24 21:50:58 2010 +1000

    spectrum: support arbitrary bit depth
    
    Partially fixes #619045