Re: [FFmpeg-devel] [PATCH 1/3] avformat/mux: add a format flag which ensure parsed and standardized creation time
On Thu, 25 Feb 2016 02:11:00 +0100 Marton Balint wrote: > This can be used for formats which write all format metadata as string to > files, therefore non-standard creation times such as 'now' will be parsed. > > The standardized creation time is UTC ISO 8601 with microsecond precision. > > Signed-off-by: Marton Balint > --- > libavformat/avformat.h | 1 + > libavformat/mux.c | 24 > libavformat/version.h | 2 +- > 3 files changed, 26 insertions(+), 1 deletion(-) > > diff --git a/libavformat/avformat.h b/libavformat/avformat.h > index 2b6533c..ea82181 100644 > --- a/libavformat/avformat.h > +++ b/libavformat/avformat.h > @@ -502,6 +502,7 @@ typedef struct AVProbeData { > The user or muxer can override this > through > AVFormatContext.avoid_negative_ts > */ > +#define AVFMT_NEED_PARSED_CREATION_TIME 0x8 /**< Format needs > pre-parsed standardized creation time */ > > #define AVFMT_SEEK_TO_PTS 0x400 /**< Seeking is based on PTS */ > > diff --git a/libavformat/mux.c b/libavformat/mux.c > index 789c811..9a39064 100644 > --- a/libavformat/mux.c > +++ b/libavformat/mux.c > @@ -36,10 +36,12 @@ > #include "libavutil/mathematics.h" > #include "libavutil/parseutils.h" > #include "libavutil/time.h" > +#include "libavutil/time_internal.h" > #include "riff.h" > #include "audiointerleave.h" > #include "url.h" > #include > +#include > #if CONFIG_NETWORK > #include "network.h" > #endif > @@ -383,6 +385,28 @@ FF_ENABLE_DEPRECATION_WARNINGS > } > } > > +/* pre-parse creation time for formats that need it */ > +if (s->oformat->flags & AVFMT_NEED_PARSED_CREATION_TIME) { > +int64_t timestamp; > +if (ff_parse_creation_time_metadata(s, ×tamp, 0) == 1) { > +time_t seconds = timestamp / 100; > +struct tm *ptm, tmbuf; > +ptm = gmtime_r(&seconds, &tmbuf); > +if (ptm) { > +char buf[32]; > +if (!strftime(buf, sizeof(buf), "%Y-%m-%dT%H:%M:%S", ptm)) { > +ret = AVERROR_EXTERNAL; > +goto fail; > +} > +av_strlcatf(buf, sizeof(buf), ".%06dZ", (int)(timestamp % > 100)); > +av_dict_set(&s->metadata, "creation_time", buf, 0); > +} else { > +ret = AVERROR_EXTERNAL; > +goto fail; > +} > +} > +} > + > /* set muxer identification string */ > if (!(s->flags & AVFMT_FLAG_BITEXACT)) { > av_dict_set(&s->metadata, "encoder", LIBAVFORMAT_IDENT, 0); > diff --git a/libavformat/version.h b/libavformat/version.h > index 82a8892..9f4ede7 100644 > --- a/libavformat/version.h > +++ b/libavformat/version.h > @@ -31,7 +31,7 @@ > > #define LIBAVFORMAT_VERSION_MAJOR 57 > #define LIBAVFORMAT_VERSION_MINOR 26 > -#define LIBAVFORMAT_VERSION_MICRO 100 > +#define LIBAVFORMAT_VERSION_MICRO 101 > > #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \ > LIBAVFORMAT_VERSION_MINOR, \ Wouldn't it be better to make this a function, and adding a function call to the muxers which need this, instead of adding yet another flag and adding that flag to the muxers? ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] avfilter: add datascope filter
Hi, patch attached. From 79bae07c716341eb4215c3b61b02c93ff7a9f6ec Mon Sep 17 00:00:00 2001 From: Paul B Mahol Date: Tue, 23 Feb 2016 22:41:07 +0100 Subject: [PATCH] avfilter: add datascope filter Signed-off-by: Paul B Mahol --- doc/filters.texi | 37 + libavfilter/Makefile | 1 + libavfilter/allfilters.c | 1 + libavfilter/vf_datascope.c | 386 + 4 files changed, 425 insertions(+) create mode 100644 libavfilter/vf_datascope.c diff --git a/doc/filters.texi b/doc/filters.texi index 344e140..fac9395 100644 --- a/doc/filters.texi +++ b/doc/filters.texi @@ -5240,6 +5240,43 @@ curves=psfile='MyCurvesPresets/purple.acv':green='0.45/0.53' @end example @end itemize +@section datascope + +Video data analysis filter. + +This filter shows hexadecimal pixel values of part of video. + +The filter accepts the following options: + +@table @option +@item size, s +Set output video size. + +@item x +Set x offset from where to pick pixels. + +@item y +Set y offset from where to pick pixels. + +@item mode +Set scope mode, can be one of the following: +@table @samp +@item mono +Draw hexadecimal pixel values with white color on black background. + +@item color +Draw hexadecimal pixel values with input video pixel color on black +background. + +@item color2 +Draw hexadecimal pixel values on color background picked from input video, +the text color is picked in such way so its always visible. +@end table + +@item axis +Draw rows and columns numbers on left and top of video. +@end table + @section dctdnoiz Denoise frames using 2D DCT (frequency domain filtering). diff --git a/libavfilter/Makefile b/libavfilter/Makefile index c451737..1f12329 100644 --- a/libavfilter/Makefile +++ b/libavfilter/Makefile @@ -135,6 +135,7 @@ OBJS-$(CONFIG_COVER_RECT_FILTER) += vf_cover_rect.o lavfutils.o OBJS-$(CONFIG_CROP_FILTER) += vf_crop.o OBJS-$(CONFIG_CROPDETECT_FILTER) += vf_cropdetect.o OBJS-$(CONFIG_CURVES_FILTER) += vf_curves.o +OBJS-$(CONFIG_DATASCOPE_FILTER) += vf_datascope.o OBJS-$(CONFIG_DCTDNOIZ_FILTER) += vf_dctdnoiz.o OBJS-$(CONFIG_DEBAND_FILTER) += vf_deband.o OBJS-$(CONFIG_DECIMATE_FILTER) += vf_decimate.o diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c index fecccfc..c8978a5 100644 --- a/libavfilter/allfilters.c +++ b/libavfilter/allfilters.c @@ -156,6 +156,7 @@ void avfilter_register_all(void) REGISTER_FILTER(CROP, crop, vf); REGISTER_FILTER(CROPDETECT, cropdetect, vf); REGISTER_FILTER(CURVES, curves, vf); +REGISTER_FILTER(DATASCOPE, datascope, vf); REGISTER_FILTER(DCTDNOIZ, dctdnoiz, vf); REGISTER_FILTER(DEBAND, deband, vf); REGISTER_FILTER(DECIMATE, decimate, vf); diff --git a/libavfilter/vf_datascope.c b/libavfilter/vf_datascope.c new file mode 100644 index 000..c15c4ce --- /dev/null +++ b/libavfilter/vf_datascope.c @@ -0,0 +1,386 @@ +/* + * Copyright (c) 2016 Paul B Mahol + * + * 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 + */ + +#include "libavutil/avassert.h" +#include "libavutil/intreadwrite.h" +#include "libavutil/opt.h" +#include "libavutil/parseutils.h" +#include "libavutil/pixdesc.h" +#include "libavutil/xga_font_data.h" +#include "avfilter.h" +#include "drawutils.h" +#include "formats.h" +#include "internal.h" +#include "video.h" + +typedef struct DatascopeContext { +const AVClass *class; +int ow, oh; +int x, y; +int mode; +int axis; + +int nb_planes; +int nb_comps; +int chars; +FFDrawContext draw; +FFDrawColor yellow; +FFDrawColor white; +FFDrawColor black; + +int (*filter)(AVFilterContext *ctx, void *arg, int jobnr, int nb_jobs); +} DatascopeContext; + +#define OFFSET(x) offsetof(DatascopeContext, x) +#define FLAGS AV_OPT_FLAG_FILTERING_PARAM|AV_OPT_FLAG_VIDEO_PARAM + +static const AVOption datascope_options[] = { +{ "size", "set output size", OFFSET(ow), AV_OPT_TYPE_IMAGE_SIZE, {.str="hd720"}, 0, 0, FLAGS }, +{ "s","set output size", OFFSET(ow), AV_OPT_TYPE_IMAGE_SIZE, {.str="hd720"}, 0, 0,
Re: [FFmpeg-devel] [PATCH] lavf/mux: do not fail in case of non monotonically increasing DTS values for data packets
On date Tuesday 2016-02-23 20:32:23 +0100, Michael Niedermayer encoded: > On Tue, Feb 23, 2016 at 07:34:16PM +0100, Stefano Sabatini wrote: [...] > > > I have an use case with data packets that have the same PTS (which > > > makes sense in that specific case). Since I want to fix this issue, > > > and dropping the check inconditionally may not be acceptable, I > > > propose the following variants: > > > > > > * I add an option to drop strict monotonicity checks on data packets > > > > > > * I enable a format level capability which disables the strict > > > monotonicity requirement for data packets depending on the format > > > > > > What do you find acceptable? > > i think its fine to treat data the same as subtitles In general, we can assume that makes no sense to have more than one packet with the same PTS/DTS for audio or video and for the same stream. As for subtitles, I'm not sure we should accept that as well, but at least in theory I can imagine an use case with multiple packets (and thus frames) conveying text information pertaining to the same time and stream. As for data, I think we can safely assume several piece of generic data in different packets but with the same timestamp, since they are meant to be processed by an automated agent. So, while the check makes sense for audio and video, in the case of subtitles and data we are in the fuzzy area. In principle, I agree with the fact that data and subtitles should be treated in the same way, so I propose the following options: 1. disable the strict monotonicity check for both data and subtitles inconditionally 2. add a libavformat muxing option to disable strictness in the timestamps monotonicity check for both data and subtitles 3. generalize 2. and provide a libavformat muxing option to disable strictness in the timestamps monotonicity check for some selection of output streams (e.g. using stream specifiers) I have no special preference, so I'd leave this decision to the libavformat maintainer (which I believe it's still you ;-)). In case any of these solutions is not considered acceptable, I need to come with an application level solution (but this would mean that ffmpeg the tool cannot be used for that). -- FFmpeg = Fast and Formidable MultiPurpose Efficient Goblin ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavf/mux: do not fail in case of non monotonically increasing DTS values for data packets
Le septidi 7 ventôse, an CCXXIV, Stefano Sabatini a écrit : > As for subtitles, I'm not sure we should accept that as well, but at > least in theory I can imagine an use case with multiple packets (and > thus frames) conveying text information pertaining to the same time > and stream. Simultaneous subtitles happens in practice quite frequently with ASS: people talking at the same time, background dialog, translation of visible signs and other texts, karaoke and translation of songs, credits, etc. The strict monotonicity test was initially bypassed by merging the ASS dialog events together, but this was an ugly hack and Clément is trying to finally get rid of its last traces. Regards, -- Nicolas George 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/3] lavu/rational: use av_unlikely in av_d2q
Le sextidi 6 ventôse, an CCXXIV, Ganesh Ajjanagadde a écrit : > Actual performance benefit is impossible to accurately quantify due to the > context-dependence of the branch predictor. Nonetheless, as a ballpark > estimate, it yields ~ 5% improvements in testing via FATE on x86-64, > Haswell+GCC. Five percent is huge, if it is five percent of something relevant. Can you give a few more details? Regards, -- Nicolas George signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]lavc/mjpegdec: Fix decoding images with Adobe_CM tag
Michael Niedermayer niedermayer.cc> writes: > On Wed, Feb 24, 2016 at 06:52:13PM +0100, Carl Eugen Hoyos wrote: > > Hi! > > > > Attached patch fixes ticket #5267 for me. > LGTM Pushed with show_bits_long() to show the content more clearly. Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavf/mux: do not fail in case of non monotonically increasing DTS values for data packets
On Thu, Feb 25, 2016 at 01:11:36PM +0100, Stefano Sabatini wrote: > On date Tuesday 2016-02-23 20:32:23 +0100, Michael Niedermayer encoded: > > On Tue, Feb 23, 2016 at 07:34:16PM +0100, Stefano Sabatini wrote: > [...] > > > > I have an use case with data packets that have the same PTS (which > > > > makes sense in that specific case). Since I want to fix this issue, > > > > and dropping the check inconditionally may not be acceptable, I > > > > propose the following variants: > > > > > > > > * I add an option to drop strict monotonicity checks on data packets > > > > > > > > * I enable a format level capability which disables the strict > > > > monotonicity requirement for data packets depending on the format > > > > > > > > What do you find acceptable? > > > > > i think its fine to treat data the same as subtitles > > In general, we can assume that makes no sense to have more than one > packet with the same PTS/DTS for audio or video and for the same > stream. > > As for subtitles, I'm not sure we should accept that as well, but at > least in theory I can imagine an use case with multiple packets (and > thus frames) conveying text information pertaining to the same time > and stream. As for data, I think we can safely assume several piece > of generic data in different packets but with the same timestamp, > since they are meant to be processed by an automated agent. > > So, while the check makes sense for audio and video, in the case of > subtitles and data we are in the fuzzy area. In principle, I agree > with the fact that data and subtitles should be treated in the same > way, so I propose the following options: > > 1. disable the strict monotonicity check for both data and subtitles >inconditionally yes, do with data what is done with subtitles already IIRC > > 2. add a libavformat muxing option to disable strictness in the >timestamps monotonicity check for both data and subtitles > > 3. generalize 2. and provide a libavformat muxing option to disable >strictness in the timestamps monotonicity check for some selection >of output streams (e.g. using stream specifiers) > > I have no special preference, so I'd leave this decision to the > libavformat maintainer (which I believe it's still you ;-)). > > In case any of these solutions is not considered acceptable, I need to > come with an application level solution (but this would mean that > ffmpeg the tool cannot be used for that). > -- > FFmpeg = Fast and Formidable MultiPurpose Efficient Goblin > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel -- 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: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] build: add install_name_dir option
From: Clément Bœsch This option is typically useful when cross-compiling dynamic libraries for iOS, with something such as --install_name_dir=@rpath --- configure | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/configure b/configure index 8436b09..64750e4 100755 --- a/configure +++ b/configure @@ -91,6 +91,7 @@ Standard options: --enable-rpath use rpath to allow installing libraries in paths not part of the dynamic linker search path use rpath when linking programs [USE WITH CARE] + --install_name_dir=DIR Darwin directory name for installed targets Licensing options: --enable-gpl allow use of GPL code, the resulting libs @@ -2075,6 +2076,7 @@ PATHS_LIST=" pkgconfigdir prefix shlibdir +install_name_dir " CMDLINE_SET=" @@ -4516,7 +4518,8 @@ case $target_os in ;; darwin) enabled ppc && add_asflags -force_cpusubtype_ALL -SHFLAGS='-dynamiclib -Wl,-single_module -Wl,-install_name,$(SHLIBDIR)/$(SLIBNAME_WITH_MAJOR),-current_version,$(LIBVERSION),-compatibility_version,$(LIBMAJOR)' +install_name_dir_default='$(SHLIBDIR)' +SHFLAGS='-dynamiclib -Wl,-single_module -Wl,-install_name,$(INSTALL_NAME_DIR)/$(SLIBNAME_WITH_MAJOR),-current_version,$(LIBVERSION),-compatibility_version,$(LIBMAJOR)' enabled x86_32 && append SHFLAGS -Wl,-read_only_relocs,suppress strip="${strip} -x" add_ldflags -Wl,-dynamic,-search_paths_first @@ -6292,6 +6295,7 @@ DATADIR=\$(DESTDIR)$datadir DOCDIR=\$(DESTDIR)$docdir MANDIR=\$(DESTDIR)$mandir PKGCONFIGDIR=\$(DESTDIR)$pkgconfigdir +INSTALL_NAME_DIR=$install_name_dir SRC_PATH=$source_path SRC_LINK=$source_link ifndef MAIN_MAKEFILE -- 2.7.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] lavu/attributes: introduce av_likely, av_unlikely
On 2/25/2016 7:27 AM, wm4 wrote: > They also make code ugly as fuck, and are more cargo-cult than anything > else. They might possibly be ok in some very critical parts of the > code, but otherwise not at all. +1 to this. I will absolutely *not* ACK any patch sets that start littering likely/unlikely all over the codebase. I am indifferent to very limited use, but you must include tests showing it actually does something better. - Derek ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] build: add install_name_dir option
On Thu, Feb 25, 2016 at 2:35 PM, Clément Bœsch wrote: > From: Clément Bœsch > > This option is typically useful when cross-compiling dynamic libraries > for iOS, with something such as --install_name_dir=@rpath I don't know about any other things and their options for iOS building, but that option name in configure looks rather out of place. Can't we at least name it somewhat more fitting, maybe with dashes instead of underscores? - Hendrik ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] lavu/attributes: introduce av_likely, av_unlikely
On Thu, Feb 25, 2016 at 8:27 AM, wm4 wrote: > On Thu, 25 Feb 2016 15:48:17 +1100 > Matt Oliver wrote: > >> On 25 February 2016 at 13:20, Ganesh Ajjanagadde wrote: >> >> > From: Ganesh Ajjanagadde >> > >> > These use __builtin_expect, and may be useful for optimizing nearly >> > certain branches, to yield size and/or speed improvements. >> > >> > Note that this is used in the Linux kernel for the same purpose. For >> > some idea as to potential benefits, see e.g >> > http://blog.man7.org/2012/10/how-much-do-builtinexpect-likely-and.html. >> > >> > Signed-off-by: Ganesh Ajjanagadde >> > --- >> > libavutil/attributes.h | 8 >> > 1 file changed, 8 insertions(+) >> > >> > diff --git a/libavutil/attributes.h b/libavutil/attributes.h >> > index 5c6b9de..1547033 100644 >> > --- a/libavutil/attributes.h >> > +++ b/libavutil/attributes.h >> > @@ -58,6 +58,14 @@ >> > #define av_warn_unused_result >> > #endif >> > >> > +#if AV_GCC_VERSION_AT_LEAST(3,0) >> > +#define av_likely(x) __builtin_expect(!!(x), 1) >> > +#define av_unlikely(x) __builtin_expect(!!(x), 0) >> > +#else >> > +#define av_likely(x) (x) >> > +#define av_unlikely(x) (x) >> > +#endif >> > + >> > #if AV_GCC_VERSION_AT_LEAST(3,1) >> > #define av_noinline __attribute__((noinline)) >> > #elif defined(_MSC_VER) >> > >> >> Ive used these builtins before and can confirm that they are useful >> (although requires devs to use them obviously). I will also point out that >> these are also supported by ICC/ICL as well. > > They also make code ugly as fuck, and are more cargo-cult than anything > else. They might possibly be ok in some very critical parts of the > code, but otherwise not at all. > I agree with wm4 on the ugly aspect, I always hate reading code from projects that use it, its terrible on readability. If its used at all, it should be proven that (a) the function is performance relevant, and (b) that the improvement is actually tangible in the grand scheme. - Hendrik ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 3/3] lavu/rational: add more info regarding floor(x+0.5) usage
On 2/25/2016 2:20 AM, Ganesh Ajjanagadde wrote: > -// (int64_t)rint() and llrint() do not work with gcc on ia64 and sparc64 > +// (int64_t)rint() and llrint() do not work with gcc on ia64 and sparc64, > +// see Ticket2713 for affected gcc/glibc versions I've never seen this comment before, but it makes me wonder: how the heck do these functions work in the rest of the codebase? - Derek ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] build: add install_name_dir option
On Thu, Feb 25, 2016 at 02:47:45PM +0100, Hendrik Leppkes wrote: > On Thu, Feb 25, 2016 at 2:35 PM, Clément Bœsch wrote: > > From: Clément Bœsch > > > > This option is typically useful when cross-compiling dynamic libraries > > for iOS, with something such as --install_name_dir=@rpath > > I don't know about any other things and their options for iOS > building, but that option name in configure looks rather out of place. > Can't we at least name it somewhat more fitting, maybe with dashes > instead of underscores? > Sure, --install-name-dir is better. I didn't find really more appropriate section in the helper, but feel free to suggest any other position. -- 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] configure: only check dispatch header on darwin
From: Matthieu Bouron Fixes build of lavd/jack on linux if dispatch happens to be available on this platform. dispatch, as well as its dependencies kqueue and pwq are generally not installed / distribued on linux systems. If it happens to be the case, you want to explicitely link against the libraries (using -ldispatch) as opposed to darwin where it is part of the standard library and -ldispatch doesn't work. --- configure | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure b/configure index 45e751f..37c4277 100755 --- a/configure +++ b/configure @@ -4532,6 +4532,7 @@ case $target_os in enabled x86_64 && objformat="macho64" enabled_any pic shared x86_64 || { check_cflags -mdynamic-no-pic && add_asflags -mdynamic-no-pic; } +check_header dispatch/dispatch.h ;; msys*) die "Native MSYS builds are discouraged, please use the MINGW environment."; @@ -5346,7 +5347,6 @@ check_func_headers glob.h glob enabled xlib && check_func_headers "X11/Xlib.h X11/extensions/Xvlib.h" XvGetPortAttribute -lXv -lX11 -lXext -check_header dispatch/dispatch.h check_header direct.h check_header dirent.h check_header dlfcn.h -- 2.7.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] configure: only check dispatch header on darwin
On 25/02/2016 14:00, Matthieu Bouron wrote: From: Matthieu Bouron Fixes build of lavd/jack on linux if dispatch happens to be available on this platform. dispatch, as well as its dependencies kqueue and pwq are generally not installed / distribued on linux systems. If it happens to be the case, you want to explicitely link against the libraries (using -ldispatch) as opposed to darwin where it is part of the standard library and -ldispatch doesn't work. Systems other than OSX/darwin could potentially have a working libdispatch/GCD as well, I know FreeBSD has a port[1]. Maybe it would be a good idea to check if pthreads are available (or the system is OSX), and if not then properly check for libdispatch. Making it OSX-only is just going to require another patch when someone using libdispatch on another system wants it. - Josh [1] https://wiki.freebsd.org/GCD ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] configure: only check dispatch header on darwin
On Thu, Feb 25, 2016 at 03:34:37PM +, Josh de Kock wrote: > On 25/02/2016 14:00, Matthieu Bouron wrote: > >From: Matthieu Bouron > > > >Fixes build of lavd/jack on linux if dispatch happens to be available on > >this platform. dispatch, as well as its dependencies kqueue and pwq are > >generally not installed / distribued on linux systems. If it happens to > >be the case, you want to explicitely link against the libraries (using > >-ldispatch) as opposed to darwin where it is part of the standard > >library and -ldispatch doesn't work. > Systems other than OSX/darwin could potentially have a working > libdispatch/GCD as well, I know FreeBSD has a port[1]. In this case, is sem_timedwait present? In which case it wouldn't matter immediately. > Maybe it would be a good idea to check if pthreads are available (or the > system is OSX), and if not then properly check for libdispatch. > Making it OSX-only is just going to require another patch when someone using > libdispatch on another system wants it. I think this patch is just meant to fix a regression in the build. Regards, -- 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] configure: only check dispatch header on darwin
On 25/02/2016 15:44, Clément Bœsch wrote: On Thu, Feb 25, 2016 at 03:34:37PM +, Josh de Kock wrote: On 25/02/2016 14:00, Matthieu Bouron wrote: From: Matthieu Bouron Fixes build of lavd/jack on linux if dispatch happens to be available on this platform. dispatch, as well as its dependencies kqueue and pwq are generally not installed / distribued on linux systems. If it happens to be the case, you want to explicitely link against the libraries (using -ldispatch) as opposed to darwin where it is part of the standard library and -ldispatch doesn't work. Systems other than OSX/darwin could potentially have a working libdispatch/GCD as well, I know FreeBSD has a port[1]. In this case, is sem_timedwait present? In which case it wouldn't matter immediately. Yes, OSX/Darwin is the only OS, that I know of, which actually requires libdispatch as an alternative to pthreads. Maybe it would be a good idea to check if pthreads are available (or the system is OSX), and if not then properly check for libdispatch. Making it OSX-only is just going to require another patch when someone using libdispatch on another system wants it. I think this patch is just meant to fix a regression in the build. Just thought it wouldn't hurt to fix the real problem--that libdispatch isn't being checked for correctly, rather than a quick fix. Either is fine, I guess, although the quick fix would just require another patch, as I previously stated. - Josh ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] configure: only check dispatch header on darwin
Le septidi 7 ventôse, an CCXXIV, Josh de Kock a écrit : > Just thought it wouldn't hurt to fix the real problem--that libdispatch > isn't being checked for correctly, rather than a quick fix. Either is fine, > I guess, although the quick fix would just require another patch, as I > previously stated. Can someone test the patch that does things "correctly" on the OSes where it applies? If not, odds are it will not be exactly correct and will require still another patch. Regards, -- Nicolas George 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] img2dec: Add mime_type to image formats
On 2/24/2016 3:19 PM, Carl Eugen Hoyos wrote: > (Do you have an example where the probing fails?) > > The idea of these additional demuxers is that they probe > the content instead of relying on the file suffix. > File suffix provides a score of _EXTENSION (50), so if probing > is possible and uses more than 16 bit, _EXTENSION + 1 is > returned to allow a better detection. > AV_SCORE_MIME is 75, so iiuc this patch would just disable > autodetection (except for png, dds and webp which compare > 64 bit and return _MAX) in favour of the mimetype. > Or am I reading the code incorrectly and mimetype doesn't > lead to a score of 75? > > Why aren't the mimetypes added to the (original) image2 > demuxer if they are really needed? From the author: First I'll address the image2 suggestion. It certainly is possible to put a large list of image MIME types in the image2 demuxer, however, a MIME type is associated with a specific image format, so it is a waste of information not to use it to detect the specific format when appropriate. The primary purpose of image2 is for image sequences, so using MIME types for that is not applicable at all. Just because image2 calls the other probe functions internally to select the codec doesn't mean that the other image formats shouldn't be first-class citizens in terms of detection. In terms of how the score for a MIME type match compares with those of the individual content matching probe functions, I'd say it makes sense. The stronger probing functions have a score which reflects their reliability. If a MIME type is specified, that does have significant weight in terms of probability, given that it was set somewhere either by a client or server program doing a content check or explicitly by some user. So maybe it should override the weaker content checks. It is similar to extension, but it is less likely to be arbitrary if present and matching a specific media format. - Derek ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] configure: only check dispatch header on darwin
On 25/02/2016 15:53, Nicolas George wrote: Le septidi 7 ventôse, an CCXXIV, Josh de Kock a écrit : Just thought it wouldn't hurt to fix the real problem--that libdispatch isn't being checked for correctly, rather than a quick fix. Either is fine, I guess, although the quick fix would just require another patch, as I previously stated. Can someone test the patch that does things "correctly" on the OSes where it applies? If not, odds are it will not be exactly correct and will require still another patch. It works on OSX. - Josh ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/2] img2dec: Add mime_type to image formats
Justin Ruggles writes: > In terms of how the score for a MIME type match compares with > those of the individual content matching probe functions, I'd > say it makes sense. The stronger probing functions have a > score which reflects their reliability. But even _EXTENSION + 1 is correct in practically all cases (the exception are mpeg streams that start with the needed four bytes) and should not be beaten by mime_type. > Improves probing, especially over http when there is a > Content-Type header Please give an example of a failing stream. If you cannot share, please provide console output and hexdump of the relevant bytes. If this cannot be fixed differently, we should either increase the score for the existing 32bit probe functions or reduce the score for mime types before this gets applied. Several probe functions (outside of img2dec.c) return MAX for 32 bits, I just wanted to give problematic mpeg streams a better chance to get detected. The alternative is to only use mime type for jpeg: I am assuming this is the only problematic case, note that we have to make sure it's not mjpeg, that's why the probe function is so long. > If a MIME type is specified, that does have significant weight > in terms of probability, given that it was set somewhere either > by a client or server program doing a content check or > explicitly by some user. Wouldn't the server simply run "file" doing less checks than we already do now? (If it doesn't trust the extension.) Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/2] img2dec: Support Progressive JPEG in jpeg_probe
Carl Eugen Hoyos ag.or.at> writes: > Derek Buitenhuis gmail.com> writes: > > > From: Justin Ruggles gmail.com> > > > > There can be multiple SOS markers, so do not return 0 in that case. > > Looks good to me. The patch was merged by Derek. Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/2] img2dec: Add mime_type to image formats
On 2/25/2016 4:30 PM, Carl Eugen Hoyos wrote: Replies are mine. >> In terms of how the score for a MIME type match compares with >> those of the individual content matching probe functions, I'd >> say it makes sense. The stronger probing functions have a >> score which reflects their reliability. > > But even _EXTENSION + 1 is correct in practically all cases > (the exception are mpeg streams that start with the needed four > bytes) and should not be beaten by mime_type. URLs with no extension at all, or extensions with query params after them are very normal. This argument is silly. >> Improves probing, especially over http when there is a >> Content-Type header > > Please give an example of a failing stream. > If you cannot share, please provide console output and > hexdump of the relevant bytes. ... your argument is "it's not 100% broken so let's not improve it"? Really? > If this cannot be fixed differently, we should either increase > the score for the existing 32bit probe functions or reduce the > score for mime types before this gets applied. Several probe > functions (outside of img2dec.c) return MAX for 32 bits, I just > wanted to give problematic mpeg streams a better chance to get > detected. [...] > The alternative is to only use mime type for jpeg: I am > assuming this is the only problematic case, note that we have > to make sure it's not mjpeg, that's why the probe function is > so long. Um, what? Why the heck would you only use mime-type for JPEG? It is definitely not the only problematic case, and this sounds utterly silly. >> If a MIME type is specified, that does have significant weight >> in terms of probability, given that it was set somewhere either >> by a client or server program doing a content check or >> explicitly by some user. > > Wouldn't the server simply run "file" doing less checks than we > already do now? (If it doesn't trust the extension.) No. This is especially not true on modern CDNs (such as AWS or GCS), where files are stored as 'blob's and mimetype can be set properly during the push to storage. It's not 1999 anymore. - Derek ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/2] img2dec: Add mime_type to image formats
On 2/25/2016 4:30 PM, Carl Eugen Hoyos wrote: > If this cannot be fixed differently, we should either increase > the score for the existing 32bit probe functions or reduce the > score for mime types before this gets applied. Several probe > functions (outside of img2dec.c) return MAX for 32 bits, I just > wanted to give problematic mpeg streams a better chance to get > detected. I forgot to mention that lowering the score of mimetype-detected stuff is still OK, to me, at least. - Derek ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavf/mux: do not fail in case of non monotonically increasing DTS values for data packets
On date Thursday 2016-02-25 13:41:02 +0100, Michael Niedermayer encoded: > On Thu, Feb 25, 2016 at 01:11:36PM +0100, Stefano Sabatini wrote: [...] > > So, while the check makes sense for audio and video, in the case of > > subtitles and data we are in the fuzzy area. In principle, I agree > > with the fact that data and subtitles should be treated in the same > > way, so I propose the following options: > > > > 1. disable the strict monotonicity check for both data and subtitles > >inconditionally > > yes, do with data what is done with subtitles already IIRC See attached patch. [...] -- FFmpeg = Fiendish and Fierce Mastering Political Empowered Geisha >From 0996189555f4a2402b396801da318b4ffcfb1a13 Mon Sep 17 00:00:00 2001 From: Stefano Sabatini Date: Thu, 17 Dec 2015 20:51:42 +0100 Subject: [PATCH] lavf/mux: do not fail in case of non strictly monotonically increasing DTS values for data packets Consistent with what we already do with subtitles since ac08c5c0adcb7f2f9b5ea3eb473d1c2b9659aab2. --- libavformat/mux.c | 1 + 1 file changed, 1 insertion(+) diff --git a/libavformat/mux.c b/libavformat/mux.c index 789c811..eb0b973 100644 --- a/libavformat/mux.c +++ b/libavformat/mux.c @@ -554,6 +554,7 @@ static int compute_muxer_pkt_fields(AVFormatContext *s, AVStream *st, AVPacket * if (st->cur_dts && st->cur_dts != AV_NOPTS_VALUE && ((!(s->oformat->flags & AVFMT_TS_NONSTRICT) && st->codec->codec_type != AVMEDIA_TYPE_SUBTITLE && + st->codec->codec_type != AVMEDIA_TYPE_DATA && st->cur_dts >= pkt->dts) || st->cur_dts > pkt->dts)) { av_log(s, AV_LOG_ERROR, "Application provided invalid, non monotonically increasing dts to muxer in stream %d: %s >= %s\n", -- 1.9.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 3/3] lavu/rational: add more info regarding floor(x+0.5) usage
On Wed, Feb 24, 2016 at 09:20:11PM -0500, Ganesh Ajjanagadde wrote: > Add some more verbose info regarding why the imprecise and slow floor(x+0.5) > hack > is used; helpful for future maintenance. > > Signed-off-by: Ganesh Ajjanagadde > --- > libavutil/rational.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) LGTM thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Frequently ignored answer#1 FFmpeg bugs should be sent to our bugtracker. User questions about the command line tools should be sent to the ffmpeg-user ML. And questions about how to use libav* should be sent to the libav-user ML. signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] avformat/format: Print debug info when probe score is increased due to mime type
Signed-off-by: Michael Niedermayer --- libavformat/format.c |8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/libavformat/format.c b/libavformat/format.c index 15fe167..7fa06a6 100644 --- a/libavformat/format.c +++ b/libavformat/format.c @@ -223,8 +223,12 @@ AVInputFormat *av_probe_input_format3(AVProbeData *pd, int is_opened, if (av_match_ext(lpd.filename, fmt1->extensions)) score = AVPROBE_SCORE_EXTENSION; } -if (av_match_name(lpd.mime_type, fmt1->mime_type)) -score = FFMAX(score, AVPROBE_SCORE_MIME); +if (av_match_name(lpd.mime_type, fmt1->mime_type)) { +if (AVPROBE_SCORE_MIME > score) { +av_log(NULL, AV_LOG_DEBUG, "Probing %s score:%d increased to %d due to MIME type\n", fmt1->name, score, AVPROBE_SCORE_MIME); +score = AVPROBE_SCORE_MIME; +} +} if (score > score_max) { score_max = score; fmt = fmt1; -- 1.7.9.5 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] Move the |die| member of FrameThreadContext to PerThreadContext.
This fixes a data race warning by ThreadSanitizer. FrameThreadContext.die is read by all the worker threads but is not protected by any mutex. Move it to PerThreadContext so that each worker thread reads its own copy of |die|, which can then be protected with PerThreadContext.mutex. --- libavcodec/pthread_frame.c | 11 +-- 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c index b77dd1e..c5ac44f 100644 --- a/libavcodec/pthread_frame.c +++ b/libavcodec/pthread_frame.c @@ -93,6 +93,8 @@ typedef struct PerThreadContext { const enum AVPixelFormat *available_formats; ///< Format array for get_format() enum AVPixelFormat result_format;///< get_format() result + +int die;///< Set when the thread should exit. } PerThreadContext; /** @@ -111,8 +113,6 @@ typedef struct FrameThreadContext { * Set for the first N packets, where N is the number of threads. * While it is set, ff_thread_en/decode_frame won't return any results. */ - -int die; ///< Set when threads should exit. } FrameThreadContext; #define THREAD_SAFE_CALLBACKS(avctx) \ @@ -134,10 +134,10 @@ static attribute_align_arg void *frame_worker_thread(void *arg) pthread_mutex_lock(&p->mutex); while (1) { -while (p->state == STATE_INPUT_READY && !fctx->die) +while (p->state == STATE_INPUT_READY && !p->die) pthread_cond_wait(&p->input_cond, &p->mutex); -if (fctx->die) break; +if (p->die) break; if (!codec->update_thread_context && THREAD_SAFE_CALLBACKS(avctx)) ff_thread_finish_setup(avctx); @@ -553,12 +553,11 @@ void ff_frame_thread_free(AVCodecContext *avctx, int thread_count) fctx->threads->avctx->internal->is_copy = 1; } -fctx->die = 1; - for (i = 0; i < thread_count; i++) { PerThreadContext *p = &fctx->threads[i]; pthread_mutex_lock(&p->mutex); +p->die = 1; pthread_cond_signal(&p->input_cond); pthread_mutex_unlock(&p->mutex); -- 2.7.0.rc3.207.g0ac5344 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Move the |die| member of FrameThreadContext to PerThreadContext.
Hi, On Thu, Feb 25, 2016 at 2:51 PM, Wan-Teh Chang wrote: > This fixes a data race warning by ThreadSanitizer. But does it fix an actual bug? (Is there a sample this fixes?) If anything, you can use atomic ints instead of regular ints if we don't already [1]. Ronald [1] https://ffmpeg.org/doxygen/trunk/atomic_8h_source.html ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] mss2: Fix buffer overflow.
Reported as https://trac.mplayerhq.hu/ticket/2264 but have not been able to reproduce with FFmpeg-only. I have no idea what coded_height is used for here exactly, so this might not be the best fix. Fixes the following chain of events: ff_mss12_decode_init sets coded_height while not setting height. ff_mpv_decode_init then copies coded_height into MpegEncContext height. This is then used by init_context_frame to allocate the data structures. However the wmv9rects are validated/initialized based on avctx->height, not avctx->coded_height. Thus the decode_wmv9 function will try to decode a larger video that we allocated data structures for, causing out-of-bounds writes. Signed-off-by: Reimar Döffinger --- libavcodec/mss12.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libavcodec/mss12.c b/libavcodec/mss12.c index 6b58aa29..d42093b 100644 --- a/libavcodec/mss12.c +++ b/libavcodec/mss12.c @@ -581,8 +581,8 @@ av_cold int ff_mss12_decode_init(MSS12Context *c, int version, return AVERROR_INVALIDDATA; } -avctx->coded_width = AV_RB32(avctx->extradata + 20); -avctx->coded_height = AV_RB32(avctx->extradata + 24); +avctx->coded_width = FFMAX(AV_RB32(avctx->extradata + 20), avctx->width); +avctx->coded_height = FFMAX(AV_RB32(avctx->extradata + 24), avctx->height); if (avctx->coded_width > 4096 || avctx->coded_height > 4096) { av_log(avctx, AV_LOG_ERROR, "Frame dimensions %dx%d too large", avctx->coded_width, avctx->coded_height); -- 2.7.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] mss2: Fix buffer overflow.
On Thu, 25 Feb 2016 21:06:46 +0100 Reimar Döffinger wrote: > Reported as https://trac.mplayerhq.hu/ticket/2264 but have > not been able to reproduce with FFmpeg-only. > I have no idea what coded_height is used for here exactly, > so this might not be the best fix. > Fixes the following chain of events: > ff_mss12_decode_init sets coded_height while not setting height. > ff_mpv_decode_init then copies coded_height into MpegEncContext height. > This is then used by init_context_frame to allocate the data structures. > However the wmv9rects are validated/initialized based on avctx->height, not > avctx->coded_height. > Thus the decode_wmv9 function will try to decode a larger video that we > allocated data structures for, causing out-of-bounds writes. > > Signed-off-by: Reimar Döffinger > --- > libavcodec/mss12.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/libavcodec/mss12.c b/libavcodec/mss12.c > index 6b58aa29..d42093b 100644 > --- a/libavcodec/mss12.c > +++ b/libavcodec/mss12.c > @@ -581,8 +581,8 @@ av_cold int ff_mss12_decode_init(MSS12Context *c, int > version, > return AVERROR_INVALIDDATA; > } > > -avctx->coded_width = AV_RB32(avctx->extradata + 20); > -avctx->coded_height = AV_RB32(avctx->extradata + 24); > +avctx->coded_width = FFMAX(AV_RB32(avctx->extradata + 20), > avctx->width); > +avctx->coded_height = FFMAX(AV_RB32(avctx->extradata + 24), > avctx->height); > if (avctx->coded_width > 4096 || avctx->coded_height > 4096) { > av_log(avctx, AV_LOG_ERROR, "Frame dimensions %dx%d too large", > avctx->coded_width, avctx->coded_height); Just a side remark... I'm wondering why ff_set_dimensions doesn't have coded_width/height arguments and checks them. I bet this situation happens often. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] configure: add missing --strip option to show_help()
On Wed, Feb 24, 2016 at 05:26:57PM +0100, Michael Niedermayer wrote: > On Wed, Feb 24, 2016 at 11:40:12AM +0100, Matthieu Bouron wrote: > > From: Matthieu Bouron > > > > --- > > configure | 1 + > > 1 file changed, 1 insertion(+) > > probably ok Pushed. Thanks. [...] ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] configure: only check dispatch header on darwin
On Thu, Feb 25, 2016 at 04:09:34PM +, Josh de Kock wrote: > On 25/02/2016 15:53, Nicolas George wrote: > >Le septidi 7 ventôse, an CCXXIV, Josh de Kock a écrit : > >>Just thought it wouldn't hurt to fix the real problem--that libdispatch > >>isn't being checked for correctly, rather than a quick fix. Either is fine, > >>I guess, although the quick fix would just require another patch, as I > >>previously stated. > > > >Can someone test the patch that does things "correctly" on the OSes where it > >applies? If not, odds are it will not be exactly correct and will require > >still another patch. > > > It works on OSX. If nobody objects, I will push the patch in a few hours. Regarding the later remarks, the patch addresses a regression in the build of lavd/jack on a linux system that has libdispatch installed and working. I agree that a better fix would be to support libdispatch on systems != darwin but I don't have the time to do that at the moment. Matthieu ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] New filter: remap?
If it doesn exist I would like to develop a new filter and would like to check if nobody is already working on it or that it exists under a different name. I have been researching the following usecase: Similar tot the displace filter, I would need a filter to remap pixels (the difference being that displace displaces the pixels relative to where it was, remap would specify where the target pixel originates from): Description: Remap pixels as indicated by second and third input stream. It takes three input streams and outputs one stream, the first input is the source, and second and third input are targetmaps. The second and third input specifies the x.y coordinate where the pixel in the target stream originates from. Note that once generated, remap files can be reused over and over again. Source and target do not have to have the same size. The effect I am trying to achieve is hard to compute with the relative "displace", and a lot more straightforward with "remap". ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/2] img2dec: Add mime_type to image formats
On Thu, Feb 25, 2016 at 04:30:13PM +, Carl Eugen Hoyos wrote: > Justin Ruggles writes: > > > In terms of how the score for a MIME type match compares with > > those of the individual content matching probe functions, I'd > > say it makes sense. The stronger probing functions have a > > score which reflects their reliability. > > But even _EXTENSION + 1 is correct in practically all cases > (the exception are mpeg streams that start with the needed four > bytes) and should not be beaten by mime_type. > > > Improves probing, especially over http when there is a > > Content-Type header > > Please give an example of a failing stream. Any image here for instance: http://phpthumb.sourceforge.net/demo/demo/phpThumb.demo.demo.php#x1 But really, any random image anywhere on the web after you add ?foo=bar to the URL. [...] -- 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 2/2] img2dec: Add mime_type to image formats
On Thu, Feb 25, 2016 at 10:35:39PM +0100, Clément Bœsch wrote: > On Thu, Feb 25, 2016 at 04:30:13PM +, Carl Eugen Hoyos wrote: > > Justin Ruggles writes: > > > > > In terms of how the score for a MIME type match compares with > > > those of the individual content matching probe functions, I'd > > > say it makes sense. The stronger probing functions have a > > > score which reflects their reliability. > > > > But even _EXTENSION + 1 is correct in practically all cases > > (the exception are mpeg streams that start with the needed four > > bytes) and should not be beaten by mime_type. > > > > > Improves probing, especially over http when there is a > > > Content-Type header > > > > Please give an example of a failing stream. > > Any image here for instance: > http://phpthumb.sourceforge.net/demo/demo/phpThumb.demo.demo.php#x1 > > But really, any random image anywhere on the web after you add ?foo=bar to > the URL. > My bad, it works. -- 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] New filter: remap?
On 2/25/16, F.Sluiter wrote: > If it doesn exist I would like to develop a new filter and would like to > check if nobody is already working on it or that it exists under a > different name. > > I have been researching the following usecase: > > Similar tot the displace filter, I would need a filter to remap pixels (the > difference being that displace displaces the pixels relative to where it > was, remap would specify where the target pixel originates from): > > Description: Remap pixels as indicated by second and third input stream. > > It takes three input streams and outputs one stream, the first input is the > source, and second and third input are targetmaps. The second and third > input specifies the x.y coordinate where the pixel in the target stream > originates from. Note that once generated, remap files can be reused over > and over again. Source and target do not have to have the same size. > The effect I am trying to achieve is hard to compute with the relative > "displace", and a lot more straightforward with "remap". With 16bit support too? Go ahead, nice idea :-) I'm interested in motion estimation/interpolation/compensation filters too. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/2] img2dec: Add mime_type to image formats
On Thu, Feb 25, 2016 at 10:37:40PM +0100, Clément Bœsch wrote: > On Thu, Feb 25, 2016 at 10:35:39PM +0100, Clément Bœsch wrote: > > On Thu, Feb 25, 2016 at 04:30:13PM +, Carl Eugen Hoyos wrote: > > > Justin Ruggles writes: > > > > > > > In terms of how the score for a MIME type match compares with > > > > those of the individual content matching probe functions, I'd > > > > say it makes sense. The stronger probing functions have a > > > > score which reflects their reliability. > > > > > > But even _EXTENSION + 1 is correct in practically all cases > > > (the exception are mpeg streams that start with the needed four > > > bytes) and should not be beaten by mime_type. > > > > > > > Improves probing, especially over http when there is a > > > > Content-Type header > > > > > > Please give an example of a failing stream. > > > > Any image here for instance: > > http://phpthumb.sourceforge.net/demo/demo/phpThumb.demo.demo.php#x1 > > > > But really, any random image anywhere on the web after you add ?foo=bar to > > the URL. > > > > My bad, it works. > http://lucy.pkh.me/samples/img.jpg?foo=bar This one doesn't for some reason, unless you drop ?foo=bar -- 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] New filter: remap?
It would also be relatively easy to implement anti-aliasing (probably in version 2). 2016-02-25 22:38 GMT+01:00 Paul B Mahol : > On 2/25/16, F.Sluiter wrote: > > If it doesn exist I would like to develop a new filter and would like to > > check if nobody is already working on it or that it exists under a > > different name. > > > > I have been researching the following usecase: > > > > Similar tot the displace filter, I would need a filter to remap pixels > (the > > difference being that displace displaces the pixels relative to where it > > was, remap would specify where the target pixel originates from): > > > > Description: Remap pixels as indicated by second and third input stream. > > > > It takes three input streams and outputs one stream, the first input is > the > > source, and second and third input are targetmaps. The second and third > > input specifies the x.y coordinate where the pixel in the target stream > > originates from. Note that once generated, remap files can be reused over > > and over again. Source and target do not have to have the same size. > > The effect I am trying to achieve is hard to compute with the relative > > "displace", and a lot more straightforward with "remap". > > With 16bit support too? Go ahead, nice idea :-) > > I'm interested in motion estimation/interpolation/compensation filters too. > ___ > 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
Re: [FFmpeg-devel] [PATCH] mss2: Fix buffer overflow.
On Thu, Feb 25, 2016 at 09:25:08PM +0100, wm4 wrote: > On Thu, 25 Feb 2016 21:06:46 +0100 > Reimar Döffinger wrote: > > > Reported as https://trac.mplayerhq.hu/ticket/2264 but have > > not been able to reproduce with FFmpeg-only. > > I have no idea what coded_height is used for here exactly, > > so this might not be the best fix. > > Fixes the following chain of events: > > ff_mss12_decode_init sets coded_height while not setting height. > > ff_mpv_decode_init then copies coded_height into MpegEncContext height. > > This is then used by init_context_frame to allocate the data structures. > > However the wmv9rects are validated/initialized based on avctx->height, not > > avctx->coded_height. > > Thus the decode_wmv9 function will try to decode a larger video that we > > allocated data structures for, causing out-of-bounds writes. > > > > Signed-off-by: Reimar Döffinger > > --- > > libavcodec/mss12.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/libavcodec/mss12.c b/libavcodec/mss12.c > > index 6b58aa29..d42093b 100644 > > --- a/libavcodec/mss12.c > > +++ b/libavcodec/mss12.c > > @@ -581,8 +581,8 @@ av_cold int ff_mss12_decode_init(MSS12Context *c, int > > version, > > return AVERROR_INVALIDDATA; > > } > > > > -avctx->coded_width = AV_RB32(avctx->extradata + 20); > > -avctx->coded_height = AV_RB32(avctx->extradata + 24); > > +avctx->coded_width = FFMAX(AV_RB32(avctx->extradata + 20), > > avctx->width); > > +avctx->coded_height = FFMAX(AV_RB32(avctx->extradata + 24), > > avctx->height); > > if (avctx->coded_width > 4096 || avctx->coded_height > 4096) { > > av_log(avctx, AV_LOG_ERROR, "Frame dimensions %dx%d too large", > > avctx->coded_width, avctx->coded_height); > > Just a side remark... > > I'm wondering why ff_set_dimensions doesn't have coded_width/height > arguments and checks them. I bet this situation happens often. I am not sure there is a fixed requirement for the relation between coded_height and height that necessarily applies to all codecs. For example I don't know if for mss1 that case I am filtering out is actually invalid! In which case the FFMAX might need to go into mss2.c instead... Note that this mostly happens because the mss2 codec essentially instantiates multiple (partial) decoders into a single avctx, and they might have differing requirements. Though I admit I am not sure the mss12 code can handle this case correctly either. I guess in that way checking it might be good since it means you at least don't have to guess if the check was just forgotten or left out intentionally. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] New filter: remap?
Could you actually be more specific, what would be needed for 16bit support? I was naively thinking to just a binary copy from source pixel to target for starters. 2016-02-25 22:40 GMT+01:00 F.Sluiter : > It would also be relatively easy to implement anti-aliasing (probably in > version 2). > > 2016-02-25 22:38 GMT+01:00 Paul B Mahol : > >> On 2/25/16, F.Sluiter wrote: >> > If it doesn exist I would like to develop a new filter and would like to >> > check if nobody is already working on it or that it exists under a >> > different name. >> > >> > I have been researching the following usecase: >> > >> > Similar tot the displace filter, I would need a filter to remap pixels >> (the >> > difference being that displace displaces the pixels relative to where it >> > was, remap would specify where the target pixel originates from): >> > >> > Description: Remap pixels as indicated by second and third input stream. >> > >> > It takes three input streams and outputs one stream, the first input is >> the >> > source, and second and third input are targetmaps. The second and third >> > input specifies the x.y coordinate where the pixel in the target stream >> > originates from. Note that once generated, remap files can be reused >> over >> > and over again. Source and target do not have to have the same size. >> > The effect I am trying to achieve is hard to compute with the relative >> > "displace", and a lot more straightforward with "remap". >> >> With 16bit support too? Go ahead, nice idea :-) >> >> I'm interested in motion estimation/interpolation/compensation filters >> too. >> ___ >> 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
Re: [FFmpeg-devel] New filter: remap?
On 2/25/16, F.Sluiter wrote: > Could you actually be more specific, what would be needed for 16bit > support? I was naively thinking to just a binary copy from source pixel to > target for starters. For example, first input is 8bit depth and 2nd and 3rd are 16bit depth so remap works for > 256 dimensions. Maybe you want to work with nonsubsampled formats only and not have option to remap y/u/v differently from each other? ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH]lavf/img2dec: Skip SOF size when probing jpeg
Hi! Attached patch fixes the sample Clément kindly provided: http://lucy.pkh.me/samples/img.jpg?foo=bar Please comment, Carl Eugen diff --git a/libavformat/img2dec.c b/libavformat/img2dec.c index a755b6f..ce40aa5 100644 --- a/libavformat/img2dec.c +++ b/libavformat/img2dec.c @@ -706,6 +706,7 @@ static int jpeg_probe(AVProbeData *p) case 0xC1: case 0xC2: case 0xC3: +i += AV_RB16(&b[i + 2]) + 1; case 0xC5: case 0xC6: case 0xC7: ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/2] img2dec: Add mime_type to image formats
Clément Bœsch pkh.me> writes: > http://lucy.pkh.me/samples/img.jpg?foo=bar Patch sent. Thank you, Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] avformat/mux: add a format flag which ensure parsed and standardized creation time
On Thu, 25 Feb 2016, wm4 wrote: On Thu, 25 Feb 2016 02:11:00 +0100 Marton Balint wrote: This can be used for formats which write all format metadata as string to files, therefore non-standard creation times such as 'now' will be parsed. The standardized creation time is UTC ISO 8601 with microsecond precision. Signed-off-by: Marton Balint --- +#define AVFMT_NEED_PARSED_CREATION_TIME 0x8 /**< Format needs pre-parsed standardized creation time */ Wouldn't it be better to make this a function, and adding a function call to the muxers which need this, instead of adding yet another flag and adding that flag to the muxers? Well, I am not sure, I have no strong argument for or against it. If that is what you and other people prefer, obviously I can change the patch. Regards, Marton ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] New filter: remap?
I am not considering a format change (yet), maybe I wasn't clear enough on how this filter would work. Displace works now with a image stream and two in puts, x_displacement, y_dispacement. x and y are relative so they are added to the x,y location of the source pixel. the pixel x,y currently being copied ends up at (x+displacement_x,y+displacement_y). However, my remap filter would do and absolute "displacement" or rather "remap", ampping from source pixels to target pixels. so the x,y found in the second and third input would determine which source pixel is copied to the current location.The filter would be almost similar code to the displace filter but it would copy the source pixel directly from the given location in stead of displacing it with a relative value.. pseudo code: /*displace*/ inputimage source[][]; input displace_x[][]; input displace_y[][]; output target[][]; for (int i; i : > On 2/25/16, F.Sluiter wrote: > > Could you actually be more specific, what would be needed for 16bit > > support? I was naively thinking to just a binary copy from source pixel > to > > target for starters. > > For example, first input is 8bit depth and 2nd and 3rd are 16bit depth > so remap works for > 256 dimensions. > > Maybe you want to work with nonsubsampled formats only and not have option > to remap y/u/v differently from each other? > ___ > 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
Re: [FFmpeg-devel] New filter: remap?
So not color mapping (that would be another awesome filter indeed) but location mapping 2016-02-25 23:42 GMT+01:00 F.Sluiter : > I am not considering a format change (yet), maybe I wasn't clear enough on > how this filter would work. > Displace works now with a image stream and two in puts, x_displacement, > y_dispacement. x and y are relative so they are added to the x,y location > of the source pixel. the pixel x,y currently being copied ends up at > (x+displacement_x,y+displacement_y). > However, my remap filter would do and absolute "displacement" or rather > "remap", ampping from source pixels to target pixels. > so the x,y found in the second and third input would determine which > source pixel is copied to the current location.The filter would be almost > similar code to the displace filter but it would copy the source pixel > directly from the given location in stead of displacing it with a relative > value.. > pseudo code: > > /*displace*/ > inputimage source[][]; > input displace_x[][]; > input displace_y[][]; > output target[][]; > > for (int i; ifor int j; j target[i+displace_x[i][j]][j+displace_y[i][j]] = source[i][j]; > }} > > /*remap*/ > inputimage source[][]; > input remap_x[][]; > input remap_y[][]; > output target[][]; > > for (int i; ifor int j; j target[i][j] = source[remap_x[i][j]][remap_y[i][j]]; > }} > > It might sound trivial, but with "remap" the target can be for example > bigger or with a different aspect ratio, and remap_x,remap_y would just > tell you were the pixels are coming from. So the same pixel from the source > could be copied multiple times to different locations in the target. With > displace, you can move a pixel only once It is "Push" vs "Pull". > > > 2016-02-25 23:06 GMT+01:00 Paul B Mahol : > >> On 2/25/16, F.Sluiter wrote: >> > Could you actually be more specific, what would be needed for 16bit >> > support? I was naively thinking to just a binary copy from source pixel >> to >> > target for starters. >> >> For example, first input is 8bit depth and 2nd and 3rd are 16bit depth >> so remap works for > 256 dimensions. >> >> Maybe you want to work with nonsubsampled formats only and not have option >> to remap y/u/v differently from each other? >> ___ >> 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
Re: [FFmpeg-devel] New filter: remap?
Dana 25. 2. 2016. 23:43 osoba "F.Sluiter" napisala je: > > I am not considering a format change (yet), maybe I wasn't clear enough on > how this filter would work. > Displace works now with a image stream and two in puts, x_displacement, > y_dispacement. x and y are relative so they are added to the x,y location > of the source pixel. the pixel x,y currently being copied ends up at > (x+displacement_x,y+displacement_y). > However, my remap filter would do and absolute "displacement" or rather > "remap", ampping from source pixels to target pixels. > so the x,y found in the second and third input would determine which source > pixel is copied to the current location.The filter would be almost similar > code to the displace filter but it would copy the source pixel directly > from the given location in stead of displacing it with a relative value.. > pseudo code: > > /*displace*/ > inputimage source[][]; > input displace_x[][]; > input displace_y[][]; > output target[][]; > > for (int i; ifor int j; j target[i+displace_x[i][j]][j+displace_y[i][j]] = source[i][j]; > }} > > /*remap*/ > inputimage source[][]; > input remap_x[][]; > input remap_y[][]; > output target[][]; > > for (int i; ifor int j; j target[i][j] = source[remap_x[i][j]][remap_y[i][j]]; I'm looking for the source, if it doesnt appear in reasonable time frame I will do it :) > }} > > It might sound trivial, but with "remap" the target can be for example > bigger or with a different aspect ratio, and remap_x,remap_y would just > tell you were the pixels are coming from. So the same pixel from the source > could be copied multiple times to different locations in the target. With > displace, you can move a pixel only once It is "Push" vs "Pull". > > > 2016-02-25 23:06 GMT+01:00 Paul B Mahol : > > > On 2/25/16, F.Sluiter wrote: > > > Could you actually be more specific, what would be needed for 16bit > > > support? I was naively thinking to just a binary copy from source pixel > > to > > > target for starters. > > > > For example, first input is 8bit depth and 2nd and 3rd are 16bit depth > > so remap works for > 256 dimensions. > > > > Maybe you want to work with nonsubsampled formats only and not have option > > to remap y/u/v differently from each other? > > ___ > > 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
Re: [FFmpeg-devel] New filter: remap?
Give me a few weeks, I have a day job and a son in the weekends, but would be happy to work together on it! 2016-02-26 0:11 GMT+01:00 Paul B Mahol : > Dana 25. 2. 2016. 23:43 osoba "F.Sluiter" napisala > je: > > > > I am not considering a format change (yet), maybe I wasn't clear enough > on > > how this filter would work. > > Displace works now with a image stream and two in puts, x_displacement, > > y_dispacement. x and y are relative so they are added to the x,y location > > of the source pixel. the pixel x,y currently being copied ends up at > > (x+displacement_x,y+displacement_y). > > However, my remap filter would do and absolute "displacement" or rather > > "remap", ampping from source pixels to target pixels. > > so the x,y found in the second and third input would determine which > source > > pixel is copied to the current location.The filter would be almost > similar > > code to the displace filter but it would copy the source pixel directly > > from the given location in stead of displacing it with a relative value.. > > pseudo code: > > > > /*displace*/ > > inputimage source[][]; > > input displace_x[][]; > > input displace_y[][]; > > output target[][]; > > > > for (int i; i > for int j; j > target[i+displace_x[i][j]][j+displace_y[i][j]] = source[i][j]; > > }} > > > > /*remap*/ > > inputimage source[][]; > > input remap_x[][]; > > input remap_y[][]; > > output target[][]; > > > > for (int i; i > for int j; j > target[i][j] = source[remap_x[i][j]][remap_y[i][j]]; > > I'm looking for the source, if it doesnt appear in reasonable time frame I > will do it :) > > > }} > > > > It might sound trivial, but with "remap" the target can be for example > > bigger or with a different aspect ratio, and remap_x,remap_y would just > > tell you were the pixels are coming from. So the same pixel from the > source > > could be copied multiple times to different locations in the target. With > > displace, you can move a pixel only once It is "Push" vs "Pull". > > > > > > > > 2016-02-25 23:06 GMT+01:00 Paul B Mahol : > > > > > On 2/25/16, F.Sluiter wrote: > > > > Could you actually be more specific, what would be needed for 16bit > > > > support? I was naively thinking to just a binary copy from source > pixel > > > to > > > > target for starters. > > > > > > For example, first input is 8bit depth and 2nd and 3rd are 16bit depth > > > so remap works for > 256 dimensions. > > > > > > Maybe you want to work with nonsubsampled formats only and not have > option > > > to remap y/u/v differently from each other? > > > ___ > > > 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 mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]lavf/img2dec: Skip SOF size when probing jpeg
On Thu, Feb 25, 2016 at 11:27:21PM +0100, Carl Eugen Hoyos wrote: > Hi! > > Attached patch fixes the sample Clément kindly provided: > http://lucy.pkh.me/samples/img.jpg?foo=bar > > Please comment, Carl Eugen > img2dec.c |1 + > 1 file changed, 1 insertion(+) > 52bcdf49dd6f17c381cf1210bb70e807bccf14e4 patchjpegsof.diff > diff --git a/libavformat/img2dec.c b/libavformat/img2dec.c > index a755b6f..ce40aa5 100644 > --- a/libavformat/img2dec.c > +++ b/libavformat/img2dec.c > @@ -706,6 +706,7 @@ static int jpeg_probe(AVProbeData *p) > case 0xC1: > case 0xC2: > case 0xC3: > +i += AV_RB16(&b[i + 2]) + 1; > case 0xC5: > case 0xC6: > case 0xC7: shouldnt this also be done for these 3? [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Republics decline into democracies and democracies degenerate into despotisms. -- Aristotle signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] mjpegdec: Do not assume unused plane pointer are NULL.
We do neither document nor check such a requirement and for application-provided get_buffer2 they could contain the result of a malloc(0) or whatever value they had previously. This fixes a use-after-free in e.g. MPlayer: https://trac.mplayerhq.hu/ticket/2262 We might want to consider changing the (documented) API in addition though. Signed-off-by: Reimar Döffinger --- libavcodec/mjpegdec.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c index 113022f..b6fbeab 100644 --- a/libavcodec/mjpegdec.c +++ b/libavcodec/mjpegdec.c @@ -2268,7 +2268,7 @@ the_end: avctx->pix_fmt == AV_PIX_FMT_GBRAP ); avcodec_get_chroma_sub_sample(s->avctx->pix_fmt, &hshift, &vshift); -for (p = 0; p<4; p++) { +for (p = 0; pnb_components; p++) { uint8_t *line = s->picture_ptr->data[p]; int w = s->width; int h = s->height; @@ -2326,7 +2326,7 @@ the_end: avctx->pix_fmt == AV_PIX_FMT_GBRAP ); avcodec_get_chroma_sub_sample(s->avctx->pix_fmt, &hshift, &vshift); -for (p = 0; p < 4; p++) { +for (p = 0; p < s->nb_components; p++) { uint8_t *dst; int w = s->width; int h = s->height; @@ -2353,7 +2353,7 @@ the_end: if (s->flipped) { int j; avcodec_get_chroma_sub_sample(s->avctx->pix_fmt, &hshift, &vshift); -for (index=0; index<4; index++) { +for (index=0; indexnb_components; index++) { uint8_t *dst = s->picture_ptr->data[index]; int w = s->picture_ptr->width; int h = s->picture_ptr->height; @@ -2375,6 +2375,7 @@ the_end: if (s->adobe_transform == 0 && s->avctx->pix_fmt == AV_PIX_FMT_GBRAP) { int w = s->picture_ptr->width; int h = s->picture_ptr->height; + av_assert0(s->nb_components == 4); for (i=0; iadobe_transform == 2 && s->avctx->pix_fmt == AV_PIX_FMT_YUVA444P) { int w = s->picture_ptr->width; int h = s->picture_ptr->height; + av_assert0(s->nb_components == 4); for (i=0; ihttp://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] Always read frame progress values under progress_mutex.
This fixes data race warnings by ThreadSanitizer. ff_thread_report_progress and ff_thread_await_progress should read progress[field] only when holding progress_mutex because progress_mutex guards frame progress values. --- libavcodec/pthread_frame.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c index b77dd1e..b7f603d 100644 --- a/libavcodec/pthread_frame.c +++ b/libavcodec/pthread_frame.c @@ -474,7 +474,7 @@ void ff_thread_report_progress(ThreadFrame *f, int n, int field) PerThreadContext *p; volatile int *progress = f->progress ? (int*)f->progress->data : NULL; -if (!progress || progress[field] >= n) return; +if (!progress) return; p = f->owner->internal->thread_ctx; @@ -482,8 +482,10 @@ void ff_thread_report_progress(ThreadFrame *f, int n, int field) av_log(f->owner, AV_LOG_DEBUG, "%p finished %d field %d\n", progress, n, field); pthread_mutex_lock(&p->progress_mutex); -progress[field] = n; -pthread_cond_broadcast(&p->progress_cond); +if (progress[field] < n) { +progress[field] = n; +pthread_cond_broadcast(&p->progress_cond); +} pthread_mutex_unlock(&p->progress_mutex); } @@ -492,7 +494,7 @@ void ff_thread_await_progress(ThreadFrame *f, int n, int field) PerThreadContext *p; volatile int *progress = f->progress ? (int*)f->progress->data : NULL; -if (!progress || progress[field] >= n) return; +if (!progress) return; p = f->owner->internal->thread_ctx; -- 2.7.0.rc3.207.g0ac5344 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Always read frame progress values under progress_mutex.
On Fri, Feb 26, 2016 at 1:06 AM, Wan-Teh Chang wrote: > This fixes data race warnings by ThreadSanitizer. > ff_thread_report_progress and ff_thread_await_progress should read > progress[field] only when holding progress_mutex because progress_mutex > guards frame progress values. Similar to the other patch, please explain what this actually fixes, those sanitizer tools produce a lot of false positives and nonsensical warnings. - Hendrik ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Always read frame progress values under progress_mutex.
On Thu, Feb 25, 2016 at 4:15 PM, Hendrik Leppkes wrote: > > Similar to the other patch, please explain what this actually fixes, > those sanitizer tools produce a lot of false positives and nonsensical > warnings. Yes, certainly. In libavcodec/pthread_frame.c, we have: 46 /** 47 * Context used by codec threads and stored in their AVCodecInternal thread_ctx. 48 */ 49 typedef struct PerThreadContext { ... 59 pthread_mutex_t progress_mutex; ///< Mutex used to protect frame progress values and progress_cond. The comment on line 59 documents that progress_mutex guards frame progress values and progress_cond. (In fact, progress_mutex also guards the |state| field in many places, but that's not the subject of this patch.) I assume "frame progess values" mean f->progress->data, where |f| points to a ThreadFrame. I inspected libavcodec/pthread_frame.c and concluded the code (ff_thread_report_progress and ff_thread_await_progress) matches that comment, except that it also does a double-checked locking style performance optimization. For example, here is ff_thread_report_progress: 472 void ff_thread_report_progress(ThreadFrame *f, int n, int field) 473 { 474 PerThreadContext *p; 475 volatile int *progress = f->progress ? (int*)f->progress->data : NULL; 476 477 if (!progress || progress[field] >= n) return; 478 479 p = f->owner->internal->thread_ctx; 480 481 if (f->owner->debug&FF_DEBUG_THREADS) 482 av_log(f->owner, AV_LOG_DEBUG, "%p finished %d field %d\n", progress, n, field); 483 484 pthread_mutex_lock(&p->progress_mutex); 485 progress[field] = n; 486 pthread_cond_broadcast(&p->progress_cond); 487 pthread_mutex_unlock(&p->progress_mutex); 488 } progress[field] can only increase. So, both ff_thread_report_progress and ff_thread_await_progress have a fast code path (line 477 and line 495, respectively) that reads progress[field] without acquiring progress_mutex. If the speculatively read value of progress[field] is >= |n|, then the functions return immediately. Otherwise, the functions take the slow code path, which acquires progress_mutex properly and does what the functions need to do. The fast code path is in the style of double-checked locking and has a data race. ThreadSanitizer is right in this case. Wan-Teh Chang ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavf/mux: do not fail in case of non monotonically increasing DTS values for data packets
On Thu, Feb 25, 2016 at 07:02:31PM +0100, Stefano Sabatini wrote: > On date Thursday 2016-02-25 13:41:02 +0100, Michael Niedermayer encoded: > > On Thu, Feb 25, 2016 at 01:11:36PM +0100, Stefano Sabatini wrote: > [...] > > > So, while the check makes sense for audio and video, in the case of > > > subtitles and data we are in the fuzzy area. In principle, I agree > > > with the fact that data and subtitles should be treated in the same > > > way, so I propose the following options: > > > > > > 1. disable the strict monotonicity check for both data and subtitles > > >inconditionally > > > > yes, do with data what is done with subtitles already IIRC > > See attached patch. > > [...] > -- > FFmpeg = Fiendish and Fierce Mastering Political Empowered Geisha > mux.c |1 + > 1 file changed, 1 insertion(+) > 28a1647f5cd7b49f6fe439777af61241adf5c19f > 0001-lavf-mux-do-not-fail-in-case-of-non-strictly-monoton.patch > From 0996189555f4a2402b396801da318b4ffcfb1a13 Mon Sep 17 00:00:00 2001 > From: Stefano Sabatini > Date: Thu, 17 Dec 2015 20:51:42 +0100 > Subject: [PATCH] lavf/mux: do not fail in case of non strictly monotonically > increasing DTS values for data packets > > Consistent with what we already do with subtitles since > ac08c5c0adcb7f2f9b5ea3eb473d1c2b9659aab2. LGTM thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Rewriting code that is poorly written but fully understood is good. Rewriting code that one doesnt understand is a sign that one is less smart then the original author, trying to rewrite it will not make it better. signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Move the |die| member of FrameThreadContext to PerThreadContext.
Hi Ronald, On Thu, Feb 25, 2016 at 12:00 PM, Ronald S. Bultje wrote: > > But does it fix an actual bug? (Is there a sample this fixes?) I assume you agree that reading and writing the non-atomic int |die| flag without mutex protection is a data race. Code inspection verifies libavcodec/pthread_frame.c reads and writes |die| without mutex protection. To fix the data race, we can either protect |die| with a mutex, or declare |die| as an atomic int. > If anything, you can use atomic ints instead of regular ints if we don't > already [1]. > [...] > [1] https://ffmpeg.org/doxygen/trunk/atomic_8h_source.html Thanks for the link. avpriv_atomic_int_get and avpriv_atomic_int_set perform the load and store with sequential consistency, which requires a full memory barrier and is slower than simply relying on the existing per-thread mutex. The drawback of my patch is that it uses more memory. I think that's the right trade-off, but I would be happy to use an atomic int. Please let me know what you prefer. Thanks, Wan-Teh Chang PS: Here is the relevant part of the ThreadSanitizer report. I'm using an old version of ffmpeg (the code is in libavcodec/pthread.c in that version): WARNING: ThreadSanitizer: data race (pid=959745) Write of size 4 at 0x7d1800055a24 by main thread (mutexes: write M18754): #0 frame_thread_free ffmpeg/libavcodec/pthread.c:761:15 (c9c6e60608e58c16d6d8dc75627a71ed+0x00985b5a) #1 ff_thread_free ffmpeg/libavcodec/pthread.c:1103:9 (c9c6e60608e58c16d6d8dc75627a71ed+0x009859ea) #2 avcodec_close ffmpeg/libavcodec/utils.c:1825:13 (c9c6e60608e58c16d6d8dc75627a71ed+0x00a8f7a6) ... Previous read of size 4 at 0x7d1800055a24 by thread T11 (mutexes: write M18771): #0 frame_worker_thread ffmpeg/libavcodec/pthread.c:378:60 (c9c6e60608e58c16d6d8dc75627a71ed+0x0098656b) Note that although some mutexes were acquired when the write and read occurred, they were not the same mutex. Here are the relevant code snippets: 361 /** 362 * Codec worker thread. 363 * 364 * Automatically calls ff_thread_finish_setup() if the codec does 365 * not provide an update_thread_context method, or if the codec returns 366 * before calling it. 367 */ 368 static attribute_align_arg void *frame_worker_thread(void *arg) 369 { 370 PerThreadContext *p = arg; 371 FrameThreadContext *fctx = p->parent; 372 AVCodecContext *avctx = p->avctx; 373 const AVCodec *codec = avctx->codec; 374 375 pthread_mutex_lock(&p->mutex); 376 while (1) { 377 int i; 378 while (p->state == STATE_INPUT_READY && !fctx->die) 379 pthread_cond_wait(&p->input_cond, &p->mutex); 380 381 if (fctx->die) break; 382 ... 750 static void frame_thread_free(AVCodecContext *avctx, int thread_count) 751 { 752 FrameThreadContext *fctx = avctx->thread_opaque; 753 const AVCodec *codec = avctx->codec; 754 int i; 755 756 park_frame_worker_threads(fctx, thread_count); 757 758 if (fctx->prev_thread && fctx->prev_thread != fctx->threads) 759 update_context_from_thread(fctx->threads->avctx, fctx->prev_thread- >avctx, 0); 760 761 fctx->die = 1; 762 763 for (i = 0; i < thread_count; i++) { 764 PerThreadContext *p = &fctx->threads[i]; 765 766 pthread_mutex_lock(&p->mutex); 767 pthread_cond_signal(&p->input_cond); 768 pthread_mutex_unlock(&p->mutex); 769 770 if (p->thread_init) 771 pthread_join(p->thread, NULL); 772 p->thread_init=0; 773 774 if (codec->close) 775 codec->close(p->avctx); 776 777 avctx->codec = NULL; 778 779 release_delayed_buffers(p); 780 } ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] mjpegdec: Do not assume unused plane pointer are NULL.
On Fri, Feb 26, 2016 at 12:15:19AM +0100, Reimar Döffinger wrote: > We do neither document nor check such a requirement > and for application-provided get_buffer2 they could > contain the result of a malloc(0) or whatever value > they had previously. > This fixes a use-after-free in e.g. MPlayer: > https://trac.mplayerhq.hu/ticket/2262 > We might want to consider changing the (documented) > API in addition though. > > Signed-off-by: Reimar Döffinger > --- > libavcodec/mjpegdec.c | 8 +--- > 1 file changed, 5 insertions(+), 3 deletions(-) the assumtation that unused plane pointers are NULL is more widespread than mjpeg i think also, is it really a good idea to leave stale pointers in the array? [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Rewriting code that is poorly written but fully understood is good. Rewriting code that one doesnt understand is a sign that one is less smart then the original author, trying to rewrite it will not make it better. signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] Read |state| under the protection of |mutex| or |progress_mutex|.
Although not documented, |state| is guarded by either |mutex| or |progress_mutex|. In several places |state| is read without mutex protection, often as a double-checked locking style performance optimization. Fix the data races by reading |state| under mutex protection. --- libavcodec/pthread_frame.c | 44 +++- 1 file changed, 19 insertions(+), 25 deletions(-) diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c index b77dd1e..a768b1c 100644 --- a/libavcodec/pthread_frame.c +++ b/libavcodec/pthread_frame.c @@ -323,12 +323,10 @@ static int submit_packet(PerThreadContext *p, AVPacket *avpkt) if (prev_thread) { int err; -if (prev_thread->state == STATE_SETTING_UP) { -pthread_mutex_lock(&prev_thread->progress_mutex); -while (prev_thread->state == STATE_SETTING_UP) -pthread_cond_wait(&prev_thread->progress_cond, &prev_thread->progress_mutex); -pthread_mutex_unlock(&prev_thread->progress_mutex); -} +pthread_mutex_lock(&prev_thread->progress_mutex); +while (prev_thread->state == STATE_SETTING_UP) +pthread_cond_wait(&prev_thread->progress_cond, &prev_thread->progress_mutex); +pthread_mutex_unlock(&prev_thread->progress_mutex); err = update_context_from_thread(p->avctx, prev_thread->avctx, 0); if (err) { @@ -353,9 +351,9 @@ static int submit_packet(PerThreadContext *p, AVPacket *avpkt) if (!p->avctx->thread_safe_callbacks && ( p->avctx->get_format != avcodec_default_get_format || p->avctx->get_buffer2 != avcodec_default_get_buffer2)) { +pthread_mutex_lock(&p->progress_mutex); while (p->state != STATE_SETUP_FINISHED && p->state != STATE_INPUT_READY) { int call_done = 1; -pthread_mutex_lock(&p->progress_mutex); while (p->state == STATE_SETTING_UP) pthread_cond_wait(&p->progress_cond, &p->progress_mutex); @@ -374,8 +372,8 @@ static int submit_packet(PerThreadContext *p, AVPacket *avpkt) p->state = STATE_SETTING_UP; pthread_cond_signal(&p->progress_cond); } -pthread_mutex_unlock(&p->progress_mutex); } +pthread_mutex_unlock(&p->progress_mutex); } fctx->prev_thread = p; @@ -426,12 +424,10 @@ int ff_thread_decode_frame(AVCodecContext *avctx, do { p = &fctx->threads[finished++]; -if (p->state != STATE_INPUT_READY) { -pthread_mutex_lock(&p->progress_mutex); -while (p->state != STATE_INPUT_READY) -pthread_cond_wait(&p->output_cond, &p->progress_mutex); -pthread_mutex_unlock(&p->progress_mutex); -} +pthread_mutex_lock(&p->progress_mutex); +while (p->state != STATE_INPUT_READY) +pthread_cond_wait(&p->output_cond, &p->progress_mutex); +pthread_mutex_unlock(&p->progress_mutex); av_frame_move_ref(picture, p->frame); *got_picture_ptr = p->got_frame; @@ -510,13 +506,13 @@ void ff_thread_finish_setup(AVCodecContext *avctx) { if (!(avctx->active_thread_type&FF_THREAD_FRAME)) return; -if(p->state == STATE_SETUP_FINISHED){ +pthread_mutex_lock(&p->progress_mutex); +if (p->state == STATE_SETUP_FINISHED) { av_log(avctx, AV_LOG_WARNING, "Multiple ff_thread_finish_setup() calls\n"); +} else { +p->state = STATE_SETUP_FINISHED; +pthread_cond_broadcast(&p->progress_cond); } - -pthread_mutex_lock(&p->progress_mutex); -p->state = STATE_SETUP_FINISHED; -pthread_cond_broadcast(&p->progress_cond); pthread_mutex_unlock(&p->progress_mutex); } @@ -528,12 +524,10 @@ static void park_frame_worker_threads(FrameThreadContext *fctx, int thread_count for (i = 0; i < thread_count; i++) { PerThreadContext *p = &fctx->threads[i]; -if (p->state != STATE_INPUT_READY) { -pthread_mutex_lock(&p->progress_mutex); -while (p->state != STATE_INPUT_READY) -pthread_cond_wait(&p->output_cond, &p->progress_mutex); -pthread_mutex_unlock(&p->progress_mutex); -} +pthread_mutex_lock(&p->progress_mutex); +while (p->state != STATE_INPUT_READY) +pthread_cond_wait(&p->output_cond, &p->progress_mutex); +pthread_mutex_unlock(&p->progress_mutex); p->got_frame = 0; } } -- 2.7.0.rc3.207.g0ac5344 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] submit_thread should also lock the mutex that guards the source avctx (prev_thread->avctx) when calling update_context_from_thread.
This is because the codec->decode() call in frame_worker_thread may be modifying that avctx at the same time. This data race is reported by ThreadSanitizer. Although submit_thread holds two locks simultaneously, it always acquires them in the same order because |prev_thread| is always the array element before |p| (with a wraparound at the end of array), and submit_thread is the only function that acquires those two locks, so this won't result in a deadlock. --- libavcodec/pthread_frame.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c index b77dd1e..e7879e3 100644 --- a/libavcodec/pthread_frame.c +++ b/libavcodec/pthread_frame.c @@ -330,7 +330,9 @@ static int submit_packet(PerThreadContext *p, AVPacket *avpkt) pthread_mutex_unlock(&prev_thread->progress_mutex); } +pthread_mutex_lock(&prev_thread->mutex); err = update_context_from_thread(p->avctx, prev_thread->avctx, 0); +pthread_mutex_unlock(&prev_thread->mutex); if (err) { pthread_mutex_unlock(&p->mutex); return err; -- 2.7.0.rc3.207.g0ac5344 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] submit_thread should also lock the mutex that guards the source avctx (prev_thread->avctx) when calling update_context_from_thread.
On 2/25/2016 11:32 PM, Wan-Teh Chang wrote: > This is because the codec->decode() call in frame_worker_thread may be > modifying that avctx at the same time. This data race is reported by > ThreadSanitizer. > > Although submit_thread holds two locks simultaneously, it always > acquires them in the same order because |prev_thread| is always the > array element before |p| (with a wraparound at the end of array), and > submit_thread is the only function that acquires those two locks, so > this won't result in a deadlock. Did you try running the FATE suite using threadsanitizer, or just decoded random videos? I remember like half the tests were failing because of data races and most of them pointed to code changed by your patches. It would be interesting to see how they are affected. configure has the --toolchain=gcc-tsan option for this. Make sure to also add --disable-stripping, then run "make fate THREADS=4 SAMPLES=/path/to/samples" or similar. You may instead want to run a small set at a time instead of the whole suite since it's big and threadsanitizer slow. Try for example fate-h264 in that case. Thanks for working on this. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v26] lavf/movenc: Add palette to video sample description
On Wed, Feb 24, 2016 at 12:51:45PM +0100, Mats Peterson wrote: > Small fix. Sorry for my spam-like patch posting, but that's the way > I work at the moment. I'll try to limit myself in the future. > > Mats > movenc.c | 63 > +-- > movenc.h |5 + > 2 files changed, 66 insertions(+), 2 deletions(-) > 8b9ff40144318e901cab56e455d8487d7cba118b > 0001-lavf-movenc-Add-palette-to-video-sample-description.patch > From 0cf62b279e2577655e3867e8b0287c191784eeaa Mon Sep 17 00:00:00 2001 > From: Mats Peterson > Date: Wed, 24 Feb 2016 12:49:50 +0100 > Subject: [PATCH v26] lavf/movenc: Add palette to video sample description > > --- > libavformat/movenc.c | 63 > -- > libavformat/movenc.h |5 > 2 files changed, 66 insertions(+), 2 deletions(-) > > diff --git a/libavformat/movenc.c b/libavformat/movenc.c > index b9c0f7a..f9b16b4 100644 > --- a/libavformat/movenc.c > +++ b/libavformat/movenc.c > @@ -1711,10 +1711,29 @@ static int mov_write_video_tag(AVIOContext *pb, > MOVMuxContext *mov, MOVTrack *tr > avio_write(pb, compressor_name, 31); > > if (track->mode == MODE_MOV && track->enc->bits_per_coded_sample) > -avio_wb16(pb, track->enc->bits_per_coded_sample); > +avio_wb16(pb, track->enc->bits_per_coded_sample | > + (track->enc->pix_fmt == AV_PIX_FMT_GRAY8 ? 0x20 : 0)); > else > avio_wb16(pb, 0x18); /* Reserved */ > -avio_wb16(pb, 0x); /* Reserved */ > + > +if (track->is_unaligned_qt_rgb && track->enc->pix_fmt == > AV_PIX_FMT_PAL8) { > +int i; > +avio_wb16(pb, 0); /* Color table ID */ > +avio_wb32(pb, 0); /* Color table seed */ > +avio_wb16(pb, 0x8000);/* Color table flags */ > +avio_wb16(pb, 255); /* Color table size (zero-relative) */ > +for (i = 0; i < 256; i++) { > +uint16_t r = (track->palette[i] >> 16) & 0xff; > +uint16_t g = (track->palette[i] >> 8) & 0xff; > +uint16_t b = track->palette[i] & 0xff; fails on big endian (mips) ./ffmpeg -i matrixbench_mpeg2.mpg -pix_fmt pal8 -vcodec rawvideo -t 10 test.mov i assume (but didnt try) that reading the palette with AV_RL32 or bytewise would work [...] -- 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: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH v27] lavf/movenc: Add palette to video sample description
Should hopefully fix the big-endian issue. -- Mats Peterson http://matsp888.no-ip.org/~mats/ >From 534cf88e5e7540bcd3d93b57f559dfa82aaa5f7c Mon Sep 17 00:00:00 2001 From: Mats Peterson Date: Fri, 26 Feb 2016 05:02:02 +0100 Subject: [PATCH v27] lavf/movenc: Add palette to video sample description --- libavformat/movenc.c | 63 -- libavformat/movenc.h |5 2 files changed, 66 insertions(+), 2 deletions(-) diff --git a/libavformat/movenc.c b/libavformat/movenc.c index b9c0f7a..ff58b4d 100644 --- a/libavformat/movenc.c +++ b/libavformat/movenc.c @@ -1711,10 +1711,29 @@ static int mov_write_video_tag(AVIOContext *pb, MOVMuxContext *mov, MOVTrack *tr avio_write(pb, compressor_name, 31); if (track->mode == MODE_MOV && track->enc->bits_per_coded_sample) -avio_wb16(pb, track->enc->bits_per_coded_sample); +avio_wb16(pb, track->enc->bits_per_coded_sample | + (track->enc->pix_fmt == AV_PIX_FMT_GRAY8 ? 0x20 : 0)); else avio_wb16(pb, 0x18); /* Reserved */ -avio_wb16(pb, 0x); /* Reserved */ + +if (track->is_unaligned_qt_rgb && track->enc->pix_fmt == AV_PIX_FMT_PAL8) { +int i; +avio_wb16(pb, 0); /* Color table ID */ +avio_wb32(pb, 0); /* Color table seed */ +avio_wb16(pb, 0x8000);/* Color table flags */ +avio_wb16(pb, 255); /* Color table size (zero-relative) */ +for (i = 0; i < 256; i++) { +uint16_t r = (AV_RL32(&track->palette[i]) >> 16) & 0xff; +uint16_t g = (AV_RL32(&track->palette[i]) >> 8) & 0xff; +uint16_t b = AV_RL32(&track->palette[i]) & 0xff; +avio_wb16(pb, 0); +avio_wb16(pb, (r << 8) | r); +avio_wb16(pb, (g << 8) | g); +avio_wb16(pb, (b << 8) | b); +} +} else +avio_wb16(pb, 0x); /* Reserved */ + if (track->tag == MKTAG('m','p','4','v')) mov_write_esds_tag(pb, track); else if (track->enc->codec_id == AV_CODEC_ID_H263) @@ -4703,6 +4722,7 @@ static int mov_write_packet(AVFormatContext *s, AVPacket *pkt) } else { int i; MOVMuxContext *mov = s->priv_data; +MOVTrack *trk = &mov->tracks[pkt->stream_index]; if (!pkt->size) return mov_write_single_packet(s, pkt); /* Passthrough. */ @@ -4739,6 +4759,31 @@ static int mov_write_packet(AVFormatContext *s, AVPacket *pkt) } } +if (trk->is_unaligned_qt_rgb) { +const uint8_t *data = pkt->data; +int size = pkt->size; +int64_t bpc = trk->enc->bits_per_coded_sample != 15 ? trk->enc->bits_per_coded_sample : 16; +int expected_stride = ((trk->enc->width * bpc + 15) >> 4)*2; +int ret = ff_reshuffle_raw_rgb(s, &pkt, trk->enc, expected_stride); +if (ret < 0) +return ret; +if (ret == CONTAINS_PAL && !trk->pal_done) { +int pal_size = 1 << trk->enc->bits_per_coded_sample; +memset(trk->palette, 0, AVPALETTE_SIZE); +memcpy(trk->palette, data + size - 4*pal_size, 4*pal_size); +trk->pal_done++; +} else if (trk->enc->pix_fmt == AV_PIX_FMT_GRAY8 || + trk->enc->pix_fmt == AV_PIX_FMT_MONOBLACK) { +for (i = 0; i < pkt->size; i++) +pkt->data[i] = ~pkt->data[i]; +} +if (ret) { +ret = mov_write_single_packet(s, pkt); +av_packet_free(&pkt); +return ret; +} +} + return mov_write_single_packet(s, pkt); } } @@ -5249,6 +5294,20 @@ static int mov_write_header(AVFormatContext *s) "WARNING codec timebase is very high. If duration is too long,\n" "file may not be playable by quicktime. Specify a shorter timebase\n" "or choose different container.\n"); +if (track->mode == MODE_MOV && +track->enc->codec_id == AV_CODEC_ID_RAWVIDEO && +track->tag == MKTAG('r','a','w',' ')) { +enum AVPixelFormat pix_fmt = track->enc->pix_fmt; +if (pix_fmt == AV_PIX_FMT_NONE && track->enc->bits_per_coded_sample == 1) +pix_fmt = AV_PIX_FMT_MONOWHITE; +track->is_unaligned_qt_rgb = +pix_fmt == AV_PIX_FMT_RGB24 || +pix_fmt == AV_PIX_FMT_BGR24 || +pix_fmt == AV_PIX_FMT_PAL8 || +pix_fmt == AV_PIX_FMT_GRAY8 || +pix_fmt == AV_PIX_FMT_MONOWHITE || +pix_fmt == AV_PIX_FMT_MONOBLACK; +} } else if (st->codec->codec_type == AVMEDIA_TYPE_AUDIO) { track->timescale = st->codec->sample_rate; if (!st->codec->f
Re: [FFmpeg-devel] [PATCH v27] lavf/movenc: Add palette to video sample description
On 02/26/2016 05:03 AM, Mats Peterson wrote: Should hopefully fix the big-endian issue. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel I'll do it in a somewhat more elegant way. -- Mats Peterson http://matsp888.no-ip.org/~mats/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH v28] lavf/movenc: Add palette to video sample description
Should hopefully fix the big-endian issue. -- Mats Peterson http://matsp888.no-ip.org/~mats/ >From 750f7e27518aa7cc9fb37240ba33a024deaead34 Mon Sep 17 00:00:00 2001 From: Mats Peterson Date: Fri, 26 Feb 2016 05:07:28 +0100 Subject: [PATCH v28] lavf/movenc: Add palette to video sample description --- libavformat/movenc.c | 64 -- libavformat/movenc.h |5 2 files changed, 67 insertions(+), 2 deletions(-) diff --git a/libavformat/movenc.c b/libavformat/movenc.c index b9c0f7a..e0223b2 100644 --- a/libavformat/movenc.c +++ b/libavformat/movenc.c @@ -1711,10 +1711,30 @@ static int mov_write_video_tag(AVIOContext *pb, MOVMuxContext *mov, MOVTrack *tr avio_write(pb, compressor_name, 31); if (track->mode == MODE_MOV && track->enc->bits_per_coded_sample) -avio_wb16(pb, track->enc->bits_per_coded_sample); +avio_wb16(pb, track->enc->bits_per_coded_sample | + (track->enc->pix_fmt == AV_PIX_FMT_GRAY8 ? 0x20 : 0)); else avio_wb16(pb, 0x18); /* Reserved */ -avio_wb16(pb, 0x); /* Reserved */ + +if (track->is_unaligned_qt_rgb && track->enc->pix_fmt == AV_PIX_FMT_PAL8) { +int i; +avio_wb16(pb, 0); /* Color table ID */ +avio_wb32(pb, 0); /* Color table seed */ +avio_wb16(pb, 0x8000);/* Color table flags */ +avio_wb16(pb, 255); /* Color table size (zero-relative) */ +for (i = 0; i < 256; i++) { +uint32_t rgb = AV_RL32(&track->palette[i]); +uint16_t r = (rgb >> 16) & 0xff; +uint16_t g = (rgb >> 8) & 0xff; +uint16_t b = rgb & 0xff; +avio_wb16(pb, 0); +avio_wb16(pb, (r << 8) | r); +avio_wb16(pb, (g << 8) | g); +avio_wb16(pb, (b << 8) | b); +} +} else +avio_wb16(pb, 0x); /* Reserved */ + if (track->tag == MKTAG('m','p','4','v')) mov_write_esds_tag(pb, track); else if (track->enc->codec_id == AV_CODEC_ID_H263) @@ -4703,6 +4723,7 @@ static int mov_write_packet(AVFormatContext *s, AVPacket *pkt) } else { int i; MOVMuxContext *mov = s->priv_data; +MOVTrack *trk = &mov->tracks[pkt->stream_index]; if (!pkt->size) return mov_write_single_packet(s, pkt); /* Passthrough. */ @@ -4739,6 +4760,31 @@ static int mov_write_packet(AVFormatContext *s, AVPacket *pkt) } } +if (trk->is_unaligned_qt_rgb) { +const uint8_t *data = pkt->data; +int size = pkt->size; +int64_t bpc = trk->enc->bits_per_coded_sample != 15 ? trk->enc->bits_per_coded_sample : 16; +int expected_stride = ((trk->enc->width * bpc + 15) >> 4)*2; +int ret = ff_reshuffle_raw_rgb(s, &pkt, trk->enc, expected_stride); +if (ret < 0) +return ret; +if (ret == CONTAINS_PAL && !trk->pal_done) { +int pal_size = 1 << trk->enc->bits_per_coded_sample; +memset(trk->palette, 0, AVPALETTE_SIZE); +memcpy(trk->palette, data + size - 4*pal_size, 4*pal_size); +trk->pal_done++; +} else if (trk->enc->pix_fmt == AV_PIX_FMT_GRAY8 || + trk->enc->pix_fmt == AV_PIX_FMT_MONOBLACK) { +for (i = 0; i < pkt->size; i++) +pkt->data[i] = ~pkt->data[i]; +} +if (ret) { +ret = mov_write_single_packet(s, pkt); +av_packet_free(&pkt); +return ret; +} +} + return mov_write_single_packet(s, pkt); } } @@ -5249,6 +5295,20 @@ static int mov_write_header(AVFormatContext *s) "WARNING codec timebase is very high. If duration is too long,\n" "file may not be playable by quicktime. Specify a shorter timebase\n" "or choose different container.\n"); +if (track->mode == MODE_MOV && +track->enc->codec_id == AV_CODEC_ID_RAWVIDEO && +track->tag == MKTAG('r','a','w',' ')) { +enum AVPixelFormat pix_fmt = track->enc->pix_fmt; +if (pix_fmt == AV_PIX_FMT_NONE && track->enc->bits_per_coded_sample == 1) +pix_fmt = AV_PIX_FMT_MONOWHITE; +track->is_unaligned_qt_rgb = +pix_fmt == AV_PIX_FMT_RGB24 || +pix_fmt == AV_PIX_FMT_BGR24 || +pix_fmt == AV_PIX_FMT_PAL8 || +pix_fmt == AV_PIX_FMT_GRAY8 || +pix_fmt == AV_PIX_FMT_MONOWHITE || +pix_fmt == AV_PIX_FMT_MONOBLACK; +} } else if (st->codec->codec_type == AVMEDIA_TYPE_AUDIO) { track->timescale = st->codec->sample_rate; if (!st->codec->frame_size && !a
Re: [FFmpeg-devel] [PATCH] Always read frame progress values under progress_mutex.
On Thu, Feb 25, 2016 at 04:42:22PM -0800, Wan-Teh Chang wrote: > On Thu, Feb 25, 2016 at 4:15 PM, Hendrik Leppkes wrote: > > > > Similar to the other patch, please explain what this actually fixes, > > those sanitizer tools produce a lot of false positives and nonsensical > > warnings. > > Yes, certainly. > > In libavcodec/pthread_frame.c, we have: > > 46 /** > 47 * Context used by codec threads and stored in their > AVCodecInternal thread_ctx. > 48 */ > 49 typedef struct PerThreadContext { > ... > 59 pthread_mutex_t progress_mutex; ///< Mutex used to protect > frame progress values and progress_cond. > > The comment on line 59 documents that progress_mutex guards frame > progress values and progress_cond. (In fact, progress_mutex also > guards the |state| field in many places, but that's not the subject of > this patch.) > > I assume "frame progess values" mean f->progress->data, where |f| > points to a ThreadFrame. I inspected libavcodec/pthread_frame.c and > concluded the code (ff_thread_report_progress and > ff_thread_await_progress) matches that comment, except that it also > does a double-checked locking style performance optimization. For > example, here is ff_thread_report_progress: > > 472 void ff_thread_report_progress(ThreadFrame *f, int n, int field) > 473 { > 474 PerThreadContext *p; > 475 volatile int *progress = f->progress ? (int*)f->progress->data : NULL; > 476 > 477 if (!progress || progress[field] >= n) return; > 478 > 479 p = f->owner->internal->thread_ctx; > 480 > 481 if (f->owner->debug&FF_DEBUG_THREADS) > 482 av_log(f->owner, AV_LOG_DEBUG, "%p finished %d field > %d\n", progress, n, field); > 483 > 484 pthread_mutex_lock(&p->progress_mutex); > 485 progress[field] = n; > 486 pthread_cond_broadcast(&p->progress_cond); > 487 pthread_mutex_unlock(&p->progress_mutex); > 488 } > > progress[field] can only increase. So, both ff_thread_report_progress > and ff_thread_await_progress have a fast code path (line 477 and line > 495, respectively) that reads progress[field] without acquiring > progress_mutex. If the speculatively read value of progress[field] is > >= |n|, then the functions return immediately. Otherwise, the > functions take the slow code path, which acquires progress_mutex > properly and does what the functions need to do. > > The fast code path is in the style of double-checked locking and has a > data race. ThreadSanitizer is right in this case. ThreadSanitizer is right about that this is a data race yes, but isnt all lock-free code technically containing a data race But is there a bug? Its basically a simple 2 thread thing, one says "iam finished" the other checks if that is set. In what case does this benefit from a lock ? also note the write is always under a lock, which id assume does any needed memory barriers also considering this is performance relevant code, please provide a benchmark [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Why not whip the teacher when the pupil misbehaves? -- Diogenes of Sinope signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v28] lavf/movenc: Add palette to video sample description
On 02/26/2016 05:08 AM, Mats Peterson wrote: Should hopefully fix the big-endian issue. Not that I understand why. I thought the palette was already in "big-endian" order, with the A, R, G, B bytes in that order, and that the previous patch would rather fail instead, so to say. But alright. Mats ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Always read frame progress values under progress_mutex.
On Thu, Feb 25, 2016 at 8:14 PM, Michael Niedermayer wrote: > > also considering this is performance relevant code, please provide > a benchmark I'll be happy to provide a benchmark. I don't see the instructions for running benchmarks in the patch submission checklist: https://ffmpeg.org/developer.html#patch-submission-checklist How do I run FFmpeg benchmarks? Thanks, Wan-Teh Chang ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v28] lavf/movenc: Add palette to video sample description
On 02/26/2016 05:26 AM, Mats Peterson wrote: On 02/26/2016 05:08 AM, Mats Peterson wrote: Should hopefully fix the big-endian issue. Not that I understand why. I thought the palette was already in "big-endian" order, with the A, R, G, B bytes in that order, and that the previous patch would rather fail instead, so to say. But alright. Mats ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel After applying this patch, would you like to try "ffmpeg -i 8bpp_129.mov -vcodec rawvideo out.mov" with the file below on that mips machine, just to see that nothing is wrong with the palette storage on the demuxer side? https://drive.google.com/open?id=0B3_pEBoLs0fabUdRcTVxRl9yVWM Mats -- Mats Peterson http://matsp888.no-ip.org/~mats/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v28] lavf/movenc: Add palette to video sample description
On 02/26/2016 06:24 AM, Mats Peterson wrote: On 02/26/2016 05:26 AM, Mats Peterson wrote: On 02/26/2016 05:08 AM, Mats Peterson wrote: Should hopefully fix the big-endian issue. Not that I understand why. I thought the palette was already in "big-endian" order, with the A, R, G, B bytes in that order, and that the previous patch would rather fail instead, so to say. But alright. Mats ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel After applying this patch, would you like to try "ffmpeg -i 8bpp_129.mov -vcodec rawvideo out.mov" with the file below on that mips machine, just to see that nothing is wrong with the palette storage on the demuxer side? https://drive.google.com/open?id=0B3_pEBoLs0fabUdRcTVxRl9yVWM Mats Look at this snippet from libavformat/qtpalette.c that stores a palette entry (palette[] is uint32_t): palette[i] = (a << 24 ) | (r << 16) | (g << 8) | (b); The way it is stored in memory is obviously dependent on the endianness of the machine. For little-endian machines, it will be BGRA, and for big-endian ones ARGB. Mats -- Mats Peterson http://matsp888.no-ip.org/~mats/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] lavf/qtpalette: Store the palette in little endian
Michael, since before I started messing with the QuickTime palette, the palette has usually been used in conjunction with AVI, which uses little endian order of the entries (BGRA), here is a proposed patch that forces little-endian order when storing the QuickTime palette, in case the movenc.c patch won't work with "ffmpeg -i 8bpp_129.mov -vcodec rawvideo out.mov" on that mips machine, which I suspect will be the case without knowing beforehand. Mats -- Mats Peterson http://matsp888.no-ip.org/~mats/ >From bf471c33a2e05c86410b5a8c22ec75dbcce70a2a Mon Sep 17 00:00:00 2001 From: Mats Peterson Date: Fri, 26 Feb 2016 07:02:57 +0100 Subject: [PATCH] lavf/qtpalette: Store the palette in little endian --- libavformat/qtpalette.c |6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/libavformat/qtpalette.c b/libavformat/qtpalette.c index 666c6b7..d53093e 100644 --- a/libavformat/qtpalette.c +++ b/libavformat/qtpalette.c @@ -61,7 +61,7 @@ int ff_get_qtpalette(int codec_id, AVIOContext *pb, uint32_t *palette) color_dec = 256 / (color_count - 1); for (i = 0; i < color_count; i++) { r = g = b = color_index; -palette[i] = (0xFFU << 24) | (r << 16) | (g << 8) | (b); +AV_WL32(&palette[i], (0xFFU << 24) | (r << 16) | (g << 8) | (b)); color_index -= color_dec; if (color_index < 0) color_index = 0; @@ -84,7 +84,7 @@ int ff_get_qtpalette(int codec_id, AVIOContext *pb, uint32_t *palette) r = color_table[i * 3 + 0]; g = color_table[i * 3 + 1]; b = color_table[i * 3 + 2]; -palette[i] = (0xFFU << 24) | (r << 16) | (g << 8) | (b); +AV_WL32(&palette[i], (0xFFU << 24) | (r << 16) | (g << 8) | (b)); } } else { /* The color table ID is 0; the color table is in the sample @@ -104,7 +104,7 @@ int ff_get_qtpalette(int codec_id, AVIOContext *pb, uint32_t *palette) avio_r8(pb); b = avio_r8(pb); avio_r8(pb); -palette[i] = (a << 24 ) | (r << 16) | (g << 8) | (b); +AV_WL32(&palette[i], (a << 24 ) | (r << 16) | (g << 8) | (b)); } } } -- 1.7.10.4 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v28] lavf/movenc: Add palette to video sample description
On 02/26/2016 06:40 AM, Mats Peterson wrote: On 02/26/2016 06:24 AM, Mats Peterson wrote: On 02/26/2016 05:26 AM, Mats Peterson wrote: On 02/26/2016 05:08 AM, Mats Peterson wrote: Should hopefully fix the big-endian issue. Not that I understand why. I thought the palette was already in "big-endian" order, with the A, R, G, B bytes in that order, and that the previous patch would rather fail instead, so to say. But alright. Mats ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel After applying this patch, would you like to try "ffmpeg -i 8bpp_129.mov -vcodec rawvideo out.mov" with the file below on that mips machine, just to see that nothing is wrong with the palette storage on the demuxer side? https://drive.google.com/open?id=0B3_pEBoLs0fabUdRcTVxRl9yVWM Mats Look at this snippet from libavformat/qtpalette.c that stores a palette entry (palette[] is uint32_t): palette[i] = (a << 24 ) | (r << 16) | (g << 8) | (b); The way it is stored in memory is obviously dependent on the endianness of the machine. For little-endian machines, it will be BGRA, and for big-endian ones ARGB. Mats This shouldn't have any relevance, though. Something is possibly wrong with the palette storage endian-wise when converting that matrix file from yuv420p to pal8. Mats -- Mats Peterson http://matsp888.no-ip.org/~mats/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v28] lavf/movenc: Add palette to video sample description
On 02/26/2016 07:41 AM, Mats Peterson wrote: Look at this snippet from libavformat/qtpalette.c that stores a palette entry (palette[] is uint32_t): palette[i] = (a << 24 ) | (r << 16) | (g << 8) | (b); The way it is stored in memory is obviously dependent on the endianness of the machine. For little-endian machines, it will be BGRA, and for big-endian ones ARGB. Mats This shouldn't have any relevance, though. Something is possibly wrong with the palette storage endian-wise when converting that matrix file from yuv420p to pal8. Mats My final spam for some time. The palette is stored in host byte order internally, isn't it? Then I don't understand whatsoever why v26 of my movenc patch will fail on a big-endian machine. No AV_RL32() should be needed. Mats -- Mats Peterson http://matsp888.no-ip.org/~mats/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavf/qtpalette: Store the palette in little endian
On 02/26/2016 07:09 AM, Mats Peterson wrote: Michael, since before I started messing with the QuickTime palette, the palette has usually been used in conjunction with AVI, which uses little endian order of the entries (BGRA), here is a proposed patch that forces little-endian order when storing the QuickTime palette, in case the movenc.c patch won't work with "ffmpeg -i 8bpp_129.mov -vcodec rawvideo out.mov" on that mips machine, which I suspect will be the case without knowing beforehand. Mats ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel Forget this one, it is probably incorrect. -- Mats Peterson http://matsp888.no-ip.org/~mats/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] lavf/matroska: expose stream encoding as global side-data
Some demuxers can't handle Matroska compression, so this lets API users check if a file uses it and determine whether those players will fail. --- libavcodec/avcodec.h | 35 ++- libavcodec/avpacket.c | 1 + libavcodec/version.h | 2 +- libavformat/matroskadec.c | 23 +-- 4 files changed, 53 insertions(+), 8 deletions(-) diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h index 7a56899..bd42f58 100644 --- a/libavcodec/avcodec.h +++ b/libavcodec/avcodec.h @@ -1227,6 +1227,34 @@ typedef struct AVCPBProperties { uint64_t vbv_delay; } AVCPBProperties; +/** + * This structure describes the Matroska encoding (compression or encryption) applied to + * a stream. It can be useful when determining whether or not other demuxers can handle + * a file, since some players don't support header compression. + */ +typedef struct AVMatroskaEncoding { +/** + * Type of content encoding. 0 for compression; 1 for encryption. + */ +uint64_t type; +/** + * Algorithm used for the compression or encryption. + * For compression: + * 0 - zlib, + * 1 - bzlib, + * 2 - lzo1x + * 3 - Header Stripping + * For encryption: + * 0 - Signing only + * 1 - DES + * 2 - 3DES + * 3 - Twofish + * 4 - Blowfish + * 5 - AES + */ +uint64_t algorithm; +} AVMatroskaEncoding; + #if FF_API_QSCALE_TYPE #define FF_QSCALE_TYPE_MPEG1 0 #define FF_QSCALE_TYPE_MPEG2 1 @@ -1414,7 +1442,12 @@ enum AVPacketSideDataType { * should be associated with a video stream and containts data in the form * of the AVMasteringDisplayMetadata struct. */ -AV_PKT_DATA_MASTERING_DISPLAY_METADATA +AV_PKT_DATA_MASTERING_DISPLAY_METADATA, + +/** + * Corresponds to the AVMatroskaEncoding struct. + */ +AV_PKT_DATA_MATROSKA_ENCODING, }; #define AV_PKT_DATA_QUALITY_FACTOR AV_PKT_DATA_QUALITY_STATS //DEPRECATED diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c index 97f079f..6a9811b 100644 --- a/libavcodec/avpacket.c +++ b/libavcodec/avpacket.c @@ -353,6 +353,7 @@ const char *av_packet_side_data_name(enum AVPacketSideDataType type) case AV_PKT_DATA_METADATA_UPDATE:return "Metadata Update"; case AV_PKT_DATA_MPEGTS_STREAM_ID: return "MPEGTS Stream ID"; case AV_PKT_DATA_MASTERING_DISPLAY_METADATA: return "Mastering display metadata"; +case AV_PKT_DATA_MATROSKA_ENCODING: return "Matroska Encoding"; } return NULL; } diff --git a/libavcodec/version.h b/libavcodec/version.h index 0856b19..c937eaf 100644 --- a/libavcodec/version.h +++ b/libavcodec/version.h @@ -29,7 +29,7 @@ #define LIBAVCODEC_VERSION_MAJOR 57 #define LIBAVCODEC_VERSION_MINOR 25 -#define LIBAVCODEC_VERSION_MICRO 101 +#define LIBAVCODEC_VERSION_MICRO 102 #define LIBAVCODEC_VERSION_INT AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \ LIBAVCODEC_VERSION_MINOR, \ diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c index d788232..d2172ac 100644 --- a/libavformat/matroskadec.c +++ b/libavformat/matroskadec.c @@ -1769,6 +1769,10 @@ static int matroska_parse_tracks(AVFormatContext *s) if (!track->codec_id) continue; +st = track->stream = avformat_new_stream(s, NULL); +if (!st) +return AVERROR(ENOMEM); + if (track->audio.samplerate < 0 || track->audio.samplerate > INT_MAX || isnan(track->audio.samplerate)) { av_log(matroska->ctx, AV_LOG_WARNING, @@ -1794,7 +1798,16 @@ static int matroska_parse_tracks(AVFormatContext *s) av_log(matroska->ctx, AV_LOG_ERROR, "Multiple combined encodings not supported"); } else if (encodings_list->nb_elem == 1) { +AVMatroskaEncoding *side_data = (void*)av_stream_new_side_data(st, + AV_PKT_DATA_MATROSKA_ENCODING, + sizeof(AVMatroskaEncoding)); +if (!side_data) +return AVERROR(ENOMEM); + +side_data->type = encodings[0].type; + if (encodings[0].type) { +side_data->algorithm = encodings[0].encryption.algo; if (encodings[0].encryption.key_id.size > 0) { /* Save the encryption key id to be stored later as a metadata tag. */ @@ -1822,10 +1835,12 @@ static int matroska_parse_tracks(AVFormatContext *s) encodings[0].compression.algo != MATROSKA_TRACK_ENCODING_COMP_LZO && #endif encodings[0].compression.algo != MATROSKA_TRACK_ENCODING_COMP_HEADERSTRIP) { +side_data->algorithm = encodings[0].compression.algo; encodings[0].scope = 0; av_log(matroska->ctx, AV_LOG_ERROR, "Unsupported encoding type
Re: [FFmpeg-devel] [PATCH v28] lavf/movenc: Add palette to video sample description
On 02/26/2016 05:08 AM, Mats Peterson wrote: Should hopefully fix the big-endian issue. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel Forget this one as well. Using AV_RL32() shouldn't be needed, since the palette is already stored in host byte order as far as I know. And I don't understand whatsoever why it will fail on a big-endian machine, Michael. Mats -- Mats Peterson http://matsp888.no-ip.org/~mats/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v28] lavf/movenc: Add palette to video sample description
On 02/26/2016 08:28 AM, Mats Peterson wrote: On 02/26/2016 05:08 AM, Mats Peterson wrote: Should hopefully fix the big-endian issue. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel Forget this one as well. Using AV_RL32() shouldn't be needed, since the palette is already stored in host byte order as far as I know. And I don't understand whatsoever why it will fail on a big-endian machine, Michael. Mats Snippet from avidec.c: ast->pal[i] = 0xFFU<<24 | AV_RL32(pal_src+4*i); And from qtpalette.c (mov and matroska): palette[i] = (a << 24 ) | (r << 16) | (g << 8) | (b); Something obviously goes wrong when storing the palette during yuv420p to pal8 conversion of that matrix file. Mats -- Mats Peterson http://matsp888.no-ip.org/~mats/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel