Re: [FFmpeg-devel] Rename RSHIFT macro to ROUNDED_RSHIFT

2019-01-22 Thread myp...@gmail.com
On Wed, Jan 23, 2019 at 3:31 AM Uoti Urpala  wrote:
>
> On Tue, 2019-01-22 at 15:35 -0300, James Almer wrote:
> > I'm not against renaming it to AV_ROUNDED_RSHIFT or similar, but other
> > than an entry in APIChanges we have no way to let library users that
> > RSHIFT will be removed two or so years from now.
>
> How about:
> static inline void  __attribute__ ((deprecated)) RSHIFT_is_deprecated(void) {}
>
> #define RSHIFT(a, b) (RSHIFT_is_deprecated(), AV_ROUNDED_SHIFT(a, b))
>
I  like this idear
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] Rename RSHIFT macro to ROUNDED_RSHIFT

2019-01-22 Thread Uoti Urpala
On Tue, 2019-01-22 at 15:35 -0300, James Almer wrote:
> I'm not against renaming it to AV_ROUNDED_RSHIFT or similar, but other
> than an entry in APIChanges we have no way to let library users that
> RSHIFT will be removed two or so years from now.

How about:
static inline void  __attribute__ ((deprecated)) RSHIFT_is_deprecated(void) {}

#define RSHIFT(a, b) (RSHIFT_is_deprecated(), AV_ROUNDED_SHIFT(a, b))


___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] Rename RSHIFT macro to ROUNDED_RSHIFT

2019-01-22 Thread James Almer
On 1/22/2019 2:17 PM, Vittorio Giovara wrote:
> On Mon, Jan 21, 2019 at 2:15 PM FeRD  wrote:
> 
>> On Mon, Jan 21, 2019 at 1:55 PM Moritz Barsnick  wrote:
>>
>>> On Mon, Jan 21, 2019 at 12:38:58 -0500, FeRD (Frank Dana) wrote:
>>>
 After applying both patches, 'make fate' succeeds and ffmpeg is still
 functional.
>>>
>>> You're not allowed to break fate (or compilation). So the two pathes
>>> need to be merged.
>>
>>
>> Aha, thanks. I'll resubmit squashed into a single patch.
>>
> 
> maybe it would be a good opportunity to expose the symbol publicly, and
> prefix it with AV_ instead of FF_, like it was done for AV_CEIL_RSHIFT
> (21f946840260da150affd9a20cc35b3d56194ba6)

That's not a good example seeing FF_CEIL_RSHIFT is still defined for
backwards compatibility. The idea here is to remove RSHIFT altogether.

I'm not against renaming it to AV_ROUNDED_RSHIFT or similar, but other
than an entry in APIChanges we have no way to let library users that
RSHIFT will be removed two or so years from now.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] Rename RSHIFT macro to ROUNDED_RSHIFT

2019-01-22 Thread Vittorio Giovara
On Mon, Jan 21, 2019 at 2:15 PM FeRD  wrote:

> On Mon, Jan 21, 2019 at 1:55 PM Moritz Barsnick  wrote:
>
> > On Mon, Jan 21, 2019 at 12:38:58 -0500, FeRD (Frank Dana) wrote:
> >
> > > After applying both patches, 'make fate' succeeds and ffmpeg is still
> > > functional.
> >
> > You're not allowed to break fate (or compilation). So the two pathes
> > need to be merged.
>
>
> Aha, thanks. I'll resubmit squashed into a single patch.
>

maybe it would be a good opportunity to expose the symbol publicly, and
prefix it with AV_ instead of FF_, like it was done for AV_CEIL_RSHIFT
(21f946840260da150affd9a20cc35b3d56194ba6)
-- 
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] Rename RSHIFT macro to ROUNDED_RSHIFT

2019-01-21 Thread FeRD
On Mon, Jan 21, 2019 at 1:55 PM Moritz Barsnick  wrote:

> On Mon, Jan 21, 2019 at 12:38:58 -0500, FeRD (Frank Dana) wrote:
>
> > After applying both patches, 'make fate' succeeds and ffmpeg is still
> > functional.
>
> You're not allowed to break fate (or compilation). So the two pathes
> need to be merged.


Aha, thanks. I'll resubmit squashed into a single patch.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] Rename RSHIFT macro to ROUNDED_RSHIFT

2019-01-21 Thread Moritz Barsnick
On Mon, Jan 21, 2019 at 12:38:58 -0500, FeRD (Frank Dana) wrote:

> After applying both patches, 'make fate' succeeds and ffmpeg is still
> functional.

You're not allowed to break fate (or compilation). So the two pathes
need to be merged.

If, OTOH, the libraries are to be considered independent, you would
first introduce the new macro alongside the old one, change all uses,
then drop the old macro in a third commit.

Moritz
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] Rename RSHIFT macro to ROUNDED_RSHIFT

2019-01-21 Thread FeRD (Frank Dana)
Patches to follow:

[PATCH 1/2] avutil: Rename RSHIFT macro to ROUNDED_RSHIFT
[PATCH 2/2] avcodec: Change uses of RSHIFT to ROUNDED_RSHIFT

This is my first patch submission to ffmpeg (and at a tense
moment, it seems, but soldiering on...), please bear with me.

The RSHIFT macro in libavutil/common.h does not actually perform
a bitwise right-shift, but rather a rounded version of the same
operation, as is noted by a comment above the macro. The rounded
divsion macro on the very next line is named ROUNDED_DIV, which
seems far more clear.

So, the first of these two patches renames RSHIFT to ROUNDED_RSHIFT
for clarity. The second updates all uses of the macro which are
internal to the ffmpeg source tree (which occur in only three
codecs under libavcodec/). After applying both patches, 'make fate'
succeeds and ffmpeg is still functional.

An example of the name causing issues (due to a conflict with
the RSHIFT macro in the Ruby source, which does perform a
standard bitwise right-shift) can be found at [1].

[1]: https://github.com/OpenShot/libopenshot/issues/164

Signed-off-by: FeRD (Frank Dana) 

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel