[Kdenlive-devel] Could you PLEASE document your code.

Tue Dec 14 14:02:30 UTC 2010

Hi Marco,

I think of interest would be the average and the maximum value; the
max to detect clipping, and the average to have an average. And a dB
scale; This can be calculated with
20*log10(signal)
after normalizing it to [-1,1].

Simon

2010/11/21 Marco Gittler <marco at gitma.de>:
> Hi
>
> yes it is correct, that it show only 200 samples.
> num_samples can be set simple to "samples", but this takes much more cpu usage.
> the correct  way would be to read all samples and take the average (or max ? ).
> Since we can only show 1 level for each frame,i used first the first samples.
>
> The best way would be:
> copy all samples to an QByteArray for each channel and emit this.
> Each audio frame consumer can the decide what to do with the samples.
> But i had not the time last  to change this.
>
> If you want to help changing this, you'r welcome.
>
> regards marcco
>
> Am 21.11.2010 um 13:26 schrieb Simon Eugster:
>
>> I wanted to continue working on my audio spectrum scope. So I made use of
>> void Render::showAudioSignal(const QByteArray);
>> until I found out that this wasn't delivering the audio signal at all
>> because I only received two values per frame.
>>
>> So I searched for the position where this signal was emitted:
>> void Render::showAudio(Mlt::Frame& frame)
>> {
>> if (!frame.is_valid() || frame.get_int("test_audio") != 0) return;
>> mlt_audio_format audio_format = mlt_audio_s16;
>> int freq = 0;
>> int num_channels = 0;
>> int samples = 0;
>> int16_t* data = (int16_t*)frame.get_audio(audio_format, freq,
>> num_channels, samples);
>> if (!data)
>>   return;
>> int num_samples = samples > 200 ? 200 : samples;
>> QByteArray channels;
>> for (int i = 0; i < num_channels; i++) {
>>   long val = 0;
>>   for (int s = 0; s < num_samples; s ++) {
>>       val += abs(data[i+s*num_channels] / 128);
>>   }
>>   channels.append(val / num_samples);
>> }
>> if (samples > 0)
>>   emit showAudioSignal(channels);
>> else
>>   emit showAudioSignal(QByteArray());
>> }
>> After skimming the entire function's code I found out that this only
>> shows the mean of the (at most) first 200 audio sample points.
>> Which I doubt is not correct anyway for a volume meter.
>>
>>
>>
>>
>>
>> So –
>> . Could you, please, document the code you write.
>>
>> Some parts look like «if it was hard to write, then it should be hard
>> to read». This is okay for me if it is your own code which is not made
>> public. But this is a collaborative Open Source project. It DOES
>> occasionally happen that others have to take a look at your code,
>> understand it, use it, fix bugs, extend it. Reading undocumented code
>> sucks. If you don't know what methods are good for and why, you have
>> to read everything, look out for possible side effects which might
>> crash everything, and so on.
>>
>> Example: Understand this code within 2 minutes:
>> http://git.gnome.org/browse/gimp/tree/app/base/gimphistogram.c
>>
>> And it is not unlikely that even _you_ will eventually have to
>> re-visit your own code after some weeks/months/years. As soon as it
>> reaches a certain complexity I'm usually very glad when I can just
>> read some comments to understand the workflow again because I usually
>> don't remember everything in detail.
>>
>> By documenting I don't mean:
>>       // Add the current value, divided by 128
>>       val += abs(data[i+s*num_channels] / 128);
>> i.e. writing what a command does. This is overhead and needs to be
>> maintained in parallel to the code (i.e. when 128 changes to 256 –
>> What would make sense here is a line explaining why we divide by 128
>> and not by Pi or by 42.)
>> But consider this:
>> // Calculate the mean of the first 200 samples to approximate the
>> audio volume
>> int num_samples = samples > 200 ? 200 : samples;
>> {...}
>> This explains WHAT a code section does and WHY. I can read one line
>> and immediately know how a whole code section works.
>>
>> In my experience, documenting code this way adds to a deeper
>> understanding of the code for both parties (author and reader), and I
>> occasionally even found some bugs simply by again having to think
>> about what the code does (and hereby sparing some time wasting on
>> debugging).
>> Lots of clever people have been writing about source code comments.
>> (One good book I can recommend is «Code Quality» from Spinellis where
>> he talks about maintainability (which is the core idea for writing
>> comments; A maintainable piece of software can be
>> extended/bugfixed/etc much easier) as well.) They all agree that it is
>> a good thing if done well.
>> I also see it as an estimation opposite to other programmers.
>>
>>
>>
>>
>> Regarding the remark about the volume meter not being correct: Our
>> ears are not linear, and there is also a peak sensitivity around
>> 300-3000 Hz (Human voice). So I guess taking the mean is okay for an
>> approximation but is not very accurate.
>>
>>
>> Thanks for reading, happy Sunday.
>> Simon
>>
>> ------------------------------------------------------------------------------
>> Beautiful is writing same markup. Internet Explorer 9 supports
>> standards for HTML5, CSS3, SVG 1.1,  ECMAScript5, and DOM L2 & L3.
>> Spend less time writing and  rewriting code and more time creating great
>> experiences on the web. Be a part of the beta today
>> http://p.sf.net/sfu/msIE9-sfdev2dev
>> _______________________________________________
>> Kdenlive-devel mailing list
>> Kdenlive-devel at lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/kdenlive-devel
>
>
> ------------------------------------------------------------------------------
> Beautiful is writing same markup. Internet Explorer 9 supports
> standards for HTML5, CSS3, SVG 1.1,  ECMAScript5, and DOM L2 & L3.
> Spend less time writing and  rewriting code and more time creating great
> experiences on the web. Be a part of the beta today
> http://p.sf.net/sfu/msIE9-sfdev2dev
> _______________________________________________
> Kdenlive-devel mailing list
> Kdenlive-devel at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/kdenlive-devel
>
>