[FFmpeg-devel] [PATCH] mpeg12enc: Use all Closed Captions side data

[Hendrik Leppkes]

On Mon, May 20, 2019 at 9:44 AM Reimar Döffinger
<Reimar.Doeffinger at gmx.de> wrote:
>
>
>
> On 20.05.2019, at 09:23, Mathieu Duponchelle <mathieu at centricular.com> wrote:
>
> > Thanks :)
> >
> > On 5/19/19 7:00 PM, Devin Heitmueller wrote:
> >> Hello Paul,
> >>
> >> On Fri, May 17, 2019 at 4:44 AM Paul B Mahol <onemda at gmail.com> wrote:
> >>> On 5/17/19, Mathieu Duponchelle <mathieu at centricular.com> wrote:
> >>>> There isn't one, as I said the added indentation is because of the new loop!
> >>> To get this committed to tree you need to comply to review requests.
> >> I think Mathieu's point is that the code indentation change was not
> >> cosmetic - it's because the code in question is now inside a for loop,
> >> and thus it needed to be indented another level.
> >>
> >> Are you suggesting he should make a patch which results in the
> >> indentation being wrong, and then submit a second patch which fixes
> >> the incorrect indentation introduced by the first patch?
>
> I think it should probably be up to the maintainer, but possibly yes.
> A lot of review still happens primarily by email, and if you re-indent code
> it becomes impossible to see what, if anything, you changed in that block.
> Enabling reviews of the patch via email means not re-indenting even if
> it becomes "wrong".
> Not everyone does reviews that way though, so some maintainers nowadays might prefer it differently?

My main concern with such requests is that its added effort to even
make these patches, since many developers will just automatically
indent where appropriate (as it should be), or even have tooling to do
it for them.
Personally, I would have to use another editor to "un-indent" the
code, since my main editor automatically corrects this stuff for me in
newly written code. As such, I'll personally never comply with such
requests, because I think its silly, and it would be extraordinary
effort to even do it, first restoring original indent with a second
editor.

In so many cases the "cosmetic" second commit was also quite simply
forgotten, resulting in an ugly codebase. Nevermind the added noise in
the commit log.

I don't think its that much to ask for a reviewer to also use a tool
that can ignore whitespace. Maybe we can teach patchwork to offer a
button to view diffs without whitespace, if people don't want to use a
simple tool.

- Hendrik