GNOME Bugzilla – Bug 767505
audiovisualizer: produces wrong timestamps with non-16 bit audio formats
Last modified: 2016-06-11 08:50:26 UTC
Created attachment 329581 [details] [review]
Patch that should fix the problem
gstaudiovisualizer computes a number of bytes from a number of samples like this:
sbpf = scope->req_spf * channels * sizeof (gint16);
This fails for obvious reasons if the actual audio data isn't 2 bytes per sample, fortunately GstAudioInfo has exactly the information we need to do this calculation correctly.
sbpf = scope->req_spf * GST_AUDIO_INFO_BPF (&scope->ainfo);
This issue is present in all the 1.6 visualizer's too but it didn't matter because the concrete classes that used it all only took 16 bit audio.
Patch attached, but because my machine doesn't have GStreamer 1.8 and I don't really want to go through setting up a sandbox for this I haven't tested it.
Review of attachment 329581 [details] [review]:
Seems mostly correct, but can you update the patch so it actually compiles? Thanks :)
@@ +1060,3 @@
rate = GST_AUDIO_INFO_RATE (&scope->ainfo);
bps = GST_AUDIO_INFO_BPS (&scope->ainfo);
+ bytes_per_samp = GST_AUDIO_INFO_BPF (&scope->ainfo);
Let's call it bpf maybe? :)
@@ +1073,2 @@
/* this is what we want */
+ sbpf = scope->req_spf bytes_per_samp;
This doesn't compile, you're missing the *
Created attachment 329600 [details] [review]
OK, this one should work
Created attachment 329601 [details] [review]
Change the variable name
Missed your comment on the variable name before, I skimmed a bit. Here's a version with the suggested name
Created attachment 329602 [details] [review]
REALLY change the variable name
Ooops forgot to commit before running git format-patch, here's one that' actually right
Author: Thomas Jones <email@example.com>
Date: Fri Jun 10 22:36:32 2016 -0400
audiovisualizer: Fix calculations for bytes<->samples conversions
Use bpf instead of channels * sizeof(gint16).
Please try to make sure your patches compile :) I fixed up the one for bug #767505 and the one for bug #767507 to compile. bps and channels was unused now, which caused compiler warnings, and a failure because -Werror.