Re: [FFmpeg-devel] [PATCH 4/7] lavfi/vf_spp: convert to the video_enc_params API
On Fri, Nov 27, 2020 at 03:38:51PM +0100, Anton Khirnov wrote:
> Quoting Michael Niedermayer (2020-10-09 19:17:06)
> > On Fri, Oct 09, 2020 at 08:13:24AM +0200, Anton Khirnov wrote:
> > > Quoting Michael Niedermayer (2020-10-06 00:15:06)
> > > > On Mon, Oct 05, 2020 at 10:26:24AM +0200, Anton Khirnov wrote:
> > > > > Quoting Michael Niedermayer (2020-10-03 20:23:02)
> > > > > > On Fri, Oct 02, 2020 at 08:03:28PM +0200, Anton Khirnov wrote:
> > > > > > > ---
> > > > > > > libavfilter/Makefile| 2 +-
> > > > > > > libavfilter/vf_spp.c| 57
> > > > > > > -
> > > > > > > libavfilter/vf_spp.h| 3 +-
> > > > > > > tests/fate/filter-video.mak | 4 +--
> > > > > > > 4 files changed, 29 insertions(+), 37 deletions(-)
> > > > > > >
> > > > > > > diff --git a/libavfilter/Makefile b/libavfilter/Makefile
> > > > > > > index d20f2937b6..2669d7b84b 100644
> > > > > > > --- a/libavfilter/Makefile
> > > > > > > +++ b/libavfilter/Makefile
> > > > > > > @@ -404,7 +404,7 @@ OBJS-$(CONFIG_SOBEL_FILTER)
> > > > > > > += vf_convolution.o
> > > > > > > OBJS-$(CONFIG_SOBEL_OPENCL_FILTER) +=
> > > > > > > vf_convolution_opencl.o opencl.o \
> > > > > > >
> > > > > > > opencl/convolution.o
> > > > > > > OBJS-$(CONFIG_SPLIT_FILTER) += split.o
> > > > > > > -OBJS-$(CONFIG_SPP_FILTER)+= vf_spp.o
> > > > > > > +OBJS-$(CONFIG_SPP_FILTER)+= vf_spp.o
> > > > > > > qp_table.o
> > > > > > > OBJS-$(CONFIG_SR_FILTER) += vf_sr.o
> > > > > > > OBJS-$(CONFIG_SSIM_FILTER) += vf_ssim.o
> > > > > > > framesync.o
> > > > > > > OBJS-$(CONFIG_STEREO3D_FILTER) += vf_stereo3d.o
> > > > > > > diff --git a/libavfilter/vf_spp.c b/libavfilter/vf_spp.c
> > > > > > > index 4bcc6429e0..2eb383be03 100644
> > > > > > > --- a/libavfilter/vf_spp.c
> > > > > > > +++ b/libavfilter/vf_spp.c
> > > > > > > @@ -36,6 +36,7 @@
> > > > > > > #include "libavutil/opt.h"
> > > > > > > #include "libavutil/pixdesc.h"
> > > > > > > #include "internal.h"
> > > > > > > +#include "qp_table.h"
> > > > > > > #include "vf_spp.h"
> > > > > > >
> > > > > > > enum mode {
> > > > > > > @@ -289,7 +290,7 @@ static void filter(SPPContext *p, uint8_t
> > > > > > > *dst, uint8_t *src,
> > > > > > > } else{
> > > > > > > const int qps = 3 + is_luma;
> > > > > > > qp = qp_table[(FFMIN(x, width - 1) >> qps) +
> > > > > > > (FFMIN(y, height - 1) >> qps) * qp_stride];
> > > > > > > -qp = FFMAX(1, ff_norm_qscale(qp,
> > > > > > > p->qscale_type));
> > > > > > > +qp = FFMAX(1, ff_norm_qscale(qp,
> > > > > > > FF_QSCALE_TYPE_MPEG2));
> > > > > >
> > > > > > wouldnt this break the cases where qscale_type is not
> > > > > > FF_QSCALE_TYPE_MPEG2 ?
> > > > >
> > > > > There should be no such cases - in the new API, only TYPE_MPEG2 exists
> > > > > (disregarding newer codecs that were not supported by this filter
> > > > > anyway).
> > > >
> > > > before the patch the code was intended to convert the quantization step
> > > > size
> > > > used in the codec to the same scale as the filter used. disregarding if
> > > > the
> > > > codec was 8x8 dct based. In fact spp should not require the input to be
> > > > 8x8 dct
> > > > based at all, why should it? It uses the DCT as a means to favor real
> > > > images
> > > > and remove "noise" that is not part of real images. It should for
> > > > example also
> > > > work if a image has blocking artifacts that look like hexagons or
> > > > triangles
> > > >
> > > > I never tried but H264 with disabled loop filter should benefit from
> > > > spp in
> > > > terms of subjective quality as long as the filter strength is set
> > > > appropriately
> > > > That is for streams where the bitrate is low enough to see artifacts
> > >
> > > Are you saying I should expand this filter to support new codecs? That
> > > does not seem appropriate for a patch that only adapts it to new API.
> >
> > no
> > but before your patch theres a central place ff_norm_qscale()
> > that has access to both the QP type (codec type) and the QP
> > and can produce a standarized QP
> > So someone who wanted to SPP support some random new codec could
> > do it there and at the same time have 4 other filters for free also
> > support the new codec.
>
> ff_norm_qscale() is still there and the filter still calls it. But if
> someone wants to extend it to support other codecs, they still need to
> add code to ff_qp_table_extract() to handle
> AVVideoEncParams.type != AV_VIDEO_ENC_PARAMS_MPEG2.
>
> It does not seem reasonable to me to pretend the code supports something
> that it actually does not.
I see what you mean but as its done by the patch the code is not clean
afterwards
at all.
For example the argument is always the same, it asks for
Re: [FFmpeg-devel] [PATCH 4/7] lavfi/vf_spp: convert to the video_enc_params API
ping Michael, do you still object to this set? If not, I am going to push it. -- Anton Khirnov ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 4/7] lavfi/vf_spp: convert to the video_enc_params API
Quoting Michael Niedermayer (2020-10-09 19:17:06)
> On Fri, Oct 09, 2020 at 08:13:24AM +0200, Anton Khirnov wrote:
> > Quoting Michael Niedermayer (2020-10-06 00:15:06)
> > > On Mon, Oct 05, 2020 at 10:26:24AM +0200, Anton Khirnov wrote:
> > > > Quoting Michael Niedermayer (2020-10-03 20:23:02)
> > > > > On Fri, Oct 02, 2020 at 08:03:28PM +0200, Anton Khirnov wrote:
> > > > > > ---
> > > > > > libavfilter/Makefile| 2 +-
> > > > > > libavfilter/vf_spp.c| 57
> > > > > > -
> > > > > > libavfilter/vf_spp.h| 3 +-
> > > > > > tests/fate/filter-video.mak | 4 +--
> > > > > > 4 files changed, 29 insertions(+), 37 deletions(-)
> > > > > >
> > > > > > diff --git a/libavfilter/Makefile b/libavfilter/Makefile
> > > > > > index d20f2937b6..2669d7b84b 100644
> > > > > > --- a/libavfilter/Makefile
> > > > > > +++ b/libavfilter/Makefile
> > > > > > @@ -404,7 +404,7 @@ OBJS-$(CONFIG_SOBEL_FILTER) +=
> > > > > > vf_convolution.o
> > > > > > OBJS-$(CONFIG_SOBEL_OPENCL_FILTER) +=
> > > > > > vf_convolution_opencl.o opencl.o \
> > > > > >
> > > > > > opencl/convolution.o
> > > > > > OBJS-$(CONFIG_SPLIT_FILTER) += split.o
> > > > > > -OBJS-$(CONFIG_SPP_FILTER)+= vf_spp.o
> > > > > > +OBJS-$(CONFIG_SPP_FILTER)+= vf_spp.o qp_table.o
> > > > > > OBJS-$(CONFIG_SR_FILTER) += vf_sr.o
> > > > > > OBJS-$(CONFIG_SSIM_FILTER) += vf_ssim.o
> > > > > > framesync.o
> > > > > > OBJS-$(CONFIG_STEREO3D_FILTER) += vf_stereo3d.o
> > > > > > diff --git a/libavfilter/vf_spp.c b/libavfilter/vf_spp.c
> > > > > > index 4bcc6429e0..2eb383be03 100644
> > > > > > --- a/libavfilter/vf_spp.c
> > > > > > +++ b/libavfilter/vf_spp.c
> > > > > > @@ -36,6 +36,7 @@
> > > > > > #include "libavutil/opt.h"
> > > > > > #include "libavutil/pixdesc.h"
> > > > > > #include "internal.h"
> > > > > > +#include "qp_table.h"
> > > > > > #include "vf_spp.h"
> > > > > >
> > > > > > enum mode {
> > > > > > @@ -289,7 +290,7 @@ static void filter(SPPContext *p, uint8_t *dst,
> > > > > > uint8_t *src,
> > > > > > } else{
> > > > > > const int qps = 3 + is_luma;
> > > > > > qp = qp_table[(FFMIN(x, width - 1) >> qps) +
> > > > > > (FFMIN(y, height - 1) >> qps) * qp_stride];
> > > > > > -qp = FFMAX(1, ff_norm_qscale(qp, p->qscale_type));
> > > > > > +qp = FFMAX(1, ff_norm_qscale(qp,
> > > > > > FF_QSCALE_TYPE_MPEG2));
> > > > >
> > > > > wouldnt this break the cases where qscale_type is not
> > > > > FF_QSCALE_TYPE_MPEG2 ?
> > > >
> > > > There should be no such cases - in the new API, only TYPE_MPEG2 exists
> > > > (disregarding newer codecs that were not supported by this filter
> > > > anyway).
> > >
> > > before the patch the code was intended to convert the quantization step
> > > size
> > > used in the codec to the same scale as the filter used. disregarding if
> > > the
> > > codec was 8x8 dct based. In fact spp should not require the input to be
> > > 8x8 dct
> > > based at all, why should it? It uses the DCT as a means to favor real
> > > images
> > > and remove "noise" that is not part of real images. It should for example
> > > also
> > > work if a image has blocking artifacts that look like hexagons or
> > > triangles
> > >
> > > I never tried but H264 with disabled loop filter should benefit from spp
> > > in
> > > terms of subjective quality as long as the filter strength is set
> > > appropriately
> > > That is for streams where the bitrate is low enough to see artifacts
> >
> > Are you saying I should expand this filter to support new codecs? That
> > does not seem appropriate for a patch that only adapts it to new API.
>
> no
> but before your patch theres a central place ff_norm_qscale()
> that has access to both the QP type (codec type) and the QP
> and can produce a standarized QP
> So someone who wanted to SPP support some random new codec could
> do it there and at the same time have 4 other filters for free also
> support the new codec.
ff_norm_qscale() is still there and the filter still calls it. But if
someone wants to extend it to support other codecs, they still need to
add code to ff_qp_table_extract() to handle
AVVideoEncParams.type != AV_VIDEO_ENC_PARAMS_MPEG2.
It does not seem reasonable to me to pretend the code supports something
that it actually does not.
>
> The patch does not replace the qp type by its replacement AVVideoEncParamsType
> but instead a wrong hardcoded value
The value is not wrong. It is correct in all the cases that code runs
in. If someone wants to handle new cases later, it is easy to grep for
ff_norm_qscale() calls and replace the value, after adapting all the
other code.
> If you use AVVideoEncParamsType or something equivalent to it / qp_type then
> that
Re: [FFmpeg-devel] [PATCH 4/7] lavfi/vf_spp: convert to the video_enc_params API
On Fri, Oct 09, 2020 at 08:13:24AM +0200, Anton Khirnov wrote:
> Quoting Michael Niedermayer (2020-10-06 00:15:06)
> > On Mon, Oct 05, 2020 at 10:26:24AM +0200, Anton Khirnov wrote:
> > > Quoting Michael Niedermayer (2020-10-03 20:23:02)
> > > > On Fri, Oct 02, 2020 at 08:03:28PM +0200, Anton Khirnov wrote:
> > > > > ---
> > > > > libavfilter/Makefile| 2 +-
> > > > > libavfilter/vf_spp.c| 57
> > > > > -
> > > > > libavfilter/vf_spp.h| 3 +-
> > > > > tests/fate/filter-video.mak | 4 +--
> > > > > 4 files changed, 29 insertions(+), 37 deletions(-)
> > > > >
> > > > > diff --git a/libavfilter/Makefile b/libavfilter/Makefile
> > > > > index d20f2937b6..2669d7b84b 100644
> > > > > --- a/libavfilter/Makefile
> > > > > +++ b/libavfilter/Makefile
> > > > > @@ -404,7 +404,7 @@ OBJS-$(CONFIG_SOBEL_FILTER) +=
> > > > > vf_convolution.o
> > > > > OBJS-$(CONFIG_SOBEL_OPENCL_FILTER) +=
> > > > > vf_convolution_opencl.o opencl.o \
> > > > > opencl/convolution.o
> > > > > OBJS-$(CONFIG_SPLIT_FILTER) += split.o
> > > > > -OBJS-$(CONFIG_SPP_FILTER)+= vf_spp.o
> > > > > +OBJS-$(CONFIG_SPP_FILTER)+= vf_spp.o qp_table.o
> > > > > OBJS-$(CONFIG_SR_FILTER) += vf_sr.o
> > > > > OBJS-$(CONFIG_SSIM_FILTER) += vf_ssim.o framesync.o
> > > > > OBJS-$(CONFIG_STEREO3D_FILTER) += vf_stereo3d.o
> > > > > diff --git a/libavfilter/vf_spp.c b/libavfilter/vf_spp.c
> > > > > index 4bcc6429e0..2eb383be03 100644
> > > > > --- a/libavfilter/vf_spp.c
> > > > > +++ b/libavfilter/vf_spp.c
> > > > > @@ -36,6 +36,7 @@
> > > > > #include "libavutil/opt.h"
> > > > > #include "libavutil/pixdesc.h"
> > > > > #include "internal.h"
> > > > > +#include "qp_table.h"
> > > > > #include "vf_spp.h"
> > > > >
> > > > > enum mode {
> > > > > @@ -289,7 +290,7 @@ static void filter(SPPContext *p, uint8_t *dst,
> > > > > uint8_t *src,
> > > > > } else{
> > > > > const int qps = 3 + is_luma;
> > > > > qp = qp_table[(FFMIN(x, width - 1) >> qps) +
> > > > > (FFMIN(y, height - 1) >> qps) * qp_stride];
> > > > > -qp = FFMAX(1, ff_norm_qscale(qp, p->qscale_type));
> > > > > +qp = FFMAX(1, ff_norm_qscale(qp,
> > > > > FF_QSCALE_TYPE_MPEG2));
> > > >
> > > > wouldnt this break the cases where qscale_type is not
> > > > FF_QSCALE_TYPE_MPEG2 ?
> > >
> > > There should be no such cases - in the new API, only TYPE_MPEG2 exists
> > > (disregarding newer codecs that were not supported by this filter
> > > anyway).
> >
> > before the patch the code was intended to convert the quantization step size
> > used in the codec to the same scale as the filter used. disregarding if the
> > codec was 8x8 dct based. In fact spp should not require the input to be 8x8
> > dct
> > based at all, why should it? It uses the DCT as a means to favor real images
> > and remove "noise" that is not part of real images. It should for example
> > also
> > work if a image has blocking artifacts that look like hexagons or triangles
> >
> > I never tried but H264 with disabled loop filter should benefit from spp in
> > terms of subjective quality as long as the filter strength is set
> > appropriately
> > That is for streams where the bitrate is low enough to see artifacts
>
> Are you saying I should expand this filter to support new codecs? That
> does not seem appropriate for a patch that only adapts it to new API.
no
but before your patch theres a central place ff_norm_qscale()
that has access to both the QP type (codec type) and the QP
and can produce a standarized QP
So someone who wanted to SPP support some random new codec could
do it there and at the same time have 4 other filters for free also
support the new codec.
The patch does not replace the qp type by its replacement AVVideoEncParamsType
but instead a wrong hardcoded value
If you use AVVideoEncParamsType or something equivalent to it / qp_type then
that resolves this concern
unless iam missing something of course ...
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Good people do not need laws to tell them to act responsibly, while bad
people will find a way around the laws. -- Plato
signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 4/7] lavfi/vf_spp: convert to the video_enc_params API
Quoting Michael Niedermayer (2020-10-06 00:15:06)
> On Mon, Oct 05, 2020 at 10:26:24AM +0200, Anton Khirnov wrote:
> > Quoting Michael Niedermayer (2020-10-03 20:23:02)
> > > On Fri, Oct 02, 2020 at 08:03:28PM +0200, Anton Khirnov wrote:
> > > > ---
> > > > libavfilter/Makefile| 2 +-
> > > > libavfilter/vf_spp.c| 57 -
> > > > libavfilter/vf_spp.h| 3 +-
> > > > tests/fate/filter-video.mak | 4 +--
> > > > 4 files changed, 29 insertions(+), 37 deletions(-)
> > > >
> > > > diff --git a/libavfilter/Makefile b/libavfilter/Makefile
> > > > index d20f2937b6..2669d7b84b 100644
> > > > --- a/libavfilter/Makefile
> > > > +++ b/libavfilter/Makefile
> > > > @@ -404,7 +404,7 @@ OBJS-$(CONFIG_SOBEL_FILTER) +=
> > > > vf_convolution.o
> > > > OBJS-$(CONFIG_SOBEL_OPENCL_FILTER) +=
> > > > vf_convolution_opencl.o opencl.o \
> > > > opencl/convolution.o
> > > > OBJS-$(CONFIG_SPLIT_FILTER) += split.o
> > > > -OBJS-$(CONFIG_SPP_FILTER)+= vf_spp.o
> > > > +OBJS-$(CONFIG_SPP_FILTER)+= vf_spp.o qp_table.o
> > > > OBJS-$(CONFIG_SR_FILTER) += vf_sr.o
> > > > OBJS-$(CONFIG_SSIM_FILTER) += vf_ssim.o framesync.o
> > > > OBJS-$(CONFIG_STEREO3D_FILTER) += vf_stereo3d.o
> > > > diff --git a/libavfilter/vf_spp.c b/libavfilter/vf_spp.c
> > > > index 4bcc6429e0..2eb383be03 100644
> > > > --- a/libavfilter/vf_spp.c
> > > > +++ b/libavfilter/vf_spp.c
> > > > @@ -36,6 +36,7 @@
> > > > #include "libavutil/opt.h"
> > > > #include "libavutil/pixdesc.h"
> > > > #include "internal.h"
> > > > +#include "qp_table.h"
> > > > #include "vf_spp.h"
> > > >
> > > > enum mode {
> > > > @@ -289,7 +290,7 @@ static void filter(SPPContext *p, uint8_t *dst,
> > > > uint8_t *src,
> > > > } else{
> > > > const int qps = 3 + is_luma;
> > > > qp = qp_table[(FFMIN(x, width - 1) >> qps) + (FFMIN(y,
> > > > height - 1) >> qps) * qp_stride];
> > > > -qp = FFMAX(1, ff_norm_qscale(qp, p->qscale_type));
> > > > +qp = FFMAX(1, ff_norm_qscale(qp,
> > > > FF_QSCALE_TYPE_MPEG2));
> > >
> > > wouldnt this break the cases where qscale_type is not
> > > FF_QSCALE_TYPE_MPEG2 ?
> >
> > There should be no such cases - in the new API, only TYPE_MPEG2 exists
> > (disregarding newer codecs that were not supported by this filter
> > anyway).
>
> before the patch the code was intended to convert the quantization step size
> used in the codec to the same scale as the filter used. disregarding if the
> codec was 8x8 dct based. In fact spp should not require the input to be 8x8
> dct
> based at all, why should it? It uses the DCT as a means to favor real images
> and remove "noise" that is not part of real images. It should for example also
> work if a image has blocking artifacts that look like hexagons or triangles
>
> I never tried but H264 with disabled loop filter should benefit from spp in
> terms of subjective quality as long as the filter strength is set
> appropriately
> That is for streams where the bitrate is low enough to see artifacts
Are you saying I should expand this filter to support new codecs? That
does not seem appropriate for a patch that only adapts it to new API.
--
Anton Khirnov
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 4/7] lavfi/vf_spp: convert to the video_enc_params API
On Mon, Oct 05, 2020 at 10:26:24AM +0200, Anton Khirnov wrote:
> Quoting Michael Niedermayer (2020-10-03 20:23:02)
> > On Fri, Oct 02, 2020 at 08:03:28PM +0200, Anton Khirnov wrote:
> > > ---
> > > libavfilter/Makefile| 2 +-
> > > libavfilter/vf_spp.c| 57 -
> > > libavfilter/vf_spp.h| 3 +-
> > > tests/fate/filter-video.mak | 4 +--
> > > 4 files changed, 29 insertions(+), 37 deletions(-)
> > >
> > > diff --git a/libavfilter/Makefile b/libavfilter/Makefile
> > > index d20f2937b6..2669d7b84b 100644
> > > --- a/libavfilter/Makefile
> > > +++ b/libavfilter/Makefile
> > > @@ -404,7 +404,7 @@ OBJS-$(CONFIG_SOBEL_FILTER) +=
> > > vf_convolution.o
> > > OBJS-$(CONFIG_SOBEL_OPENCL_FILTER) += vf_convolution_opencl.o
> > > opencl.o \
> > > opencl/convolution.o
> > > OBJS-$(CONFIG_SPLIT_FILTER) += split.o
> > > -OBJS-$(CONFIG_SPP_FILTER)+= vf_spp.o
> > > +OBJS-$(CONFIG_SPP_FILTER)+= vf_spp.o qp_table.o
> > > OBJS-$(CONFIG_SR_FILTER) += vf_sr.o
> > > OBJS-$(CONFIG_SSIM_FILTER) += vf_ssim.o framesync.o
> > > OBJS-$(CONFIG_STEREO3D_FILTER) += vf_stereo3d.o
> > > diff --git a/libavfilter/vf_spp.c b/libavfilter/vf_spp.c
> > > index 4bcc6429e0..2eb383be03 100644
> > > --- a/libavfilter/vf_spp.c
> > > +++ b/libavfilter/vf_spp.c
> > > @@ -36,6 +36,7 @@
> > > #include "libavutil/opt.h"
> > > #include "libavutil/pixdesc.h"
> > > #include "internal.h"
> > > +#include "qp_table.h"
> > > #include "vf_spp.h"
> > >
> > > enum mode {
> > > @@ -289,7 +290,7 @@ static void filter(SPPContext *p, uint8_t *dst,
> > > uint8_t *src,
> > > } else{
> > > const int qps = 3 + is_luma;
> > > qp = qp_table[(FFMIN(x, width - 1) >> qps) + (FFMIN(y,
> > > height - 1) >> qps) * qp_stride];
> > > -qp = FFMAX(1, ff_norm_qscale(qp, p->qscale_type));
> > > +qp = FFMAX(1, ff_norm_qscale(qp, FF_QSCALE_TYPE_MPEG2));
> >
> > wouldnt this break the cases where qscale_type is not FF_QSCALE_TYPE_MPEG2 ?
>
> There should be no such cases - in the new API, only TYPE_MPEG2 exists
> (disregarding newer codecs that were not supported by this filter
> anyway).
before the patch the code was intended to convert the quantization step size
used in the codec to the same scale as the filter used. disregarding if the
codec was 8x8 dct based. In fact spp should not require the input to be 8x8 dct
based at all, why should it? It uses the DCT as a means to favor real images
and remove "noise" that is not part of real images. It should for example also
work if a image has blocking artifacts that look like hexagons or triangles
I never tried but H264 with disabled loop filter should benefit from spp in
terms of subjective quality as long as the filter strength is set appropriately
That is for streams where the bitrate is low enough to see artifacts
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
The real ebay dictionary, page 1
"Used only once"- "Some unspecified defect prevented a second use"
"In good condition" - "Can be repaird by experienced expert"
"As is" - "You wouldnt want it even if you were payed for it, if you knew ..."
signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 4/7] lavfi/vf_spp: convert to the video_enc_params API
Quoting Michael Niedermayer (2020-10-03 20:23:02)
> On Fri, Oct 02, 2020 at 08:03:28PM +0200, Anton Khirnov wrote:
> > ---
> > libavfilter/Makefile| 2 +-
> > libavfilter/vf_spp.c| 57 -
> > libavfilter/vf_spp.h| 3 +-
> > tests/fate/filter-video.mak | 4 +--
> > 4 files changed, 29 insertions(+), 37 deletions(-)
> >
> > diff --git a/libavfilter/Makefile b/libavfilter/Makefile
> > index d20f2937b6..2669d7b84b 100644
> > --- a/libavfilter/Makefile
> > +++ b/libavfilter/Makefile
> > @@ -404,7 +404,7 @@ OBJS-$(CONFIG_SOBEL_FILTER) +=
> > vf_convolution.o
> > OBJS-$(CONFIG_SOBEL_OPENCL_FILTER) += vf_convolution_opencl.o
> > opencl.o \
> > opencl/convolution.o
> > OBJS-$(CONFIG_SPLIT_FILTER) += split.o
> > -OBJS-$(CONFIG_SPP_FILTER)+= vf_spp.o
> > +OBJS-$(CONFIG_SPP_FILTER)+= vf_spp.o qp_table.o
> > OBJS-$(CONFIG_SR_FILTER) += vf_sr.o
> > OBJS-$(CONFIG_SSIM_FILTER) += vf_ssim.o framesync.o
> > OBJS-$(CONFIG_STEREO3D_FILTER) += vf_stereo3d.o
> > diff --git a/libavfilter/vf_spp.c b/libavfilter/vf_spp.c
> > index 4bcc6429e0..2eb383be03 100644
> > --- a/libavfilter/vf_spp.c
> > +++ b/libavfilter/vf_spp.c
> > @@ -36,6 +36,7 @@
> > #include "libavutil/opt.h"
> > #include "libavutil/pixdesc.h"
> > #include "internal.h"
> > +#include "qp_table.h"
> > #include "vf_spp.h"
> >
> > enum mode {
> > @@ -289,7 +290,7 @@ static void filter(SPPContext *p, uint8_t *dst, uint8_t
> > *src,
> > } else{
> > const int qps = 3 + is_luma;
> > qp = qp_table[(FFMIN(x, width - 1) >> qps) + (FFMIN(y,
> > height - 1) >> qps) * qp_stride];
> > -qp = FFMAX(1, ff_norm_qscale(qp, p->qscale_type));
> > +qp = FFMAX(1, ff_norm_qscale(qp, FF_QSCALE_TYPE_MPEG2));
>
> wouldnt this break the cases where qscale_type is not FF_QSCALE_TYPE_MPEG2 ?
There should be no such cases - in the new API, only TYPE_MPEG2 exists
(disregarding newer codecs that were not supported by this filter
anyway).
--
Anton Khirnov
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 4/7] lavfi/vf_spp: convert to the video_enc_params API
On Fri, Oct 02, 2020 at 08:03:28PM +0200, Anton Khirnov wrote:
> ---
> libavfilter/Makefile| 2 +-
> libavfilter/vf_spp.c| 57 -
> libavfilter/vf_spp.h| 3 +-
> tests/fate/filter-video.mak | 4 +--
> 4 files changed, 29 insertions(+), 37 deletions(-)
>
> diff --git a/libavfilter/Makefile b/libavfilter/Makefile
> index d20f2937b6..2669d7b84b 100644
> --- a/libavfilter/Makefile
> +++ b/libavfilter/Makefile
> @@ -404,7 +404,7 @@ OBJS-$(CONFIG_SOBEL_FILTER) +=
> vf_convolution.o
> OBJS-$(CONFIG_SOBEL_OPENCL_FILTER) += vf_convolution_opencl.o
> opencl.o \
> opencl/convolution.o
> OBJS-$(CONFIG_SPLIT_FILTER) += split.o
> -OBJS-$(CONFIG_SPP_FILTER)+= vf_spp.o
> +OBJS-$(CONFIG_SPP_FILTER)+= vf_spp.o qp_table.o
> OBJS-$(CONFIG_SR_FILTER) += vf_sr.o
> OBJS-$(CONFIG_SSIM_FILTER) += vf_ssim.o framesync.o
> OBJS-$(CONFIG_STEREO3D_FILTER) += vf_stereo3d.o
> diff --git a/libavfilter/vf_spp.c b/libavfilter/vf_spp.c
> index 4bcc6429e0..2eb383be03 100644
> --- a/libavfilter/vf_spp.c
> +++ b/libavfilter/vf_spp.c
> @@ -36,6 +36,7 @@
> #include "libavutil/opt.h"
> #include "libavutil/pixdesc.h"
> #include "internal.h"
> +#include "qp_table.h"
> #include "vf_spp.h"
>
> enum mode {
> @@ -289,7 +290,7 @@ static void filter(SPPContext *p, uint8_t *dst, uint8_t
> *src,
> } else{
> const int qps = 3 + is_luma;
> qp = qp_table[(FFMIN(x, width - 1) >> qps) + (FFMIN(y,
> height - 1) >> qps) * qp_stride];
> -qp = FFMAX(1, ff_norm_qscale(qp, p->qscale_type));
> +qp = FFMAX(1, ff_norm_qscale(qp, FF_QSCALE_TYPE_MPEG2));
wouldnt this break the cases where qscale_type is not FF_QSCALE_TYPE_MPEG2 ?
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
If you think the mosad wants you dead since a long time then you are either
wrong or dead since a long time.
signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 4/7] lavfi/vf_spp: convert to the video_enc_params API
---
libavfilter/Makefile| 2 +-
libavfilter/vf_spp.c| 57 -
libavfilter/vf_spp.h| 3 +-
tests/fate/filter-video.mak | 4 +--
4 files changed, 29 insertions(+), 37 deletions(-)
diff --git a/libavfilter/Makefile b/libavfilter/Makefile
index d20f2937b6..2669d7b84b 100644
--- a/libavfilter/Makefile
+++ b/libavfilter/Makefile
@@ -404,7 +404,7 @@ OBJS-$(CONFIG_SOBEL_FILTER) +=
vf_convolution.o
OBJS-$(CONFIG_SOBEL_OPENCL_FILTER) += vf_convolution_opencl.o
opencl.o \
opencl/convolution.o
OBJS-$(CONFIG_SPLIT_FILTER) += split.o
-OBJS-$(CONFIG_SPP_FILTER)+= vf_spp.o
+OBJS-$(CONFIG_SPP_FILTER)+= vf_spp.o qp_table.o
OBJS-$(CONFIG_SR_FILTER) += vf_sr.o
OBJS-$(CONFIG_SSIM_FILTER) += vf_ssim.o framesync.o
OBJS-$(CONFIG_STEREO3D_FILTER) += vf_stereo3d.o
diff --git a/libavfilter/vf_spp.c b/libavfilter/vf_spp.c
index 4bcc6429e0..2eb383be03 100644
--- a/libavfilter/vf_spp.c
+++ b/libavfilter/vf_spp.c
@@ -36,6 +36,7 @@
#include "libavutil/opt.h"
#include "libavutil/pixdesc.h"
#include "internal.h"
+#include "qp_table.h"
#include "vf_spp.h"
enum mode {
@@ -289,7 +290,7 @@ static void filter(SPPContext *p, uint8_t *dst, uint8_t
*src,
} else{
const int qps = 3 + is_luma;
qp = qp_table[(FFMIN(x, width - 1) >> qps) + (FFMIN(y, height
- 1) >> qps) * qp_stride];
-qp = FFMAX(1, ff_norm_qscale(qp, p->qscale_type));
+qp = FFMAX(1, ff_norm_qscale(qp, FF_QSCALE_TYPE_MPEG2));
}
for (i = 0; i < count; i++) {
const int x1 = x + offset[i + count - 1][0];
@@ -374,47 +375,34 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *in)
AVFilterLink *outlink = ctx->outputs[0];
AVFrame *out = in;
int qp_stride = 0;
-const int8_t *qp_table = NULL;
+int8_t *qp_table = NULL;
const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(inlink->format);
const int depth = desc->comp[0].depth;
+int ret = 0;
/* if we are not in a constant user quantizer mode and we don't want to use
* the quantizers from the B-frames (B-frames often have a higher QP), we
* need to save the qp table from the last non B-frame; this is what the
* following code block does */
-if (!s->qp) {
-qp_table = av_frame_get_qp_table(in, _stride, >qscale_type);
-
-if (qp_table && !s->use_bframe_qp && in->pict_type !=
AV_PICTURE_TYPE_B) {
-int w, h;
-
-/* if the qp stride is not set, it means the QP are only defined on
- * a line basis */
-if (!qp_stride) {
-w = AV_CEIL_RSHIFT(inlink->w, 4);
-h = 1;
-} else {
-w = qp_stride;
-h = AV_CEIL_RSHIFT(inlink->h, 4);
-}
-
-if (w * h > s->non_b_qp_alloc_size) {
-int ret = av_reallocp_array(>non_b_qp_table, w, h);
-if (ret < 0) {
-s->non_b_qp_alloc_size = 0;
-return ret;
-}
-s->non_b_qp_alloc_size = w * h;
-}
+if (!s->qp && (s->use_bframe_qp || in->pict_type != AV_PICTURE_TYPE_B)) {
+ret = ff_qp_table_extract(in, _table, _stride, NULL);
+if (ret < 0) {
+av_frame_free();
+return ret;
+}
-av_assert0(w * h <= s->non_b_qp_alloc_size);
-memcpy(s->non_b_qp_table, qp_table, w * h);
+if (!s->use_bframe_qp && in->pict_type != AV_PICTURE_TYPE_B) {
+av_freep(>non_b_qp_table);
+s->non_b_qp_table = qp_table;
+s->non_b_qp_stride = qp_stride;
}
}
if (s->log2_count && !ctx->is_disabled) {
-if (!s->use_bframe_qp && s->non_b_qp_table)
-qp_table = s->non_b_qp_table;
+if (!s->use_bframe_qp && s->non_b_qp_table) {
+qp_table = s->non_b_qp_table;
+qp_stride = s->non_b_qp_stride;
+}
if (qp_table || s->qp) {
const int cw = AV_CEIL_RSHIFT(inlink->w, s->hsub);
@@ -429,7 +417,8 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *in)
out = ff_get_video_buffer(outlink, aligned_w, aligned_h);
if (!out) {
av_frame_free();
-return AVERROR(ENOMEM);
+ret = AVERROR(ENOMEM);
+goto finish;
}
av_frame_copy_props(out, in);
out->width = in->width;
@@ -453,7 +442,11 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *in)
inlink->w, inlink->h);
av_frame_free();
}
-return ff_filter_frame(outlink, out);
+ret =
