[FFmpeg-devel] [PATCH] libavfilter/af_hdcd.c: Collect HDCD stats and report

[Burt P.]

Thanks. I've sent a new version of the patch in reply to Carl Eugen
Hoyos, and using git send-email this time, but I saw this message and
included most of the things you mentioned.
I'm still getting used to this.

Anyway, I am curious why both of you think UPDATE_INFO should not
remain a macro? It is only used in that one function, but
unfortunately twice, so I thought a macro would do well there.



On Sun, Jul 3, 2016 at 6:34 AM, Moritz Barsnick <barsnick at gmx.net> wrote:
> On Sat, Jul 02, 2016 at 23:53:20 -0500, Burt P. wrote:
>
>>  static int integrate(hdcd_state_t *state, int *flag, const int32_t
>> *samples, int count, int stride)
>
> Your mailer is breaking likes in the patch. You may need to attach it
> as a text file, or actually use git send-email.
>
>> +/* update the user info/flags */
>> +#define UPDATE_INFO(s,pe,tg,tf) do{if (pe || tg || tf || s->sustain)
>> { s->_hdcd_detected = 1; } if (pe) { s->_peak_extend = 1; } if (tf) {
>> s->_transient_filter = 1;} if (tg < s->_gain_min) { s->_gain_min=tg; }
>>  if (tg > s->_gain_max) { s->_gain_max=tg; } }while(0);
>
> Not more readable, but more compact (and possibly more ffmpeg style):
> s->_hdcd_detected = pe || tg || tf || s->sustain;
> s->_peak_extend = !!pe;
> s->_transient_filter = !!tf;
> [...]
>
>> +#define GAINTOFLOAT(g) ((float)(g>>8) + ((float)(g>>7 & 1) * 0.5))
>
> I can think of other ways to do this, but as it's not performace
> relevant, it shouldn't matter.
>
>> +        if (state->_gain_min < _gain_min) { _gain_min = state->_gain_min; }
>> +        if (state->_gain_max > _gain_max) { _gain_max = state->_gain_max; }
>
> ffmpeg style would probably be:
>         if (state->_gain_max > _gain_max)
>             _gain_max = state->_gain_max;
>
> but for readability, I would have said:
>         _gain_max = FFMIN(_gain_max, state->_gain_max);
> (Or does ffmpeg have an explicit macro for "limit this value"?).
>
> Same above in the UPDATE_INFO macro (-> function).
>
> Moritz
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel