| Message ID | 20210224142242.911865-1-andreas.rheinhardt@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [FFmpeg-devel,1/2] avfilter/transform: Stop exporting internal functions | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | success | Make fate finished |
| andriy/PPC64_make | success | Make finished |
| andriy/PPC64_make_fate | success | Make fate finished |
lgtm
On 2/24/2021 11:22 AM, Andreas Rheinhardt wrote: > avfilter_transform, avfilter_(add|sub|mult)_matrix are not part of the > public API (transform.h is not a public header), yet they are currently > exported because of their name. This commit changes this: > avfilter_transform is renamed to ff_affine_transform; the other > functions are just removed as they have never been used at all. The symbols are exported and have been in four releases so far for this soname. If we plan on making a 4.4 release before the bump, it may be a good idea if we keep the symbols around for the sake of not affecting the ABI, so I'm inclined towards just wrapping them in a LIBAVFILTER_VERSION_MAJOR < 8 check like we do when we remove avpriv ones. > > Found-by: Anton Khirnov <anton@khirnov.net> > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> > --- > libavfilter/transform.c | 23 +---------------------- > libavfilter/transform.h | 29 +---------------------------- > libavfilter/vf_deshake.c | 5 +++-- > 3 files changed, 5 insertions(+), 52 deletions(-) > > diff --git a/libavfilter/transform.c b/libavfilter/transform.c > index f4f9e0a47d..1f91436f73 100644 > --- a/libavfilter/transform.c > +++ b/libavfilter/transform.c > @@ -122,28 +122,7 @@ void ff_get_matrix( > matrix[8] = 1; > } > > -void avfilter_add_matrix(const float *m1, const float *m2, float *result) > -{ > - int i; > - for (i = 0; i < 9; i++) > - result[i] = m1[i] + m2[i]; > -} > - > -void avfilter_sub_matrix(const float *m1, const float *m2, float *result) > -{ > - int i; > - for (i = 0; i < 9; i++) > - result[i] = m1[i] - m2[i]; > -} > - > -void avfilter_mul_matrix(const float *m1, float scalar, float *result) > -{ > - int i; > - for (i = 0; i < 9; i++) > - result[i] = m1[i] * scalar; > -} > - > -int avfilter_transform(const uint8_t *src, uint8_t *dst, > +int ff_affine_transform(const uint8_t *src, uint8_t *dst, > int src_stride, int dst_stride, > int width, int height, const float *matrix, > enum InterpolateMethod interpolate, > diff --git a/libavfilter/transform.h b/libavfilter/transform.h > index 9b0c19ceca..0344f9c228 100644 > --- a/libavfilter/transform.h > +++ b/libavfilter/transform.h > @@ -83,33 +83,6 @@ void ff_get_matrix( > float *matrix > ); > > -/** > - * Add two matrices together. result = m1 + m2. > - * > - * @param m1 9-item transformation matrix > - * @param m2 9-item transformation matrix > - * @param result 9-item transformation matrix > - */ > -void avfilter_add_matrix(const float *m1, const float *m2, float *result); > - > -/** > - * Subtract one matrix from another. result = m1 - m2. > - * > - * @param m1 9-item transformation matrix > - * @param m2 9-item transformation matrix > - * @param result 9-item transformation matrix > - */ > -void avfilter_sub_matrix(const float *m1, const float *m2, float *result); > - > -/** > - * Multiply a matrix by a scalar value. result = m1 * scalar. > - * > - * @param m1 9-item transformation matrix > - * @param scalar a number > - * @param result 9-item transformation matrix > - */ > -void avfilter_mul_matrix(const float *m1, float scalar, float *result); > - > /** > * Do an affine transformation with the given interpolation method. This > * multiplies each vector [x,y,1] by the matrix and then interpolates to > @@ -126,7 +99,7 @@ void avfilter_mul_matrix(const float *m1, float scalar, float *result); > * @param fill edge fill method > * @return negative on error > */ > -int avfilter_transform(const uint8_t *src, uint8_t *dst, > +int ff_affine_transform(const uint8_t *src, uint8_t *dst, > int src_stride, int dst_stride, > int width, int height, const float *matrix, > enum InterpolateMethod interpolate, > diff --git a/libavfilter/vf_deshake.c b/libavfilter/vf_deshake.c > index 28a541b94a..8771399351 100644 > --- a/libavfilter/vf_deshake.c > +++ b/libavfilter/vf_deshake.c > @@ -330,8 +330,9 @@ static int deshake_transform_c(AVFilterContext *ctx, > > for (i = 0; i < 3; i++) { > // Transform the luma and chroma planes > - ret = avfilter_transform(in->data[i], out->data[i], in->linesize[i], out->linesize[i], > - plane_w[i], plane_h[i], matrixs[i], interpolate, fill); > + ret = ff_affine_transform(in->data[i], out->data[i], in->linesize[i], > + out->linesize[i], plane_w[i], plane_h[i], > + matrixs[i], interpolate, fill); > if (ret < 0) > return ret; > } >
James Almer: > On 2/24/2021 11:22 AM, Andreas Rheinhardt wrote: >> avfilter_transform, avfilter_(add|sub|mult)_matrix are not part of the >> public API (transform.h is not a public header), yet they are currently >> exported because of their name. This commit changes this: >> avfilter_transform is renamed to ff_affine_transform; the other >> functions are just removed as they have never been used at all. > > The symbols are exported and have been in four releases so far for this > soname. If we plan on making a 4.4 release before the bump, it may be a > good idea if we keep the symbols around for the sake of not affecting > the ABI, so I'm inclined towards just wrapping them in a > LIBAVFILTER_VERSION_MAJOR < 8 check like we do when we remove avpriv ones. > And why? There was never a legitimate outside user of these functions. And removing avfilter_all_channel_layouts in d5e5c6862bbc834d5a036a3fa053a7569d84f928 (which also didn't have a legitimate outside user) didn't seem to break anything either. Btw: 88d80cb97528d52dac3178cf5393d6095eca6200 broke ABI for x64, because older versions of libavformat exchange a PutBitContext with libavcodec via avpriv_align_put_bits and avpriv_copy_bits. So we can't really make a 4.4 release. - Andreas >> >> Found-by: Anton Khirnov <anton@khirnov.net> >> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> >> --- >> libavfilter/transform.c | 23 +---------------------- >> libavfilter/transform.h | 29 +---------------------------- >> libavfilter/vf_deshake.c | 5 +++-- >> 3 files changed, 5 insertions(+), 52 deletions(-) >> >> diff --git a/libavfilter/transform.c b/libavfilter/transform.c >> index f4f9e0a47d..1f91436f73 100644 >> --- a/libavfilter/transform.c >> +++ b/libavfilter/transform.c >> @@ -122,28 +122,7 @@ void ff_get_matrix( >> matrix[8] = 1; >> } >> -void avfilter_add_matrix(const float *m1, const float *m2, float >> *result) >> -{ >> - int i; >> - for (i = 0; i < 9; i++) >> - result[i] = m1[i] + m2[i]; >> -} >> - >> -void avfilter_sub_matrix(const float *m1, const float *m2, float >> *result) >> -{ >> - int i; >> - for (i = 0; i < 9; i++) >> - result[i] = m1[i] - m2[i]; >> -} >> - >> -void avfilter_mul_matrix(const float *m1, float scalar, float *result) >> -{ >> - int i; >> - for (i = 0; i < 9; i++) >> - result[i] = m1[i] * scalar; >> -} >> - >> -int avfilter_transform(const uint8_t *src, uint8_t *dst, >> +int ff_affine_transform(const uint8_t *src, uint8_t *dst, >> int src_stride, int dst_stride, >> int width, int height, const float *matrix, >> enum InterpolateMethod interpolate, >> diff --git a/libavfilter/transform.h b/libavfilter/transform.h >> index 9b0c19ceca..0344f9c228 100644 >> --- a/libavfilter/transform.h >> +++ b/libavfilter/transform.h >> @@ -83,33 +83,6 @@ void ff_get_matrix( >> float *matrix >> ); >> -/** >> - * Add two matrices together. result = m1 + m2. >> - * >> - * @param m1 9-item transformation matrix >> - * @param m2 9-item transformation matrix >> - * @param result 9-item transformation matrix >> - */ >> -void avfilter_add_matrix(const float *m1, const float *m2, float >> *result); >> - >> -/** >> - * Subtract one matrix from another. result = m1 - m2. >> - * >> - * @param m1 9-item transformation matrix >> - * @param m2 9-item transformation matrix >> - * @param result 9-item transformation matrix >> - */ >> -void avfilter_sub_matrix(const float *m1, const float *m2, float >> *result); >> - >> -/** >> - * Multiply a matrix by a scalar value. result = m1 * scalar. >> - * >> - * @param m1 9-item transformation matrix >> - * @param scalar a number >> - * @param result 9-item transformation matrix >> - */ >> -void avfilter_mul_matrix(const float *m1, float scalar, float *result); >> - >> /** >> * Do an affine transformation with the given interpolation method. >> This >> * multiplies each vector [x,y,1] by the matrix and then >> interpolates to >> @@ -126,7 +99,7 @@ void avfilter_mul_matrix(const float *m1, float >> scalar, float *result); >> * @param fill edge fill method >> * @return negative on error >> */ >> -int avfilter_transform(const uint8_t *src, uint8_t *dst, >> +int ff_affine_transform(const uint8_t *src, uint8_t *dst, >> int src_stride, int dst_stride, >> int width, int height, const float *matrix, >> enum InterpolateMethod interpolate, >> diff --git a/libavfilter/vf_deshake.c b/libavfilter/vf_deshake.c >> index 28a541b94a..8771399351 100644 >> --- a/libavfilter/vf_deshake.c >> +++ b/libavfilter/vf_deshake.c >> @@ -330,8 +330,9 @@ static int deshake_transform_c(AVFilterContext *ctx, >> for (i = 0; i < 3; i++) { >> // Transform the luma and chroma planes >> - ret = avfilter_transform(in->data[i], out->data[i], >> in->linesize[i], out->linesize[i], >> - plane_w[i], plane_h[i], matrixs[i], >> interpolate, fill); >> + ret = ff_affine_transform(in->data[i], out->data[i], >> in->linesize[i], >> + out->linesize[i], plane_w[i], >> plane_h[i], >> + matrixs[i], interpolate, fill); >> if (ret < 0) >> return ret; >> } >> >
On Thu, Feb 25, 2021 at 1:32 AM Andreas Rheinhardt < andreas.rheinhardt@gmail.com> wrote: > James Almer: > > On 2/24/2021 11:22 AM, Andreas Rheinhardt wrote: > >> avfilter_transform, avfilter_(add|sub|mult)_matrix are not part of the > >> public API (transform.h is not a public header), yet they are currently > >> exported because of their name. This commit changes this: > >> avfilter_transform is renamed to ff_affine_transform; the other > >> functions are just removed as they have never been used at all. > > > > The symbols are exported and have been in four releases so far for this > > soname. If we plan on making a 4.4 release before the bump, it may be a > > good idea if we keep the symbols around for the sake of not affecting > > the ABI, so I'm inclined towards just wrapping them in a > > LIBAVFILTER_VERSION_MAJOR < 8 check like we do when we remove avpriv > ones. > > > > And why? There was never a legitimate outside user of these functions. > And removing avfilter_all_channel_layouts in > d5e5c6862bbc834d5a036a3fa053a7569d84f928 (which also didn't have a > legitimate outside user) didn't seem to break anything either. > Btw: 88d80cb97528d52dac3178cf5393d6095eca6200 broke ABI for x64, because > older versions of libavformat exchange a PutBitContext with libavcodec > via avpriv_align_put_bits and avpriv_copy_bits. So we can't really make > a 4.4 release. > 5.0 then > > - Andreas > > >> > >> Found-by: Anton Khirnov <anton@khirnov.net> > >> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> > >> --- > >> libavfilter/transform.c | 23 +---------------------- > >> libavfilter/transform.h | 29 +---------------------------- > >> libavfilter/vf_deshake.c | 5 +++-- > >> 3 files changed, 5 insertions(+), 52 deletions(-) > >> > >> diff --git a/libavfilter/transform.c b/libavfilter/transform.c > >> index f4f9e0a47d..1f91436f73 100644 > >> --- a/libavfilter/transform.c > >> +++ b/libavfilter/transform.c > >> @@ -122,28 +122,7 @@ void ff_get_matrix( > >> matrix[8] = 1; > >> } > >> -void avfilter_add_matrix(const float *m1, const float *m2, float > >> *result) > >> -{ > >> - int i; > >> - for (i = 0; i < 9; i++) > >> - result[i] = m1[i] + m2[i]; > >> -} > >> - > >> -void avfilter_sub_matrix(const float *m1, const float *m2, float > >> *result) > >> -{ > >> - int i; > >> - for (i = 0; i < 9; i++) > >> - result[i] = m1[i] - m2[i]; > >> -} > >> - > >> -void avfilter_mul_matrix(const float *m1, float scalar, float *result) > >> -{ > >> - int i; > >> - for (i = 0; i < 9; i++) > >> - result[i] = m1[i] * scalar; > >> -} > >> - > >> -int avfilter_transform(const uint8_t *src, uint8_t *dst, > >> +int ff_affine_transform(const uint8_t *src, uint8_t *dst, > >> int src_stride, int dst_stride, > >> int width, int height, const float *matrix, > >> enum InterpolateMethod interpolate, > >> diff --git a/libavfilter/transform.h b/libavfilter/transform.h > >> index 9b0c19ceca..0344f9c228 100644 > >> --- a/libavfilter/transform.h > >> +++ b/libavfilter/transform.h > >> @@ -83,33 +83,6 @@ void ff_get_matrix( > >> float *matrix > >> ); > >> -/** > >> - * Add two matrices together. result = m1 + m2. > >> - * > >> - * @param m1 9-item transformation matrix > >> - * @param m2 9-item transformation matrix > >> - * @param result 9-item transformation matrix > >> - */ > >> -void avfilter_add_matrix(const float *m1, const float *m2, float > >> *result); > >> - > >> -/** > >> - * Subtract one matrix from another. result = m1 - m2. > >> - * > >> - * @param m1 9-item transformation matrix > >> - * @param m2 9-item transformation matrix > >> - * @param result 9-item transformation matrix > >> - */ > >> -void avfilter_sub_matrix(const float *m1, const float *m2, float > >> *result); > >> - > >> -/** > >> - * Multiply a matrix by a scalar value. result = m1 * scalar. > >> - * > >> - * @param m1 9-item transformation matrix > >> - * @param scalar a number > >> - * @param result 9-item transformation matrix > >> - */ > >> -void avfilter_mul_matrix(const float *m1, float scalar, float *result); > >> - > >> /** > >> * Do an affine transformation with the given interpolation method. > >> This > >> * multiplies each vector [x,y,1] by the matrix and then > >> interpolates to > >> @@ -126,7 +99,7 @@ void avfilter_mul_matrix(const float *m1, float > >> scalar, float *result); > >> * @param fill edge fill method > >> * @return negative on error > >> */ > >> -int avfilter_transform(const uint8_t *src, uint8_t *dst, > >> +int ff_affine_transform(const uint8_t *src, uint8_t *dst, > >> int src_stride, int dst_stride, > >> int width, int height, const float *matrix, > >> enum InterpolateMethod interpolate, > >> diff --git a/libavfilter/vf_deshake.c b/libavfilter/vf_deshake.c > >> index 28a541b94a..8771399351 100644 > >> --- a/libavfilter/vf_deshake.c > >> +++ b/libavfilter/vf_deshake.c > >> @@ -330,8 +330,9 @@ static int deshake_transform_c(AVFilterContext *ctx, > >> for (i = 0; i < 3; i++) { > >> // Transform the luma and chroma planes > >> - ret = avfilter_transform(in->data[i], out->data[i], > >> in->linesize[i], out->linesize[i], > >> - plane_w[i], plane_h[i], matrixs[i], > >> interpolate, fill); > >> + ret = ff_affine_transform(in->data[i], out->data[i], > >> in->linesize[i], > >> + out->linesize[i], plane_w[i], > >> plane_h[i], > >> + matrixs[i], interpolate, fill); > >> if (ret < 0) > >> return ret; > >> } > >> > > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Andreas Rheinhardt (12021-02-25):
> And why? There was never a legitimate outside user of these functions.
I agree with that. Something that was not part of the API cannot be
considered part of the ABI. If some project use these functions, they
were explicitly cheating, and they knew it.
Regards,
On 2/24/2021 9:31 PM, Andreas Rheinhardt wrote: > James Almer: >> On 2/24/2021 11:22 AM, Andreas Rheinhardt wrote: >>> avfilter_transform, avfilter_(add|sub|mult)_matrix are not part of the >>> public API (transform.h is not a public header), yet they are currently >>> exported because of their name. This commit changes this: >>> avfilter_transform is renamed to ff_affine_transform; the other >>> functions are just removed as they have never been used at all. >> >> The symbols are exported and have been in four releases so far for this >> soname. If we plan on making a 4.4 release before the bump, it may be a >> good idea if we keep the symbols around for the sake of not affecting >> the ABI, so I'm inclined towards just wrapping them in a >> LIBAVFILTER_VERSION_MAJOR < 8 check like we do when we remove avpriv ones. >> > > And why? There was never a legitimate outside user of these functions. Because it's still exported. But ok, since nobody seems to think it's worth bothering with, this patch should be ok as is. > And removing avfilter_all_channel_layouts in > d5e5c6862bbc834d5a036a3fa053a7569d84f928 (which also didn't have a > legitimate outside user) didn't seem to break anything either. That function and others are mentioned as added in APIChanges yet their removal is not. Sounds like it was handled poorly... > Btw: 88d80cb97528d52dac3178cf5393d6095eca6200 broke ABI for x64, because > older versions of libavformat exchange a PutBitContext with libavcodec > via avpriv_align_put_bits and avpriv_copy_bits. So we can't really make > a 4.4 release. That commit could be reverted in the release/4.4 branch. But yeah, it should have not been committed until those two functions were removed, since they tie PutBitContext to the ABI. A crappy situation overall, but it should not prevent us from making one last release before the bump. The amount of additions to the libraries is considerable, and the first release post bump will be missing a lot of old API, so adoption could take a bit, and something newer than 4.3 should be readily available long before that.
James Almer: > On 2/24/2021 9:31 PM, Andreas Rheinhardt wrote: >> James Almer: >>> On 2/24/2021 11:22 AM, Andreas Rheinhardt wrote: >>>> avfilter_transform, avfilter_(add|sub|mult)_matrix are not part of the >>>> public API (transform.h is not a public header), yet they are currently >>>> exported because of their name. This commit changes this: >>>> avfilter_transform is renamed to ff_affine_transform; the other >>>> functions are just removed as they have never been used at all. >>> >>> The symbols are exported and have been in four releases so far for this >>> soname. If we plan on making a 4.4 release before the bump, it may be a >>> good idea if we keep the symbols around for the sake of not affecting >>> the ABI, so I'm inclined towards just wrapping them in a >>> LIBAVFILTER_VERSION_MAJOR < 8 check like we do when we remove avpriv >>> ones. >>> >> >> And why? There was never a legitimate outside user of these functions. > > Because it's still exported. But ok, since nobody seems to think it's > worth bothering with, this patch should be ok as is. > >> And removing avfilter_all_channel_layouts in >> d5e5c6862bbc834d5a036a3fa053a7569d84f928 (which also didn't have a >> legitimate outside user) didn't seem to break anything either. > > That function and others are mentioned as added in APIChanges yet their > removal is not. Sounds like it was handled poorly... > The whole formats API was scheduled to be made private in b74a1da49db5ebed51aceae6cacc2329288a92c1 in May 2012. It was removed in Libav in 1961e46c15c23a041f8d8614a25388a3ee9eff63 (less than a month after its deprecation...) which was merged into FFmpeg in 5916bc46581230c68c946c0b4733cce381eddcbd in June 2012. It was the merge commit that removed avfilter_all_channel_layouts (which is something that Libav never had). None of these three commits added any entry to APIChanges for their removal which was handled poorly; but d5e5c6862bbc834d5a036a3fa053a7569d84f928 was not (it would be wrong to add an APIChanges entry for something that hasn't been public API for years). >> Btw: 88d80cb97528d52dac3178cf5393d6095eca6200 broke ABI for x64, because >> older versions of libavformat exchange a PutBitContext with libavcodec >> via avpriv_align_put_bits and avpriv_copy_bits. So we can't really make >> a 4.4 release. > > That commit could be reverted in the release/4.4 branch. But yeah, it > should have not been committed until those two functions were removed, > since they tie PutBitContext to the ABI. That will break ABI for those people who use a libavformat version >= 88d80cb97528d52dac3178cf5393d6095eca6200 and older than the commits that removed PutBitContext from the ABI. Granted, not a release, but nevertheless. (Of course this problem would not even exist if we disallowed using libraries from different commits together.) > A crappy situation overall, but it should not prevent us from making one > last release before the bump. The amount of additions to the libraries > is considerable, and the first release post bump will be missing a lot > of old API, so adoption could take a bit, and something newer than 4.3 > should be readily available long before that.
On 2/25/2021 11:44 AM, Andreas Rheinhardt wrote: > James Almer: >> On 2/24/2021 9:31 PM, Andreas Rheinhardt wrote: >>> James Almer: >>>> On 2/24/2021 11:22 AM, Andreas Rheinhardt wrote: >>>>> avfilter_transform, avfilter_(add|sub|mult)_matrix are not part of the >>>>> public API (transform.h is not a public header), yet they are currently >>>>> exported because of their name. This commit changes this: >>>>> avfilter_transform is renamed to ff_affine_transform; the other >>>>> functions are just removed as they have never been used at all. >>>> >>>> The symbols are exported and have been in four releases so far for this >>>> soname. If we plan on making a 4.4 release before the bump, it may be a >>>> good idea if we keep the symbols around for the sake of not affecting >>>> the ABI, so I'm inclined towards just wrapping them in a >>>> LIBAVFILTER_VERSION_MAJOR < 8 check like we do when we remove avpriv >>>> ones. >>>> >>> >>> And why? There was never a legitimate outside user of these functions. >> >> Because it's still exported. But ok, since nobody seems to think it's >> worth bothering with, this patch should be ok as is. >> >>> And removing avfilter_all_channel_layouts in >>> d5e5c6862bbc834d5a036a3fa053a7569d84f928 (which also didn't have a >>> legitimate outside user) didn't seem to break anything either. >> >> That function and others are mentioned as added in APIChanges yet their >> removal is not. Sounds like it was handled poorly... >> > > The whole formats API was scheduled to be made private in > b74a1da49db5ebed51aceae6cacc2329288a92c1 in May 2012. It was removed in > Libav in 1961e46c15c23a041f8d8614a25388a3ee9eff63 (less than a month > after its deprecation...) which was merged into FFmpeg in > 5916bc46581230c68c946c0b4733cce381eddcbd in June 2012. It was the merge > commit that removed avfilter_all_channel_layouts (which is something > that Libav never had). None of these three commits added any entry to > APIChanges for their removal which was handled poorly; but > d5e5c6862bbc834d5a036a3fa053a7569d84f928 was not (it would be wrong to > add an APIChanges entry for something that hasn't been public API for > years). > >>> Btw: 88d80cb97528d52dac3178cf5393d6095eca6200 broke ABI for x64, because >>> older versions of libavformat exchange a PutBitContext with libavcodec >>> via avpriv_align_put_bits and avpriv_copy_bits. So we can't really make >>> a 4.4 release. >> >> That commit could be reverted in the release/4.4 branch. But yeah, it >> should have not been committed until those two functions were removed, >> since they tie PutBitContext to the ABI. > > That will break ABI for those people who use a libavformat version >= > 88d80cb97528d52dac3178cf5393d6095eca6200 and older than the commits that > removed PutBitContext from the ABI. Granted, not a release, but > nevertheless. Indeed. People, or rather distros, will want to drop 4.4 libraries on top of 4.3 ones without recompiling or relinking every software that depends on libav*. It's much more valuable to keep compatibility between the two releases than between intermediary commits after 88d80cb975 and 4.4. Like i said, the situation is crappy and has no single perfect solution, but has one being clearly better than the other. > (Of course this problem would not even exist if we disallowed using > libraries from different commits together.) > >> A crappy situation overall, but it should not prevent us from making one >> last release before the bump. The amount of additions to the libraries >> is considerable, and the first release post bump will be missing a lot >> of old API, so adoption could take a bit, and something newer than 4.3 >> should be readily available long before that. > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >
James Almer: > On 2/25/2021 11:44 AM, Andreas Rheinhardt wrote: >> James Almer: >>> On 2/24/2021 9:31 PM, Andreas Rheinhardt wrote: >>>> James Almer: >>>>> On 2/24/2021 11:22 AM, Andreas Rheinhardt wrote: >>>>>> avfilter_transform, avfilter_(add|sub|mult)_matrix are not part of >>>>>> the >>>>>> public API (transform.h is not a public header), yet they are >>>>>> currently >>>>>> exported because of their name. This commit changes this: >>>>>> avfilter_transform is renamed to ff_affine_transform; the other >>>>>> functions are just removed as they have never been used at all. >>>>> >>>>> The symbols are exported and have been in four releases so far for >>>>> this >>>>> soname. If we plan on making a 4.4 release before the bump, it may >>>>> be a >>>>> good idea if we keep the symbols around for the sake of not affecting >>>>> the ABI, so I'm inclined towards just wrapping them in a >>>>> LIBAVFILTER_VERSION_MAJOR < 8 check like we do when we remove avpriv >>>>> ones. >>>>> >>>> >>>> And why? There was never a legitimate outside user of these functions. >>> >>> Because it's still exported. But ok, since nobody seems to think it's >>> worth bothering with, this patch should be ok as is. >>> >>>> And removing avfilter_all_channel_layouts in >>>> d5e5c6862bbc834d5a036a3fa053a7569d84f928 (which also didn't have a >>>> legitimate outside user) didn't seem to break anything either. >>> >>> That function and others are mentioned as added in APIChanges yet their >>> removal is not. Sounds like it was handled poorly... >>> >> >> The whole formats API was scheduled to be made private in >> b74a1da49db5ebed51aceae6cacc2329288a92c1 in May 2012. It was removed in >> Libav in 1961e46c15c23a041f8d8614a25388a3ee9eff63 (less than a month >> after its deprecation...) which was merged into FFmpeg in >> 5916bc46581230c68c946c0b4733cce381eddcbd in June 2012. It was the merge >> commit that removed avfilter_all_channel_layouts (which is something >> that Libav never had). None of these three commits added any entry to >> APIChanges for their removal which was handled poorly; but >> d5e5c6862bbc834d5a036a3fa053a7569d84f928 was not (it would be wrong to >> add an APIChanges entry for something that hasn't been public API for >> years). >> >>>> Btw: 88d80cb97528d52dac3178cf5393d6095eca6200 broke ABI for x64, >>>> because >>>> older versions of libavformat exchange a PutBitContext with libavcodec >>>> via avpriv_align_put_bits and avpriv_copy_bits. So we can't really make >>>> a 4.4 release. >>> >>> That commit could be reverted in the release/4.4 branch. But yeah, it >>> should have not been committed until those two functions were removed, >>> since they tie PutBitContext to the ABI. >> >> That will break ABI for those people who use a libavformat version >= >> 88d80cb97528d52dac3178cf5393d6095eca6200 and older than the commits that >> removed PutBitContext from the ABI. Granted, not a release, but >> nevertheless. > > Indeed. People, or rather distros, will want to drop 4.4 libraries on > top of 4.3 ones without recompiling or relinking every software that > depends on libav*. It's much more valuable to keep compatibility between > the two releases than between intermediary commits after 88d80cb975 and > 4.4. We have only broken the avpriv API, not the public API. As long as distros compile all libraries from the same snapshot, they will be fine either way. So the "every software that depends on libav*" point is moot. > Like i said, the situation is crappy and has no single perfect solution, > but has one being clearly better than the other. > But this does not mean that I disagree with you here. >> (Of course this problem would not even exist if we disallowed using >> libraries from different commits together.) >> >>> A crappy situation overall, but it should not prevent us from making one >>> last release before the bump. The amount of additions to the libraries >>> is considerable, and the first release post bump will be missing a lot >>> of old API, so adoption could take a bit, and something newer than 4.3 >>> should be readily available long before that.
diff --git a/libavfilter/transform.c b/libavfilter/transform.c index f4f9e0a47d..1f91436f73 100644 --- a/libavfilter/transform.c +++ b/libavfilter/transform.c @@ -122,28 +122,7 @@ void ff_get_matrix( matrix[8] = 1; } -void avfilter_add_matrix(const float *m1, const float *m2, float *result) -{ - int i; - for (i = 0; i < 9; i++) - result[i] = m1[i] + m2[i]; -} - -void avfilter_sub_matrix(const float *m1, const float *m2, float *result) -{ - int i; - for (i = 0; i < 9; i++) - result[i] = m1[i] - m2[i]; -} - -void avfilter_mul_matrix(const float *m1, float scalar, float *result) -{ - int i; - for (i = 0; i < 9; i++) - result[i] = m1[i] * scalar; -} - -int avfilter_transform(const uint8_t *src, uint8_t *dst, +int ff_affine_transform(const uint8_t *src, uint8_t *dst, int src_stride, int dst_stride, int width, int height, const float *matrix, enum InterpolateMethod interpolate, diff --git a/libavfilter/transform.h b/libavfilter/transform.h index 9b0c19ceca..0344f9c228 100644 --- a/libavfilter/transform.h +++ b/libavfilter/transform.h @@ -83,33 +83,6 @@ void ff_get_matrix( float *matrix ); -/** - * Add two matrices together. result = m1 + m2. - * - * @param m1 9-item transformation matrix - * @param m2 9-item transformation matrix - * @param result 9-item transformation matrix - */ -void avfilter_add_matrix(const float *m1, const float *m2, float *result); - -/** - * Subtract one matrix from another. result = m1 - m2. - * - * @param m1 9-item transformation matrix - * @param m2 9-item transformation matrix - * @param result 9-item transformation matrix - */ -void avfilter_sub_matrix(const float *m1, const float *m2, float *result); - -/** - * Multiply a matrix by a scalar value. result = m1 * scalar. - * - * @param m1 9-item transformation matrix - * @param scalar a number - * @param result 9-item transformation matrix - */ -void avfilter_mul_matrix(const float *m1, float scalar, float *result); - /** * Do an affine transformation with the given interpolation method. This * multiplies each vector [x,y,1] by the matrix and then interpolates to @@ -126,7 +99,7 @@ void avfilter_mul_matrix(const float *m1, float scalar, float *result); * @param fill edge fill method * @return negative on error */ -int avfilter_transform(const uint8_t *src, uint8_t *dst, +int ff_affine_transform(const uint8_t *src, uint8_t *dst, int src_stride, int dst_stride, int width, int height, const float *matrix, enum InterpolateMethod interpolate, diff --git a/libavfilter/vf_deshake.c b/libavfilter/vf_deshake.c index 28a541b94a..8771399351 100644 --- a/libavfilter/vf_deshake.c +++ b/libavfilter/vf_deshake.c @@ -330,8 +330,9 @@ static int deshake_transform_c(AVFilterContext *ctx, for (i = 0; i < 3; i++) { // Transform the luma and chroma planes - ret = avfilter_transform(in->data[i], out->data[i], in->linesize[i], out->linesize[i], - plane_w[i], plane_h[i], matrixs[i], interpolate, fill); + ret = ff_affine_transform(in->data[i], out->data[i], in->linesize[i], + out->linesize[i], plane_w[i], plane_h[i], + matrixs[i], interpolate, fill); if (ret < 0) return ret; }
avfilter_transform, avfilter_(add|sub|mult)_matrix are not part of the public API (transform.h is not a public header), yet they are currently exported because of their name. This commit changes this: avfilter_transform is renamed to ff_affine_transform; the other functions are just removed as they have never been used at all. Found-by: Anton Khirnov <anton@khirnov.net> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> --- libavfilter/transform.c | 23 +---------------------- libavfilter/transform.h | 29 +---------------------------- libavfilter/vf_deshake.c | 5 +++-- 3 files changed, 5 insertions(+), 52 deletions(-)