[FFmpeg-devel] [PATCH] AAC Encoder: clipping avoidance

[Rostislav Pehlivanov]

>I don't like it to be done automatically since it could (and in
>general would) modify parts of the code I didn't touch should they
>contain whitespace.
I haven't seen any trailing spaces in any ffmpeg code. In the worst case
scenario you remove one existing trailing space, you see it in git diff and
you leave it in the patch and maybe mention it if it's 2 or 3 times.
If you want to do it manually consider having any trailing spaces
highlighted, most editors already do that by default.

>AFAIR, it's not possible to select different scalefactors for the
>different windows in the window group, and a lot of code assumes that
>only the first window has the proper scalefactor set.
Yeah, I though so. It's rare to see (w+w2)*16.

You did forget to do it in encode_window_bands_info() though, the code
still reads
>sce->sf_idx[(win+w)*16+swb]
You should change it there too.


On 18 July 2015 at 00:19, Claudio Freire <klaussfreire at gmail.com> wrote:

> On Fri, Jul 17, 2015 at 7:09 PM, Rostislav Pehlivanov
> <atomnuker at gmail.com> wrote:
> >>attachment.bin:357: trailing whitespace.
> >>attachment.bin:436: trailing whitespace.
> >>attachment.bin:454: trailing whitespace.
>
> Oops
>
> > You should probably configure your editor to strip that off
> automatically.
>
> I don't like it to be done automatically since it could (and in
> general would) modify parts of the code I didn't touch should they
> contain whitespace.
>
> I prefer doing it manually. I guess those slipped past me. I thought I
> had taken care of patchcheck issues already.
>
> >
> >>sce->sf_idx[win*16+swb],
> > Why the change from (win+w)*2? Shouldn't you remove the entire loop
> around
> > w because now nothing depends on it?
>
> AFAIR, it's not possible to select different scalefactors for the
> different windows in the window group, and a lot of code assumes that
> only the first window has the proper scalefactor set.
>
> I'm not sure, but I'd rather err on the safe side. Indexing on sf_idx
> should always index the first group to avoid inconsistencies.
>
> The loop however isn't rendered pointless by that change, there's
> still the scoefs pointer being advanced by w (and that's the whole
> point of the loop in fact).
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>