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

[Peter Ross]

On Sat, Jan 26, 2019 at 02:46:59PM -0500, FeRD wrote:
> 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.)

only recently came into contact with this macro, as its used in vp56. the
name confused me initially me, and had to grep for it, and then read then
expansion.

i support renaming it to something more clear (ROUNDED_RSHIFT) and adding the
FF_ or AV_ namespace prefix.

-- Peter
(A907 E02F A6E5 0CD2 34CD 20D2 6760 79C5 AC40 DD6B)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20190127/0ae03544/attachment.sig>