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 454121 - bsewavetool command for listing per chunk infos
bsewavetool command for listing per chunk infos
Status: RESOLVED FIXED
Product: beast
Classification: Other
Component: tools
SVN trunk
Other Linux
: High enhancement
: m0.7
Assigned To: Beast Maintainers
Beast Maintainers
Depends on:
Blocks:
 
 
Reported: 2007-07-05 23:34 UTC by Stefan Westerfeld
Modified: 2007-10-13 20:53 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Proposed implementation: info command (9.51 KB, patch)
2007-07-06 00:06 UTC, Stefan Westerfeld
rejected Details | Review
new info command implementation (21.76 KB, patch)
2007-07-12 23:02 UTC, Stefan Westerfeld
none Details | Review
improved info implementation (19.43 KB, patch)
2007-08-24 19:08 UTC, Stefan Westerfeld
none Details | Review

Description Stefan Westerfeld 2007-07-05 23:34:27 UTC
The intention of this enhancement is to be able to query chunk specific information of a bsewave file, either in a human readable way, or in a script readable way.

Script readable output should allow for instance a script to perform specific operations, such as adding volume adjustment or downsampling, only under certain conditions.

Human readable output should allow humans to get an overview over the chunks in a bsewave file, without having to resort to a text editor.

The original proposal from the TODO suggests

   --info=all # human readable
   --info=mix-freq # script readable

as options.
Comment 1 Stefan Westerfeld 2007-07-06 00:06:59 UTC
Created attachment 91284 [details] [review]
Proposed implementation: info command

Remarks:
 - there is no --show=midi-note, since there is not always a midi note; show is for information that is always there; midi notes need to be queried by --xinfo=midi-note
 - blank lines have been choosen as "not there" for xinfos, this allows deterministic parsing in shell scripts like this

bsewavetool info foo.bsewave --show=osc-freq --xinfo=volume --xinfo=midi-note | while read osc_freq
do
  read volume
  read midi_note
  # do something (volume and midi_note may or may not be set)
  echo "chunk: osc-freq=$osc_freq volume=$volume midi_note=$midi_note"
done

Some human readable sample output listing three chunks of a drumkit:

stefan@lotrien:/usr/local/src/beast/tools$ bsewavetool info ../r.bsewave -m42 -m43 -m44
LOAD: ../r.bsewave
EXEC: info

Chunk
  Osc Freq      92.50 Hz
  Mix Freq   48000.00 Hz
  MIDI Note        42     (Fis-1)
  Samples       68219     (1.42 s)
  Stored as  ogg data

Chunk
  Osc Freq      98.00 Hz
  Mix Freq   48000.00 Hz
  MIDI Note        43     (G-1)
  Samples       68219     (1.42 s)
  Stored as  ogg data

Chunk
  Osc Freq     103.83 Hz
  Mix Freq   48000.00 Hz
  MIDI Note        44     (Gis-1)
  Samples       68219     (1.42 s)
  Stored as  ogg data

SAVE: ../r.bsewave

Things that might be worth adding:
 - per wave information (such as the number of channels) is not covered by that patch
 - xinfos that are not known to the "info" command are not shown as part of the human readable output, nor is it possible to iterate over all xinfos of a chunk with the shell
Comment 2 Stefan Westerfeld 2007-07-07 15:39:06 UTC
Another thing that might be worth adding is:
 - show loop information in human readable form
Comment 3 Tim Janik 2007-07-09 23:12:47 UTC
awaiting new patch from Stefan that uses a single read staement for multiple infos, has --script option etc, after phone call 2007-07-08.
Comment 4 Stefan Westerfeld 2007-07-12 23:02:00 UTC
Created attachment 91703 [details] [review]
new info command implementation

I won't try to summarize the contents of the phone call here. But I think I implemented it all. Here is a new script example output:

stefan@lotrien:/usr/local/src/beast/tools$ bsewavetool info ../cr8000_kit.bsewave --script --show=chunk-key --show=osc-freq --show=midi-note --show=length --compute=avg-energy --compute=avg-energy-raw --show=label --show=blurb
LOAD: ../cr8000_kit.bsewave
EXEC: info
tjkgGMJ 65.406 36 11662 -14.910 -14.910 \  \
uDXeN6L 69.296 37 3276 -24.110 -24.110 \  \
DozIFf6 73.416 38 5922 -15.551 -15.551 \  \
5RJistS 77.782 39 18513 -31.361 -20.903 \  \
BEqhdkK 92.499 42 6806 -28.788 -20.152 \  \
o1KoLh7 116.541 46 19874 -27.785 -17.888 \  \
szgfjWM 123.471 47 22970 -23.076 -16.140 \  \
LilgBiL 146.832 50 24738 -23.292 -16.356 \  \
6roEhSa 155.564 51 35725 -31.320 -23.785 \  \
a0dZplX 207.652 56 20643 -27.901 -22.221 \  \
wp5prN5 311.127 63 11216 -17.027 -13.418 \  \
b9LORuA 329.628 64 19225 -18.532 -14.923 M64\ Sample Recorded\ at\ Noon
WGqikRS 622.254 75 1438 -19.199 -11.664 \  \
SAVE: ../cr8000_kit.bsewave

Note that only one of the chunks has a label and blurb. Here is the human readable output for the same file.

stefan@lotrien:/usr/local/src/beast/tools$ bsewavetool info ../cr8000_kit.bsewave --pretty=full -m63 -m64
LOAD: ../cr8000_kit.bsewave
EXEC: info

Wave
  Label             cr8000_kit
  Comment           CR8000 Drum Kit created out of entierly free samples
  Channels          1

Chunk 'wp5prN5'
  Osc Freq     311.13 Hz
  Mix Freq   44100.00 Hz
  MIDI Note        63     (Dis+1)
  Samples       11216     (0.25 s)
  Volume        66.00%    (-3.61 dB)
  Stored as  raw data
  Avg Energy   -17.03 dB  (-13.42 dB before volume adjustment)

Chunk 'b9LORuA'
  Label             M64 Sample
  Comment           Recorded at Noon
  Osc Freq     329.63 Hz
  Mix Freq   44100.00 Hz
  MIDI Note        64     (E+1)
  Samples       19225     (0.44 s)
  Volume        66.00%    (-3.61 dB)
  Stored as  raw data
  Avg Energy   -18.53 dB  (-14.92 dB before volume adjustment)

SAVE: ../cr8000_kit.bsewave

Remarks:
* I added a FIXME when I found that unrelated code was using the wrong data type to iterate over a data handle

* We could get the argument format more compact, if we treated --show and --compute arguments just as field names. Then the field list could be rewritten:

bsewavetool info ...
  --script --show=chunk-key --show=osc-freq --show=midi-note --show=length --compute=avg-energy --compute=avg-energy-raw --show=label --show=blurb

would become

bsewavetool info ...
  --script=chunk-key,osc-freq,midi-note,length,avg-energy,avg-energy-raw,label,blurb

which looks a lot better to me. Note that you cannot select fields in the human output anyway, and actually I don't see the need for that.

* With the patch, info now can be used to replace list-chunks, as info --script --show=chunk-key is equivalent to list-chunks. I'd still keep list-chunks for the sake of readability.

* Note that the documentation and bsewavetool assumes that the wave xinfo name will be changed to label, which was not done yet.
Comment 5 Tim Janik 2007-08-15 00:31:07 UTC
Comment on attachment 91703 [details] [review]
new info command implementation

just skimming over the usage parts of the patch:

>+    g_print ("      The following fields are valid for both, the wave and chunks:\n");
>+    g_print ("        channels        number of channels of a chunk/wave\n");
>+    g_print ("        label           label of a chunk/wave (when available)\n");
>+    g_print ("        blurb           comment associated with a chunk/wave (when available)\n");

"when" has time-conotation, "if" or "where" is more appropriate.

>+    g_print ("      The following fields can only be shown for chunks:\n");
>+    g_print ("        osc-freq        frequency of the chunk\n");
>+    g_print ("        mix-freq        mix freq (sampling rate) of the chunk\n");
>+    g_print ("        midi-note       midi note of a chunk (when available)\n");
>+    g_print ("        length          length of the chunk in samples\n");
>+    g_print ("        volume          volume at which the chunk is to be played\n");
>+    g_print ("        format          storage format used to save the chunk data\n");
>+    g_print ("        loop-type       whether the chunk is to be looped\n");
>+    g_print ("        loop-start      offset in samples for the start of the loop\n");
>+    g_print ("        loop-end        offset in samples for the end of the loop\n");
>+    g_print ("        loop-count      maximum limit for how often the loop should be repeated\n");
>+    g_print ("    The following chunk features can be computed and shown:\n");
>+    g_print ("      avg-energy-raw    average signal energy (dB) of the raw data of the chunk\n");
>+    g_print ("      avg-energy        average signal energy (dB) using volume xinfo\n");

i think the above indentation levels is really stretching it for --help on --show=<fieldname>. this would be good enough:
Show wave or chunk fields:
  [...]
Show wave fields:
  [...]
Show chunk fields:
  [...]

allthough, i think --field=<fieldname> would be slightly better namer for this option.

the reason to have a parameterized argument syntax --show=* in the first place is that * corresponds to arbitrary field types contained in a bsewave file. the same cannot be said for the computed features, we always know what features can be computed. also, the user doesn't really care that they are "computed", "extracted", "calculated", "crunched", "averaged", "table-looked-up", "decrypted", whatever...
it'd be much easier to just offer:
  --avg-energy-raw
  --avg-energy

> -  uint i;
> +  uint i; // FIXME

this is very useless. a "FIXME" should *never* be added without explanation, it makes the code look like there'd be something wrong with "uint i;" in itself. (if you think the data type is wrong here, the missing comment fails to state what would be correct instead; share your wisdom if it is substantial in any way ;)
Comment 6 Stefan Westerfeld 2007-08-24 19:08:49 UTC
Created attachment 94282 [details] [review]
improved info implementation

Changes:
- fixed the uint i; FIXME
- changed script usage - an example for the new syntax would be

stefan@lotrien:/usr/local/src/beast/tools$ bsewavetool info ../cr8000_kit.bsewave --script=chunk-key,osc-freq,midi-note,length,+avg-energy,+avg-energy-raw,label,blurb
LOAD: ../cr8000_kit.bsewave
EXEC: info
tjkgGMJ 65.406 36 11662 -14.910 -14.910 \  \
uDXeN6L 69.296 37 3276 -24.110 -24.110 \  \
DozIFf6 73.416 38 5922 -15.551 -15.551 \  \
5RJistS 77.782 39 18513 -31.361 -20.903 \  \
BEqhdkK 92.499 42 6806 -28.788 -20.152 \  \
o1KoLh7 116.541 46 19874 -27.785 -17.888 \  \
szgfjWM 123.471 47 22970 -23.076 -16.140 \  \
LilgBiL 146.832 50 24738 -23.292 -16.356 \  \
6roEhSa 155.564 51 35725 -31.320 -23.785 \  \
a0dZplX 207.652 56 20643 -27.901 -22.221 \  \
wp5prN5 311.127 63 11216 -17.027 -13.418 \  \
b9LORuA 329.628 64 19225 -18.532 -14.923 M64\ Sample Recorded\ at\ Noon
WGqikRS 622.254 75 1438 -19.199 -11.664 \  \
SAVE: ../cr8000_kit.bsewave

- rewrote the documentation part related to info --script

Remarks:

- string_tokenize (or a superset thereof, with configurable field separators, like strtok) could be useful elsewhere, and therefore could be moved to rapicorn string utils
Comment 7 Krzysztof Foltman 2007-09-20 21:59:20 UTC
The only problem I see for now is that unknown options (like --flying-cows) are ignored instead of reported as invocation error.

Plus the FIXME, but that should probably be fixed in that place and then filed as a separate bug, potentially with very low priority (as it's not related to this particular bug, and the chance of having individual samples >4 GB is so low it might not even be supported).

Also, as loop points seem to be represented as SfiNum, how does that relate to 4GB limit?
Comment 8 Tim Janik 2007-10-13 20:47:18 UTC
(In reply to comment #7)
> The only problem I see for now is that unknown options (like --flying-cows) are
> ignored instead of reported as invocation error.

not sure where that applies, trying to pass it in to info yields:
** BSE-TOOLS-Error: 
** extra argument given to command "info": --flying-cows

> Plus the FIXME, but that should probably be fixed in that place and then filed
> as a separate bug, potentially with very low priority (as it's not related to
> this particular bug, and the chance of having individual samples >4 GB is so
> low it might not even be supported).

right, this should be treated/fixed as a seperate issue.

> Also, as loop points seem to be represented as SfiNum, how does that relate to
> 4GB limit?

sfi/sfitypes.h defines SfiNum as int64.
Comment 9 Tim Janik 2007-10-13 20:53:13 UTC
(In reply to comment #6)
> Created an attachment (id=94282) [edit]
> improved info implementation

thanks, patch applied. please take a look at the source code history for the
subsequent fixups and try to avoid them for future patches.

as for the actual behavior, considering that --pretty=medium is the shortes available output format (i.e. this is lacking the "oneline" and "short" variants that e.g. git-log has), it had to be more compact which a subsequent commit did. also, saving the bsewave after reading out info was quite bogus behavior and has been fixed.