Re: [FFmpeg-devel] [PATCH 1/3] avformat/mux: add a format flag which ensure parsed and standardized creation time

2016-02-25 Thread wm4
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

2016-02-25 Thread Paul B Mahol
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

2016-02-25 Thread Stefano Sabatini
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

2016-02-25 Thread Nicolas George
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

2016-02-25 Thread Nicolas George
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

2016-02-25 Thread Carl Eugen Hoyos
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

2016-02-25 Thread Michael Niedermayer
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

2016-02-25 Thread Clément Bœsch
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

2016-02-25 Thread Derek Buitenhuis
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

2016-02-25 Thread Hendrik Leppkes
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

2016-02-25 Thread Hendrik Leppkes
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

2016-02-25 Thread Derek Buitenhuis
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

2016-02-25 Thread Clément Bœsch
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

2016-02-25 Thread Matthieu Bouron
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

2016-02-25 Thread Josh de Kock

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

2016-02-25 Thread Clément Bœsch
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

2016-02-25 Thread Josh de Kock

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

2016-02-25 Thread Nicolas George
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

2016-02-25 Thread Derek Buitenhuis
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

2016-02-25 Thread Josh de Kock

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

2016-02-25 Thread Carl Eugen Hoyos
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

2016-02-25 Thread Carl Eugen Hoyos
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

2016-02-25 Thread Derek Buitenhuis
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

2016-02-25 Thread Derek Buitenhuis
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

2016-02-25 Thread Stefano Sabatini
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

2016-02-25 Thread Michael Niedermayer
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

2016-02-25 Thread Michael Niedermayer
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.

2016-02-25 Thread Wan-Teh Chang
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.

2016-02-25 Thread Ronald S. Bultje
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.

2016-02-25 Thread Reimar Döffinger
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.

2016-02-25 Thread wm4
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()

2016-02-25 Thread Matthieu Bouron
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

2016-02-25 Thread Matthieu Bouron
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?

2016-02-25 Thread F.Sluiter
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

2016-02-25 Thread Clément Bœsch
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

2016-02-25 Thread Clément Bœsch
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?

2016-02-25 Thread 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


Re: [FFmpeg-devel] [PATCH 2/2] img2dec: Add mime_type to image formats

2016-02-25 Thread Clément Bœsch
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?

2016-02-25 Thread 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] [PATCH] mss2: Fix buffer overflow.

2016-02-25 Thread Reimar Döffinger
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?

2016-02-25 Thread F.Sluiter
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?

2016-02-25 Thread 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] [PATCH]lavf/img2dec: Skip SOF size when probing jpeg

2016-02-25 Thread Carl Eugen Hoyos
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

2016-02-25 Thread Carl Eugen Hoyos
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

2016-02-25 Thread Marton Balint



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?

2016-02-25 Thread 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; 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?

2016-02-25 Thread F.Sluiter
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?

2016-02-25 Thread 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; 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?

2016-02-25 Thread F.Sluiter
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

2016-02-25 Thread Michael Niedermayer
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.

2016-02-25 Thread Reimar Döffinger
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.

2016-02-25 Thread Wan-Teh Chang
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.

2016-02-25 Thread Hendrik Leppkes
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.

2016-02-25 Thread Wan-Teh Chang
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

2016-02-25 Thread Michael Niedermayer
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.

2016-02-25 Thread Wan-Teh Chang
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.

2016-02-25 Thread Michael Niedermayer
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|.

2016-02-25 Thread Wan-Teh Chang
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.

2016-02-25 Thread Wan-Teh Chang
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.

2016-02-25 Thread James Almer
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

2016-02-25 Thread Michael Niedermayer
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

2016-02-25 Thread Mats Peterson

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

2016-02-25 Thread Mats Peterson

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

2016-02-25 Thread Mats Peterson

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.

2016-02-25 Thread Michael Niedermayer
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

2016-02-25 Thread Mats Peterson

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.

2016-02-25 Thread Wan-Teh Chang
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

2016-02-25 Thread Mats Peterson

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

2016-02-25 Thread Mats Peterson

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

2016-02-25 Thread Mats Peterson
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

2016-02-25 Thread Mats Peterson

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

2016-02-25 Thread Mats Peterson

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

2016-02-25 Thread Mats Peterson

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

2016-02-25 Thread Rodger Combs
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

2016-02-25 Thread Mats Peterson

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

2016-02-25 Thread Mats Peterson

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