Re: [FFmpeg-devel] [PATCH V2 1/4] lavc/vaapi_encode: Change the slice/parameter buffers to dynamic alloc.
On 2017/8/10 5:11, Mark Thompson wrote:
> On 02/08/17 06:53, Jun Zhao wrote:
>> V2: Change the slice/parameter buffers to dynamic alloc and split
>> the mutil-slice support for AVC/HEVC.
>>
>> From 39fd7852df0c96217310c525107da06a7ec0a062 Mon Sep 17 00:00:00 2001
>> From: Jun Zhao
>> Date: Mon, 31 Jul 2017 22:46:23 -0400
>> Subject: [PATCH V2 1/4] lavc/vaapi_encode: Change the slice/parameter buffers
>> to dynamic alloc.
>>
>> Change the slice/parameter buffers to be allocated dynamically.
>>
>> Signed-off-by: Wang, Yi A
>> Signed-off-by: Jun Zhao
>> ---
>> libavcodec/vaapi_encode.c | 27 ---
>> libavcodec/vaapi_encode.h | 6 ++
>> 2 files changed, 26 insertions(+), 7 deletions(-)
>>
>> diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
>> index 2de5f76cab..11d9803b5d 100644
>> --- a/libavcodec/vaapi_encode.c
>> +++ b/libavcodec/vaapi_encode.c
>> @@ -36,13 +36,19 @@ static int
>> vaapi_encode_make_packed_header(AVCodecContext *avctx,
>> VAAPIEncodeContext *ctx = avctx->priv_data;
>> VAStatus vas;
>> VABufferID param_buffer, data_buffer;
>> +VABufferID *tmp;
>> VAEncPackedHeaderParameterBuffer params = {
>> .type = type,
>> .bit_length = bit_len,
>> .has_emulation_bytes = 1,
>> };
>>
>> -av_assert0(pic->nb_param_buffers + 2 <= MAX_PARAM_BUFFERS);
>> +tmp = av_realloc_array(pic->param_buffers, sizeof(*tmp),
>> (pic->nb_param_buffers + 2));
>
> Redundant parentheses?
>
>> +if (!tmp) {
>> +av_freep(&pic->param_buffers);
>
> This leaks the actual buffers which were allocated
> (pic->param_buffers[0..pic->nb_param_buffers-1]).
After double-check the code, I think if av_realloc_array fail in
vaapi_encode_make_packed_header/vaapi_encode_make_param_buffer,
we can just return error number ENOMEM, then vaapi_encode_issue will free the
VA buffer/pic->param_buffers/...in error handling.
>
>> +return AVERROR(ENOMEM);
>> +}
>> +pic->param_buffers = tmp;
>>
>> vas = vaCreateBuffer(ctx->hwctx->display, ctx->va_context,
>> VAEncPackedHeaderParameterBufferType,
>> @@ -77,9 +83,15 @@ static int vaapi_encode_make_param_buffer(AVCodecContext
>> *avctx,
>> {
>> VAAPIEncodeContext *ctx = avctx->priv_data;
>> VAStatus vas;
>> +VABufferID *tmp;
>> VABufferID buffer;
>>
>> -av_assert0(pic->nb_param_buffers + 1 <= MAX_PARAM_BUFFERS);
>> +tmp = av_realloc_array(pic->param_buffers, sizeof(*tmp),
>> (pic->nb_param_buffers + 1));
>> +if (!tmp) {
>> +av_freep(&pic->param_buffers);
>
> Same here.
>
>> +return AVERROR(ENOMEM);
>> +}
>> +pic->param_buffers = tmp;
>>
>> vas = vaCreateBuffer(ctx->hwctx->display, ctx->va_context,
>> type, len, 1, data, &buffer);
>> @@ -122,6 +134,8 @@ static int vaapi_encode_wait(AVCodecContext *avctx,
>> // Input is definitely finished with now.
>> av_frame_free(&pic->input_image);
>>
>> +av_freep(&pic->param_buffers);
>
> Maybe it would be cleaner to free param_buffers at the end of
> vaapi_encode_issue() - the buffers themselves are no longer usable at that
> point.
Ok
>
>> +
>> pic->encode_complete = 1;
>> return 0;
>> }
>> @@ -313,7 +327,10 @@ static int vaapi_encode_issue(AVCodecContext *avctx,
>> }
>> }
>>
>> -av_assert0(pic->nb_slices <= MAX_PICTURE_SLICES);
>> +pic->slices = (VAAPIEncodeSlice **)av_malloc(sizeof(VAAPIEncodeSlice *)
>> * pic->nb_slices);
>> +if (pic->slices == NULL)
>> +goto fail;
>> +
>> for (i = 0; i < pic->nb_slices; i++) {
>> slice = av_mallocz(sizeof(*slice));
>
> How about just making pic->slices be VAAPIEncodeSlice*, pointing to an array
> of the structures? This code is now allocating both an array of pointers and
> then each element of that array individually.
>
>> if (!slice) {
>> @@ -427,6 +444,8 @@ fail:
>> vaDestroyBuffer(ctx->hwctx->display, pic->param_buffers[i]);
>> fail_at_end:
>> av_freep(&pic->codec_picture_params);
>> +av_freep(&pic->param_buffers);
>> +av_freep(&pic->slices);
>> av_frame_free(&pic->recon_image);
>> av_buffer_unref(&pic->output_buffer_ref);
>> pic->output_buffer = VA_INVALID_ID;
>> @@ -544,6 +563,8 @@ static int vaapi_encode_free(AVCodecContext *avctx,
>> av_frame_free(&pic->input_image);
>> av_frame_free(&pic->recon_image);
>>
>> +av_freep(&pic->param_buffers);
>> +av_freep(&pic->slices);
>> // Output buffer should already be destroyed.
>> av_assert0(pic->output_buffer == VA_INVALID_ID);
>>
>> diff --git a/libavcodec/vaapi_encode.h b/libavcodec/vaapi_encode.h
>> index 0edf27e4cb..b542772aed 100644
>> --- a/libavcodec/vaapi_encode.h
>> +++ b/libavcodec/vaapi_encode.h
>> @@ -35,8 +35,6 @@ enum {
>> MAX_CONFIG_ATTRIBUTES = 4,
>> MAX_GLOBAL_PARAMS = 4,
>> MAX_PICTURE_REFERENCES = 2,
>>
Re: [FFmpeg-devel] [PATCH 01/16] doc/filters: document the unstability of the shorthand options notation.
On 8/11/17, Michael Niedermayer wrote: > On Thu, Aug 10, 2017 at 04:15:55PM +0200, Nicolas George wrote: >> Le tridi 23 thermidor, an CCXXV, Michael Niedermayer a écrit : >> > Please limit the notes in filters.texi and Changelog to the filters and >> > options you intend to change. >> >> That would defeat the purpose. Doubly so: >> >> - Being free to change the options as need requires. That means any >> filter and any option. Not all, not many, but any. >> >> - Having a rule that is simple to remember instead of a long list. >> >> Please bring new arguments to the discussion if you want to continue it. > > well > First, I dont think a single developer should declare a whole class of > interfaces spaning the areas other developers work on unstable against > one or more objections from them. Or if one has that right then everyone > else should as well. > > I maintain several filters and clearly stated that this doc-text does not > apply to them. None of them would benefit from breaking the > order or inserting options in the middle. > In fact with a unstable order there is no benefit from adding options > in the middle, noone could reliably use the new order. > > as in if you link to libavfilter 99.123 you can write a:b:c , if you > link to libavfilter 99.124 you can write a:c:b. Update your distro > package and it could change in otherwise unchanged applications. > > and none of this in the example above is neccesarily a syntax error > and gives an error message. the different order could just give > different results. thats bad design > > I like to re iterate, i do not agree to declaring the option order > of the filters i maintain as unstable. > > Even if we could reorder them without disadvantages, > there is no benefit in doing so. > > Its trivial to change your docs/changeog patch to avoid my concern, and > its trivial to change MAINTAINERs as well if people value declaring the > interfaces as unstable more than a maintainer who fanatically insist on > maintaining a stable interface in the filters he maintains. > > Also there is no long list, just a entry in the changelog and in the > documentation of the specific filter which was changed. > I think we both agree that this is a rare event. And i would argue > it is NOT a event specific to the shorthand interface. Am i not > correct that you would similarly change the named interface if a > cleanup you do benefits from it ? What about keeping old options intact? ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 01/16] doc/filters: document the unstability of the shorthand options notation.
Le quartidi 24 thermidor, an CCXXV, Michael Niedermayer a écrit : > First, I dont think a single developer should declare a whole class of > interfaces spaning the areas other developers work on unstable against > one or more objections from them. Or if one has that right then everyone > else should as well. So they post a patch on the mailing-list and everybody can discuss it. That is the way it is supposed to work, and that is exactly what I did. Are you accusing me of perverting the process or something, or is this paragraph here only to muddy things more? > I maintain several filters and clearly stated that this doc-text does not > apply to them. Well, good for you. Keep the options on your filter in order. I do not intend to prevent you from doing so. What are you fussing about? > order or inserting options in the middle. > In fact with a unstable order there is no benefit from adding options > in the middle, noone could reliably use the new order. > > as in if you link to libavfilter 99.123 you can write a:b:c , if you > link to libavfilter 99.124 you can write a:c:b. Update your distro > package and it could change in otherwise unchanged applications. Everybody is already aware of that. > and none of this in the example above is neccesarily a syntax error > and gives an error message. the different order could just give > different results. thats bad design Yes, that is bad design. And that applies to somebody misremembering a:c:b when it should have been a:b:c: the whole shorthand notation was bad design from the start. Thanks for making my point for me. > I like to re iterate, i do not agree to declaring the option order > of the filters i maintain as unstable. You re-iterate, but you do not bring new arguments to the discussion. I have brought many and you have not addressed half of them. I can re-state them if you have forgotten. > Also there is no long list, just a entry in the changelog and in the > documentation of the specific filter which was changed. You fail to see from a user's point of view. Try, for one second, to imagine you are a simple user learning how to use that tool. You read in the documentation that this filter has stable shorthand notation, and this one not, this other yes, these two no, and these five give no information at all. What will you remember? Which filter is stable and which is not? No, you will remember not to use the shorthand notation, because writing "x=", "y=" even when it is not necessary is easier than remembering when it is necessary and when it is not. Having exceptions and categories is worse than having everything declared unstable. > I think we both agree that this is a rare event. And i would argue > it is NOT a event specific to the shorthand interface. Am i not > correct that you would similarly change the named interface if a > cleanup you do benefits from it ? No, you are not correct at all. Quite the contrary, I have already explained why it is not true. The users want stability, I know that. They have it: the named options. Since they have something that will always work, I can propose to break something that is just a convenience to type a few less keys. I would not propose to break the named interface, because there is no backup stable interface. Regards, -- Nicolas George ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 01/16] doc/filters: document the unstability of the shorthand options notation.
Le tridi 23 thermidor, an CCXXV, Marton Balint a écrit : > I suggest you push the patch series without this patch, Michael can fix the > overlay and blend/tblend parameter order. If later the needed additional > compatibility code becomes too much of a burden, we can discuss this > further. I could do that if people agree, but wouldn't breaking then repairing be akin to breaking twice, and as such worse than breaking once? Regards, -- Nicolas George ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 01/16] doc/filters: document the unstability of the shorthand options notation.
Le quartidi 24 thermidor, an CCXXV, Paul B Mahol a écrit : > What about keeping old options intact? That requires glue code and more importantly testing. I do not intend to do it, and it blocks progress. Regards, -- Nicolas George ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 01/16] doc/filters: document the unstability of the shorthand options notation.
On Thu, Aug 10, 2017 at 01:46:27PM +0200, Nicolas George wrote: > It does not mean that we intend to break the order of options > at a whim, but it gives us more freedom to make necessary > changes without extra unnecessary burden while giving stability > to users that require it. [...] I'd rather make such changes justified and documented as exceptional in the Changelog (or in APIchanges) when we can't get around it cleanly, than documenting a free for all area. You're saying documenting the risk or potential changes helps us make changes more easily with more transparency, but no matter the wording, to me it looks like it's going to be a slippery slope where developers interpret it differently and will abuse that declared rule whenever possible. Indeed, it's the perfect defense against users and other developers when breaking the interface. So yeah, I'm in favor of "no API breakage" (of course, major version bumps allow to bypass this), whatever the form, and we can always make documented exceptions for obscures options after a discussion. -- Clément B. signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 2/2] libavdevice: Add SQV screen capture device
For making it more reasonable QSV screen capture is added to
device list, so it can be read as usual device, producing
packets to its output. Nevertheless it is only wrapper around QSV decoder
with screen capture plugin loading
---
configure| 2 +
libavdevice/Makefile | 1 +
libavdevice/alldevices.c | 1 +
libavdevice/qsvscreen.c | 216 +++
4 files changed, 220 insertions(+)
create mode 100644 libavdevice/qsvscreen.c
diff --git a/configure b/configure
index 942765e8a3..c5119cf672 100755
--- a/configure
+++ b/configure
@@ -3035,6 +3035,8 @@ oss_indev_deps_any="soundcard_h sys_soundcard_h"
oss_outdev_deps_any="soundcard_h sys_soundcard_h"
pulse_indev_deps="libpulse"
pulse_outdev_deps="libpulse"
+screen_qsv_indev_deps="libmfx"
+screen_qsv_indev_select="screen_qsv_decoder"
qtkit_indev_extralibs="-framework QTKit -framework Foundation -framework
QuartzCore"
qtkit_indev_select="qtkit"
sdl2_outdev_deps="sdl2"
diff --git a/libavdevice/Makefile b/libavdevice/Makefile
index c055d6718d..327cbb0a6c 100644
--- a/libavdevice/Makefile
+++ b/libavdevice/Makefile
@@ -38,6 +38,7 @@ OBJS-$(CONFIG_PULSE_INDEV) += pulse_audio_dec.o
\
pulse_audio_common.o timefilter.o
OBJS-$(CONFIG_PULSE_OUTDEV) += pulse_audio_enc.o \
pulse_audio_common.o
+OBJS-$(CONFIG_SCREEN_QSV_INDEV) += qsvscreen.o
OBJS-$(CONFIG_QTKIT_INDEV) += qtkit.o
OBJS-$(CONFIG_SDL2_OUTDEV) += sdl2.o
OBJS-$(CONFIG_SNDIO_INDEV) += sndio_dec.o sndio.o
diff --git a/libavdevice/alldevices.c b/libavdevice/alldevices.c
index a8ed53ae5d..bcc46c2691 100644
--- a/libavdevice/alldevices.c
+++ b/libavdevice/alldevices.c
@@ -57,6 +57,7 @@ static void register_all(void)
REGISTER_OUTDEV (OPENGL, opengl);
REGISTER_INOUTDEV(OSS, oss);
REGISTER_INOUTDEV(PULSE,pulse);
+ REGISTER_INDEV (SCREEN_QSV, screen_qsv);
REGISTER_INDEV (QTKIT,qtkit);
REGISTER_OUTDEV (SDL2, sdl2);
REGISTER_INOUTDEV(SNDIO,sndio);
diff --git a/libavdevice/qsvscreen.c b/libavdevice/qsvscreen.c
new file mode 100644
index 00..475a9a4c50
--- /dev/null
+++ b/libavdevice/qsvscreen.c
@@ -0,0 +1,216 @@
+/*
+ * Intel QSV screen capture interface
+ *
+ * This file is part of FFmpeg.
+ *
+ * Copyright (C) 2017 Alexander Bilyak
+ *
+ * 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 QSV Screen capture demuxer interface
+ * @author Alexander Bilyak
+ * @note As Intel Media SDK provide screen capture as plug-in to QSV decoder
+ * so this "device" is nothing more than wrapper around QSV decoder
+ * with appropiate settings.
+ */
+
+#include "config.h"
+#include "libavformat/internal.h"
+#include "libavutil/opt.h"
+#include "libavcodec/avcodec.h"
+#include "libavutil/time.h"
+#include "libavutil/imgutils.h"
+
+/**
+ * QSV screen capture context
+ */
+typedef struct QSVScreenContext {
+
+const AVClass *class; /**< Class for private options */
+
+AVCodecContext *avctx; /**< Codec context used for fake decoding */
+AVCodec *codec; /**< Codec used for fake decoding */
+
+int draw_mouse; /**< Draw mouse cursor (private option) */
+int width; /**< Width of the grab frame (private option)
*/
+int height; /**< Height of the grab frame (private option)
*/
+} QSVScreenContext;
+
+/**
+ * Initializes the QSV screen device demuxer (public device demuxer API).
+ *
+ * @param avctx Context from avformat core
+ *
+ * @return 0 on success, a negative AVERROR on error
+ */
+static int screen_qsv_read_header(AVFormatContext *avctx)
+{
+int ret;
+AVStream *st;
+QSVScreenContext *ctx = avctx->priv_data;
+AVDictionary* opts = NULL;
+
+ctx->codec = avcodec_find_decoder_by_name("screen_qsv");
+if (!ctx->codec) {
+ret = AVERROR_DECODER_NOT_FOUND;
+goto error;
+}
+
+ctx->avctx = avcodec_alloc_context3(ctx->codec);
+if (!ctx->avctx) {
+ret = AVERROR(ENOMEM);
+goto error;
+}
+
+ctx->avctx->width = ctx->width;
+ct
Re: [FFmpeg-devel] [PATCH 01/16] doc/filters: document the unstability of the shorthand options notation.
Le quartidi 24 thermidor, an CCXXV, Clement Boesch a écrit : > I'd rather make such changes justified and documented as exceptional in > the Changelog (or in APIchanges) when we can't get around it cleanly, than > documenting a free for all area. > > You're saying documenting the risk or potential changes helps us make > changes more easily with more transparency, but no matter the wording, to > me it looks like it's going to be a slippery slope where developers > interpret it differently and will abuse that declared rule whenever > possible. Indeed, it's the perfect defense against users and other > developers when breaking the interface. Can you explain what you mean exactly? I am very doubtful about "slippery slope" arguments, they usually have at their core the fallacy of trusting your now-self more than your later-self, while actually your later-self will have more information and thus be able of better decisions. > So yeah, I'm in favor of "no API breakage" (of course, major version bumps > allow to bypass this), whatever the form, and we can always make > documented exceptions for obscures options after a discussion. This would be nice, in principle. But that does not tell me what to do in practice. Will you implement and test the compatibility code, soon so that the patch series can be pushed, fixing the bugs it is fixing and allowing work to continue? Will you do that in the future when filters are converted to the new options system? Will you go over all the filter's documentation to fix the places where the order does not match? The last paragraph is one of my strongest arguments, and nobody addressed it yet: the shorthand notation is not stable. Right now. We have on occasion inserted options out of order, and the order is in fact not documented since the documentation does not match the implementation. This patch is not changing the policy, it is documenting it. Regards, -- Nicolas George ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 1/2] libavcodec: Add support for QSV screen capture plugin
Intel QSV SDK provide screen capture plugin starting from API ver 1.17
as runtime loadable plugin for QSV decoder.
* add API version selection while initialization of QSV context
(default is still 1.1 for usual encoding/decoding)
---
configure | 2 +
libavcodec/Makefile| 1 +
libavcodec/allcodecs.c | 1 +
libavcodec/qsv.c | 6 +-
libavcodec/qsv_internal.h | 2 +-
libavcodec/qsvdec.c| 12 ++-
libavcodec/qsvdec.h| 5 +
libavcodec/qsvdec_screen.c | 250 +
libavcodec/qsvenc.c| 3 +-
9 files changed, 272 insertions(+), 10 deletions(-)
create mode 100644 libavcodec/qsvdec_screen.c
diff --git a/configure b/configure
index 3a146ed857..942765e8a3 100755
--- a/configure
+++ b/configure
@@ -2520,6 +2520,8 @@ rv20_decoder_select="h263_decoder"
rv20_encoder_select="h263_encoder"
rv30_decoder_select="golomb h264pred h264qpel mpegvideo rv34dsp"
rv40_decoder_select="golomb h264pred h264qpel mpegvideo rv34dsp"
+screen_qsv_decoder_deps="libmfx"
+screen_qsv_decoder_select="qsvdec"
screenpresso_decoder_select="zlib"
shorten_decoder_select="bswapdsp"
sipr_decoder_select="lsp"
diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index 59029a853c..7a2052d4c7 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -519,6 +519,7 @@ OBJS-$(CONFIG_S302M_DECODER) += s302m.o
OBJS-$(CONFIG_S302M_ENCODER) += s302menc.o
OBJS-$(CONFIG_SANM_DECODER)+= sanm.o
OBJS-$(CONFIG_SCPR_DECODER)+= scpr.o
+OBJS-$(CONFIG_SCREEN_QSV_DECODER) += qsvdec_screen.o
OBJS-$(CONFIG_SCREENPRESSO_DECODER)+= screenpresso.o
OBJS-$(CONFIG_SDX2_DPCM_DECODER) += dpcm.o
OBJS-$(CONFIG_SGI_DECODER) += sgidec.o
diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
index 0243f47358..d572c3f27c 100644
--- a/libavcodec/allcodecs.c
+++ b/libavcodec/allcodecs.c
@@ -315,6 +315,7 @@ static void register_all(void)
REGISTER_ENCDEC (S302M, s302m);
REGISTER_DECODER(SANM, sanm);
REGISTER_DECODER(SCPR, scpr);
+ REGISTER_DECODER(SCREEN_QSV,screen_qsv);
REGISTER_DECODER(SCREENPRESSO, screenpresso);
REGISTER_DECODER(SDX2_DPCM, sdx2_dpcm);
REGISTER_ENCDEC (SGI, sgi);
diff --git a/libavcodec/qsv.c b/libavcodec/qsv.c
index b9e2cd990d..80d0d4af37 100644
--- a/libavcodec/qsv.c
+++ b/libavcodec/qsv.c
@@ -242,15 +242,13 @@ load_plugin_fail:
}
int ff_qsv_init_internal_session(AVCodecContext *avctx, mfxSession *session,
- const char *load_plugins)
+ const char *load_plugins, mfxVersion api_ver)
{
mfxIMPL impl = MFX_IMPL_AUTO_ANY;
-mfxVersion ver = { { QSV_VERSION_MINOR, QSV_VERSION_MAJOR } };
-
const char *desc;
int ret;
-ret = MFXInit(impl, &ver, session);
+ret = MFXInit(impl, &api_ver, session);
if (ret < 0)
return ff_qsv_print_error(avctx, ret,
"Error initializing an internal MFX
session");
diff --git a/libavcodec/qsv_internal.h b/libavcodec/qsv_internal.h
index c0305508dd..ebc3082e22 100644
--- a/libavcodec/qsv_internal.h
+++ b/libavcodec/qsv_internal.h
@@ -88,7 +88,7 @@ int ff_qsv_profile_to_mfx(enum AVCodecID codec_id, int
profile);
int ff_qsv_map_pixfmt(enum AVPixelFormat format, uint32_t *fourcc);
int ff_qsv_init_internal_session(AVCodecContext *avctx, mfxSession *session,
- const char *load_plugins);
+ const char *load_plugins, mfxVersion api_ver);
int ff_qsv_init_session_device(AVCodecContext *avctx, mfxSession *psession,
AVBufferRef *device_ref, const char
*load_plugins);
diff --git a/libavcodec/qsvdec.c b/libavcodec/qsvdec.c
index c00817f1d9..9aa139b1e8 100644
--- a/libavcodec/qsvdec.c
+++ b/libavcodec/qsvdec.c
@@ -41,8 +41,9 @@
#include "qsv_internal.h"
#include "qsvdec.h"
-static int qsv_init_session(AVCodecContext *avctx, QSVContext *q, mfxSession
session,
-AVBufferRef *hw_frames_ref, AVBufferRef
*hw_device_ref)
+int ff_qsv_init_session(AVCodecContext *avctx, QSVContext *q, mfxSession
session,
+AVBufferRef *hw_frames_ref, AVBufferRef *hw_device_ref,
+mfxVersion api_ver)
{
int ret;
@@ -83,7 +84,7 @@ static int qsv_init_session(AVCodecContext *avctx, QSVContext
*q, mfxSession ses
} else {
if (!q->internal_session) {
ret = ff_qsv_init_internal_session(avctx, &q->internal_session,
- q->load_plugins);
+ q->load_plugins, api_ver);
if (ret < 0)
return ret;
}
@@ -145,7 +146,10 @@ static int qsv_decode_init(AVCodecContext *avctx,
QSVContext *q)
iopattern =
Re: [FFmpeg-devel] [PATCH] libavcodec/mips: Improve avc idct8 msa function
Please review the patch.
-Original Message-
From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf Of
Kaustubh Raste
Sent: Friday, August 4, 2017 5:24 PM
To: FFmpeg development discussions and patches
Subject: Re: [FFmpeg-devel] [PATCH] libavcodec/mips: Improve avc idct8 msa
function
Ping.
-Original Message-
From: Manojkumar Bhosale
Sent: Monday, July 31, 2017 3:43 PM
To: FFmpeg development discussions and patches
Cc: Kaustubh Raste
Subject: RE: [FFmpeg-devel] [PATCH] libavcodec/mips: Improve avc idct8 msa
function
LGTM
thx
-Original Message-
From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf Of
kaustubh.ra...@imgtec.com
Sent: Monday, July 31, 2017 12:07 PM
To: ffmpeg-devel@ffmpeg.org
Cc: Kaustubh Raste
Subject: [FFmpeg-devel] [PATCH] libavcodec/mips: Improve avc idct8 msa function
From: Kaustubh Raste
Replace memset call with msa stores.
Signed-off-by: Kaustubh Raste
---
libavcodec/mips/h264idct_msa.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/libavcodec/mips/h264idct_msa.c b/libavcodec/mips/h264idct_msa.c
index 861befe..1e1a5c8 100644
--- a/libavcodec/mips/h264idct_msa.c
+++ b/libavcodec/mips/h264idct_msa.c
@@ -120,11 +120,12 @@ static void avc_idct8_addblk_msa(uint8_t *dst, int16_t
*src, int32_t dst_stride)
v4i32 res0_r, res1_r, res2_r, res3_r, res4_r, res5_r, res6_r, res7_r;
v4i32 res0_l, res1_l, res2_l, res3_l, res4_l, res5_l, res6_l, res7_l;
v16i8 dst0, dst1, dst2, dst3, dst4, dst5, dst6, dst7;
-v16i8 zeros = { 0 };
+v8i16 zeros = { 0 };
src[0] += 32;
LD_SH8(src, 8, src0, src1, src2, src3, src4, src5, src6, src7);
+ST_SH8(zeros, zeros, zeros, zeros, zeros, zeros, zeros, zeros, src,
+ 8);
vec0 = src0 + src4;
vec1 = src0 - src4;
@@ -318,7 +319,6 @@ void ff_h264_idct8_addblk_msa(uint8_t *dst, int16_t *src,
int32_t dst_stride) {
avc_idct8_addblk_msa(dst, src, dst_stride);
-memset(src, 0, 64 * sizeof(dctcoef));
}
void ff_h264_idct4x4_addblk_dc_msa(uint8_t *dst, int16_t *src,
--
1.7.9.5
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] lavf/movdec: flag chapter streams as such, even when not reading them
This allows the use of the `ignore_chapters` option to avoid performing
extra seeks at startup without producing "subtitle" chapter streams.
---
libavformat/mov.c | 16
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/libavformat/mov.c b/libavformat/mov.c
index 63f84be782..5f83c695e6 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -5762,6 +5762,7 @@ static void mov_read_chapters(AVFormatContext *s)
int64_t cur_pos;
int i, j;
int chapter_track;
+int read_contents = (s->pb->seekable & AVIO_SEEKABLE_NORMAL) &&
!mov->ignore_chapters;
for (j = 0; j < mov->nb_chapter_tracks; j++) {
chapter_track = mov->chapter_tracks[j];
@@ -5781,7 +5782,7 @@ static void mov_read_chapters(AVFormatContext *s)
if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO) {
st->disposition |= AV_DISPOSITION_ATTACHED_PIC |
AV_DISPOSITION_TIMED_THUMBNAILS;
-if (st->nb_index_entries) {
+if (st->nb_index_entries && read_contents) {
// Retrieve the first frame, if possible
AVPacket pkt;
AVIndexEntry *sample = &st->index_entries[0];
@@ -5801,6 +5802,10 @@ static void mov_read_chapters(AVFormatContext *s)
st->codecpar->codec_type = AVMEDIA_TYPE_DATA;
st->codecpar->codec_id = AV_CODEC_ID_BIN_DATA;
st->discard = AVDISCARD_ALL;
+
+if (!read_contents)
+continue;
+
for (i = 0; i < st->nb_index_entries; i++) {
AVIndexEntry *sample = &st->index_entries[i];
int64_t end = i+1 < st->nb_index_entries ?
st->index_entries[i+1].timestamp : st->duration;
@@ -5851,7 +5856,8 @@ static void mov_read_chapters(AVFormatContext *s)
}
}
finish:
-avio_seek(sc->pb, cur_pos, SEEK_SET);
+if (read_contents)
+avio_seek(sc->pb, cur_pos, SEEK_SET);
}
}
@@ -6181,9 +6187,11 @@ static int mov_read_header(AVFormatContext *s)
}
av_log(mov->fc, AV_LOG_TRACE, "on_parse_exit_offset=%"PRId64"\n",
avio_tell(pb));
+
+if (mov->nb_chapter_tracks > 0)
+mov_read_chapters(s);
+
if (pb->seekable & AVIO_SEEKABLE_NORMAL) {
-if (mov->nb_chapter_tracks > 0 && !mov->ignore_chapters)
-mov_read_chapters(s);
for (i = 0; i < s->nb_streams; i++)
if (s->streams[i]->codecpar->codec_tag == AV_RL32("tmcd")) {
mov_read_timecode_track(s, s->streams[i]);
--
2.14.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 01/16] doc/filters: document the unstability of the shorthand options notation.
On Fri, Aug 11, 2017 at 11:12:32AM +0200, Nicolas George wrote: > Le quartidi 24 thermidor, an CCXXV, Clement Boesch a écrit : > > I'd rather make such changes justified and documented as exceptional in > > the Changelog (or in APIchanges) when we can't get around it cleanly, than > > documenting a free for all area. > > > > You're saying documenting the risk or potential changes helps us make > > changes more easily with more transparency, but no matter the wording, to > > me it looks like it's going to be a slippery slope where developers > > interpret it differently and will abuse that declared rule whenever > > possible. Indeed, it's the perfect defense against users and other > > developers when breaking the interface. > > Can you explain what you mean exactly? I am very doubtful about > "slippery slope" arguments, they usually have at their core the fallacy > of trusting your now-self more than your later-self, while actually your > later-self will have more information and thus be able of better > decisions. > I'm afraid of the situation where a developer will feel like the order of the options is not ideal, or an option could be renamed for consistency with other filters, and will take the easy way out "oh well, we documented it's unstable, users can only blame themselves for relying on it, not even mentioning the new order/name is much better for everyone". > > So yeah, I'm in favor of "no API breakage" (of course, major version bumps > > allow to bypass this), whatever the form, and we can always make > > documented exceptions for obscures options after a discussion. > > This would be nice, in principle. But that does not tell me what to do > in practice. > > Will you implement and test the compatibility code, soon so that the > patch series can be pushed, fixing the bugs it is fixing and allowing > work to continue? If you're referring to this current patch serie, can you list here all the filters that are affected by an API break and to what degree? We can discuss here if it's an acceptable change or not, and should require a compatibility code (maybe it will be for one or two filters only). > Will you do that in the future when filters are converted to the new > options system? We could consider an API to deprecate an AVOption for future similar issue. I think we already had such discussion in the past (maybe that was around the time we changed "user-agent" into "user_agent"), involving the presence of a flag. > Will you go over all the filter's documentation to fix the places where > the order does not match? I consider that one a long standing and deeper issue we haven't solved yet. My position here is that I do believe all the long-description of option belongs in the source code itself (under NULL_IF_CONFIG_SMALL() maybe) and the final documentation should be generated from it. That way, we won't have to worry about option orders or inconsistencies between code and documentation. It will also allow having a cleaner documentation wrt option types (mainly thinking about bool values here). That's a large project, but I don't think it's hard. > The last paragraph is one of my strongest arguments, and nobody > addressed it yet: the shorthand notation is not stable. Right now. We > have on occasion inserted options out of order, and the order is in fact > not documented since the documentation does not match the > implementation. I think we've been careful about avoiding such mistakes. I have the feeling this may only affect options at the end of already long options list, so the impact is mitigated. > This patch is not changing the policy, it is documenting it. And my position is that it's currently not that bad handled and should be improved. In the past, we've been breaking the C API and ABI several times by "mistakes". Our documentation never was perfect either, and still isn't. Does that mean we should document that users should never expect any API stability from the FFmpeg project because we're just shitty at it? -- Clément B. signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/mpeg4videodec: Clear mcsel before decoding an image
On Sun, Aug 06, 2017 at 02:38:29PM +0200, Michael Niedermayer wrote: > Fixes: runtime error: signed integer overflow: 2146467840 + 1032192 cannot be > represented in type 'int' > Fixes: 2826/clusterfuzz-testcase-minimized-5901511613743104 > > Found-by: continuous fuzzing process > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer > --- > libavcodec/mpeg4videodec.c | 1 + > 1 file changed, 1 insertion(+) applied [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Elect your leaders based on what they did after the last election, not based on what they say before an election. signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/avidec: Move packet skip after prefix and related checks
On Tue, Aug 08, 2017 at 02:51:07AM +0200, Michael Niedermayer wrote: > This fixes loosing packets > Fixes: big.avi > > Signed-off-by: Michael Niedermayer > --- > libavformat/avidec.c | 25 - > 1 file changed, 12 insertions(+), 13 deletions(-) applied [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB I do not agree with what you have to say, but I'll defend to the death your right to say it. -- Voltaire signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/2] avcodec/tests/dct: Add peak mean error check
On Sun, Jul 09, 2017 at 03:52:08AM +0200, Michael Niedermayer wrote: > based on quotes of IEEE 1180 / ISO/IEC 23002-1 > > Signed-off-by: Michael Niedermayer > --- > libavcodec/tests/dct.c | 2 ++ > 1 file changed, 2 insertions(+) applied [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Avoid a single point of failure, be that a person or equipment. signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/2] avcodec/tests/dct: Add Mean square error test
On Sun, Jul 09, 2017 at 03:52:09AM +0200, Michael Niedermayer wrote: > based on quotes of IEEE 1180 / ISO/IEC 23002-1 > > Signed-off-by: Michael Niedermayer > --- > libavcodec/tests/dct.c | 10 +++--- > 1 file changed, 7 insertions(+), 3 deletions(-) applied [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Elect your leaders based on what they did after the last election, not based on what they say before an election. signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 01/16] doc/filters: document the unstability of the shorthand options notation.
Le quartidi 24 thermidor, an CCXXV, Clement Boesch a écrit : > I'm afraid of the situation where a developer will feel like the order of > the options is not ideal, or an option could be renamed for consistency > with other filters, and will take the easy way out "oh well, we documented > it's unstable, users can only blame themselves for relying on it, not even > mentioning the new order/name is much better for everyone". In my mind, even with this documentation commit, changing the order of an option would still require posting a patch on the mailing-list to discuss it. It could be stated explicitly, but I do not know where (the user documentation is not the place). As for renaming options, this is absolutely not allowed. > If you're referring to this current patch serie, can you list here all the > filters that are affected by an API break and to what degree? > > We can discuss here if it's an acceptable change or not, and should > require a compatibility code (maybe it will be for one or two filters > only). All the filters that use framesync2.h and have any of the "shortest", "eof_action" and "repeatlast" options lose the possibility to set these options by shorthand notation. I.e. you can no longer put 1 in the fifth or sixth position in the arguments list and hope it will fall on "shortest". > We could consider an API to deprecate an AVOption for future similar > issue. I think we already had such discussion in the past (maybe that was > around the time we changed "user-agent" into "user_agent"), involving the > presence of a flag. Yes, we could, and that takes time that could be dedicated to fixing actual bugs or implementing new features. Ask any user: "if you agree to always write x=, y= in your overlay filters, I can fix your bug faster, what do you prefer?", I know what they will answer. > I consider that one a long standing and deeper issue we haven't solved > yet. Yes, but it still makes my point: the shorthand notation is currently broken, and we might as well take the benefits from it. > My position here is that I do believe all the long-description of > option belongs in the source code itself (under NULL_IF_CONFIG_SMALL() > maybe) and the final documentation should be generated from it. > > That way, we won't have to worry about option orders or inconsistencies > between code and documentation. It will also allow having a cleaner > documentation wrt option types (mainly thinking about bool values here). > > That's a large project, but I don't think it's hard. I agree. In fact, a few months ago I posted a detailed plan of an API to replace the AVOption system that included that and many other things, including getting rid of escaping hell. > I think we've been careful about avoiding such mistakes. I have the > feeling this may only affect options at the end of already long options > list, so the impact is mitigated. Yes, but when it comes to trust, it is all or nothing: right now, users cannot trust the order given in the documentation, and they are not warned of it. > And my position is that it's currently not that bad handled and should be > improved. > > In the past, we've been breaking the C API and ABI several times by > "mistakes". Our documentation never was perfect either, and still isn't. > Does that mean we should document that users should never expect any API > stability from the FFmpeg project because we're just shitty at it? No, but that means that when a specific has been in fact unstable and people did not complain about it, we can get rid of it with less hassle. Regards, -- Nicolas George ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] Mov/mp4 demuxer failing on mp4 file (Sample size is too large)
On Thu, Aug 10, 2017 at 1:38 PM, Paul B Mahol wrote: > On 8/10/17, Robert Krüger wrote: > > On Thu, Aug 10, 2017 at 10:21 AM, Paul B Mahol wrote: > > > >> On 8/9/17, Robert Krüger wrote: > >> > Hi, > >> > > >> > can someone tell me what this hints at? Is this more likely a broken > >> > file > >> > or a missing feature of the demuxer? Unfortunately I cannot make the > >> > file > >> > available due to legal issues but if I know where to look, I could try > >> > to > >> > find a sample with similar characteristics. > >> > > >> > [mov,mp4,m4a,3gp,3g2,mj2 @ 0x7fb4b7002200] Sample size 2147484020 is > too > >> > large > >> > > >> > [mov,mp4,m4a,3gp,3g2,mj2 @ 0x7fb4b7002200] Invalid > >> sample_count=-2147483519 > >> > > >> > [mov,mp4,m4a,3gp,3g2,mj2 @ 0x7fb4b7002200] error reading header > >> > >> Does it play in QuickTime? > > > > > > Yes it plays in Quicktime X on OSX but not in the old Quicktime 7. > > Than it should be reported as ffmpeg bug. > I would love to do that but at the moment I can only share the sample privately due to rights issues. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] JPEG2000: SSE optimisation of DWT decoding
> From: Maxime Taisant > > > From: Ivan Kalvachev > > > > On 8/8/17, maxime taisant wrote: > > > From: Maxime Taisant > > > > > > +movups m2, [lineq+2*j0q-24] > > > +movups m5, [lineq+2*j0q-8] > > > +shufps m2, m5, 0xDD > > > +addps m2, m1 > > > +mulps m2, m3 > > > +subps m0, m2 > > > +movups m4, m1 > > > +shufps m1, m0, 0x44 ; 0100'0100 q1010 > > Is that movlhps m1, m0 ? > > No, this command place the first two values of m1 in the last two > doublewords of m1, and the first two values of m0 in the first two > doublewords of m1. > Movhlps would simply replace the first two values of m1 by the ones > of m0. > Ok, so everything I said here is wrong... You were right, that IS movlhps m1, m0. Will change that, sorry. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 0/3] tonemap filter v2
Changes since v1: - changed pixel format from yuv to rgb, to sync with mpv's implementation - slightly changed a couple of the algorithms - improved log messages, using the right output context Vittorio Vittorio Giovara (3): Add single precision planar RGB pixel formats zscale: Enable single precision input/ouput filtering Add tonemap filter doc/filters.texi | 108 libavfilter/Makefile | 1 + libavfilter/allfilters.c | 1 + libavfilter/version.h| 2 +- libavfilter/vf_tonemap.c | 349 +++ libavfilter/vf_zscale.c | 7 +- libavutil/pixdesc.c | 54 ++ libavutil/pixdesc.h | 5 + libavutil/pixfmt.h | 7 + tests/ref/fate/sws-pixdesc-query | 20 +++ 10 files changed, 550 insertions(+), 4 deletions(-) create mode 100644 libavfilter/vf_tonemap.c -- 2.13.2 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 2/3] zscale: Enable single precision input/ouput filtering
--- libavfilter/vf_zscale.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/libavfilter/vf_zscale.c b/libavfilter/vf_zscale.c index 466689dbc5..1617db7cf0 100644 --- a/libavfilter/vf_zscale.c +++ b/libavfilter/vf_zscale.c @@ -179,6 +179,7 @@ static int query_formats(AVFilterContext *ctx) AV_PIX_FMT_GBRP, AV_PIX_FMT_GBRP9, AV_PIX_FMT_GBRP10, AV_PIX_FMT_GBRP12, AV_PIX_FMT_GBRP14, AV_PIX_FMT_GBRP16, AV_PIX_FMT_GBRAP, AV_PIX_FMT_GBRAP16, +AV_PIX_FMT_GBRPF32, AV_PIX_FMT_GBRAPF32, AV_PIX_FMT_NONE }; int ret; @@ -429,7 +430,7 @@ static void format_init(zimg_image_format *format, AVFrame *frame, const AVPixFm format->subsample_w = desc->log2_chroma_w; format->subsample_h = desc->log2_chroma_h; format->depth = desc->comp[0].depth; -format->pixel_type = desc->comp[0].depth > 8 ? ZIMG_PIXEL_WORD : ZIMG_PIXEL_BYTE; +format->pixel_type = (desc->flags & AV_PIX_FMT_FLAG_FLOAT) ? ZIMG_PIXEL_FLOAT : desc->comp[0].depth > 8 ? ZIMG_PIXEL_WORD : ZIMG_PIXEL_BYTE; format->color_family = (desc->flags & AV_PIX_FMT_FLAG_RGB) ? ZIMG_COLOR_RGB : ZIMG_COLOR_YUV; format->matrix_coefficients = (desc->flags & AV_PIX_FMT_FLAG_RGB) ? ZIMG_MATRIX_RGB : colorspace == -1 ? convert_matrix(frame->colorspace) : colorspace; format->color_primaries = primaries == -1 ? convert_primaries(frame->color_primaries) : primaries; @@ -573,13 +574,13 @@ static int filter_frame(AVFilterLink *link, AVFrame *in) s->alpha_src_format.width = in->width; s->alpha_src_format.height = in->height; s->alpha_src_format.depth = desc->comp[0].depth; -s->alpha_src_format.pixel_type = desc->comp[0].depth > 8 ? ZIMG_PIXEL_WORD : ZIMG_PIXEL_BYTE; +s->alpha_src_format.pixel_type = (desc->flags & AV_PIX_FMT_FLAG_FLOAT) ? ZIMG_PIXEL_FLOAT : desc->comp[0].depth > 8 ? ZIMG_PIXEL_WORD : ZIMG_PIXEL_BYTE; s->alpha_src_format.color_family = ZIMG_COLOR_GREY; s->alpha_dst_format.width = out->width; s->alpha_dst_format.height = out->height; s->alpha_dst_format.depth = odesc->comp[0].depth; -s->alpha_dst_format.pixel_type = odesc->comp[0].depth > 8 ? ZIMG_PIXEL_WORD : ZIMG_PIXEL_BYTE; +s->alpha_dst_format.pixel_type = (desc->flags & AV_PIX_FMT_FLAG_FLOAT) ? ZIMG_PIXEL_FLOAT : odesc->comp[0].depth > 8 ? ZIMG_PIXEL_WORD : ZIMG_PIXEL_BYTE; s->alpha_dst_format.color_family = ZIMG_COLOR_GREY; zimg_filter_graph_free(s->alpha_graph); -- 2.13.2 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] Add tonemap filter
Based off mpv automatic tonemapping capabilities.
Signed-off-by: Vittorio Giovara
---
doc/filters.texi | 108 +++
libavfilter/Makefile | 1 +
libavfilter/allfilters.c | 1 +
libavfilter/version.h| 2 +-
libavfilter/vf_tonemap.c | 349 +++
5 files changed, 460 insertions(+), 1 deletion(-)
create mode 100644 libavfilter/vf_tonemap.c
diff --git a/doc/filters.texi b/doc/filters.texi
index eedc7b5896..4d96f5572d 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -14436,6 +14436,113 @@ Vertical low-pass filtering can only be enabled for
@option{mode}
@end table
+@section tonemap
+Tone map colors from different dynamic ranges.
+
+This filter expects data in single precision floating point, as it needs to
+operate on (and can output) out-of-range values. Another filter, such as
+@ref{zscale}, is needed to convert the resulting frame to a usable format.
+
+The tonemapping algorithms implemented only work on linear light, so input
+data should be linearized beforehand (and possibly correctly tagged).
+
+@example
+ffmpeg -i INPUT -vf
zscale=transfer=linear,tonemap=clip,zscale=transfer=bt709,format=yuv420p OUTPUT
+@end example
+
+@subsection Options
+The filter accepts the following options.
+
+@table @option
+@item tonemap
+Set the tone map algorithm to use.
+
+Possible values are:
+@table @var
+@item none
+Do not apply any tone map, only desaturate overbright pixels.
+
+@item clip
+Hard-clip any out-of-range values. Use it for perfect color accuracy for
+in-range values, while distorting out-of-range values.
+
+@item linear
+Stretch the entire reference gamut to a linear multiple of the display.
+
+@item gamma
+Fit a logarithmic transfer between the tone curves.
+
+@item reinhard
+Preserve overall image brightness with a simple curve, using nonlinear
+contrast, which results in flattening details and degrading color accuracy.
+
+@item hable
+Peserve both dark and bright details better than @var{reinhard}, at the cost
+of slightly darkening everything. Use it when detail preservation is more
+important than color and brightness accuracy.
+
+@item mobius
+Smoothly map out-of-range values, while retaining contrast and colors for
+in-range material as much as possible. Use it when color accuracy is more
+important than detail preservation.
+@end table
+
+Default is none.
+
+@item param
+Tune the tone mapping algorithm.
+
+This affects the following algorithms:
+@table @var
+@item none
+Ignored.
+
+@item linear
+Specifies the scale factor to use while stretching.
+Default to 1.0.
+
+@item gamma
+Specifies the exponent of the function.
+Default to 1.8.
+
+@item clip
+Specify an extra linear coefficient to multiply into the signal before
clipping.
+Default to 1.0.
+
+@item reinhard
+Specify the local contrast coefficient at the display peak.
+Default to 0.5, which means that in-gamut values will be about half as bright
+as when clipping.
+
+@item hable
+Ignored.
+
+@item mobius
+Specify the transition point from linear to mobius transform. Every value
+below this point is guaranteed to be mapped 1:1. The higher the value, the
+more accurate the result will be, at the cost of losing bright details.
+Default to 0.3, which due to the steep initial slope still preserves in-range
+colors fairly accurately.
+@end table
+
+@item desat
+Apply desaturation for highlights that exceed this level of brightness. The
+higher the parameter, the more color information will be preserved. This
+setting helps prevent unnaturally blown-out colors for super-highlights, by
+(smoothly) turning into white instead. This makes images feel more natural,
+at the cost of reducing information about out-of-range colors.
+
+The default of 2.0 is somewhat conservative and will mostly just apply to
+skies or directly sunlit surfaces. A setting of 0.0 disables this option.
+
+This option works only if the input frame has a supported color tag.
+
+@item peak
+Override signal/nominal/reference peak with this value. Useful when the
+embedded peak information in display metadata is not reliable or when tone
+mapping from a lower range to a higher range.
+@end table
+
@section transpose
Transpose rows with columns in the input video and optionally flip it.
@@ -15621,6 +15728,7 @@
zoompan=z='min(max(zoom,pzoom)+0.0015,1.5)':d=1:x='iw/2-(iw/zoom/2)':y='ih/2-(ih
@end example
@end itemize
+@anchor{zscale}
@section zscale
Scale (resize) the input video, using the z.lib library:
https://github.com/sekrit-twc/zimg.
diff --git a/libavfilter/Makefile b/libavfilter/Makefile
index 06b915fc91..545b871cd7 100644
--- a/libavfilter/Makefile
+++ b/libavfilter/Makefile
@@ -314,6 +314,7 @@ OBJS-$(CONFIG_THUMBNAIL_FILTER) +=
vf_thumbnail.o
OBJS-$(CONFIG_TILE_FILTER) += vf_tile.o
OBJS-$(CONFIG_TINTERLACE_FILTER) += vf_tinterlace.o
OBJS-$(CONFIG_TLUT2_FILTER) += vf_lut2.o framesync2.o
+OBJS-$(CONFIG_TONEMAP_FILTER
[FFmpeg-devel] [PATCH 1/3] Add single precision planar RGB pixel formats
Signed-off-by: Vittorio Giovara
---
libavutil/pixdesc.c | 54
libavutil/pixdesc.h | 5
libavutil/pixfmt.h | 7 ++
tests/ref/fate/sws-pixdesc-query | 20 +++
4 files changed, 86 insertions(+)
diff --git a/libavutil/pixdesc.c b/libavutil/pixdesc.c
index 59587328ea..d45eae5772 100644
--- a/libavutil/pixdesc.c
+++ b/libavutil/pixdesc.c
@@ -2183,6 +2183,60 @@ static const AVPixFmtDescriptor
av_pix_fmt_descriptors[AV_PIX_FMT_NB] = {
.name = "d3d11",
.flags = AV_PIX_FMT_FLAG_HWACCEL,
},
+[AV_PIX_FMT_GBRPF32BE] = {
+.name = "gbrpf32be",
+.nb_components = 3,
+.log2_chroma_w = 0,
+.log2_chroma_h = 0,
+.comp = {
+{ 2, 4, 0, 0, 32, 3, 31, 1 },/* R */
+{ 0, 4, 0, 0, 32, 3, 31, 1 },/* G */
+{ 1, 4, 0, 0, 32, 3, 31, 1 },/* B */
+},
+.flags = AV_PIX_FMT_FLAG_BE | AV_PIX_FMT_FLAG_PLANAR |
+ AV_PIX_FMT_FLAG_RGB | AV_PIX_FMT_FLAG_FLOAT,
+},
+[AV_PIX_FMT_GBRPF32LE] = {
+.name = "gbrpf32le",
+.nb_components = 3,
+.log2_chroma_w = 0,
+.log2_chroma_h = 0,
+.comp = {
+{ 2, 4, 0, 0, 32, 3, 31, 1 },/* R */
+{ 0, 4, 0, 0, 32, 3, 31, 1 },/* G */
+{ 1, 4, 0, 0, 32, 3, 31, 1 },/* B */
+},
+.flags = AV_PIX_FMT_FLAG_PLANAR | AV_PIX_FMT_FLAG_FLOAT |
AV_PIX_FMT_FLAG_RGB,
+},
+[AV_PIX_FMT_GBRAPF32BE] = {
+.name = "gbrapf32be",
+.nb_components = 4,
+.log2_chroma_w = 0,
+.log2_chroma_h = 0,
+.comp = {
+{ 2, 4, 0, 0, 32, 3, 31, 1 },/* R */
+{ 0, 4, 0, 0, 32, 3, 31, 1 },/* G */
+{ 1, 4, 0, 0, 32, 3, 31, 1 },/* B */
+{ 3, 4, 0, 0, 32, 3, 31, 1 },/* A */
+},
+.flags = AV_PIX_FMT_FLAG_BE | AV_PIX_FMT_FLAG_PLANAR |
+ AV_PIX_FMT_FLAG_ALPHA | AV_PIX_FMT_FLAG_RGB |
+ AV_PIX_FMT_FLAG_FLOAT,
+},
+[AV_PIX_FMT_GBRAPF32LE] = {
+.name = "gbrapf32le",
+.nb_components = 4,
+.log2_chroma_w = 0,
+.log2_chroma_h = 0,
+.comp = {
+{ 2, 4, 0, 0, 32, 3, 31, 1 },/* R */
+{ 0, 4, 0, 0, 32, 3, 31, 1 },/* G */
+{ 1, 4, 0, 0, 32, 3, 31, 1 },/* B */
+{ 3, 4, 0, 0, 32, 3, 31, 1 },/* A */
+},
+.flags = AV_PIX_FMT_FLAG_PLANAR | AV_PIX_FMT_FLAG_ALPHA |
+ AV_PIX_FMT_FLAG_RGB | AV_PIX_FMT_FLAG_FLOAT,
+},
};
#if FF_API_PLUS1_MINUS1
FF_ENABLE_DEPRECATION_WARNINGS
diff --git a/libavutil/pixdesc.h b/libavutil/pixdesc.h
index c3a6f27f49..294555260a 100644
--- a/libavutil/pixdesc.h
+++ b/libavutil/pixdesc.h
@@ -178,6 +178,11 @@ typedef struct AVPixFmtDescriptor {
#define AV_PIX_FMT_FLAG_BAYER(1 << 8)
/**
+ * The pixel format contains IEEE-754 single precision floating point values.
+ */
+#define AV_PIX_FMT_FLAG_FLOAT(1 << 10)
+
+/**
* Return the number of bits per pixel used by the pixel format
* described by pixdesc. Note that this is not the same as the number
* of bits per sample.
diff --git a/libavutil/pixfmt.h b/libavutil/pixfmt.h
index d4a39dcc01..6012bcebde 100644
--- a/libavutil/pixfmt.h
+++ b/libavutil/pixfmt.h
@@ -329,6 +329,11 @@ enum AVPixelFormat {
AV_PIX_FMT_GRAY9BE, ///
Re: [FFmpeg-devel] [PATCH] JPEG2000: SSE optimisation of DWT decoding
On 8/10/17, maxime taisant wrote:
>> From: Ivan Kalvachev
>> On 8/8/17, maxime taisant wrote:
>> > From: Maxime Taisant
>> >
>> > Hi,
>> >
>> > Here is some SSE optimisations for the dwt function used to decode
>> > JPEG2000.
>> > I tested this code by using the time command while reading a JPEG2000
>> > encoded video with ffmpeg and, on average, I observed a 4.05% general
>> > improvement, and a 12.67% improvement on the dwt decoding part alone.
BTW, forgot to tell you that FFmpeg has its own benchmarking macros
that counts the cpu cycles that it takes for a execution of block of C code.
Use it (in .c files) like this
#include "libavutil/timer.h"
{
START_TIMER
function();
STOP_TIMER("function")
}
The functions would output the results to stderr,
and they would use the name you provide to them.
(So you can benchmark more than one thing at a time).
Make sure the function(s) runs a lot per benchmark run.
The macro would show results at log2 measures.
Do more (3 or 5) separate benchmark runs, since
the final results always slightly differs. (function could
be interrupted, and if detected the measurement
would be discarded/skipped).
Also, try the macro without any function inside,
so you have the "NULL" function. The stop_timer
has an instruction fence opcode, that blocks until
all prior microcodes are executed and this could
take a while. Your benchmarks could never get
faster than the NULL function.
>> > In the nasm code, you can notice that the SR1DFLOAT macro appear
>> > twice. One version is called in the nasm code by the HORSD macro and
>> > the other is called in the C code of the dwt function, I couldn't
>> > figure out a way to make only one macro.
>>
>> You want to use the same macro at two locations or you want to have
>> 1 function and "call" it from 2 places?
>>
>> For the former, I'd guess that you might have been getting errors
>> about duplicated labels, since you use the local to the file form instead
>> local to the macro form. aka: ".loop" vs "%%loop".
>
> Currently I have one function declared with "cglobal" and called in the C
> code, and one macro with exactly the same behavior used in the nasm code.
> So I guess I would like to keep only one of the two and call it from both
> places. (Sorry if it's still not clear, English is not my native language).
Now I'm even more confused... ;)
Don't worry, I'm also not native English speaker.
>> > I also couldn't figure out a good way to optimize the VER_SD part, so
>> > that is why I left it unchanged, with just a SSE-optimized version of
>> > the SR_1D_FLOAT function.
>>
>> [...]
>> > +.extend:
>> > +shl i0d, 2
>> > +shl i1d, 2
>> > +mov j0q, i0q
>> > +mov j1q, i1q
>> > +movups m0, [lineq+j0q+4]
>> > +shufps m0, m0, 0x1B
>>
>> The x86inc provides with readable method for the shuffle constant.
>> q where X is index in the source reg.
>> Using q3210 would generate constant that leaves all elements at their
>> original places.
>> The 0x1B is q0123 , that is swap, isn't it?.
>>
>> Also, minor cosmetic nitpick.
>> usually the first parameters are placed so their commas are vertically
>> aligned.
>> This applies only when the parameter is register (so no jmp labels or []
>> addresses ).
>>
>
> Ok, I will change all that.
>
>> [...]
>> > +;line{2*i,2*(i+1),2*(i+2),2*(i+3)} -=
>> > F_LFTG_DELTA*(line{2*i-1,2*(i+1)-1,2*(i+2)-1,2*(i+3)-
>> 1}+line{2*i+1,2*(
>> > i+1)+1,2*(i+2)+1,2*(i+3)+1})
>> > +movups m0, [lineq+2*j0q-28]
>> > +movups m4, [lineq+2*j0q-12]
>> > +movups m1, m0
>> > +shufps m0, m4, 0xDD
>> > +shufps m1, m4, 0x88
>>
>> The x86inc provides with a way to emulate 3 operand avx.
>> This means it hides one of the movaps (use 'a' for reg reg).
>> shufps m1, m0, m4, 0x88
>> shufps m0, m4, 0xDD
>
> I know, but I figured that I would do a sse version first and add avx
> support afterwards.
On 8/11/17, maxime taisant wrote:
>
>> From: Maxime Taisant
>>
>> > From: Ivan Kalvachev
>> >
>> > On 8/8/17, maxime taisant wrote:
>> > > From: Maxime Taisant
>> > >
>> > > +movups m2, [lineq+2*j0q-24]
>> > > +movups m5, [lineq+2*j0q-8]
>> > > +shufps m2, m5, 0xDD
>> > > +addps m2, m1
>> > > +mulps m2, m3
>> > > +subps m0, m2
>> > > +movups m4, m1
>> > > +shufps m1, m0, 0x44 ; 0100'0100 q1010
>> > Is that movlhps m1, m0 ?
>>
>> No, this command place the first two values of m1 in the last two
>> doublewords of m1, and the first two values of m0 in the first two
>> doublewords of m1.
>> Movhlps would simply replace the first two values of m1 by the ones
>> of m0.
>>
>
> Ok, so everything I said here is wrong...
> You were right, that IS movlhps m1, m0.
> Will change that, sorry.
I do not insist replacing the shufps with movlhps,
most modern CPU execute both for the same cycles.
(Well, some old AMD cpu do have movlhps faster and
it might execute at different subdomain...
Run some benchmarks.)
Just make sure your shuffles are correct
Re: [FFmpeg-devel] [PATCH] JPEG2000: SSE optimisation of DWT decoding
On Fri, Aug 11, 2017 at 06:32:37PM +0300, Ivan Kalvachev wrote:
> On 8/10/17, maxime taisant wrote:
> >> From: Ivan Kalvachev
> >> On 8/8/17, maxime taisant wrote:
> >> > From: Maxime Taisant
> >> >
> >> > Hi,
> >> >
> >> > Here is some SSE optimisations for the dwt function used to decode
> >> > JPEG2000.
> >> > I tested this code by using the time command while reading a JPEG2000
> >> > encoded video with ffmpeg and, on average, I observed a 4.05% general
> >> > improvement, and a 12.67% improvement on the dwt decoding part alone.
>
> BTW, forgot to tell you that FFmpeg has its own benchmarking macros
> that counts the cpu cycles that it takes for a execution of block of C code.
> Use it (in .c files) like this
>
> #include "libavutil/timer.h"
> {
> START_TIMER
> function();
> STOP_TIMER("function")
>}
>
> The functions would output the results to stderr,
> and they would use the name you provide to them.
> (So you can benchmark more than one thing at a time).
>
> Make sure the function(s) runs a lot per benchmark run.
> The macro would show results at log2 measures.
>
> Do more (3 or 5) separate benchmark runs, since
> the final results always slightly differs. (function could
> be interrupted, and if detected the measurement
> would be discarded/skipped).
>
> Also, try the macro without any function inside,
> so you have the "NULL" function. The stop_timer
> has an instruction fence opcode, that blocks until
> all prior microcodes are executed and this could
> take a while. Your benchmarks could never get
> faster than the NULL function.
>
j2k is already in checkasm; I'd suggest to integrate test(s) for the
functions in that place (tests/checkasm/jpeg2000dsp.c).
make checkasm && tests/checkasm/checkasm --test=jpeg2000dsp --bench
This will make use of the {START,STOP}_TIMER code and provide clean
performance comparisons which you can share here.
--
Clément B.
signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 01/16] doc/filters: document the unstability of the shorthand options notation.
On Fri, Aug 11, 2017 at 12:33:25PM +0200, Nicolas George wrote:
> Le quartidi 24 thermidor, an CCXXV, Clement Boesch a écrit :
> > I'm afraid of the situation where a developer will feel like the order of
> > the options is not ideal, or an option could be renamed for consistency
> > with other filters, and will take the easy way out "oh well, we documented
> > it's unstable, users can only blame themselves for relying on it, not even
> > mentioning the new order/name is much better for everyone".
>
> In my mind, even with this documentation commit, changing the order of
> an option would still require posting a patch on the mailing-list to
> discuss it. It could be stated explicitly, but I do not know where (the
> user documentation is not the place).
>
> As for renaming options, this is absolutely not allowed.
>
> > If you're referring to this current patch serie, can you list here all the
> > filters that are affected by an API break and to what degree?
> >
> > We can discuss here if it's an acceptable change or not, and should
> > require a compatibility code (maybe it will be for one or two filters
> > only).
>
> All the filters that use framesync2.h and have any of the "shortest",
> "eof_action" and "repeatlast" options lose the possibility to set these
> options by shorthand notation. I.e. you can no longer put 1 in the fifth
> or sixth position in the arguments list and hope it will fall on
> "shortest".
>
That looks like it has a marginal effect in this particular case. I'd
agree with just documenting it in the Changelog and still not making it
"the rule" (that is, NAK on the doc patch). Also please bump minor or
micro.
If other people insist on compatibility, I'd suggest the following:
In ff_framesync2_init(), run av_opt_find() with "eof_action", "shortest"
and "repeatlast" on parent (AVFilterContext); if you can't find the option
you fallback on the framesync opts values. Then you wrap the fields and
options in the filters with ifdefery, and announce the future breakage in
doc/APIchanges.
I don't think that's worth the hassle (even though I don't think that's
much more work), but people may disagree.
> > We could consider an API to deprecate an AVOption for future similar
> > issue. I think we already had such discussion in the past (maybe that was
> > around the time we changed "user-agent" into "user_agent"), involving the
> > presence of a flag.
>
> Yes, we could, and that takes time that could be dedicated to fixing
> actual bugs or implementing new features. Ask any user: "if you agree to
> always write x=, y= in your overlay filters, I can fix your bug faster,
> what do you prefer?", I know what they will answer.
You know the answer of the user with the bug, not the hundreds others who
don't have the bug, rely on the shorthand, read a SO/reddit/forum post
with a now non-working example, or execute a random plugin of an app which
suddenly stop working.
But your patch doesn't seem to affect the x and y of overlay, so everything is
fine, right? :)
[...]
BTW, now that I think about it, how about this:
We add a flag to AVOption such as AV_OPT_FLAG_FILTER_STABLE_SHORTHAND and
flag all the options that we believe won't need to move in the future and
for which we can always rely on (typically x and y in overlay, or w and h
in scale).
Then we simply warn the user for every shorthand use of other options
("using short-hand form with '' may cause issue in the future as
it could be swapped around").
That way, we:
- give clear visibility of the instability of (some of) the shorthands
- affect only marginal uses (that is, late options because we are
generally going to flag the few first ones)
- gain flexibility to indeed swap around the options at will in the future
(we initially wait for a release or two such that the users gets aware
of their potential misuses with the introduced warning)
- keep shorthands useful for the most essential cases
- provide trust on the current use of shorthands.
Regards,
--
Clément B.
signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 1/3] avcodec/ffv1dec_template: Fix undefined shift
Fixes: runtime error: left shift of negative value -127
Fixes: 2834/clusterfuzz-testcase-minimized-5988039123795968
Found-by: continuous fuzzing process
https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer
---
libavcodec/ffv1dec_template.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavcodec/ffv1dec_template.c b/libavcodec/ffv1dec_template.c
index 61cdc90116..d41d807e64 100644
--- a/libavcodec/ffv1dec_template.c
+++ b/libavcodec/ffv1dec_template.c
@@ -149,7 +149,7 @@ static void RENAME(decode_rgb_frame)(FFV1Context *s,
uint8_t *src[3], int w, int
}
if (lbd)
-*((uint32_t*)(src[0] + x*4 + stride[0]*y)) = b + (g<<8) +
(r<<16) + (a<<24);
+*((uint32_t*)(src[0] + x*4 + stride[0]*y)) = b +
((unsigned)g<<8) + ((unsigned)r<<16) + ((unsigned)a<<24);
else if (sizeof(TYPE) == 4) {
*((uint16_t*)(src[0] + x*2 + stride[0]*y)) = g;
*((uint16_t*)(src[1] + x*2 + stride[1]*y)) = b;
--
2.13.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 2/3] avcodec/gdv: Check available space before reading palette
Fixes: Timeout
Fixes: 2926/clusterfuzz-testcase-498711001458278
Found-by: continuous fuzzing process
https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer
---
libavcodec/gdv.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/libavcodec/gdv.c b/libavcodec/gdv.c
index b324e4f26e..dc91869edf 100644
--- a/libavcodec/gdv.c
+++ b/libavcodec/gdv.c
@@ -427,6 +427,8 @@ static int gdv_decode_frame(AVCodecContext *avctx, void
*data,
case 1:
memset(gdv->frame + PREAMBLE_SIZE, 0, gdv->frame_size - PREAMBLE_SIZE);
case 0:
+if (bytestream2_get_bytes_left(gb) < 256*3)
+return AVERROR_INVALIDDATA;
for (i = 0; i < 256; i++) {
unsigned r = bytestream2_get_byte(gb);
unsigned g = bytestream2_get_byte(gb);
--
2.13.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 3/3] avcodec/lagarith: Detect end of input in lag_decode_line() loop
Fixes: timeout
Fixes: 2933/clusterfuzz-testcase-5124990208835584
Found-by: continuous fuzzing process
https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer
---
libavcodec/lagarith.c| 6 --
libavcodec/lagarithrac.c | 1 +
libavcodec/lagarithrac.h | 5 +
3 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/libavcodec/lagarith.c b/libavcodec/lagarith.c
index 860381746d..0f4aa89486 100644
--- a/libavcodec/lagarith.c
+++ b/libavcodec/lagarith.c
@@ -455,10 +455,12 @@ static int lag_decode_arith_plane(LagarithContext *l,
uint8_t *dst,
return -1;
ff_lag_rac_init(&rac, &gb, length - stride);
-
-for (i = 0; i < height; i++)
+for (i = 0; i < height; i++) {
+if (rac.overread > MAX_OVERREAD)
+return AVERROR_INVALIDDATA;
read += lag_decode_line(l, &rac, dst + (i * stride), width,
stride, esc_count);
+}
if (read > length)
av_log(l->avctx, AV_LOG_WARNING,
diff --git a/libavcodec/lagarithrac.c b/libavcodec/lagarithrac.c
index 3d36d1b9e9..cdda67fb81 100644
--- a/libavcodec/lagarithrac.c
+++ b/libavcodec/lagarithrac.c
@@ -46,6 +46,7 @@ void ff_lag_rac_init(lag_rac *l, GetBitContext *gb, int
length)
l->range= 0x80;
l->low = *l->bytestream >> 1;
l->hash_shift = FFMAX(l->scale, 10) - 10;
+l->overread = 0;
for (i = j = 0; i < 1024; i++) {
unsigned r = i << l->hash_shift;
diff --git a/libavcodec/lagarithrac.h b/libavcodec/lagarithrac.h
index dfdfea0db3..ee836d01db 100644
--- a/libavcodec/lagarithrac.h
+++ b/libavcodec/lagarithrac.h
@@ -47,6 +47,9 @@ typedef struct lag_rac {
const uint8_t *bytestream;/**< Current position in input
bytestream. */
const uint8_t *bytestream_end;/**< End position of input bytestream. */
+int overread;
+#define MAX_OVERREAD 4
+
uint32_t prob[258]; /**< Table of cumulative probability for each
symbol. */
uint8_t range_hash[1024]; /**< Hash table mapping upper byte to
approximate symbol. */
} lag_rac;
@@ -62,6 +65,8 @@ static inline void lag_rac_refill(lag_rac *l)
l->low |= 0xff & (AV_RB16(l->bytestream) >> 1);
if (l->bytestream < l->bytestream_end)
l->bytestream++;
+else
+l->overread++;
}
}
--
2.13.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 01/16] doc/filters: document the unstability of the shorthand options notation.
Le quartidi 24 thermidor, an CCXXV, Clement Boesch a écrit :
> That looks like it has a marginal effect in this particular case. I'd
I would not have proposed something worse.
> agree with just documenting it in the Changelog and still not making it
> "the rule" (that is, NAK on the doc patch). Also please bump minor or
> micro.
Ok.
> If other people insist on compatibility, I'd suggest the following:
>
> In ff_framesync2_init(), run av_opt_find() with "eof_action", "shortest"
> and "repeatlast" on parent (AVFilterContext); if you can't find the option
> you fallback on the framesync opts values. Then you wrap the fields and
> options in the filters with ifdefery, and announce the future breakage in
> doc/APIchanges.
>
> I don't think that's worth the hassle (even though I don't think that's
> much more work), but people may disagree.
I will not oppose if somebody does it, but I think it would be a waste
of time.
> BTW, now that I think about it, how about this:
>
> We add a flag to AVOption such as AV_OPT_FLAG_FILTER_STABLE_SHORTHAND and
> flag all the options that we believe won't need to move in the future and
> for which we can always rely on (typically x and y in overlay, or w and h
> in scale).
>
> Then we simply warn the user for every shorthand use of other options
> ("using short-hand form with '' may cause issue in the future as
> it could be swapped around").
>
> That way, we:
>
> - give clear visibility of the instability of (some of) the shorthands
> - affect only marginal uses (that is, late options because we are
> generally going to flag the few first ones)
> - gain flexibility to indeed swap around the options at will in the future
> (we initially wait for a release or two such that the users gets aware
> of their potential misuses with the introduced warning)
> - keep shorthands useful for the most essential cases
> - provide trust on the current use of shorthands.
It looks nice on principle, but I think you forget one consideration
about convenience: learning which option is stable and which is not
requires more effort than just putting the names of the options always.
So as soon as we acknowledge that some options are not stable with
shorthand, it becomes more convenient to use the names, and any
mitigation scheme that can be implemented will just be mostly unused.
Think of it that way: if we had designed the filters with an
AVOption-based system from the start instead of going for a simple
string and a custom parser, same as every time, would we have
implemented the shorthand system? No, we would just have used the
standard key=value:key=value... parser. The shorthand system only exists
because we thought we could get away with parsing the filters options
with sscanf(opt_str, "%d:%d", &w, &h) and we did not have the
clairvoyance to make a clean break when switching to AVOption.
Regards,
--
Nicolas George
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 01/16] doc/filters: document the unstability of the shorthand options notation.
Hi all,
sorry for jumping into the discussion late. I think it is very
important though. Please pardon me if I missed anything from
the previous discussions on this topic.
On 2017-08-11 23:34 +0200, Nicolas George wrote:
> Le quartidi 24 thermidor, an CCXXV, Clement Boesch a écrit :
[...]
> > BTW, now that I think about it, how about this:
> >
> > We add a flag to AVOption such as AV_OPT_FLAG_FILTER_STABLE_SHORTHAND and
> > flag all the options that we believe won't need to move in the future and
> > for which we can always rely on (typically x and y in overlay, or w and h
> > in scale).
> >
> > Then we simply warn the user for every shorthand use of other options
> > ("using short-hand form with '' may cause issue in the future as
> > it could be swapped around").
Besides AV_OPT_FLAG_FILTER_STABLE_SHORTHAND being an awfully long name
for it, I think it is a good idea.
Maybe even explicitly marking the options that can be used without name
and have a stable order and not allowing the others to be used without
name would be even better. Documentation should clearly indicate for
which options the name could be omitted. I guess we would have to come
up with a clean notation for that, but that shouldn't be too hard.
> > That way, we:
> >
> > - give clear visibility of the instability of (some of) the shorthands
> > - affect only marginal uses (that is, late options because we are
> > generally going to flag the few first ones)
> > - gain flexibility to indeed swap around the options at will in the future
> > (we initially wait for a release or two such that the users gets aware
> > of their potential misuses with the introduced warning)
> > - keep shorthands useful for the most essential cases
> > - provide trust on the current use of shorthands.
>
> It looks nice on principle, but I think you forget one consideration
> about convenience: learning which option is stable and which is not
> requires more effort than just putting the names of the options always.
> So as soon as we acknowledge that some options are not stable with
> shorthand, it becomes more convenient to use the names, and any
> mitigation scheme that can be implemented will just be mostly unused.
I do not think so. For many use cases *not* using named options will
clearly be more attractive, e.g. only one stupid and simple example is
vf fps.
I think if it is not allowed (like I suggested above) to give some
arguments with the shorthand notation, this will support the use cases
where using the shorthand notation is most useful and convenient.
> Think of it that way: if we had designed the filters with an
> AVOption-based system from the start instead of going for a simple
> string and a custom parser, same as every time, would we have
> implemented the shorthand system? No, we would just have used the
> standard key=value:key=value... parser. The shorthand system only exists
> because we thought we could get away with parsing the filters options
> with sscanf(opt_str, "%d:%d", &w, &h) and we did not have the
> clairvoyance to make a clean break when switching to AVOption.
To me this reads like a huge over simplification, though it is
partially correct. But I strongly believe the shorthand notation
was always much more than a side effect of choosing the wrong
way to implement things.
Alexander
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
