[FFmpeg-devel] [PATCH] Implement hdcd filtering

[Rostislav Pehlivanov]

On 22 March 2016 at 11:22, Benjamin St <benjaminst123 at gmail.com> wrote:

> This patch applies filtering/decoding for hdcds(see ticket #4441) . The
> filter is heavily based on
> https://github.com/kode54/foo_hdcd/. (Is this ok? Copyright?)
>
> Discuss, Review
>
> Thank you, Benjamin
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>
Some style issues:

>+static int hdcd_scan(hdcd_state_t *state, int const *samples, int max,
int stride) {
In FFmpeg we always put the opening function bracket on a new line.

>+#define GAIN_APPLY(sample, gain) (sample) *= (gain)
This is getting into #define INCREMENT(x) (x++) territory, could you remove
it and just use sample *= gain everywhere?

>+static const uint32_t peaktab[9856] =
>+static uint8_t readaheadtab[1 << 8] = {
>+static float gaintab[(15 << 7) + 1] = {
>++static float gaintabup[(15 << 7) + 1] = {
No need to specify exactly how many entries the array has when you define
it, just leave the brackets empty []. It doesn't matter that much, but it
makes it easier to extend the array later on if you need to.

>out = ff_get_audio_buffer(outlink, in->nb_samples);;
Duplicated ;;

>+static void hdcd_process(hdcd_state_t *state, int *samples, int count,
>+        int stride) {
Not sure if the attachmed is messed up, but align the argument on the
newline to be after the bracket like:
>+static void hdcd_process(hdcd_state_t *state, int *samples, int count,
>+                                        int stride) {

Also, consider dropping the entire CHANNEL_NUM define, HDCDs will always
carry a stereo signal, so there's never going to be a need to change
CHANNEL_NUM.