Re: [FFmpeg-devel] [PATCH] lavc/aarch64/sbrdsp_neon: fix build on old binutils
On Thu, Jan 25, 2018 at 09:44:55PM -0600, Rodger Combs wrote:
> ---
> libavcodec/aarch64/sbrdsp_neon.S | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavcodec/aarch64/sbrdsp_neon.S
> b/libavcodec/aarch64/sbrdsp_neon.S
> index d1d79b749c..d23717e760 100644
> --- a/libavcodec/aarch64/sbrdsp_neon.S
> +++ b/libavcodec/aarch64/sbrdsp_neon.S
> @@ -287,7 +287,7 @@ endfunc
> zip1v4.4S, v4.4S, v4.4S
> fmlav6.4S, v1.4S, v3.4S
> fmlav2.4S, v5.4S, v4.4S
> -fcmeq v7.4S, v3.4S, #0.0
> +fcmeq v7.4S, v3.4S, #0
> bif v2.16B, v6.16B, v7.16B
> st1 {v2.4S}, [x0], #16
> subsx5, x5, #2
> --
> 2.15.1
LGTM.
--
Matthieu B.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]: Change Stack Frame Limit in Cuda Context
Thanks for the review Mark.
On Thu, Jan 25, 2018 at 4:13 PM, Mark Thompson wrote:
>
> > diff --git a/libavcodec/nvenc.c b/libavcodec/nvenc.c
> > index 4a91d99..2da251b 100644
> > --- a/libavcodec/nvenc.c
> > +++ b/libavcodec/nvenc.c
> > @@ -420,6 +420,12 @@ static av_cold int nvenc_check_device(AVCodecContext
> *avctx, int idx)
> > goto fail;
> > }
> >
> > +cu_res = dl_fn->cuda_dl->cuCtxSetLimit(CU_LIMIT_STACK_SIZE, 128);
> > +if (cu_res != CUDA_SUCCESS) {
> > +av_log(avctx, AV_LOG_FATAL, "Failed reducing CUDA context stack
> limit for NVENC: 0x%x\n", (int)cu_res);
> > +goto fail;
> > +}
> > +
> > ctx->cu_context = ctx->cu_context_internal;
> >
> > if ((ret = nvenc_pop_context(avctx)) < 0)
>
> Does this actually have any effect? I was under the impression that the
> CUDA context created inside the NVENC encoder wouldn't actually be used for
> any CUDA operations at all (really just a GPU device handle).
>
There are some cuda kernels in the driver that may be invoked depending on
the nvenc operations specified in the commandline. My observation from
looking at the nvcc statistics is that most stack frame size for these cuda
kernels are 0 (highest observed was 120 bytes).
>
> > diff --git a/libavutil/hwcontext_cuda.c b/libavutil/hwcontext_cuda.c
> > index 37827a7..1f022fa 100644
> > --- a/libavutil/hwcontext_cuda.c
> > +++ b/libavutil/hwcontext_cuda.c
> > @@ -386,6 +386,12 @@ static int cuda_device_create(AVHWDeviceContext
> *ctx, const char *device,
> > goto error;
> > }
> >
> > +err = cu->cuCtxSetLimit(CU_LIMIT_STACK_SIZE, 128);
> > +if (err != CUDA_SUCCESS) {
> > +av_log(ctx, AV_LOG_ERROR, "Error reducing CUDA context stack
> limit\n");
> > +goto error;
> > +}
> > +
> > cu->cuCtxPopCurrent(&dummy);
> >
> > hwctx->internal->is_allocated = 1;
> > --
> > 2.9.1
> >
>
> This is technically a user-visible change, since it will apply to all user
> programs run on the CUDA context created here as well as those inside
> ffmpeg. I'm not sure how many people actually use that, though, so maybe
> it won't affect anyone.
>
In ffmpeg, I see vf_thumbnail_cuda and vf_scale_cuda available (not sure if
there is more, but these two should not be affected by this reduction).
User can always raise the stack limit size if their own custom kernel
require higher stack frame size.
>
> If the stack limit is violated, what happens? Will that be undefined
> behaviour with random effects (crash / incorrect results), or is it likely
> to be caught at program compile/load-time?
>
Stack will likely overflow and kernel will terminate (though I have yet
encounter this before).
Thanks,
Ben
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH v3] avcodec/libopus: support disabling phase inversion.
From: Menno
This supports disabling phase inversion in both the libopus encoder and the
decoder.
Signed-off-by: Menno
---
doc/encoders.texi | 5 +
libavcodec/libopusdec.c | 33 +
libavcodec/libopusenc.c | 14 ++
3 files changed, 52 insertions(+)
diff --git a/doc/encoders.texi b/doc/encoders.texi
index 6a410a8cb6..c5dfc646d9 100644
--- a/doc/encoders.texi
+++ b/doc/encoders.texi
@@ -981,6 +981,11 @@ Other values include 0 for mono and stereo, 1 for surround
sound with masking
and LFE bandwidth optimizations, and 255 for independent streams with an
unspecified channel layout.
+@item apply_phase_inv (N.A.) (requires libopus >= 1.2)
+If set to 0, disables the use of phase inversion for intensity stereo,
+improving the quality of mono downmixes, but slightly reducing normal stereo
+quality. The default is 1 (phase inversion enabled).
+
@end table
@anchor{libshine}
diff --git a/libavcodec/libopusdec.c b/libavcodec/libopusdec.c
index 4f7f4755c2..f82ae57e54 100644
--- a/libavcodec/libopusdec.c
+++ b/libavcodec/libopusdec.c
@@ -25,6 +25,7 @@
#include "libavutil/internal.h"
#include "libavutil/intreadwrite.h"
#include "libavutil/ffmath.h"
+#include "libavutil/opt.h"
#include "avcodec.h"
#include "internal.h"
@@ -33,11 +34,15 @@
#include "libopus.h"
struct libopus_context {
+AVClass *class;
OpusMSDecoder *dec;
int pre_skip;
#ifndef OPUS_SET_GAIN
union { int i; double d; } gain;
#endif
+#ifdef OPUS_SET_PHASE_INVERSION_DISABLED_REQUEST
+int apply_phase_inv;
+#endif
};
#define OPUS_HEAD_SIZE 19
@@ -136,6 +141,15 @@ static av_cold int libopus_decode_init(AVCodecContext *avc)
}
#endif
+#ifdef OPUS_SET_PHASE_INVERSION_DISABLED_REQUEST
+ret = opus_multistream_decoder_ctl(opus->dec,
+
OPUS_SET_PHASE_INVERSION_DISABLED(!opus->apply_phase_inv));
+if (ret != OPUS_OK)
+av_log(avc, AV_LOG_WARNING,
+ "Unable to set phase inversion: %s\n",
+ opus_strerror(ret));
+#endif
+
/* Decoder delay (in samples) at 48kHz */
avc->delay = avc->internal->skip_samples = opus->pre_skip;
@@ -209,6 +223,24 @@ static void libopus_flush(AVCodecContext *avc)
avc->internal->skip_samples = opus->pre_skip;
}
+
+#define OFFSET(x) offsetof(struct libopus_context, x)
+#define FLAGS AV_OPT_FLAG_AUDIO_PARAM | AV_OPT_FLAG_DECODING_PARAM
+static const AVOption libopusdec_options[] = {
+#ifdef OPUS_SET_PHASE_INVERSION_DISABLED_REQUEST
+{ "apply_phase_inv", "Apply intensity stereo phase inversion",
OFFSET(apply_phase_inv), AV_OPT_TYPE_BOOL, { .i64 = 1 }, 0, 1, FLAGS },
+{ NULL },
+#endif
+};
+
+static const AVClass libopusdec_class = {
+.class_name = "libopusdec",
+.item_name = av_default_item_name,
+.option = libopusdec_options,
+.version= LIBAVUTIL_VERSION_INT,
+};
+
+
AVCodec ff_libopus_decoder = {
.name = "libopus",
.long_name = NULL_IF_CONFIG_SMALL("libopus Opus"),
@@ -223,5 +255,6 @@ AVCodec ff_libopus_decoder = {
.sample_fmts= (const enum AVSampleFormat[]){ AV_SAMPLE_FMT_FLT,
AV_SAMPLE_FMT_S16,
AV_SAMPLE_FMT_NONE },
+.priv_class = &libopusdec_class,
.wrapper_name = "libopus",
};
diff --git a/libavcodec/libopusenc.c b/libavcodec/libopusenc.c
index b449497d15..4ae81b0bb2 100644
--- a/libavcodec/libopusenc.c
+++ b/libavcodec/libopusenc.c
@@ -39,6 +39,9 @@ typedef struct LibopusEncOpts {
int packet_size;
int max_bandwidth;
int mapping_family;
+#ifdef OPUS_SET_PHASE_INVERSION_DISABLED_REQUEST
+int apply_phase_inv;
+#endif
} LibopusEncOpts;
typedef struct LibopusEncContext {
@@ -154,6 +157,14 @@ static int libopus_configure_encoder(AVCodecContext
*avctx, OpusMSEncoder *enc,
"Unable to set maximum bandwidth: %s\n",
opus_strerror(ret));
}
+#ifdef OPUS_SET_PHASE_INVERSION_DISABLED_REQUEST
+ret = opus_multistream_encoder_ctl(enc,
+
OPUS_SET_PHASE_INVERSION_DISABLED(!opts->apply_phase_inv));
+if (ret != OPUS_OK)
+av_log(avctx, AV_LOG_WARNING,
+ "Unable to set phase inversion: %s\n",
+ opus_strerror(ret));
+#endif
return OPUS_OK;
}
@@ -530,6 +541,9 @@ static const AVOption libopus_options[] = {
{ "on", "Use variable bit rate", 0, AV_OPT_TYPE_CONST, {
.i64 = 1 }, 0, 0, FLAGS, "vbr" },
{ "constrained","Use constrained VBR", 0, AV_OPT_TYPE_CONST, {
.i64 = 2 }, 0, 0, FLAGS, "vbr" },
{ "mapping_family", "Channel Mapping Family",
OFFSET(mapping_family), AV_OPT_TYPE_INT, { .i64 = -1 }, -1, 255, FLAGS,
"mapping_family" },
+#ifdef OPUS_SET_PHASE_INVERSION_DISABLED_REQUEST
+{ "apply_phase_inv", "Apply intensity stereo phase inversion",
OFFSET(apply_phase_inv), AV_O
Re: [FFmpeg-devel] [PATCH] lavc/qsv: skip the packet if decoding failure.
On 26/01/18 07:33, Song, Ruiling wrote:
>
>
>> -Original Message-
>> From: Jun Zhao [mailto:mypopy...@gmail.com]
>> Sent: Friday, January 26, 2018 1:02 PM
>> To: FFmpeg development discussions and patches ;
>> Song, Ruiling
>> Subject: Re: [FFmpeg-devel] [PATCH] lavc/qsv: skip the packet if decoding
>> failure.
>>
>>
>>
>> On 2018/1/25 16:37, Ruiling Song wrote:
>>> From: "Ruiling, Song"
>>>
>>> MediaSDK may fail to decode some frame, just skip it.
>>> Otherwise, it will keep decoding the failure packet repeatedly
>>> without processing any packet afterwards.
>>>
>>> v2:
>>> switch to using av_packet_unref().
>>>
>>> Signed-off-by: Ruiling Song
>>> ---
>>> libavcodec/qsvdec_h2645.c | 6 +-
>>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/libavcodec/qsvdec_h2645.c b/libavcodec/qsvdec_h2645.c
>>> index 5e00673..d92a150 100644
>>> --- a/libavcodec/qsvdec_h2645.c
>>> +++ b/libavcodec/qsvdec_h2645.c
>>> @@ -153,8 +153,12 @@ static int qsv_decode_frame(AVCodecContext *avctx,
>> void *data,
>>> }
>>>
>>> ret = ff_qsv_process_data(avctx, &s->qsv, frame, got_frame, &s-
>>> buffer_pkt);
>>> -if (ret < 0)
>>> +if (ret < 0) {
>>> +/* Drop buffer_pkt when failed to decode the packet. Otherwise,
>>> + the decoder will keep decoding the failure packet. */
>>> +av_packet_unref(&s->buffer_pkt);
>>
>> I think add a warning message more better than only drop the packet on
>> the quiet.
> What do you think "packet decoding failed, skip it."?
> Usually some error message has already been printed out before skipping the
> packet.
Yeah. I think the only ways we get here without having already logged the
failure are either an internal bug (the setup is broken somehow) or
out-of-memory. Neither of those would be improved by a generic "decoding
failed" message here.
Anyway, this was applied in libav and has already been applied here as a merge.
Thanks,
- Mark
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]: Change Stack Frame Limit in Cuda Context
On 26/01/18 09:06, Ben Chang wrote:
> Thanks for the review Mark.
>
> On Thu, Jan 25, 2018 at 4:13 PM, Mark Thompson wrote:
>>
>>> diff --git a/libavcodec/nvenc.c b/libavcodec/nvenc.c
>>> index 4a91d99..2da251b 100644
>>> --- a/libavcodec/nvenc.c
>>> +++ b/libavcodec/nvenc.c
>>> @@ -420,6 +420,12 @@ static av_cold int nvenc_check_device(AVCodecContext
>> *avctx, int idx)
>>> goto fail;
>>> }
>>>
>>> +cu_res = dl_fn->cuda_dl->cuCtxSetLimit(CU_LIMIT_STACK_SIZE, 128);
>>> +if (cu_res != CUDA_SUCCESS) {
>>> +av_log(avctx, AV_LOG_FATAL, "Failed reducing CUDA context stack
>> limit for NVENC: 0x%x\n", (int)cu_res);
>>> +goto fail;
>>> +}
>>> +
>>> ctx->cu_context = ctx->cu_context_internal;
>>>
>>> if ((ret = nvenc_pop_context(avctx)) < 0)
>>
>> Does this actually have any effect? I was under the impression that the
>> CUDA context created inside the NVENC encoder wouldn't actually be used for
>> any CUDA operations at all (really just a GPU device handle).
>>
> There are some cuda kernels in the driver that may be invoked depending on
> the nvenc operations specified in the commandline. My observation from
> looking at the nvcc statistics is that most stack frame size for these cuda
> kernels are 0 (highest observed was 120 bytes).
Right, that makes sense. If Nvidia is happy that this will always work in
drivers compatible with this API version (including any future ones) then sure.
>>
>>> diff --git a/libavutil/hwcontext_cuda.c b/libavutil/hwcontext_cuda.c
>>> index 37827a7..1f022fa 100644
>>> --- a/libavutil/hwcontext_cuda.c
>>> +++ b/libavutil/hwcontext_cuda.c
>>> @@ -386,6 +386,12 @@ static int cuda_device_create(AVHWDeviceContext
>> *ctx, const char *device,
>>> goto error;
>>> }
>>>
>>> +err = cu->cuCtxSetLimit(CU_LIMIT_STACK_SIZE, 128);
>>> +if (err != CUDA_SUCCESS) {
>>> +av_log(ctx, AV_LOG_ERROR, "Error reducing CUDA context stack
>> limit\n");
>>> +goto error;
>>> +}
>>> +
>>> cu->cuCtxPopCurrent(&dummy);
>>>
>>> hwctx->internal->is_allocated = 1;
>>> --
>>> 2.9.1
>>>
>>
>> This is technically a user-visible change, since it will apply to all user
>> programs run on the CUDA context created here as well as those inside
>> ffmpeg. I'm not sure how many people actually use that, though, so maybe
>> it won't affect anyone.
>>
> In ffmpeg, I see vf_thumbnail_cuda and vf_scale_cuda available (not sure if
> there is more, but these two should not be affected by this reduction).
> User can always raise the stack limit size if their own custom kernel
> require higher stack frame size.
I don't mean filters inside ffmpeg, I mean a user program which probably uses
NVDEC and/or NVENC (and possibly other things) from libavcodec but then does
its own CUDA processing with the same context. This is silently changing the
setup underneath it, and 128 feels like a very small number.
>>
>> If the stack limit is violated, what happens? Will that be undefined
>> behaviour with random effects (crash / incorrect results), or is it likely
>> to be caught at program compile/load-time?
>>
> Stack will likely overflow and kernel will terminate (though I have yet
> encounter this before).
As long as the user gets a clear message that a stack overflow has occurred so
that they can realise that they need to raise the value then it should be fine.
Thanks,
- Mark
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH V2] example/vaapi_transcode: Add a VA-API transcode example.
V2: - deduce output format from file extension, because VP8/VP9 encoder
need to a muxer (e,g ivf or/and mp4).
From 9828a8889d5279202c2e8ef9429eb06ad869842b Mon Sep 17 00:00:00 2001
From: Jun Zhao
Date: Thu, 11 Jan 2018 15:00:30 +0800
Subject: [PATCH V2] example/vaapi_transcode: Add a VA-API transcode example.
Add VA-API transcoding example, useage like:
./vaapi_transcode input_stream h264_vaapi output_stream,
and the example can't handle resolution change case.
test case like:
- ./vaapi_transcode input.mp4 h264_vaapi output_h264.mp4
- ./vaapi_transcode input.mp4 vp8_vaapi output_vp8.ivf
Signed-off-by: Jun Zhao
Signed-off-by: Liu, Kaixuan
---
configure | 2 +
doc/examples/Makefile | 1 +
doc/examples/vaapi_transcode.c | 301 +
3 files changed, 304 insertions(+)
create mode 100644 doc/examples/vaapi_transcode.c
diff --git a/configure b/configure
index fcfa7aa442..92aef31cbe 100755
--- a/configure
+++ b/configure
@@ -1526,6 +1526,7 @@ EXAMPLE_LIST="
transcode_aac_example
transcoding_example
vaapi_encode_example
+vaapi_transcode_example
"
EXTERNAL_AUTODETECT_LIBRARY_LIST="
@@ -3320,6 +3321,7 @@ scaling_video_example_deps="avutil swscale"
transcode_aac_example_deps="avcodec avformat swresample"
transcoding_example_deps="avfilter avcodec avformat avutil"
vaapi_encode_example_deps="avcodec avutil h264_vaapi_encoder"
+vaapi_transcode_example_deps="avcodec avformat avutil"
# EXTRALIBS_LIST
cpu_init_extralibs="pthreads_extralibs"
diff --git a/doc/examples/Makefile b/doc/examples/Makefile
index da5af36532..928ff306b3 100644
--- a/doc/examples/Makefile
+++ b/doc/examples/Makefile
@@ -20,6 +20,7 @@ EXAMPLES-$(CONFIG_SCALING_VIDEO_EXAMPLE) += scaling_video
EXAMPLES-$(CONFIG_TRANSCODE_AAC_EXAMPLE) += transcode_aac
EXAMPLES-$(CONFIG_TRANSCODING_EXAMPLE) += transcoding
EXAMPLES-$(CONFIG_VAAPI_ENCODE_EXAMPLE) += vaapi_encode
+EXAMPLES-$(CONFIG_VAAPI_TRANSCODE_EXAMPLE) += vaapi_transcode
EXAMPLES := $(EXAMPLES-yes:%=doc/examples/%$(PROGSSUF)$(EXESUF))
EXAMPLES_G := $(EXAMPLES-yes:%=doc/examples/%$(PROGSSUF)_g$(EXESUF))
diff --git a/doc/examples/vaapi_transcode.c b/doc/examples/vaapi_transcode.c
new file mode 100644
index 00..af1550f5f2
--- /dev/null
+++ b/doc/examples/vaapi_transcode.c
@@ -0,0 +1,301 @@
+/*
+ * Video Acceleration API (video transcoding) transcode sample
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+/**
+ * @file
+ * Intel VAAPI-accelerated transcoding example.
+ *
+ * @example vaapi_transcode.c
+ * This example shows how to do VAAPI-accelerated transcoding.
+ * Usage: vaapi_transcode input_stream h264_vaapi output_stream
+ * e.g: - vaapi_transcode input.mp4 h264_vaapi output_h264.mp4
+ * - vaapi_transcode input.mp4 vp9_vaapi output_vp9.ivf
+ */
+
+#include
+#include
+
+#include
+#include
+#include
+
+static AVFormatContext *ifmt_ctx = NULL, *ofmt_ctx = NULL;
+static AVBufferRef *hw_device_ctx = NULL;
+static AVCodecContext *decoder_ctx = NULL, *encoder_ctx = NULL;
+static int video_stream = -1;
+static AVStream *ost;
+static int initialized = 0;
+
+static enum AVPixelFormat get_vaapi_format(AVCodecContext *ctx,
+ const enum AVPixelFormat *pix_fmts)
+{
+const enum AVPixelFormat *p;
+
+for (p = pix_fmts; *p != AV_PIX_FMT_NONE; p++) {
+if (*p == AV_PIX_FMT_VAAPI)
+return *p;
+}
+
+fprintf(stderr, "Unable to decode this file using VA-API.\n");
+return AV_PIX_FMT_NONE;
+}
+
+static int open_input_file(const char *filename)
+{
+int ret;
+AVCodec *decoder = NULL;
+AVStream *video = NULL;
+
+if ((ret = avformat_open_input(&ifmt_ctx, filename, NULL, NULL)) < 0) {
+fprintf(stderr, "Cannot open input file '%s', Error code: %s\n",
+filename, av_err2str(ret));
+return ret;
+}
+
+if ((ret = avformat_find_stream_info(ifmt_ctx, NULL)) < 0) {
+fprintf(stderr, "Cannot find input stream information. Error code:
%s\n",
+av_err2str(ret));
+return ret;
+}
+
+ret = av_find_best_stream(ifmt_ctx, AVMEDIA_TYPE_VIDEO, -1, -1, &decoder,
0);
+if (ret < 0) {
+f
[FFmpeg-devel] [PATCH] avformat/mpegenc - fix PCM 16BE muxing and disallow, unsupported audio codecs
From 7c31072230e392ca8be218df4a1f5a2c1b5d Mon Sep 17 00:00:00 2001
From: Gyan Doshi
Date: Fri, 26 Jan 2018 19:15:28 +0530
Subject: [PATCH] avformat/mpegenc - fix PCM 16BE muxing and disallow
unsupported audio codecs
PCM_S16BE streams in MPEG-PS are recognized as PCM_DVD by the demuxer
which prevents their proper remuxing in MPEG-1/2 PS.
In addition, the muxer only supports five audio codecs but will silently
go ahead and process any codec. Demuxing of these other streams fails.
Couple of error messages added and one error typo (VBV size) corrected.
---
libavformat/mpegenc.c | 27 +--
1 file changed, 21 insertions(+), 6 deletions(-)
diff --git a/libavformat/mpegenc.c b/libavformat/mpegenc.c
index c77c3dfe41..1b20cb7282 100644
--- a/libavformat/mpegenc.c
+++ b/libavformat/mpegenc.c
@@ -353,7 +353,8 @@ static av_cold int mpeg_mux_init(AVFormatContext *ctx)
if (!s->is_mpeg2 &&
(st->codecpar->codec_id == AV_CODEC_ID_AC3 ||
st->codecpar->codec_id == AV_CODEC_ID_DTS ||
- st->codecpar->codec_id == AV_CODEC_ID_PCM_S16BE))
+ st->codecpar->codec_id == AV_CODEC_ID_PCM_S16BE ||
+ st->codecpar->codec_id == AV_CODEC_ID_PCM_DVD))
av_log(ctx, AV_LOG_WARNING,
"%s in MPEG-1 system streams is not widely supported, "
"consider using the vob or the dvd muxer "
@@ -363,20 +364,34 @@ static av_cold int mpeg_mux_init(AVFormatContext *ctx)
stream->id = ac3_id++;
} else if (st->codecpar->codec_id == AV_CODEC_ID_DTS) {
stream->id = dts_id++;
-} else if (st->codecpar->codec_id == AV_CODEC_ID_PCM_S16BE) {
+} else if (st->codecpar->codec_id == AV_CODEC_ID_PCM_S16BE ||
+ st->codecpar->codec_id == AV_CODEC_ID_PCM_DVD) {
stream->id = lpcm_id++;
for (j = 0; j < 4; j++) {
if (lpcm_freq_tab[j] == st->codecpar->sample_rate)
break;
}
-if (j == 4)
+if (j == 4) {
+int sr;
+av_log(ctx, AV_LOG_ERROR, "Invalid sampling rate for PCM
stream.\n");
+av_log(ctx, AV_LOG_INFO, "Allowed sampling rates:");
+for (sr = 0; sr < 4; sr++)
+av_log(ctx, AV_LOG_INFO, " %d", lpcm_freq_tab[sr]);
+av_log(ctx, AV_LOG_INFO, "\n");
goto fail;
-if (st->codecpar->channels > 8)
-return -1;
+}
+if (st->codecpar->channels > 8) {
+av_log(ctx, AV_LOG_ERROR, "At most 8 channels allowed for
LPCM stream.\n");
+return AVERROR(EINVAL);
+}
stream->lpcm_header[0] = 0x0c;
stream->lpcm_header[1] = (st->codecpar->channels - 1) | (j <<
4);
stream->lpcm_header[2] = 0x80;
stream->lpcm_align = st->codecpar->channels * 2;
+} else if (st->codecpar->codec_id != AV_CODEC_ID_MP2 &&
+ st->codecpar->codec_id != AV_CODEC_ID_MP3) {
+ av_log(ctx, AV_LOG_ERROR, "Unsupported audio codec.
Must be one of mp2, mp3, pcm_s16be, ac3 or dts.\n");
+ return AVERROR(EINVAL);
} else {
stream->id = mpa_id++;
}
@@ -397,7 +412,7 @@ static av_cold int mpeg_mux_init(AVFormatContext *ctx)
stream->max_buffer_size = 6 * 1024 + props->buffer_size / 8;
else {
av_log(ctx, AV_LOG_WARNING,
- "VBV buffer size not set, using default size of 130KB\n"
+ "VBV buffer size not set, using default size of 230KB\n"
"If you want the mpeg file to be compliant to some
specification\n"
"Like DVD, VCD or others, make sure you set the correct
buffer size\n");
// FIXME: this is probably too small as default
--
2.11.1.windows.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] rtsp: rename certain options after a deprecation period
On Fri, Jan 26, 2018 at 02:45:47AM +0200, Jan Ekström wrote: > On Fri, Jan 26, 2018 at 1:56 AM, Carl Eugen Hoyos wrote: > > 2018-01-26 0:52 GMT+01:00 wm4 : > >> (and I already wrote that on IRC too, where he lurks as > >> michaelni) > > > > Could one of the native speakers please try to convince > > me that this is not a disparaging term? > > > > Hi, > > I am not an English native, but the verb "to lurk" is colloquially utilized > as: > > "read the postings in an Internet forum without actively contributing." > (quote from whatever dictionary Google utilizes) > > ...and this IMHO is applicable enough as Michael is busy just like > many of us are busy most of the day. > > Now, please, all parties. Stop. We've had enough rudeness in this > thread. And now, back to on-topic: > > * This change set offers to rename some options to make them similar > to how the TCP protocol does things > (http://git.videolan.org/?p=ffmpeg.git;a=blob;f=libavformat/tcp.c;h=8773493df1efebac33cff5d203c8c9ff17299d4b;hb=HEAD#l52). > UDP protocol seems to follow suite regarding the "timeout" parameter, > even matching the microsecond part? > * It's imperfect since it doesn't change any of the types of the > options (listen_timeout is now name-wise matched, but type-wise is > supposed to contain seconds for rtsp, milliseconds (! yet another time > scale) in tcp). > * But if we do not rename the current timeout parameter (which does a > different thing than timeout for both TCP and UDP - and name-wise > matches "listen_timeout" as far as it can be seen), we cannot have the > matching name for the matching type of - which should be under the > option "timeout". > > So, as far as I can see for now applying this or a slightly modified > version of this as per James' comments doesn't seem to be a too bad > initial step. It brings at least one option in line. We can then start > the larger job of normalizing the timeout parameter across FFmpeg, as > it seems like mentioning this area made multiple people notice > possible improvements in this area. But this is just my 2 eurocents. I think we first should take a step back and decide how we want a consistent API to look exactly. Then make changes towards that. This patch introduces a parameter that very possibly will have to be deprecated when things are changed to consistent second based timeouts. I mean this patch makes rtsp consistent with parameters that we may have to deprecate. Lets just for a moment assume we agree that we want to have consistent second based timeouts throughout the codebase. To achive this with minimal confusion it requires a new parameter name that has not been used for micro or nano or milli seconds before. If now a new parameter for timeouts is added in seconds then each piece of code can independantly add support to it one by one never conflicting with anything or depending on anything. And all existing timeouts that are not in seconds would then become deprecated. I think thats a meaningfull way forward and i would suggest that we go that direction. Its very easy to do too once there is agreement on the name of the new parameter(s) We can also apply this patch but its not really moving us closer to consistent second based timeouts. thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB When you are offended at any man's fault, turn to yourself and study your own failings. Then you will forget your anger. -- Epictetus signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/2] lavc: Add coded_w/h to AVCodecParameters
On Fri, Jan 19, 2018 at 01:15:13PM -0300, James Almer wrote:
> On 1/19/2018 7:12 AM, Hendrik Leppkes wrote:
> > On Fri, Jan 19, 2018 at 4:19 AM, Li, Zhong wrote:
> >>> -Original Message-
> >>> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> >>> Of James Almer
> >>> Sent: Thursday, January 18, 2018 1:15 PM
> >>> To: ffmpeg-devel@ffmpeg.org
> >>> Subject: Re: [FFmpeg-devel] [PATCH 1/2] lavc: Add coded_w/h to
> >>> AVCodecParameters
> >>>
> >>> On 1/18/2018 2:03 AM, Zhong Li wrote:
> coded_width/height may be different from width/height sometimes
> >>>
> (e.g, crop or lowres cases).
> >>>
> >>> Which is why it's not a field that belongs to AVCodecParameters.
> >>>
> >>> Codec level cropping has nothing to do with containers. Same with lowres,
> >>> which is an internal feature, and scheduled for removal.
> >>
> >> Got it. How about fixing ticket #6958 as below?
> >>
> >> diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
> >> index 0e7a771..233760d 100644
> >> --- a/fftools/ffprobe.c
> >> +++ b/fftools/ffprobe.c
> >> @@ -2512,10 +2512,12 @@ static int show_stream(WriterContext *w,
> >> AVFormatContext *fmt_ctx, int stream_id
> >> case AVMEDIA_TYPE_VIDEO:
> >> print_int("width",par->width);
> >> print_int("height", par->height);
> >> +#if FF_API_LAVF_AVCTX
> >> if (dec_ctx) {
> >> print_int("coded_width", dec_ctx->coded_width);
> >> print_int("coded_height", dec_ctx->coded_height);
> >> }
> >> +#endif
> >> print_int("has_b_frames", par->video_delay);
> >> sar = av_guess_sample_aspect_ratio(fmt_ctx, stream, NULL);
> >> if (sar.den) {
> >> @@ -2912,6 +2914,10 @@ static int open_input_file(InputFile *ifile, const
> >> char *filename)
> >>
> >> ist->dec_ctx->pkt_timebase = stream->time_base;
> >> ist->dec_ctx->framerate = stream->avg_frame_rate;
> >> +#if FF_API_LAVF_AVCTX
> >> +ist->dec_ctx->coded_width = stream->codec->coded_width;
> >> +ist->dec_ctx->coded_height = stream->codec->coded_height;
> >> +#endif
> >>
> >
> > As mentioned in the thread for that patch already, writing new code
> > using deprecated API should really be avoided.
> >
> > The way I see it, if someone really needs to know coded w/h (which is
> > typically an internal technical detail of no relevance to users), they
> > should decode a frame and get it from the decoder.
> >
> > - Hendrik
>
> This specific approach is IMO acceptable. It basically recovers the
> pre-codecpar behavior until AVStream->codec is removed, and effectively
> fixes the "regression".
> Once that's gone, ffprobe will stop reporting coded_width/height
> altogether instead of doing so with a bogus value, as everything is
> being wrapped in the proper checks.
+1
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Many things microsoft did are stupid, but not doing something just because
microsoft did it is even more stupid. If everything ms did were stupid they
would be bankrupt already.
signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] GSoC 2018
On 15 January 2018 at 22:08, Pedro Arthur wrote: > Added an entry in the ideas page for the super resolution project. I'd like > to know if any one could be co-mentor with me? just in case my studies > conflicts with mentoring as gsoc overlaps with half of my phd period. > Also I need to think about a reasonable qualification task, if anyone have > any idea let me know. > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > I think actually writing and reading papers for the filter is pointless - RAVU already exists in shader form, its well known in the community and we'll essentially get it for free once the Vulkan backend is complete. What the task should be would be to make a CPU-only version of the shaders. Also the qualification task you just put in is already done - there's already a filter to convolve 2 images. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] GSoC 2018
2018-01-26 14:26 GMT-02:00 Rostislav Pehlivanov : > I think actually writing and reading papers for the filter is pointless - > RAVU already exists in shader form, its well known in the community and > we'll essentially get it for free once the Vulkan backend is complete. > What the task should be would be to make a CPU-only version of the shaders. > I didn't know about these filters, I did a quick search and found a github repo "mvp-prefilters" but there isn't any documentation explaining the internals of it. Can you point me some resources where I can find for example which models are they using, motivation and maybe some benchmark with other models? My initial idea was that the student would evaluate a few SR models, make a balance based on quality/performance and pick the best fit. If it's already done there is only the porting task, but I guess maybe there is newer and unevaluated models or we could improve something. Also the qualification task you just put in is already done - there's > already a filter to convolve 2 images. > Should I revert the wiki? or you have an idea for a qualification task? > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > Thanks, Pedro. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] GSoC 2018
Am 26.01.18 um 18:15 schrieb Pedro Arthur: > 2018-01-26 14:26 GMT-02:00 Rostislav Pehlivanov : > >> I think actually writing and reading papers for the filter is pointless - >> RAVU already exists in shader form, its well known in the community and >> we'll essentially get it for free once the Vulkan backend is complete. >> What the task should be would be to make a CPU-only version of the shaders. >> > I didn't know about these filters, I did a quick search and found a github > repo "mvp-prefilters" but there isn't any documentation explaining the > internals of it. > Can you point me some resources where I can find for example which models > are they using, motivation and maybe some benchmark with other models? > My initial idea was that the student would evaluate a few SR models, make a > balance based on quality/performance and pick the best fit. > If it's already done there is only the porting task, but I guess maybe > there is newer and unevaluated models or we could improve something. > > Also the qualification task you just put in is already done - there's >> already a filter to convolve 2 images. >> > Should I revert the wiki? or you have an idea for a qualification task? No need to revert it. Just update the qual task one a better one is found. Thanks, Thilo ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]: Change Stack Frame Limit in Cuda Context
Am 26.01.2018 um 10:06 schrieb Ben Chang:
Thanks for the review Mark.
On Thu, Jan 25, 2018 at 4:13 PM, Mark Thompson wrote:
diff --git a/libavcodec/nvenc.c b/libavcodec/nvenc.c
index 4a91d99..2da251b 100644
--- a/libavcodec/nvenc.c
+++ b/libavcodec/nvenc.c
@@ -420,6 +420,12 @@ static av_cold int nvenc_check_device(AVCodecContext
*avctx, int idx)
goto fail;
}
+cu_res = dl_fn->cuda_dl->cuCtxSetLimit(CU_LIMIT_STACK_SIZE, 128);
+if (cu_res != CUDA_SUCCESS) {
+av_log(avctx, AV_LOG_FATAL, "Failed reducing CUDA context stack
limit for NVENC: 0x%x\n", (int)cu_res);
+goto fail;
+}
+
ctx->cu_context = ctx->cu_context_internal;
if ((ret = nvenc_pop_context(avctx)) < 0)
Does this actually have any effect? I was under the impression that the
CUDA context created inside the NVENC encoder wouldn't actually be used for
any CUDA operations at all (really just a GPU device handle).
There are some cuda kernels in the driver that may be invoked depending on
the nvenc operations specified in the commandline. My observation from
looking at the nvcc statistics is that most stack frame size for these cuda
kernels are 0 (highest observed was 120 bytes).
Wouldn't it affect potential future CUDA filters, which might make more
use of the stack?
smime.p7s
Description: S/MIME Cryptographic Signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] avcodec/nvenc: refcount input frame mappings
If some logic like vsync in ffmpeg.c duplicates frames, it might pass
the same frame twice, which will result in a crash due it being
effectively mapped and unmapped twice.
Signed-off-by: Timo Rothenpieler
---
libavcodec/nvenc.c | 39 +++
libavcodec/nvenc.h | 2 +-
2 files changed, 24 insertions(+), 17 deletions(-)
diff --git a/libavcodec/nvenc.c b/libavcodec/nvenc.c
index 4a91d99720..0ecaa15162 100644
--- a/libavcodec/nvenc.c
+++ b/libavcodec/nvenc.c
@@ -1389,12 +1389,9 @@ av_cold int ff_nvenc_encode_close(AVCodecContext *avctx)
av_fifo_freep(&ctx->unused_surface_queue);
if (ctx->surfaces && (avctx->pix_fmt == AV_PIX_FMT_CUDA || avctx->pix_fmt
== AV_PIX_FMT_D3D11)) {
-for (i = 0; i < ctx->nb_surfaces; ++i) {
-if (ctx->surfaces[i].input_surface) {
- p_nvenc->nvEncUnmapInputResource(ctx->nvencoder,
ctx->surfaces[i].in_map.mappedResource);
-}
-}
for (i = 0; i < ctx->nb_registered_frames; i++) {
+if (ctx->registered_frames[i].mapped)
+p_nvenc->nvEncUnmapInputResource(ctx->nvencoder,
ctx->registered_frames[i].in_map.mappedResource);
if (ctx->registered_frames[i].regptr)
p_nvenc->nvEncUnregisterResource(ctx->nvencoder,
ctx->registered_frames[i].regptr);
}
@@ -1629,19 +1626,23 @@ static int nvenc_upload_frame(AVCodecContext *avctx,
const AVFrame *frame,
if (res < 0)
return res;
-nvenc_frame->in_map.version = NV_ENC_MAP_INPUT_RESOURCE_VER;
-nvenc_frame->in_map.registeredResource =
ctx->registered_frames[reg_idx].regptr;
-nv_status = p_nvenc->nvEncMapInputResource(ctx->nvencoder,
&nvenc_frame->in_map);
-if (nv_status != NV_ENC_SUCCESS) {
-av_frame_unref(nvenc_frame->in_ref);
-return nvenc_print_error(avctx, nv_status, "Error mapping an input
resource");
+if (!ctx->registered_frames[reg_idx].mapped) {
+ctx->registered_frames[reg_idx].in_map.version =
NV_ENC_MAP_INPUT_RESOURCE_VER;
+ctx->registered_frames[reg_idx].in_map.registeredResource =
ctx->registered_frames[reg_idx].regptr;
+nv_status = p_nvenc->nvEncMapInputResource(ctx->nvencoder,
&ctx->registered_frames[reg_idx].in_map);
+if (nv_status != NV_ENC_SUCCESS) {
+av_frame_unref(nvenc_frame->in_ref);
+return nvenc_print_error(avctx, nv_status, "Error mapping an
input resource");
+}
}
-ctx->registered_frames[reg_idx].mapped = 1;
+ctx->registered_frames[reg_idx].mapped += 1;
+
nvenc_frame->reg_idx = reg_idx;
-nvenc_frame->input_surface =
nvenc_frame->in_map.mappedResource;
-nvenc_frame->format=
nvenc_frame->in_map.mappedBufferFmt;
+nvenc_frame->input_surface =
ctx->registered_frames[reg_idx].in_map.mappedResource;
+nvenc_frame->format=
ctx->registered_frames[reg_idx].in_map.mappedBufferFmt;
nvenc_frame->pitch = frame->linesize[0];
+
return 0;
} else {
NV_ENC_LOCK_INPUT_BUFFER lockBufferParams = { 0 };
@@ -1793,9 +1794,15 @@ static int process_output_surface(AVCodecContext *avctx,
AVPacket *pkt, NvencSur
if (avctx->pix_fmt == AV_PIX_FMT_CUDA || avctx->pix_fmt ==
AV_PIX_FMT_D3D11) {
-p_nvenc->nvEncUnmapInputResource(ctx->nvencoder,
tmpoutsurf->in_map.mappedResource);
+ctx->registered_frames[tmpoutsurf->reg_idx].mapped -= 1;
+if (ctx->registered_frames[tmpoutsurf->reg_idx].mapped == 0) {
+p_nvenc->nvEncUnmapInputResource(ctx->nvencoder,
ctx->registered_frames[tmpoutsurf->reg_idx].in_map.mappedResource);
+} else if (ctx->registered_frames[tmpoutsurf->reg_idx].mapped < 0) {
+res = AVERROR_BUG;
+goto error;
+}
+
av_frame_unref(tmpoutsurf->in_ref);
-ctx->registered_frames[tmpoutsurf->reg_idx].mapped = 0;
tmpoutsurf->input_surface = NULL;
}
diff --git a/libavcodec/nvenc.h b/libavcodec/nvenc.h
index 2e51f1e946..ab6825f633 100644
--- a/libavcodec/nvenc.h
+++ b/libavcodec/nvenc.h
@@ -44,7 +44,6 @@ typedef struct NvencSurface
{
NV_ENC_INPUT_PTR input_surface;
AVFrame *in_ref;
-NV_ENC_MAP_INPUT_RESOURCE in_map;
int reg_idx;
int width;
int height;
@@ -131,6 +130,7 @@ typedef struct NvencContext
int ptr_index;
NV_ENC_REGISTERED_PTR regptr;
int mapped;
+NV_ENC_MAP_INPUT_RESOURCE in_map;
} registered_frames[MAX_REGISTERED_FRAMES];
int nb_registered_frames;
--
2.15.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] mpeg4video: Add support for MPEG-4 Simple Studio Profile.
On Mon, Jan 22, 2018 at 2:07 AM, Michael Niedermayer wrote: > On Sun, Jan 21, 2018 at 12:37:21PM +, Kieran Kunhya wrote: > > On Mon, Jan 1, 2018 at 7:01 PM, Michael Niedermayer > > > wrote: > > > > > Hi > > > > > > > Patch updated. > > Some of the review comments I decided not to implement in order to keep > > closer to the spec. > > honestly, this reasoning makes no sense to me. > that spec does IIRC not even fully describe some parts in DPCM > > also all existing mpeg&h26x code we have was designed for speed > basically ignoring if its close to some spec implementation style or not > > I would prefer we try to make our code as good as we can and not try to > be similar to some specification or reference implementation. > except for maybe some niche codecs where performance totally doesnt matter > It does describe DPCM, just in an odd way. I am going to implement your ricing changes but for the record your passive aggressiveness to many developers is awful. Please learn to compromise. Kieran ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] mpeg4video: Add support for MPEG-4 Simple Studio Profile.
On Mon, Jan 22, 2018 at 10:53 AM, Carl Eugen Hoyos wrote: > 2018-01-22 11:41 GMT+01:00 Marton Balint : > > > Your command line shows gcc 4.7. Maybe this is > > some compiler issue then? > > I can also reproduce the issue with vanilla gcc 6.3.0, > so I don't think this is a compiler issue. Do you plan to produce a valid testcase or not? I would note that if this was a trac ticket you would have closed your own report. Kieran ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] rtsp: rename certain options after a deprecation period
On Thu, 25 Jan 2018 19:00:43 +0100 wm4 wrote: > The names inherently clash with the meanings of the HTTP libavformat > protocol options. Rename them after a deprecation period to make them > compatible with the HTTP ones. > --- > I see no better way that wouldn't require more effort than justified. > The incompatible semantics of the "timeout" option while still clashing > with the HTTP one caused major problems to me as API user, and I'm > hoping that this will solve itself in 2 years. > --- I'll push this in a few days or so with James Almers change requests applied. There was a lot of other bikeshedding and personal insults, but no actual rejection or further mandatory change requests. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]: Change Stack Frame Limit in Cuda Context
On Fri, Jan 26, 2018 at 3:32 AM, Mark Thompson wrote: > On 26/01/18 09:06, Ben Chang wrote: > > Thanks for the review Mark. > > > > There are some cuda kernels in the driver that may be invoked depending > on > > the nvenc operations specified in the commandline. My observation from > > looking at the nvcc statistics is that most stack frame size for these > cuda > > kernels are 0 (highest observed was 120 bytes). > > Right, that makes sense. If Nvidia is happy that this will always work in > drivers compatible with this API version (including any future ones) then > sure. > I am not saying this should be the "permanent" value for stack frame size per GPU thread. However, at this moment (looking at existing cuda kernels that devs have control over), I do not see this reduction as an issue. > > >> > >> > >> This is technically a user-visible change, since it will apply to all > user > >> programs run on the CUDA context created here as well as those inside > >> ffmpeg. I'm not sure how many people actually use that, though, so > maybe > >> it won't affect anyone. > >> > > In ffmpeg, I see vf_thumbnail_cuda and vf_scale_cuda available (not sure > if > > there is more, but these two should not be affected by this reduction). > > User can always raise the stack limit size if their own custom kernel > > require higher stack frame size. > > I don't mean filters inside ffmpeg, I mean a user program which probably > uses NVDEC and/or NVENC (and possibly other things) from libavcodec but > then does its own CUDA processing with the same context. This is silently > changing the setup underneath it, and 128 feels like a very small number. > Yes, this is really a trade off between reducing memory usage (since there are numerous complaints of high memory usage preventing having more ffmpeg instances) and user convenience (custom cuda implementation may be impacted). My thought (which can be wrong) is that users who implement their own cuda kernel may have better knowledge about cuda (eg. how much stack frame size their kernel needs or use cuda debugger to find out what issue they may have). The size of the kernels are really implementation dependent (eg, allocating arrays in stacks or heap, recursions, how much register spills, etc) so stack frame sizes may vary widely. The default, 1024 bytes, may not be enough at times and user will need to adjust the stack limit accordingly anyway. > > >> > >> If the stack limit is violated, what happens? Will that be undefined > >> behaviour with random effects (crash / incorrect results), or is it > likely > >> to be caught at program compile/load-time? > >> > > Stack will likely overflow and kernel will terminate (though I have yet > > encounter this before). > > As long as the user gets a clear message that a stack overflow has > occurred so that they can realise that they need to raise the value then it > should be fine. I believe you will see stack overflow if attached to cuda debugger. But the default error may just be kernel launch error/failure. This goes back to my opinion that cuda developer should figure this out relatively easy if they want to customize the cuda part of their program. Copying Timo's comment from another thread to consolidate discussion. >>Wouldn't it affect potential future CUDA filters, which might make more use of the stack? If nvidia introduces a new kernel that exceed this limit, changes will need to be made (but I do not think is anytime soon). Thanks, Ben ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] mov: do not mark timed thumbnail tracks as attached picture
The AV_DISPOSITION_ATTACHED_PIC flag is meant only for exporting a
static picture (such as embedded cover art) as pseudo video track. The
requirement is that there is exactly 1 packet, and that normal demuxing
does not return it (otherwise avformat_queue_attached_pictures() would
add it a second time). It should never used on actual video tracks that
return packets when demuxing.
I considered keeping the current behavior if there is exactly 1 frame
(according to nb_index_entries), but that would require additional work
to make sure avformat_queue_attached_pictures() does not add it a second
time, so I didn't bother.
Reverts part of 697400eac07c0614f6b9f2e7615563982dbcbe4a and fixes
regressions with certain API users with such mp4 files.
---
libavformat/mov.c | 18 +-
1 file changed, 1 insertion(+), 17 deletions(-)
diff --git a/libavformat/mov.c b/libavformat/mov.c
index 22faecfc17..f74be03359 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -6335,23 +6335,7 @@ static void mov_read_chapters(AVFormatContext *s)
cur_pos = avio_tell(sc->pb);
if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO) {
-st->disposition |= AV_DISPOSITION_ATTACHED_PIC |
AV_DISPOSITION_TIMED_THUMBNAILS;
-if (st->nb_index_entries) {
-// Retrieve the first frame, if possible
-AVPacket pkt;
-AVIndexEntry *sample = &st->index_entries[0];
-if (avio_seek(sc->pb, sample->pos, SEEK_SET) != sample->pos) {
-av_log(s, AV_LOG_ERROR, "Failed to retrieve first
frame\n");
-goto finish;
-}
-
-if (av_get_packet(sc->pb, &pkt, sample->size) < 0)
-goto finish;
-
-st->attached_pic = pkt;
-st->attached_pic.stream_index = st->index;
-st->attached_pic.flags |= AV_PKT_FLAG_KEY;
-}
+st->disposition |= AV_DISPOSITION_TIMED_THUMBNAILS;
} else {
st->codecpar->codec_type = AVMEDIA_TYPE_DATA;
st->codecpar->codec_id = AV_CODEC_ID_BIN_DATA;
--
2.15.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]: Change Stack Frame Limit in Cuda Context
On 26/01/18 20:51, Ben Chang wrote: > On Fri, Jan 26, 2018 at 3:32 AM, Mark Thompson wrote: > >> On 26/01/18 09:06, Ben Chang wrote: >>> Thanks for the review Mark. >>> To clarify, since it is less clear now with the trimmed context: my two comments about this change are completely independent. (Given that, maybe it should be split into two parts - one for hwcontext and one for nvenc?) This part is about the change to NVENC: >>> There are some cuda kernels in the driver that may be invoked depending >> on >>> the nvenc operations specified in the commandline. My observation from >>> looking at the nvcc statistics is that most stack frame size for these >> cuda >>> kernels are 0 (highest observed was 120 bytes). >> >> Right, that makes sense. If Nvidia is happy that this will always work in >> drivers compatible with this API version (including any future ones) then >> sure. >> > I am not saying this should be the "permanent" value for stack frame size > per GPU thread. However, at this moment (looking at existing cuda kernels > that devs have control over), I do not see this reduction as an issue. I think you should be confident that the chosen value here will last well into the future for NVENC use. Consider that this will end up in releases - if a future Nvidia driver update happens to need a larger stack then all previous releases and binaries will stop working for all users. This part is about the change to the hwcontext device creation: This is technically a user-visible change, since it will apply to all >> user programs run on the CUDA context created here as well as those inside ffmpeg. I'm not sure how many people actually use that, though, so >> maybe it won't affect anyone. >>> In ffmpeg, I see vf_thumbnail_cuda and vf_scale_cuda available (not sure >> if >>> there is more, but these two should not be affected by this reduction). >>> User can always raise the stack limit size if their own custom kernel >>> require higher stack frame size. >> >> I don't mean filters inside ffmpeg, I mean a user program which probably >> uses NVDEC and/or NVENC (and possibly other things) from libavcodec but >> then does its own CUDA processing with the same context. This is silently >> changing the setup underneath it, and 128 feels like a very small number. >> > Yes, this is really a trade off between reducing memory usage (since there > are numerous complaints of high memory usage preventing having more ffmpeg > instances) and user convenience (custom cuda implementation may be > impacted). My thought (which can be wrong) is that users who implement > their own cuda kernel may have better knowledge about cuda (eg. how much > stack frame size their kernel needs or use cuda debugger to find out what > issue they may have). The size of the kernels are really implementation > dependent (eg, allocating arrays in stacks or heap, recursions, how much > register spills, etc) so stack frame sizes may vary widely. The default, > 1024 bytes, may not be enough at times and user will need to adjust the > stack limit accordingly anyway. Note that since you are changing a library the users in this context are all ffmpeg library users. So, it means any program or library which uses ffmpeg, and transitively anyone who uses them. The end-user need not be informed about CUDA at all. (What you've said also makes it sound like it can change by compiler version, but I guess such changes should be small.) If the stack limit is violated, what happens? Will that be undefined behaviour with random effects (crash / incorrect results), or is it >> likely to be caught at program compile/load-time? >>> Stack will likely overflow and kernel will terminate (though I have yet >>> encounter this before). >> >> As long as the user gets a clear message that a stack overflow has >> occurred so that they can realise that they need to raise the value then it >> should be fine. > > I believe you will see stack overflow if attached to cuda debugger. But the > default error may just be kernel launch error/failure. This goes back to my > opinion that cuda developer should figure this out relatively easy if they > want to customize the cuda part of their program. Ok, sure. It's possibly unfortunate if a user who knows nothing about CUDA rebuilds a program from source with newer ffmpeg libraries including this change because something could stop working in an opaque way, but if it errors out then it won't silently give the wrong answer and should be diagnosable by the original developer. Thanks, - Mark ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH V2 2/2] Don't overwrite previously setup dimensions for all codecs
On Thu, Jan 18, 2018 at 01:03:34PM +0800, Zhong Li wrote:
> Currently a hacky way is used for some specific codecs such as
> H264/VP6F/DXV (and "lowres" case is broken now).
> Replace with a more generic way(an evolution based on a libav commit
> 9de9b828 but hasn't been merged since it breaks lowres).
>
> V1->V2: add "lowres" handle code
>
> Signed-off-by: Zhong Li
> ---
> libavcodec/utils.c | 8 ++--
> 1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> index 427f612..fdd1b46 100644
> --- a/libavcodec/utils.c
> +++ b/libavcodec/utils.c
> @@ -684,16 +684,12 @@ int attribute_align_arg avcodec_open2(AVCodecContext
> *avctx, const AVCodec *code
> goto free_and_end;
> }
>
> -// only call ff_set_dimensions() for non H.264/VP6F/DXV codecs so as not
> to overwrite previously setup dimensions
> -if (!(avctx->coded_width && avctx->coded_height && avctx->width &&
> avctx->height &&
> - (avctx->codec_id == AV_CODEC_ID_H264 || avctx->codec_id ==
> AV_CODEC_ID_VP6F || avctx->codec_id == AV_CODEC_ID_DXV))) {
> -if (avctx->coded_width && avctx->coded_height)
> +if (avctx->coded_width && avctx->coded_height && (!avctx->width &&
> !avctx->height || avctx->lowles))
> ret = ff_set_dimensions(avctx, avctx->coded_width,
> avctx->coded_height);
> -else if (avctx->width && avctx->height)
> +else if (avctx->width && avctx->height && (!avctx->coded_width &&
> !avctx->coded_height || avctx->lowles))
> ret = ff_set_dimensions(avctx, avctx->width, avctx->height);
> if (ret < 0)
> goto free_and_end;
> -}
This has typos in variable names, it will not work nor build
please make sure that submitted patches have been tested before
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
When you are offended at any man's fault, turn to yourself and study your
own failings. Then you will forget your anger. -- Epictetus
signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] mpeg4video: Add support for MPEG-4 Simple Studio Profile.
On Fri, Jan 26, 2018 at 08:04:39PM +, Kieran Kunhya wrote: > On Mon, Jan 22, 2018 at 2:07 AM, Michael Niedermayer > wrote: > > > On Sun, Jan 21, 2018 at 12:37:21PM +, Kieran Kunhya wrote: > > > On Mon, Jan 1, 2018 at 7:01 PM, Michael Niedermayer > > > > > wrote: > > > > > > > Hi > > > > > > > > > > > Patch updated. > > > Some of the review comments I decided not to implement in order to keep > > > closer to the spec. > > > > honestly, this reasoning makes no sense to me. > > that spec does IIRC not even fully describe some parts in DPCM > > > > also all existing mpeg&h26x code we have was designed for speed > > basically ignoring if its close to some spec implementation style or not > > > > I would prefer we try to make our code as good as we can and not try to > > be similar to some specification or reference implementation. > > except for maybe some niche codecs where performance totally doesnt matter > > > > It does describe DPCM, just in an odd way. The specification i read did not specify DPCM in a way one could implement aka the description was not complete. and it also was not understandable to you as you asked on IRC about it ages ago Jän 22 03:12:54michaelni: don't know if I am missing something here but does it say anywhere what the length of dpcm_residual is? Jän 22 03:41:31I'm guessing it should read 8-12? Jän 22 03:41:41in 14496-2 ... Dez 23 01:55:30atomnuker: any idea how to calculate the length of dpcm_residual Dez 23 01:57:19it just says "dpcm_residual: This is an unsigned integer that indicates the value of a DPCM residual" Dez 23 01:58:08 shouldnt the definiton of "uimsbf" tell you how to parse those values Dez 23 01:58:25sure, but doesn't explain the 4-12 Dez 23 01:58:44 how did you figure out the length of say block_mean. its 8-12 uimsbf Dez 23 01:59:10block mean is the average of the block and the supported bitdepths are 8-12 Dez 23 02:01:31 so basically the format syntax is just bonkers Dez 23 02:10:12there must be something I'm missing If the spec makes sense to you now, please explain because it made no sense to me last i looked. I genuinely do not understand what it meant > I am going to implement your ricing changes but for the record your passive > aggressiveness to many developers is awful. i appologize if something i said sounded passive aggerssive, it was not intended to. > Please learn to compromise. you refer to myself pushing towards a optimial implementation and not the spec? as in " I would prefer we try to make our code as good as we can and not try to be similar to some specification or reference implementation." ? Dont you agree that this is the better choice for teh codebase even though yes it is certainly more work ? [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB If a bugfix only changes things apparently unrelated to the bug with no further explanation, that is a good sign that the bugfix is wrong. signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] mov: do not mark timed thumbnail tracks as attached picture
On Sat, 27 Jan 2018, wm4 wrote:
The AV_DISPOSITION_ATTACHED_PIC flag is meant only for exporting a
static picture (such as embedded cover art) as pseudo video track. The
requirement is that there is exactly 1 packet, and that normal demuxing
does not return it (otherwise avformat_queue_attached_pictures() would
add it a second time). It should never used on actual video tracks that
return packets when demuxing.
Docs in avformat.h only says that AV_DISPOSITION_ATTACHED_PIC "usually"
contains one packet.
I considered keeping the current behavior if there is exactly 1 frame
(according to nb_index_entries), but that would require additional work
to make sure avformat_queue_attached_pictures() does not add it a second
time, so I didn't bother.
Reverts part of 697400eac07c0614f6b9f2e7615563982dbcbe4a and fixes
regressions with certain API users with such mp4 files.
---
libavformat/mov.c | 18 +-
1 file changed, 1 insertion(+), 17 deletions(-)
diff --git a/libavformat/mov.c b/libavformat/mov.c
index 22faecfc17..f74be03359 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -6335,23 +6335,7 @@ static void mov_read_chapters(AVFormatContext *s)
cur_pos = avio_tell(sc->pb);
if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO) {
-st->disposition |= AV_DISPOSITION_ATTACHED_PIC |
AV_DISPOSITION_TIMED_THUMBNAILS;
-if (st->nb_index_entries) {
-// Retrieve the first frame, if possible
-AVPacket pkt;
-AVIndexEntry *sample = &st->index_entries[0];
-if (avio_seek(sc->pb, sample->pos, SEEK_SET) != sample->pos) {
-av_log(s, AV_LOG_ERROR, "Failed to retrieve first
frame\n");
-goto finish;
-}
-
-if (av_get_packet(sc->pb, &pkt, sample->size) < 0)
-goto finish;
-
-st->attached_pic = pkt;
-st->attached_pic.stream_index = st->index;
-st->attached_pic.flags |= AV_PKT_FLAG_KEY;
-}
+st->disposition |= AV_DISPOSITION_TIMED_THUMBNAILS;
AV_DISPOSITION_TIMED_THUMBNAILS is only ever used with
AV_DISPOSITION_ATTACHED_PIC according to the docs in avformat.h.
Like it or not, that is how the flag was introduced, so I'd rather not
change it now. It made sense to introduce it this way, because checking
for the ATTACHED_PIC flag was used (for example in ffmpeg when using the
capital V stream speicifier) to search for ordinary video streams. By
using this flag for timed thumbnails as well, applications did not have to
check for an additional flag when searching for ordinary video streams.
So I am against this patch, probably better to fix the regression in the
API user side, because it seems to me this is one of those cases where
something will regress no matter what we do, so it is better to not cause
new regressions and advise the API user to work around existing ones
according to the slightly changed semantics of the API.
Regards,
Marton
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] mov: do not mark timed thumbnail tracks as attached picture
On Sat, 27 Jan 2018 01:44:04 +0100 (CET)
Marton Balint wrote:
> On Sat, 27 Jan 2018, wm4 wrote:
>
> > The AV_DISPOSITION_ATTACHED_PIC flag is meant only for exporting a
> > static picture (such as embedded cover art) as pseudo video track. The
> > requirement is that there is exactly 1 packet, and that normal demuxing
> > does not return it (otherwise avformat_queue_attached_pictures() would
> > add it a second time). It should never used on actual video tracks that
> > return packets when demuxing.
>
> Docs in avformat.h only says that AV_DISPOSITION_ATTACHED_PIC "usually"
> contains one packet.
It also implies AVStream.attached_pic is set, which can be only one
picture.
> >
> > I considered keeping the current behavior if there is exactly 1 frame
> > (according to nb_index_entries), but that would require additional work
> > to make sure avformat_queue_attached_pictures() does not add it a second
> > time, so I didn't bother.
> >
> > Reverts part of 697400eac07c0614f6b9f2e7615563982dbcbe4a and fixes
> > regressions with certain API users with such mp4 files.
> > ---
> > libavformat/mov.c | 18 +-
> > 1 file changed, 1 insertion(+), 17 deletions(-)
> >
> > diff --git a/libavformat/mov.c b/libavformat/mov.c
> > index 22faecfc17..f74be03359 100644
> > --- a/libavformat/mov.c
> > +++ b/libavformat/mov.c
> > @@ -6335,23 +6335,7 @@ static void mov_read_chapters(AVFormatContext *s)
> > cur_pos = avio_tell(sc->pb);
> >
> > if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO) {
> > -st->disposition |= AV_DISPOSITION_ATTACHED_PIC |
> > AV_DISPOSITION_TIMED_THUMBNAILS;
> > -if (st->nb_index_entries) {
> > -// Retrieve the first frame, if possible
> > -AVPacket pkt;
> > -AVIndexEntry *sample = &st->index_entries[0];
> > -if (avio_seek(sc->pb, sample->pos, SEEK_SET) !=
> > sample->pos) {
> > -av_log(s, AV_LOG_ERROR, "Failed to retrieve first
> > frame\n");
> > -goto finish;
> > -}
> > -
> > -if (av_get_packet(sc->pb, &pkt, sample->size) < 0)
> > -goto finish;
> > -
> > -st->attached_pic = pkt;
> > -st->attached_pic.stream_index = st->index;
> > -st->attached_pic.flags |= AV_PKT_FLAG_KEY;
> > -}
> > +st->disposition |= AV_DISPOSITION_TIMED_THUMBNAILS;
>
> AV_DISPOSITION_TIMED_THUMBNAILS is only ever used with
> AV_DISPOSITION_ATTACHED_PIC according to the docs in avformat.h.
Well then there can be only 1 timed thumbnail. As documented, this flag
has been nonsense, and not even libavformat itself respected it. As I
wrote in the commit message, the first packet is added twice, once
injected by utils.c, and then again returned by mov.c.
How does this make any sense?
> Like it or not, that is how the flag was introduced, so I'd rather not
> change it now. It made sense to introduce it this way, because checking
> for the ATTACHED_PIC flag was used (for example in ffmpeg when using the
> capital V stream speicifier) to search for ordinary video streams. By
> using this flag for timed thumbnails as well, applications did not have to
> check for an additional flag when searching for ordinary video streams.
This is also nonsense. ATTACHED_PIC is not meant for any stream
selection stuff that ffmpeg.c might do, it means that it's a cover art
picture imported from metadata. (To be honest that doesn't make sense
either, it's just a dumb hack that should never have existed in the
first place and that broke a lot of things when it was introduced.)
If you want some flag that means "ordinary video stream", it should
probably be introduced separately, instead of abusing ATTACHED_PIC for
it.
> So I am against this patch, probably better to fix the regression in the
> API user side, because it seems to me this is one of those cases where
> something will regress no matter what we do, so it is better to not cause
> new regressions and advise the API user to work around existing ones
> according to the slightly changed semantics of the API.
It's already "regressed" because it's been in a permanent state of
being buggy and making no sense at all.
I can fix the avformat.h description of AV_DISPOSITION_TIMED_THUMBNAILS.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] rtsp: rename certain options after a deprecation period
On Fri, Jan 26, 2018 at 12:52:19AM +0100, wm4 wrote: > On Fri, 26 Jan 2018 00:21:14 +0100 > Carl Eugen Hoyos wrote: > > > 2018-01-26 0:00 GMT+01:00 Hendrik Leppkes : > > > On Thu, Jan 25, 2018 at 11:35 PM, Carl Eugen Hoyos > > > wrote: [...] > > I'd also like to point out that it _did_ happen in the past that > Michael went off tangents in patch reviews and asked for unreasonable > extra work because of minor issues. Ive written a huge number of reviews over the years, and i certainly did what you claim above in one of these many reviews. Not in the case you refer here to though. And if it happens, that a unreasonable request is made, discussion is whats needed. In fact many developers have made requests that others have found unreasonable. Many if not most of the more active developers have. Its not a uncommon source of long discussions. > The AVFrame.opaque_ref fiasco > (whose non-sensical later solution which he demanded was finally > pushed, even though it still does not solve the issues he claimed his > approach would solve) comes to mind, so much you say in here ill skip over "non-sensical" and "demanded" these are not really true in the way you present it here. the solution choosen IIRC does indeed not achive everything. It was in fact a compromise that resulted from the discussion of many developers. IIRC a solution that would solve all teh listed issues was not liked by multiple people > and I considered his general > conduct on this issue harassment (not like I'd get an apology). If anything i said felt like harrasment, i appologize. That said a review or comment or objection to a patch is not harrasment and I will continue to review and when it is needed object to changes. Maybe it is just my point of view here but you seem very sensitive to anything that is not an approval of your changes. While at the same time your replies are often aggressive or offensive. Just as random examples, your single line objection here certainly came over in a passive aggressive tone: https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2018-January/224488.html and your next reply, that basically told me farewell when i said that i wouldnt continue to maintain or help maintain hevc when error messages would have to be ommited (not one specific one but in general) https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2018-January/224506.html And that was just a day or 2 ago. The very mail here i reply to is then also accusing me of harrasment in patch review from last year. I dont exactly enjoy having to reply to these kind of accusations. More so considering how long ago the review is that refers to. and on IRC 2 days ago Jän 25 23:47:53<+wm4> also michaelni harassed me a lot in the past (like making me go through all that pointless crap when getting rid of the side data merging Jän 25 23:48:02can you drop that already? it was not pointless Jän 25 23:48:22i was careless and did a merge wrong, and google came out of the woodworks because i broke chromium Jän 25 23:48:41the abi concerns had a reason I would really like to see these accusations stop, this is not something anyone else that i remember has done in FFmpeg or Libav, not at this scale at least. Thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB When the tyrant has disposed of foreign enemies by conquest or treaty, and there is nothing more to fear from them, then he is always stirring up some war or other, in order that the people may require a leader. -- Plato signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avfilter/vf_transpose: Fix used plane count.
On Wed, Jan 24, 2018 at 09:51:42PM +0100, Michael Niedermayer wrote: > Fixes out of array access > Fixes: poc.mp4 > > Found-by: GwanYeong Kim > Signed-off-by: Michael Niedermayer > --- > libavfilter/vf_transpose.c | 8 +++- > 1 file changed, 7 insertions(+), 1 deletion(-) applied [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Modern terrorism, a quick summary: Need oil, start war with country that has oil, kill hundread thousand in war. Let country fall into chaos, be surprised about raise of fundamantalists. Drop more bombs, kill more people, be surprised about them taking revenge and drop even more bombs and strip your own citizens of their rights and freedoms. to be continued signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] rtsp: rename certain options after a deprecation period
On Sat, 27 Jan 2018 02:25:49 +0100 Michael Niedermayer wrote: > On Fri, Jan 26, 2018 at 12:52:19AM +0100, wm4 wrote: > > On Fri, 26 Jan 2018 00:21:14 +0100 > > Carl Eugen Hoyos wrote: > > > > > 2018-01-26 0:00 GMT+01:00 Hendrik Leppkes : > > > > On Thu, Jan 25, 2018 at 11:35 PM, Carl Eugen Hoyos > > > > wrote: > [...] > > > > > I'd also like to point out that it _did_ happen in the past that > > Michael went off tangents in patch reviews and asked for unreasonable > > extra work because of minor issues. > > Ive written a huge number of reviews over the years, and i certainly > did what you claim above in one of these many reviews. Not in the case > you refer here to though. And if it happens, that a unreasonable request > is made, discussion is whats needed. > In fact many developers have made requests that others have found > unreasonable. Many if not most of the more active developers have. > Its not a uncommon source of long discussions. And so we're having a long discussion again. You could just have left it alone. > > > > The AVFrame.opaque_ref fiasco > > (whose non-sensical later solution which he demanded was finally > > pushed, even though it still does not solve the issues he claimed his > > approach would solve) comes to mind, > > so much you say in here > ill skip over "non-sensical" and "demanded" these are not really true in > the way you present it here. The "demanded" was definitely true. You blocked my patch, even though it was just a Libav merge to begin with. There were never big discussions about such tiny merges before, and nobody would have given a shit had I pushed those merges directly to git (like it's normally done with merges). But you started a big discussion and eventually achieved that a sub-optimal solution was added. The current solution _is_ nonsense. There is now a semi-public field (AVFrame.private_ref) in a libavutil struct that can be used only by libavcodec, something which we always avoid, because the libraries are supposed to use only the public API of each other (except avpriv_ and headers). It still doesn't solve the problem you claimed there was: if libavcodec fails to "process" the private_ref contents, the returned frame to the user will be broken and make the user application crash. Oh, and of course the doxygen says AVFrame.private_ref can be used by other ffmpeg libs too. So what happens if libavfilter's use of private_ref clashes with libavcodec's? Please explain. Absolutely NOTHING is helped by using a separate field instead of opaque_ref, and you were the only one who insisted on doing this (the others at best unaware of the details and vaguely trusting your claims). You even ignored my solution, which was consistently checking all entrypoints for opaque_ref (and wrapping the user use of it), which my patches did. You didn't even care and somehow claimed there were holes in it, which there were not, and even if there were, YOUR CURRENT CODE WOULD FAIL TOO. At this point I seriously had enough of your shit and left the project for a while. > the solution choosen IIRC does indeed not achive everything. It was > in fact a compromise that resulted from the discussion of many developers. > IIRC a solution that would solve all teh listed issues was not liked by > multiple people Because apparently they still wanted it to be merged, so they just did whatever you wanted, whether it made sense or not. I on the other hand was seeking a solution that was technically closer to ideal. But it was impossible to get through you. > > and I considered his general > > conduct on this issue harassment (not like I'd get an apology). > > If anything i said felt like harrasment, i appologize. That said > a review or comment or objection to a patch is not harrasment and > I will continue to review and when it is needed object to changes. > > Maybe it is just my point of view here but you seem very sensitive > to anything that is not an approval of your changes. > While at the same time your replies are often aggressive or offensive. > > Just as random examples, your single line objection here certainly came > over in a passive aggressive tone: > https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2018-January/224488.html That's not passive aggressive. You know certain devs (including me) don't like excessive logging code for fuzzing fixes, and that was extensively discussed in the past. But there you go again. (And sure, I can be convinced that the logging is actually OK in this specific case, but you preferred doing something else instead.) However, your reply was passive aggressive to the max: > > and your next reply, that basically told me farewell when i said that i > wouldnt continue to maintain or help maintain hevc when error messages > would have to be ommited (not one specific one but in general) > https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2018-January/224506.html You basically threatened that you'd stop doing important wo
Re: [FFmpeg-devel] [PATCH] mpeg4video: Add support for MPEG-4 Simple Studio Profile.
On Sat, Jan 27, 2018 at 12:33 AM, Michael Niedermayer < mich...@niedermayer.cc> wrote: > On Fri, Jan 26, 2018 at 08:04:39PM +, Kieran Kunhya wrote: > > On Mon, Jan 22, 2018 at 2:07 AM, Michael Niedermayer > > > wrote: > > > > > On Sun, Jan 21, 2018 at 12:37:21PM +, Kieran Kunhya wrote: > > > > On Mon, Jan 1, 2018 at 7:01 PM, Michael Niedermayer > > > > > > > wrote: > > > > > > > > > Hi > > > > > > > > > > > > > > > Patch updated. > > > > Some of the review comments I decided not to implement in order to > keep > > > > closer to the spec. > > > > > > honestly, this reasoning makes no sense to me. > > > that spec does IIRC not even fully describe some parts in DPCM > > > > > > also all existing mpeg&h26x code we have was designed for speed > > > basically ignoring if its close to some spec implementation style or > not > > > > > > I would prefer we try to make our code as good as we can and not try to > > > be similar to some specification or reference implementation. > > > except for maybe some niche codecs where performance totally doesnt > matter > > > > > > > It does describe DPCM, just in an odd way. > > The specification i read did not specify DPCM in a way one could implement > aka the description was not complete. > > and it also was not understandable to you as you asked on IRC about it > ages ago > > Jän 22 03:12:54michaelni: don't know if I am missing > something here but does it say anywhere what the length of dpcm_residual is? > Jän 22 03:41:31I'm guessing it should read 8-12? > Jän 22 03:41:41in 14496-2 > ... > Dez 23 01:55:30atomnuker: any idea how to calculate the > length of dpcm_residual > Dez 23 01:57:19it just says "dpcm_residual: This is an > unsigned integer that indicates the value of a DPCM residual" > Dez 23 01:58:08 shouldnt the definiton of "uimsbf" tell > you how to parse those values > Dez 23 01:58:25sure, but doesn't explain the 4-12 > Dez 23 01:58:44 how did you figure out the length of say > block_mean. its 8-12 uimsbf > Dez 23 01:59:10block mean is the average of the block and > the supported bitdepths are 8-12 > Dez 23 02:01:31 so basically the format syntax is just > bonkers > Dez 23 02:10:12there must be something I'm missing > > If the spec makes sense to you now, please explain because it made no sense > to me last i looked. > I genuinely do not understand what it meant It is possible to implement, I will send a patch when I am done. Please do not quote things from 2017, it is now 2018. > > I am going to implement your ricing changes but for the record your > passive > > aggressiveness to many developers is awful. > > i appologize if something i said sounded passive aggerssive, it was not > intended > to. > > > > Please learn to compromise. > > you refer to myself pushing towards a optimial implementation and not the > spec? > as in " I would prefer we try to make our code as good as we can and not > try to > be similar to some specification or reference implementation." > ? > > Dont you agree that this is the better choice for teh codebase even though > yes > it is certainly more work ? > Changing two lines of code to be less readable and "faster" makes such little difference but instead you bully contributors on a daily basis backed up by your nasty lieutenants. When will you realise your inability to compromise is a major reason why this project is dysfunctional and viewed as a joke in the rest of the OSS world. Kieran ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] rtsp: rename certain options after a deprecation period
On 1/26/2018 11:23 PM, wm4 wrote: > On Sat, 27 Jan 2018 02:25:49 +0100 > Michael Niedermayer wrote: > >> On Fri, Jan 26, 2018 at 12:52:19AM +0100, wm4 wrote: >>> On Fri, 26 Jan 2018 00:21:14 +0100 >>> Carl Eugen Hoyos wrote: >>> 2018-01-26 0:00 GMT+01:00 Hendrik Leppkes : > On Thu, Jan 25, 2018 at 11:35 PM, Carl Eugen Hoyos > wrote: >> [...] >> >>> >>> I'd also like to point out that it _did_ happen in the past that >>> Michael went off tangents in patch reviews and asked for unreasonable >>> extra work because of minor issues. >> >> Ive written a huge number of reviews over the years, and i certainly >> did what you claim above in one of these many reviews. Not in the case >> you refer here to though. And if it happens, that a unreasonable request >> is made, discussion is whats needed. >> In fact many developers have made requests that others have found >> unreasonable. Many if not most of the more active developers have. >> Its not a uncommon source of long discussions. > > And so we're having a long discussion again. You could just have left > it alone. Then drop it, the lot of you. Stop replying, throwing contradicting accusations in every direction, and let this thread die already. Nothing, absolutely nothing good will come out of this if you keep going. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] rtsp: rename certain options after a deprecation period
On Fri, 26 Jan 2018 23:26:20 -0300 James Almer wrote: > On 1/26/2018 11:23 PM, wm4 wrote: > > On Sat, 27 Jan 2018 02:25:49 +0100 > > Michael Niedermayer wrote: > > > >> On Fri, Jan 26, 2018 at 12:52:19AM +0100, wm4 wrote: > >>> On Fri, 26 Jan 2018 00:21:14 +0100 > >>> Carl Eugen Hoyos wrote: > >>> > 2018-01-26 0:00 GMT+01:00 Hendrik Leppkes : > > On Thu, Jan 25, 2018 at 11:35 PM, Carl Eugen Hoyos > > wrote: > >> [...] > >> > >>> > >>> I'd also like to point out that it _did_ happen in the past that > >>> Michael went off tangents in patch reviews and asked for unreasonable > >>> extra work because of minor issues. > >> > >> Ive written a huge number of reviews over the years, and i certainly > >> did what you claim above in one of these many reviews. Not in the case > >> you refer here to though. And if it happens, that a unreasonable request > >> is made, discussion is whats needed. > >> In fact many developers have made requests that others have found > >> unreasonable. Many if not most of the more active developers have. > >> Its not a uncommon source of long discussions. > > > > And so we're having a long discussion again. You could just have left > > it alone. > > Then drop it, the lot of you. Stop replying, throwing contradicting > accusations in every direction, and let this thread die already. > > Nothing, absolutely nothing good will come out of this if you keep going. I'm just defending myself. I don't want to be accused of throwing around baseless accusations. That includes your post too. There are _real_ issues and they will come up again and again if we don't solve them. Pretending they don't exist isn't a solution. But whatever. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/2] avcodec/hevc_cabac: Check prefix so as to avoid invalid shifts in coeff_abs_level_remaining_decode()
On Tue, Jan 23, 2018 at 07:44:24PM +0100, Michael Niedermayer wrote: > On Tue, Jan 16, 2018 at 12:37:28AM +0100, Michael Niedermayer wrote: > > I suspect that this can be limited tighter, but i failed to find anything > > in the spec that would confirm that. > > > > Fixes: 4833/clusterfuzz-testcase-minimized-5302840101699584 > > Fixes: runtime error: left shift of 134217730 by 4 places cannot be > > represented in type 'int' > > > > Found-by: continuous fuzzing process > > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > > Signed-off-by: Michael Niedermayer > > --- > > libavcodec/hevc_cabac.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > will apply this in a few days unless someone wants me to wait applied [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB "Nothing to hide" only works if the folks in power share the values of you and everyone you know entirely and always will -- Tom Scott signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/2] avcodec/mjpegdec: Fix integer overflow in DC dequantization
On Wed, Jan 24, 2018 at 04:34:50AM +0100, Michael Niedermayer wrote: > Fixes: runtime error: signed integer overflow: -65535 * 65312 cannot be > represented in type 'int' > Fixes: 4900/clusterfuzz-testcase-minimized-5769019744321536 > > Found-by: continuous fuzzing process > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer > --- > libavcodec/mjpegdec.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) applied [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Complexity theory is the science of finding the exact solution to an approximation. Benchmarking OTOH is finding an approximation of the exact signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avfilter/vf_framerate: fix cpy_line_width calculation on >8 bits format
On Sat, Jan 20, 2018 at 03:04:58PM +0700, Muhammad Faiz wrote: > Fix tsan warnings on fate-filter-framerate-12bit-down and > fate-filter-framerate-12bit-up. > > Signed-off-by: Muhammad Faiz > --- > libavfilter/vf_framerate.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) should be ok if you checked the output thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB When you are offended at any man's fault, turn to yourself and study your own failings. Then you will forget your anger. -- Epictetus signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avfilter/vf_framerate: fix cpy_line_width calculation on >8 bits format
On Sat, 27 Jan 2018, Michael Niedermayer wrote: On Sat, Jan 20, 2018 at 03:04:58PM +0700, Muhammad Faiz wrote: Fix tsan warnings on fate-filter-framerate-12bit-down and fate-filter-framerate-12bit-up. Signed-off-by: Muhammad Faiz --- libavfilter/vf_framerate.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) should be ok if you checked the output My pending framerate patches will fix this anyway. Regards, Marton ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/opensrt: add Haivision Open SRT protocol
On Mon, Jan 15, 2018 at 09:02:01AM -0500, Nablet Developer wrote:
> protocol requires libsrt (https://github.com/Haivision/srt) to be
> installed
This overall looks good, a few issues/suggestions i spoted are below:
[...]
> +@table @option
> +@item conntimeo=@var{milliseconds}
> +Connection timeout, in milliseconds. SRT cannot connect for RTT > 1500 msec
> +(2 handshake exchanges) with the default connect timeout of 3 seconds. This
> option
> +applies to the caller and rendezvous connection modes. The connect timeout
> is 10 times
> +the value set for the rendezvous mode (which can be used as a workaround for
> this
> +connection problem with earlier versions).
newly added time based options should be in seconds
the user can achive micro or milli seconds by using suffixes
also see AV_OPT_TYPE_DURATION, it may fit here
> +
> +@item fc=@var{bytes}
> +Flight Flag Size (Window Size), in bytes. FC is actually an internal
> parameter and
> +you should set it to not less than @option{recv_buffer_size} and
> @option{mss}.
> +The default value is relatively large, therefore unless you set a very large
> +receiver buffer, you do not need to change this option. Default value is
> 25600.
> +
> +@item inputbw=@var{bytes/seconds}
> +Sender nominal input rate, in bytes per seconds. Used along with
> @option{oheadbw},
> +when @option{maxbw} is set to relative (0), to calculate maximum sending
> rate when
> +recovery packets are sent along with main media stream:
> +@option{inputbw} * (100 + @option{oheadbw}) / 100
> +if @option{inputbw} is not set while @option{maxbw} is set to relative (0),
> the actual
> +ctual input rate is evaluated inside the library. Default value is 0.
> +
> +@item iptos=@var{tos}
> +IP Type of Service. Applies to sender only. Default value is 0xB8.
> +
> +@item ipttl=@var{ttl}
> +IP Time To Live. Applies to sender only. Default value is 64.
> +
> +@item listen_timeout=@var{milliseconds}
> +Set listen timeout, expressed in milliseconds.
> +
> +@item maxbw=@var{bytes/seconds}
> +Maximum sending bandwidth, in bytes per seconds.
> +-1 infinite (CSRTCC limit is 30mbps)
> +0 relative to input rate (see @option{inputbw})
> +>0 absolute limit value
> +Default value is 0 (relative)
> +
> +@item mode=@var{0|1|2}
> +Connection mode.
> +0 (caller) opens client connection.
> +1 (listener) starts server to listen for incoming connections.
> +2 (rendezvous) use Rendez-Vous connection mode.
> +Default valus is 0 (caller).
This looks like the user would have to use litteral integers
teh code supports the named identifers in brackets directly.
> +
> +@item mss=@var{bytes}
> +Maximum Segment Size, in bytes. Used for buffer allocation and rate
> calculation using
> +packet counter assuming fully filled packets. The smallest MSS between the
> peers is
> +used. This is 1500 by default in the overall internet. This is the maximum
> size of the
> +UDP packet and can be only decreased, unless you have some unusual dedicated
> network
> +settings. Default value is 1500.
> +
> +@item nakreport=@var{1|0}
> +If set to 1, Receiver will send `UMSG_LOSSREPORT` messages periodically
> until the
> +lost packet is retransmitted or intentionally dropped. Default value is 1.
> +
> +@item oheadbw=@var{percents}
> +Recovery bandwidth overhead above input rate, in percents. See
> @option{inputbw}.
> +Default value is 25%.
> +
> +@item passphrase=@var{string}
> +HaiCrypt Encryption/Decryption Passphrase string, length from 10 to 79
> characters.
> +The passphrase is the shared secret between the sender and the receiver.
> +It is used to generate the Key Encrypting Key using PBKDF2 (Password-Based
> +Key Deriviation Function). It is used only if @option{pbkeylen} is non-zero.
> +t is used on the receiver only if the received data is encrypted.
> +The configured passphrase cannot be get back (write-only).
> +
> +@item pbkeylen=@var{bytes}
> +Sender encryption key length, in bytes. Only can be set to 0, 16, 24 and 32.
> +Enable sender encryption if not 0. Not required on receiver (set to 0),
> +key size obtained from sender in HaiCrypt handshake. Default value is 0.
> +
> +@item recv_buffer_size=@var{bytes}
> +Set receive buffer size, expressed bytes.
> +
> +@item send_buffer_size=@var{bytes}
> +Set send buffer size, expressed bytes.
> +
> +@item timeout=@var{microseconds}
> +Set raise error timeout, expressed in microseconds.
> +
> +This option is only relevant in read mode: if no data arrived in more
> +than this time interval, raise error.
> +
> +@item tlpktdrop=@var{1|0}
> +Too-late Packet Drop. When enabled on receiver, it skips missing packets that
> +have not been delivered in time and deliver the following packets to the
> application
> +when their time-to-play has come. It also send a fake ACK to sender. When
> enabled on
> +sender and enabled on the receiving peer, sender drops the older packets
> that have no
> +chance to be delivered in time. It was automatically enabled in sender if
> receiver
> +s
Re: [FFmpeg-devel] [PATCH 3/3] hls: don't print a certain warning if playlist loading is aborted
On Fri, 26 Jan 2018 11:11:46 +0800
刘歧 wrote:
> > On 24 Jan 2018, at 15:08, wm4 wrote:
> >
> > AVERROR_EXIT happens when the user's interrupt callback signals that
> > playback should be aborted. In this case, the demuxer shouldn't print a
> > warning, as it's expected that all network accesses are stopped.
> > ---
> > libavformat/hls.c | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/libavformat/hls.c b/libavformat/hls.c
> > index 6e1a2e3f1e..02e764f932 100644
> > --- a/libavformat/hls.c
> > +++ b/libavformat/hls.c
> > @@ -1422,8 +1422,9 @@ reload:
> > if (!v->finished &&
> > av_gettime_relative() - v->last_load_time >= reload_interval) {
> > if ((ret = parse_playlist(c, v->url, v, NULL)) < 0) {
> > -av_log(v->parent, AV_LOG_WARNING, "Failed to reload
> > playlist %d\n",
> > - v->index);
> > +if (ret != AVERROR_EXIT)
> > +av_log(v->parent, AV_LOG_WARNING, "Failed to reload
> > playlist %d\n",
> > + v->index);
> > return ret;
> > }
> > /* If we need to reload the playlist again below (if
> > --
> > 2.15.1
> >
> > ___
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> Patchset LGTM
Thanks. Pushed all 3 patches.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH V3] lavc: Don't overwrite previously setup dimensions for all codecs
Currently a hacky way is used for some specific codecs such as
H264/VP6F/DXV.
Replace with a more generic way(an evolution based on a libav commit
9de9b828 but hasn't been merged since it breaks lowres).
Verified it won't introduce regression of original ticket #1386 fixing.
V2: add "lowres" handle code
V3: fix the typo
Signed-off-by: Zhong Li
---
libavcodec/utils.c | 8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index 4c71843..8fe20d9 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -684,16 +684,12 @@ int attribute_align_arg avcodec_open2(AVCodecContext
*avctx, const AVCodec *code
goto free_and_end;
}
-// only call ff_set_dimensions() for non H.264/VP6F/DXV codecs so as not
to overwrite previously setup dimensions
-if (!(avctx->coded_width && avctx->coded_height && avctx->width &&
avctx->height &&
- (avctx->codec_id == AV_CODEC_ID_H264 || avctx->codec_id ==
AV_CODEC_ID_VP6F || avctx->codec_id == AV_CODEC_ID_DXV))) {
-if (avctx->coded_width && avctx->coded_height)
+if (avctx->coded_width && avctx->coded_height && (!avctx->width &&
!avctx->height || avctx->lowres))
ret = ff_set_dimensions(avctx, avctx->coded_width,
avctx->coded_height);
-else if (avctx->width && avctx->height)
+else if (avctx->width && avctx->height && (!avctx->coded_width &&
!avctx->coded_height || avctx->lowres))
ret = ff_set_dimensions(avctx, avctx->width, avctx->height);
if (ret < 0)
goto free_and_end;
-}
if ((avctx->coded_width || avctx->coded_height || avctx->width ||
avctx->height)
&& ( av_image_check_size2(avctx->coded_width, avctx->coded_height,
avctx->max_pixels, AV_PIX_FMT_NONE, 0, avctx) < 0
--
1.8.3.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] qsvdec: Relax the surface vs coded dimension check
Fix a common vp8 decoding failure.
Many vp8 clips cannot decode if hw_frames_ctx is enabled, reporting
"Error during QSV decoding.: incompatible video parameters (-14)".
It is due to mfx.FrameInfo.Width/Height not matching coded_w/coded_h.
See: -hwaccel qsv -init_hw_device qsv -c:v vp8_qsv -i
vp8-test-vectors-r1/vp80-00-comprehensive-001.ivf
-vf "hwdownload,format=nv12" -pix_fmt yuv420p -f md5 -
Signed-off-by: Zhong Li
Signed-off-by: Luca Barbato
---
libavcodec/qsv.c| 2 +-
libavcodec/qsvdec.c | 3 ---
2 files changed, 1 insertion(+), 4 deletions(-)
diff --git a/libavcodec/qsv.c b/libavcodec/qsv.c
index 250b4e6..5217adf 100644
--- a/libavcodec/qsv.c
+++ b/libavcodec/qsv.c
@@ -389,7 +389,7 @@ static mfxStatus qsv_frame_alloc(mfxHDL pthis,
mfxFrameAllocRequest *req,
mfxFrameInfo *i = &req->Info;
mfxFrameInfo *i1 = &frames_hwctx->surfaces[0].Info;
-if (i->Width != i1->Width || i->Height != i1->Height ||
+if (i->Width > i1->Width || i->Height > i1->Height ||
i->FourCC != i1->FourCC || i->ChromaFormat != i1->ChromaFormat) {
av_log(ctx->logctx, AV_LOG_ERROR, "Mismatching surface properties
in an "
"allocation request: %dx%d %d %d vs %dx%d %d %d\n",
diff --git a/libavcodec/qsvdec.c b/libavcodec/qsvdec.c
index abbede5..45bedf9 100644
--- a/libavcodec/qsvdec.c
+++ b/libavcodec/qsvdec.c
@@ -149,9 +149,6 @@ static int qsv_decode_init(AVCodecContext *avctx,
QSVContext *q)
else if (frames_hwctx->frame_type &
MFX_MEMTYPE_VIDEO_MEMORY_DECODER_TARGET)
iopattern = MFX_IOPATTERN_OUT_VIDEO_MEMORY;
}
-
-frame_width = frames_hwctx->surfaces[0].Info.Width;
-frame_height = frames_hwctx->surfaces[0].Info.Height;
}
if (!iopattern)
--
1.8.3.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH V2 2/2] Don't overwrite previously setup dimensions for all codecs
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> Of Michael Niedermayer
> Sent: Saturday, January 27, 2018 8:05 AM
> To: FFmpeg development discussions and patches
>
> Subject: Re: [FFmpeg-devel] [PATCH V2 2/2] Don't overwrite previously setup
> dimensions for all codecs
>
> On Thu, Jan 18, 2018 at 01:03:34PM +0800, Zhong Li wrote:
> > Currently a hacky way is used for some specific codecs such as
> > H264/VP6F/DXV (and "lowres" case is broken now).
> > Replace with a more generic way(an evolution based on a libav commit
> > 9de9b828 but hasn't been merged since it breaks lowres).
> >
> > V1->V2: add "lowres" handle code
> >
> > Signed-off-by: Zhong Li
> > ---
> > libavcodec/utils.c | 8 ++--
> > 1 file changed, 2 insertions(+), 6 deletions(-)
> >
> > diff --git a/libavcodec/utils.c b/libavcodec/utils.c index
> > 427f612..fdd1b46 100644
> > --- a/libavcodec/utils.c
> > +++ b/libavcodec/utils.c
> > @@ -684,16 +684,12 @@ int attribute_align_arg
> avcodec_open2(AVCodecContext *avctx, const AVCodec *code
> > goto free_and_end;
> > }
> >
> > -// only call ff_set_dimensions() for non H.264/VP6F/DXV codecs so as
> not to overwrite previously setup dimensions
> > -if (!(avctx->coded_width && avctx->coded_height && avctx->width
> && avctx->height &&
> > - (avctx->codec_id == AV_CODEC_ID_H264 || avctx->codec_id
> == AV_CODEC_ID_VP6F || avctx->codec_id == AV_CODEC_ID_DXV))) {
> > -if (avctx->coded_width && avctx->coded_height)
> > +if (avctx->coded_width && avctx->coded_height && (!avctx->width
> > + && !avctx->height || avctx->lowles))
> > ret = ff_set_dimensions(avctx, avctx->coded_width,
> avctx->coded_height);
> > -else if (avctx->width && avctx->height)
> > +else if (avctx->width && avctx->height && (!avctx->coded_width &&
> > + !avctx->coded_height || avctx->lowles))
> > ret = ff_set_dimensions(avctx, avctx->width, avctx->height);
> > if (ret < 0)
> > goto free_and_end;
> > -}
>
> This has typos in variable names, it will not work nor build
>
> please make sure that submitted patches have been tested before
Really sorry for that, I fixed it locally but forgot to submit.
I've sent an updated patch now.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
