Re: [FFmpeg-devel] Rename RSHIFT macro to ROUNDED_RSHIFT
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
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
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
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
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
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
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