[FFmpeg-devel] [PATCH] avutil: Rename RSHIFT macro to ROUNDED_RSHIFT

[FeRD]

On Tue, Jan 22, 2019 at 8:15 AM Moritz Barsnick <barsnick at gmx.net> wrote:

> On Mon, Jan 21, 2019 at 21:32:46 -0500, FeRD wrote:
> > Well... not *purely* cosmetic. See [1] for an example of one issue.
> Ruby's
>
> > `ruby/config.h` header also defines an `RSHIFT` macro, with different
> > semantics (it doesn't round), so when building code which includes
> > both headers the macro ends up being redefined.
> [...]
> > [1]: https://github.com/OpenShot/libopenshot/issues/164
>
> While I agree with the assessment that ffmpeg's macro should have been
> named "FF_RSHIFT", "AV_RSHIFT" or even something more appropriate along
> your suggestion, you failed to post an upstream patch with ruby to
> rename their macro to "RUBY_RSHIFT". ;-)


Fair, but that could be taken as a compliment! Changing it in either source
tree is sufficient to solve the conflict, so perhaps the FFMpeg project
seemed like the venue that would produce less friction. ;-)

No, honestly, the main reason is that... well, Ruby's RSHIFT defines a
classic bitwise right-shift, whereas FFMpeg's rounds. So, in my very
opinionated opinion, Ruby's RSHIFT is at least CORRECT, whereas FFMpeg's is
just plain wrong. (Wrong to be called "RSHIFT", when it's really
ROUNDED_RSHIFT.) So if there's going to be any macro defining RSHIFT, I'd
prefer it at least be one that fits the name.

Should Ruby's be named RUBY_RSHIFT? Sure, it'd certainly make life easier.
Should FFMpeg's be named AV_ROUNDED_RSHIFT? Also sure. But (again in my
opinionated opinion) not AV_RSHIFT, because that's still *not* what it
does! (And especially since the FFMpeg codebase already has AV_CEIL_RSHIFT
which always rounds upwards, setting a pattern/precedent for the naming.)