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

[Mathieu Duponchelle]

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'm confident that Mathieu is trying to comply with review requests so
> he can get his code merged.  In this particular case, Carl raised his
> concern about the indentation, Mathieu responded by suggesting that
> given there was a functional change the re-indent was correct, and
> then there was silence (i.e. neither agreement nor disagreement).
>
> I'm also asking because I have work which I'm hoping to get upstreamed
> as well at some point, and I'm sure I've got similar things in my
> patches.  And having spent 20+ years reviewing patches on lots of
> other open source projects, I wouldn't have thought twice about
> accepting the patch as-is (given the change in indentation is a result
> of a functional change and is not cosmetic).  In my experience, this
> particular patch isn't an example of what maintainers mean when they
> say "don't mix cosmetic whitespace changes with functional changes".
>
> Regards,
>
> Devin
>