Re: [FFmpeg-devel] [PATCH] avcodec: Add AV_CODEC_FLAG2_DONT_SPLIT_SIDE_DATA

2017-03-07 Thread wm4
On Wed,  8 Mar 2017 02:57:50 +0100
Michael Niedermayer  wrote:

> This allows to test or use  the code without av_packet_split_side_data() or
> allows applications to separate the side data handling out not
> running it automatically.
> 
> It also makes it easier to change the default behavior
> 
> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/avcodec.h |  5 +
>  libavcodec/utils.c   | 12 
>  2 files changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index af054f3194..92e930249e 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -910,6 +910,11 @@ typedef struct RcOverride{
>   */
>  #define AV_CODEC_FLAG2_FAST   (1 <<  0)
>  /**
> + * Do not split side data.
> + * @see AVFMT_FLAG_KEEP_SIDE_DATA
> + */
> +#define AV_CODEC_FLAG2_DONT_SPLIT_SIDE_DATA (1 << 1)
> +/**
>   * Skip bitstream encoding.
>   */
>  #define AV_CODEC_FLAG2_NO_OUTPUT  (1 <<  2)
> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> index db3adb18d4..94278c6950 100644
> --- a/libavcodec/utils.c
> +++ b/libavcodec/utils.c
> @@ -2250,7 +2250,8 @@ int attribute_align_arg 
> avcodec_decode_video2(AVCodecContext *avctx, AVFrame *pi
>  
>  if ((avctx->codec->capabilities & AV_CODEC_CAP_DELAY) || avpkt->size ||
>  (avctx->active_thread_type & FF_THREAD_FRAME)) {
> -int did_split = av_packet_split_side_data();
> +int did_split = (avctx->flags2 & 
> AV_CODEC_FLAG2_DONT_SPLIT_SIDE_DATA) ?
> +0 : av_packet_split_side_data();
>  ret = apply_param_change(avctx, );
>  if (ret < 0)
>  goto fail;
> @@ -2356,7 +2357,8 @@ int attribute_align_arg 
> avcodec_decode_audio4(AVCodecContext *avctx,
>  uint8_t discard_reason = 0;
>  // copy to ensure we do not change avpkt
>  AVPacket tmp = *avpkt;
> -int did_split = av_packet_split_side_data();
> +int did_split = (avctx->flags2 & 
> AV_CODEC_FLAG2_DONT_SPLIT_SIDE_DATA) ?
> +0 : av_packet_split_side_data();
>  ret = apply_param_change(avctx, );
>  if (ret < 0)
>  goto fail;
> @@ -2669,7 +2671,8 @@ int avcodec_decode_subtitle2(AVCodecContext *avctx, 
> AVSubtitle *sub,
>  if ((avctx->codec->capabilities & AV_CODEC_CAP_DELAY) || avpkt->size) {
>  AVPacket pkt_recoded;
>  AVPacket tmp = *avpkt;
> -int did_split = av_packet_split_side_data();
> +int did_split = (avctx->flags2 & 
> AV_CODEC_FLAG2_DONT_SPLIT_SIDE_DATA) ?
> +0 : av_packet_split_side_data();
>  //apply_param_change(avctx, );
>  
>  if (did_split) {
> @@ -2860,7 +2863,8 @@ int attribute_align_arg 
> avcodec_send_packet(AVCodecContext *avctx, const AVPacke
>  if (avctx->codec->send_packet) {
>  if (avpkt) {
>  AVPacket tmp = *avpkt;
> -int did_split = av_packet_split_side_data();
> +int did_split = (avctx->flags2 & 
> AV_CODEC_FLAG2_DONT_SPLIT_SIDE_DATA) ?
> +0 : av_packet_split_side_data();
>  ret = apply_param_change(avctx, );
>  if (ret >= 0)
>  ret = avctx->codec->send_packet(avctx, );

I don't understand, what's the purpose?

IMO this merge/split side data is a pointless waste of resources and
should be removed.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCHv3 3/3] mkv: Export bounds and padding from spherical metadata

2017-03-07 Thread wm4
On Tue, 7 Mar 2017 21:38:52 +0100
Hendrik Leppkes  wrote:

> On Tue, Mar 7, 2017 at 7:26 PM, wm4  wrote:
> > On Tue, 21 Feb 2017 17:35:53 -0500
> > Vittorio Giovara  wrote:
> >  
> >> ---
> >>  libavformat/matroskadec.c  | 64 
> >> --
> >>  tests/ref/fate/matroska-spherical-mono |  6 +++-
> >>  2 files changed, 66 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
> >> index 7223e94..0a02237 100644
> >> --- a/libavformat/matroskadec.c
> >> +++ b/libavformat/matroskadec.c
> >> @@ -1913,16 +1913,65 @@ static int mkv_parse_video_projection(AVStream 
> >> *st, const MatroskaTrack *track)
> >>  AVSphericalMapping *spherical;
> >>  enum AVSphericalProjection projection;
> >>  size_t spherical_size;
> >> +size_t l, t, r, b;
> >> +size_t padding = 0;
> >>  int ret;
> >> +GetByteContext gb;
> >> +
> >> +bytestream2_init(, track->video.projection.private.data,
> >> + track->video.projection.private.size);
> >> +
> >> +if (bytestream2_get_byte() != 0) {
> >> +av_log(NULL, AV_LOG_WARNING, "Unknown spherical metadata\n");
> >> +return 0;
> >> +}
> >> +
> >> +bytestream2_skip(, 3); // flags
> >>
> >>  switch (track->video.projection.type) {
> >>  case MATROSKA_VIDEO_PROJECTION_TYPE_EQUIRECTANGULAR:
> >> -projection = AV_SPHERICAL_EQUIRECTANGULAR;
> >> +if (track->video.projection.private.size == 0)
> >> +projection = AV_SPHERICAL_EQUIRECTANGULAR;
> >> +else if (track->video.projection.private.size == 20) {
> >> +t = bytestream2_get_be32();
> >> +b = bytestream2_get_be32();
> >> +l = bytestream2_get_be32();
> >> +r = bytestream2_get_be32();
> >> +
> >> +if (b >= UINT_MAX - t || r >= UINT_MAX - l) {
> >> +av_log(NULL, AV_LOG_ERROR,
> >> +   "Invalid bounding rectangle coordinates "
> >> +   "%zu,%zu,%zu,%zu\n", l, t, r, b);
> >> +return AVERROR_INVALIDDATA;
> >> +}
> >> +
> >> +if (l || t || r || b)
> >> +projection = AV_SPHERICAL_EQUIRECTANGULAR_TILE;
> >> +else
> >> +projection = AV_SPHERICAL_EQUIRECTANGULAR;
> >> +} else {
> >> +av_log(NULL, AV_LOG_ERROR, "Unknown spherical metadata\n");
> >> +return AVERROR_INVALIDDATA;
> >> +}
> >>  break;
> >>  case MATROSKA_VIDEO_PROJECTION_TYPE_CUBEMAP:
> >> -if (track->video.projection.private.size < 4)
> >> +if (track->video.projection.private.size < 4) {
> >> +av_log(NULL, AV_LOG_ERROR, "Missing projection private 
> >> properties\n");
> >> +return AVERROR_INVALIDDATA;
> >> +} else if (track->video.projection.private.size == 12) {
> >> +uint32_t layout = bytestream2_get_be32();
> >> +if (layout == 0) {
> >> +projection = AV_SPHERICAL_CUBEMAP;
> >> +} else {
> >> +av_log(NULL, AV_LOG_WARNING,
> >> +   "Unknown spherical cubemap layout %"PRIu32"\n", 
> >> layout);
> >> +return 0;
> >> +}
> >> +padding = bytestream2_get_be32();
> >> +} else {
> >> +av_log(NULL, AV_LOG_ERROR, "Unknown spherical metadata\n");
> >>  return AVERROR_INVALIDDATA;
> >> -projection = AV_SPHERICAL_CUBEMAP;
> >> +}
> >>  break;
> >>  default:
> >>  return 0;
> >> @@ -1937,6 +1986,15 @@ static int mkv_parse_video_projection(AVStream *st, 
> >> const MatroskaTrack *track)
> >>  spherical->pitch = (int32_t)(track->video.projection.pitch * (1 << 
> >> 16));
> >>  spherical->roll  = (int32_t)(track->video.projection.roll  * (1 << 
> >> 16));
> >>
> >> +spherical->padding = padding;
> >> +
> >> +if (spherical->projection == AV_SPHERICAL_EQUIRECTANGULAR_TILE) {
> >> +spherical->bound_left   = l;
> >> +spherical->bound_top= t;
> >> +spherical->bound_right  = r;
> >> +spherical->bound_bottom = b;
> >> +}
> >> +
> >>  ret = av_stream_add_side_data(st, AV_PKT_DATA_SPHERICAL, (uint8_t 
> >> *)spherical,
> >>spherical_size);
> >>  if (ret < 0) {
> >> diff --git a/tests/ref/fate/matroska-spherical-mono 
> >> b/tests/ref/fate/matroska-spherical-mono
> >> index 8048aff..a70d879 100644
> >> --- a/tests/ref/fate/matroska-spherical-mono
> >> +++ b/tests/ref/fate/matroska-spherical-mono
> >> @@ -8,7 +8,11 @@ inverted=0
> >>  [SIDE_DATA]
> >>  side_data_type=Spherical Mapping
> >>  side_data_size=56
> >> -projection=equirectangular
> >> +projection=tiled equirectangular
> >> +bound_left=148
> >> +bound_top=73
> >> +bound_right=147
> >> +bound_bottom=72

Re: [FFmpeg-devel] [PATCH] Revert "lavu/atomic: add support for the new memory model aware gcc built-ins"

2017-03-07 Thread James Almer
On 3/7/2017 3:47 AM, wm4 wrote:
> On Tue,  7 Mar 2017 00:29:53 -0300
> James Almer  wrote:
> 
>> This reverts commit faa9d2982969c999ab0e443a226eff116f7f8e4b.
>>
>> This change became superfluous when support for C11 atomics was introduced.
>> Reverting it will make the removal of this implementation in an upcoming
>> merge conflict free.
>>
>> Signed-off-by: James Almer 
>> ---
>>  configure  |  4 +---
>>  libavutil/atomic_gcc.h | 17 -
>>  2 files changed, 1 insertion(+), 20 deletions(-)
>>
>> diff --git a/configure b/configure
>> index 0199fec5c0..6350942ef9 100755
>> --- a/configure
>> +++ b/configure
>> @@ -1821,7 +1821,6 @@ ARCH_FEATURES="
>>  
>>  BUILTIN_LIST="
>>  atomic_cas_ptr
>> -atomic_compare_exchange
>>  machine_rw_barrier
>>  MemoryBarrier
>>  mm_empty
>> @@ -2322,7 +2321,7 @@ symver_if_any="symver_asm_label symver_gnu_asm"
>>  valgrind_backtrace_deps="!optimizations valgrind_valgrind_h"
>>  
>>  # threading support
>> -atomics_gcc_if_any="sync_val_compare_and_swap atomic_compare_exchange"
>> +atomics_gcc_if="sync_val_compare_and_swap"
>>  atomics_suncc_if="atomic_cas_ptr machine_rw_barrier"
>>  atomics_win32_if="MemoryBarrier"
>>  atomics_native_if_any="$ATOMICS_LIST"
>> @@ -5533,7 +5532,6 @@ if ! disabled network; then
>>  fi
>>  
>>  check_builtin atomic_cas_ptr atomic.h "void **ptr; void *oldval, *newval; 
>> atomic_cas_ptr(ptr, oldval, newval)"
>> -check_builtin atomic_compare_exchange "" "int *ptr, *oldval; int newval; 
>> __atomic_compare_exchange_n(ptr, oldval, newval, 0, __ATOMIC_SEQ_CST, 
>> __ATOMIC_SEQ_CST)"
>>  check_builtin machine_rw_barrier mbarrier.h "__machine_rw_barrier()"
>>  check_builtin MemoryBarrier windows.h "MemoryBarrier()"
>>  check_builtin sarestart signal.h "SA_RESTART"
>> diff --git a/libavutil/atomic_gcc.h b/libavutil/atomic_gcc.h
>> index 5f9fc49ba0..2bb43c3cea 100644
>> --- a/libavutil/atomic_gcc.h
>> +++ b/libavutil/atomic_gcc.h
>> @@ -28,40 +28,27 @@
>>  #define avpriv_atomic_int_get atomic_int_get_gcc
>>  static inline int atomic_int_get_gcc(volatile int *ptr)
>>  {
>> -#if HAVE_ATOMIC_COMPARE_EXCHANGE
>> -return __atomic_load_n(ptr, __ATOMIC_SEQ_CST);
>> -#else
>>  __sync_synchronize();
>>  return *ptr;
>> -#endif
>>  }
>>  
>>  #define avpriv_atomic_int_set atomic_int_set_gcc
>>  static inline void atomic_int_set_gcc(volatile int *ptr, int val)
>>  {
>> -#if HAVE_ATOMIC_COMPARE_EXCHANGE
>> -__atomic_store_n(ptr, val, __ATOMIC_SEQ_CST);
>> -#else
>>  *ptr = val;
>>  __sync_synchronize();
>> -#endif
>>  }
>>  
>>  #define avpriv_atomic_int_add_and_fetch atomic_int_add_and_fetch_gcc
>>  static inline int atomic_int_add_and_fetch_gcc(volatile int *ptr, int inc)
>>  {
>> -#if HAVE_ATOMIC_COMPARE_EXCHANGE
>> -return __atomic_add_fetch(ptr, inc, __ATOMIC_SEQ_CST);
>> -#else
>>  return __sync_add_and_fetch(ptr, inc);
>> -#endif
>>  }
>>  
>>  #define avpriv_atomic_ptr_cas atomic_ptr_cas_gcc
>>  static inline void *atomic_ptr_cas_gcc(void * volatile *ptr,
>> void *oldval, void *newval)
>>  {
>> -#if HAVE_SYNC_VAL_COMPARE_AND_SWAP
>>  #ifdef __ARMCC_VERSION
>>  // armcc will throw an error if ptr is not an integer type
>>  volatile uintptr_t *tmp = (volatile uintptr_t*)ptr;
>> @@ -69,10 +56,6 @@ static inline void *atomic_ptr_cas_gcc(void * volatile 
>> *ptr,
>>  #else
>>  return __sync_val_compare_and_swap(ptr, oldval, newval);
>>  #endif
>> -#else
>> -__atomic_compare_exchange_n(ptr, , newval, 0, __ATOMIC_SEQ_CST, 
>> __ATOMIC_SEQ_CST);
>> -return oldval;
>> -#endif
>>  }
>>  
>>  #endif /* AVUTIL_ATOMIC_GCC_H */
> 
> Sounds like a good thing.

Pushed.

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 1/2] libx264: remove the "x264opts" AVOption

2017-03-07 Thread Michael Niedermayer
On Wed, Mar 08, 2017 at 01:41:38AM +0200, Jan Ekstrom wrote:
> On Wed, Mar 8, 2017 at 1:01 AM, Michael Niedermayer
>  wrote:
> > the fast pskip i mentioned yesterday
> >
> > ./ffmpeg -y -i ~/videos/matrixbench_mpeg2.mpg -vcodec libx264 -x264-params 
> > fast-pskip -vframes 15 -an ref-p.nut
> > ./ffmpeg -y -i ~/videos/matrixbench_mpeg2.mpg -vcodec libx264 -x264-params 
> > no-fast-pskip -vframes 15 -an ref-np.nut
> > ./ffmpeg -y -i ~/videos/matrixbench_mpeg2.mpg -vcodec libx264 -x264opts 
> > no-fast-pskip -vframes 15 -an ref-no.nut
> > ./ffmpeg -y -i ~/videos/matrixbench_mpeg2.mpg -vcodec libx264 -x264opts 
> > fast-pskip -vframes 15 -an ref-o.nut
> >
> > ffd2c735cfb268d5f4291c2f6baab386  ref-o.nut
> > ffd2c735cfb268d5f4291c2f6baab386  ref-p.nut
> > f424075a964018aa83f98f2bf5ab2830  ref-no.nut
> > ffd2c735cfb268d5f4291c2f6baab386  ref-np.nu
> >
> > it appears *fast-pskip has no effect in x264-params
> 
> Right, as I noted in my previous message, the custom dictionary
> parsing code in there sets the value to "1" if a value is not
> available. The API always expects a key and a value (even if a NULL
> value would be valid for a boolean option).
> So to emulate what x264opts is doing you would just set "fast-pskip=1"
> or "no-fast-pskip=1". You can by this guess that fast-pskip is indeed
> the default behavior during medium preset in libx264.
> 

> In my personal opinion I'm not sure this is worth it to have a Yet
> Another Dictionary Parser inside libx264.c, esp. since
> av_dict_parse_string() behavior is already the defining one in
> multiple components where key/value lists are handled (you can see our
> Doxygen for some sort of list of usage:
> https://www.ffmpeg.org/doxygen/trunk/group__lavu__dict.html#gaca5ff7c251e60bd13164d13c82f21b79
> ).

As it is the code that uses the common parser is buggy
that is if a user specifies no-fast-pskip
its not disabled
in fact it does not even error out or give any warning

IIUC you want an implementation thats based on a common parser ?
can you write an implementation thats based on a common parser and
which works without bugs in corner cases ?
i think that should be possible but if iam missing something, please
elaborate

[...]
-- 
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] avcodec: Add AV_CODEC_FLAG2_DONT_SPLIT_SIDE_DATA

2017-03-07 Thread Michael Niedermayer
This allows to test or use  the code without av_packet_split_side_data() or
allows applications to separate the side data handling out not
running it automatically.

It also makes it easier to change the default behavior

Signed-off-by: Michael Niedermayer 
---
 libavcodec/avcodec.h |  5 +
 libavcodec/utils.c   | 12 
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index af054f3194..92e930249e 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -910,6 +910,11 @@ typedef struct RcOverride{
  */
 #define AV_CODEC_FLAG2_FAST   (1 <<  0)
 /**
+ * Do not split side data.
+ * @see AVFMT_FLAG_KEEP_SIDE_DATA
+ */
+#define AV_CODEC_FLAG2_DONT_SPLIT_SIDE_DATA (1 << 1)
+/**
  * Skip bitstream encoding.
  */
 #define AV_CODEC_FLAG2_NO_OUTPUT  (1 <<  2)
diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index db3adb18d4..94278c6950 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -2250,7 +2250,8 @@ int attribute_align_arg 
avcodec_decode_video2(AVCodecContext *avctx, AVFrame *pi
 
 if ((avctx->codec->capabilities & AV_CODEC_CAP_DELAY) || avpkt->size ||
 (avctx->active_thread_type & FF_THREAD_FRAME)) {
-int did_split = av_packet_split_side_data();
+int did_split = (avctx->flags2 & AV_CODEC_FLAG2_DONT_SPLIT_SIDE_DATA) ?
+0 : av_packet_split_side_data();
 ret = apply_param_change(avctx, );
 if (ret < 0)
 goto fail;
@@ -2356,7 +2357,8 @@ int attribute_align_arg 
avcodec_decode_audio4(AVCodecContext *avctx,
 uint8_t discard_reason = 0;
 // copy to ensure we do not change avpkt
 AVPacket tmp = *avpkt;
-int did_split = av_packet_split_side_data();
+int did_split = (avctx->flags2 & AV_CODEC_FLAG2_DONT_SPLIT_SIDE_DATA) ?
+0 : av_packet_split_side_data();
 ret = apply_param_change(avctx, );
 if (ret < 0)
 goto fail;
@@ -2669,7 +2671,8 @@ int avcodec_decode_subtitle2(AVCodecContext *avctx, 
AVSubtitle *sub,
 if ((avctx->codec->capabilities & AV_CODEC_CAP_DELAY) || avpkt->size) {
 AVPacket pkt_recoded;
 AVPacket tmp = *avpkt;
-int did_split = av_packet_split_side_data();
+int did_split = (avctx->flags2 & AV_CODEC_FLAG2_DONT_SPLIT_SIDE_DATA) ?
+0 : av_packet_split_side_data();
 //apply_param_change(avctx, );
 
 if (did_split) {
@@ -2860,7 +2863,8 @@ int attribute_align_arg 
avcodec_send_packet(AVCodecContext *avctx, const AVPacke
 if (avctx->codec->send_packet) {
 if (avpkt) {
 AVPacket tmp = *avpkt;
-int did_split = av_packet_split_side_data();
+int did_split = (avctx->flags2 & 
AV_CODEC_FLAG2_DONT_SPLIT_SIDE_DATA) ?
+0 : av_packet_split_side_data();
 ret = apply_param_change(avctx, );
 if (ret >= 0)
 ret = avctx->codec->send_packet(avctx, );
-- 
2.11.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH] lavc/movtextdec: fix incorrect offset calculation for UTF-8 characters

2017-03-07 Thread Erik Bråthen Solem
The 3GPP Timed Text (TTXT / tx3g / mov_text) specification counts multibyte 
UTF-8 characters as one single character, ffmpeg currently counts bytes. This 
patch inserts an if test such that:
1. continuation bytes are not counted during decoding
2. style boxes will not split these characters

Fixes trac #6021 (decoding part).

---
 libavcodec/movtextdec.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/libavcodec/movtextdec.c b/libavcodec/movtextdec.c
index 6de1500..2c7a204 100644
--- a/libavcodec/movtextdec.c
+++ b/libavcodec/movtextdec.c
@@ -342,6 +342,7 @@ static int text_to_ass(AVBPrint *buf, const char *text, 
const char *text_end,
 }
 
 while (text < text_end) {
+if ((*text & 0xC0) != 0x80) { /* Boxes never split multibyte chars */
 if (m->box_flags & STYL_BOX) {
 for (i = 0; i < m->style_entries; i++) {
 if (m->s[i]->style_flag && text_pos == m->s[i]->style_end) {
@@ -387,6 +388,8 @@ static int text_to_ass(AVBPrint *buf, const char *text, 
const char *text_end,
 }
 }
 }
+text_pos++;
+}
 
 switch (*text) {
 case '\r':
@@ -399,7 +402,6 @@ static int text_to_ass(AVBPrint *buf, const char *text, 
const char *text_end,
 break;
 }
 text++;
-text_pos++;
 }
 
 return 0;
-- 
1.9.5 (Apple Git-50.3)

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH] lavc/movtextenc: fix incorrect offset calculation for UTF-8 characters

2017-03-07 Thread Erik Bråthen Solem
The 3GPP Timed Text (TTXT / tx3g / mov_text) specification counts multibyte 
UTF-8 characters as one single character, ffmpeg currently counts bytes. This 
produces files where style boxes have incorrect offsets. This patch introduces:
1. a separate variable that keeps track of the byte count
2. a for loop that excludes continuation bytes from the character counting

Fixes trac #6021 (encoding part).

---
 libavcodec/movtextenc.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/libavcodec/movtextenc.c b/libavcodec/movtextenc.c
index 20e01e2..8d09ff4 100644
--- a/libavcodec/movtextenc.c
+++ b/libavcodec/movtextenc.c
@@ -70,6 +70,7 @@ typedef struct {
 uint8_t style_fontsize;
 uint32_t style_color;
 uint16_t text_pos;
+uint16_t byte_size;
 } MovTextContext;
 
 typedef struct {
@@ -302,7 +303,10 @@ static void mov_text_text_cb(void *priv, const char *text, 
int len)
 {
 MovTextContext *s = priv;
 av_bprint_append_data(>buffer, text, len);
-s->text_pos += len;
+for (int i = 0; i < len; i++)
+if ((text[i] & 0xC0) != 0x80)
+s->text_pos++; /* increase character count */
+s->byte_size += len; /* increase byte count */
 }
 
 static void mov_text_new_line_cb(void *priv, int forced)
@@ -310,6 +314,7 @@ static void mov_text_new_line_cb(void *priv, int forced)
 MovTextContext *s = priv;
 av_bprint_append_data(>buffer, "\n", 1);
 s->text_pos += 1;
+s->byte_size += 1;
 }
 
 static const ASSCodesCallbacks mov_text_callbacks = {
@@ -328,6 +333,7 @@ static int mov_text_encode_frame(AVCodecContext *avctx, 
unsigned char *buf,
 size_t j;
 
 s->text_pos = 0;
+s->byte_size = 0;
 s->count = 0;
 s->box_flags = 0;
 s->style_entries = 0;
@@ -362,7 +368,7 @@ static int mov_text_encode_frame(AVCodecContext *avctx, 
unsigned char *buf,
 }
 }
 
-AV_WB16(buf, s->text_pos);
+AV_WB16(buf, s->byte_size);
 buf += 2;
 
 if (!av_bprint_is_complete(>buffer)) {
-- 
1.9.5 (Apple Git-50.3)

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH V2] vf_hwupload: Add missing return value check

2017-03-07 Thread Jun Zhao
ping ?

On 2017/3/3 9:35, Jun Zhao wrote:
> V2: Fix the potential memory leak.2
> 
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] libavcodec/psd : add support for rgb/gray float

2017-03-07 Thread Dzung Hoang
I implemented a 32-bit float GBRAP/GBRP pixel format and used it in vf_zscale.c 
and vf_overlay.c. I also implemented an EXR encoder using miniexr that uses the 
new float pixel format. I can contribute this if there is interest.
I used the above mods to do HDR video processing that includes overlaying HDR 
and SDR video. IMHO, for ffmpeg to handle HDR video well, float and half-float 
are needed and also metadata to signal the color space and transfer function 
for each picture. It's a big job that can touch a lot of code, which is why I 
kept my changes private and limited to only what I need.


  From: Martin Vignali 
 To: FFmpeg development discussions and patches  
 Sent: Tuesday, March 7, 2017 4:16 PM
 Subject: Re: [FFmpeg-devel] libavcodec/psd : add support for rgb/gray float
   
Hello

@Carl :
I make some tests in order to avoid temp buffer, but i have some strange
result, when i not use a temp buffer.


> Time to think about a float pixfmt? I find it weird that we support
> excessively obscure pixfmts like AV_PIX_FMT_BGR4_BYTE, but then have
> hacks like this one if the codec uses float data.
>
> (I'm just commenting, not arguing for/against your actual patch.)
>
I agree with your comment.
Float pixfmt is a more clean way.
it can also be useful for image processing with filters, and encoding float
data.

Any tips to add support for float pixfmts ?
I try to take a look to the source code, but it's not very easy to
understand.

Martin
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


   
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH] avutil/tests/lfg.c: added proper normality test

2017-03-07 Thread Thomas Turner
The Chen-Shapiro(CS) test was used to test normality for
Lagged Fibonacci PRNG.

Normality Hypothesis Test:

The null hypothesis formally tests if the population
the sample represents is normally-distributed. For
CS, when the normality hypothesis is True, the
distribution of QH will have a mean close to 1.

Information on CS can be found here:

http://www.stata-journal.com/sjpdf.html?articlenum=st0264
http://www.originlab.com/doc/Origin-Help/NormalityTest-Algorithm

Signed-off-by: Thomas Turner 
---
 libavutil/tests/lfg.c|  164 +++-
 tests/fate/libavutil.mak |4 +
 tests/ref/fate/lfg   | 1007 ++
 3 files changed, 1153 insertions(+), 22 deletions(-)
 create mode 100644 tests/ref/fate/lfg

diff --git a/libavutil/tests/lfg.c b/libavutil/tests/lfg.c
index 1425e02..ad3b491 100644
--- a/libavutil/tests/lfg.c
+++ b/libavutil/tests/lfg.c
@@ -20,6 +20,85 @@
 #include "libavutil/timer.h"
 #include "libavutil/lfg.h"
 
+static const double Z_TABLE[31][10] = {
+{0.5000,  0.5040,  0.5080,  0.5120,  0.5160,  0.5199,  0.5239,  0.5279,  
0.5319,  0.5359},
+{0.5398,  0.5438,  0.5478,  0.5517,  0.5557,  0.5596,  0.5636,  0.5675,  
0.5714,  0.5753},
+{0.5793,  0.5832,  0.5871,  0.5910,  0.5948,  0.5987,  0.6026,  0.6064,  
0.6103,  0.6141},
+{0.6179,  0.6217,  0.6255,  0.6293,  0.6331,  0.6368,  0.6406,  0.6443,  
0.6480,  0.6517},
+{0.6554,  0.6591,  0.6628,  0.6664,  0.6700,  0.6736,  0.6772,  0.6808,  
0.6844,  0.6879},
+{0.6915,  0.6950,  0.6985,  0.7019,  0.7054,  0.7088,  0.7123,  0.7157,  
0.7190,  0.7224},
+{0.7257,  0.7291,  0.7324,  0.7357,  0.7389,  0.7422,  0.7454,  0.7486,  
0.7517,  0.7549},
+{0.7580,  0.7611,  0.7642,  0.7673,  0.7704,  0.7734,  0.7764,  0.7794,  
0.7823,  0.7852},
+{0.7881,  0.7910,  0.7939,  0.7967,  0.7995,  0.8023,  0.8051,  0.8078,  
0.8106,  0.8133},
+{0.8159,  0.8186,  0.8212,  0.8238,  0.8264,  0.8289,  0.8315,  0.8340,  
0.8365,  0.8389},
+{0.8413,  0.8438,  0.8461,  0.8485,  0.8508,  0.8531,  0.8554,  0.8577,  
0.8599,  0.8621},
+{0.8643,  0.8665,  0.8686,  0.8708,  0.8729,  0.8749,  0.8770,  0.8790,  
0.8810,  0.8830},
+{0.8849,  0.8869,  0.,  0.8907,  0.8925,  0.8944,  0.8962,  0.8980,  
0.8997,  0.9015},
+{0.9032,  0.9049,  0.9066,  0.9082,  0.9099,  0.9115,  0.9131,  0.9147,  
0.9162,  0.9177},
+{0.9192,  0.9207,  0.9222,  0.9236,  0.9251,  0.9265,  0.9279,  0.9292,  
0.9306,  0.9319},
+{0.9332,  0.9345,  0.9357,  0.9370,  0.9382,  0.9394,  0.9406,  0.9418,  
0.9429,  0.9441},
+{0.9452,  0.9463,  0.9474,  0.9484,  0.9495,  0.9505,  0.9515,  0.9525,  
0.9535,  0.9545},
+{0.9554,  0.9564,  0.9573,  0.9582,  0.9591,  0.9599,  0.9608,  0.9616,  
0.9625,  0.9633},
+{0.9641,  0.9649,  0.9656,  0.9664,  0.9671,  0.9678,  0.9686,  0.9693,  
0.9699,  0.9706},
+{0.9713,  0.9719,  0.9726,  0.9732,  0.9738,  0.9744,  0.9750,  0.9756,  
0.9761,  0.9767},
+{0.9772,  0.9778,  0.9783,  0.9788,  0.9793,  0.9798,  0.9803,  0.9808,  
0.9812,  0.9817},
+{0.9821,  0.9826,  0.9830,  0.9834,  0.9838,  0.9842,  0.9846,  0.9850,  
0.9854,  0.9857},
+{0.9861,  0.9864,  0.9868,  0.9871,  0.9875,  0.9878,  0.9881,  0.9884,  
0.9887,  0.9890},
+{0.9893,  0.9896,  0.9898,  0.9901,  0.9904,  0.9906,  0.9909,  0.9911,  
0.9913,  0.9916},
+{0.9918,  0.9920,  0.9922,  0.9925,  0.9927,  0.9929,  0.9931,  0.9932,  
0.9934,  0.9936},
+{0.9938,  0.9940,  0.9941,  0.9943,  0.9945,  0.9946,  0.9948,  0.9949,  
0.9951,  0.9952},
+{0.9953,  0.9955,  0.9956,  0.9957,  0.9959,  0.9960,  0.9961,  0.9962,  
0.9963,  0.9964},
+{0.9965,  0.9966,  0.9967,  0.9968,  0.9969,  0.9970,  0.9971,  0.9972,  
0.9973,  0.9974},
+{0.9974,  0.9975,  0.9976,  0.9977,  0.9977,  0.9978,  0.9979,  0.9979,  
0.9980,  0.9981},
+{0.9981,  0.9982,  0.9982,  0.9983,  0.9984,  0.9984,  0.9985,  0.9985,  
0.9986,  0.9986},
+{0.9987,  0.9987,  0.9987,  0.9988,  0.9988,  0.9989,  0.9989,  0.9989,  
0.9990,  0.9990} };
+
+// Inverse cumulative distribution function
+static double inv_cdf(double u)
+{
+const double a[4] = { 2.50662823884,
+ -18.61500062529,
+  41.39119773534,
+ -25.44106049637};
+
+const double b[4] = {-8.47351093090,
+  23.08336743743,
+ -21.06224101826,
+   3.13082909833};
+
+const double c[9] = {0.3374754822726147,
+  0.9761690190917186,
+  0.1607979714918209,
+  0.0276438810333863,
+  0.0038405729373609,
+  0.0003951896511919,
+  0.321767881768,
+  0.002888167364,
+  0.003960315187};
+
+double r;
+double x = u - 0.5;
+
+// Beasley-Springer
+ if (fabs(x) < 

[FFmpeg-devel] fate: Do not report side data size

2017-03-07 Thread Vittorio Giovara
This should address the mismatch between different archs
Please CC.
-- 
Vittorio


0001-fate-Do-not-report-side-data-size.patch
Description: Binary data
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 1/2] libx264: remove the "x264opts" AVOption

2017-03-07 Thread Jan Ekstrom
On Wed, Mar 8, 2017 at 1:01 AM, Michael Niedermayer
 wrote:
> the fast pskip i mentioned yesterday
>
> ./ffmpeg -y -i ~/videos/matrixbench_mpeg2.mpg -vcodec libx264 -x264-params 
> fast-pskip -vframes 15 -an ref-p.nut
> ./ffmpeg -y -i ~/videos/matrixbench_mpeg2.mpg -vcodec libx264 -x264-params 
> no-fast-pskip -vframes 15 -an ref-np.nut
> ./ffmpeg -y -i ~/videos/matrixbench_mpeg2.mpg -vcodec libx264 -x264opts 
> no-fast-pskip -vframes 15 -an ref-no.nut
> ./ffmpeg -y -i ~/videos/matrixbench_mpeg2.mpg -vcodec libx264 -x264opts 
> fast-pskip -vframes 15 -an ref-o.nut
>
> ffd2c735cfb268d5f4291c2f6baab386  ref-o.nut
> ffd2c735cfb268d5f4291c2f6baab386  ref-p.nut
> f424075a964018aa83f98f2bf5ab2830  ref-no.nut
> ffd2c735cfb268d5f4291c2f6baab386  ref-np.nu
>
> it appears *fast-pskip has no effect in x264-params

Right, as I noted in my previous message, the custom dictionary
parsing code in there sets the value to "1" if a value is not
available. The API always expects a key and a value (even if a NULL
value would be valid for a boolean option).
So to emulate what x264opts is doing you would just set "fast-pskip=1"
or "no-fast-pskip=1". You can by this guess that fast-pskip is indeed
the default behavior during medium preset in libx264.

In my personal opinion I'm not sure this is worth it to have a Yet
Another Dictionary Parser inside libx264.c, esp. since
av_dict_parse_string() behavior is already the defining one in
multiple components where key/value lists are handled (you can see our
Doxygen for some sort of list of usage:
https://www.ffmpeg.org/doxygen/trunk/group__lavu__dict.html#gaca5ff7c251e60bd13164d13c82f21b79
).

>
> i have no idea if thats the only issue
>

Unless the very short code around it, or av_dict_parse_string is
globally bugged in some way, most likely all "issues" with x264-params
are due to the implicit value being set for key-value pairs without a
value.

> also to keep both x264-params and x264opts working you need just
> 1 line more than if you drop one. Its just one entry in the AVOption
> array to pass both to the same implementation
>

That is another alternative and at the very least would remove the
duplicated implementation which would be a positive step forward. For
some reason I would prefer only having a single option for this so
that there would be no questions regarding why the behavior changed
for x264opts with regards to implicit values.


Best regards,
Jan Ekström
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] matroskadec: cosmetics: Rearrange checks for projection-depedendent properties

2017-03-07 Thread James Almer
On 3/7/2017 7:35 PM, Vittorio Giovara wrote:
> ---
> ... and this chunk for matroska too.
> Vittorio
> 
>  libavformat/matroskadec.c | 21 +
>  1 file changed, 9 insertions(+), 12 deletions(-)
> 
> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
> index fdc3f268aa..fdb23ab05e 100644
> --- a/libavformat/matroskadec.c
> +++ b/libavformat/matroskadec.c
> @@ -1930,9 +1930,7 @@ static int mkv_parse_video_projection(AVStream *st, 
> const MatroskaTrack *track)
>  
>  switch (track->video.projection.type) {
>  case MATROSKA_VIDEO_PROJECTION_TYPE_EQUIRECTANGULAR:
> -if (track->video.projection.private.size == 0)
> -projection = AV_SPHERICAL_EQUIRECTANGULAR;
> -else if (track->video.projection.private.size == 20) {
> +if (track->video.projection.private.size == 20) {
>  t = bytestream2_get_be32();
>  b = bytestream2_get_be32();
>  l = bytestream2_get_be32();
> @@ -1946,15 +1944,15 @@ static int mkv_parse_video_projection(AVStream *st, 
> const MatroskaTrack *track)
> l, t, r, b);
>  return AVERROR_INVALIDDATA;
>  }
> -
> -if (l || t || r || b)
> -projection = AV_SPHERICAL_EQUIRECTANGULAR_TILE;
> -else
> -projection = AV_SPHERICAL_EQUIRECTANGULAR;
> -} else {
> +} else if (track->video.projection.private.size != 0) {
>  av_log(NULL, AV_LOG_ERROR, "Unknown spherical metadata\n");
>  return AVERROR_INVALIDDATA;
>  }
> +
> +if (l || t || r || b)
> +projection = AV_SPHERICAL_EQUIRECTANGULAR_TILE;
> +else
> +projection = AV_SPHERICAL_EQUIRECTANGULAR;
>  break;
>  case MATROSKA_VIDEO_PROJECTION_TYPE_CUBEMAP:
>  if (track->video.projection.private.size < 4) {
> @@ -1962,13 +1960,12 @@ static int mkv_parse_video_projection(AVStream *st, 
> const MatroskaTrack *track)
>  return AVERROR_INVALIDDATA;
>  } else if (track->video.projection.private.size == 12) {
>  uint32_t layout = bytestream2_get_be32();
> -if (layout == 0) {
> -projection = AV_SPHERICAL_CUBEMAP;
> -} else {
> +if (layout) {
>  av_log(NULL, AV_LOG_WARNING,
> "Unknown spherical cubemap layout %"PRIu32"\n", 
> layout);
>  return 0;
>  }
> +projection = AV_SPHERICAL_CUBEMAP;
>  padding = bytestream2_get_be32();
>  } else {
>  av_log(NULL, AV_LOG_ERROR, "Unknown spherical metadata\n");
> 

LGTM

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] mov: Fix checking layout and loading padding for cubemaps

2017-03-07 Thread James Almer
On 3/7/2017 7:31 PM, Vittorio Giovara wrote:
> ---
> I missed this chunk of review in this version of the set,
> sorry, here is the proper patch.
> Vittorio
> 
>  libavformat/mov.c | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index cc098cd977..d5c3949050 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -4635,7 +4635,7 @@ static int mov_read_sv3d(MOVContext *c, AVIOContext 
> *pb, MOVAtom atom)
>  {
>  AVStream *st;
>  MOVStreamContext *sc;
> -int size;
> +int size, layout;
>  int32_t yaw, pitch, roll;
>  size_t l = 0, t = 0, r = 0, b = 0;
>  size_t padding = 0;
> @@ -4699,6 +4699,12 @@ static int mov_read_sv3d(MOVContext *c, AVIOContext 
> *pb, MOVAtom atom)
>  avio_skip(pb, 4); /*  version + flags */
>  switch (tag) {
>  case MKTAG('c','b','m','p'):
> +layout = avio_rb32(pb);

Nit: Should be uint32_t, but for a != 0 check i guess int
is sufficient.

LGTM regardless of the above.

> +if (layout) {
> +av_log(c->fc, AV_LOG_WARNING,
> +   "Unsupported cubemap layout %d\n", layout);
> +return 0;
> +}
>  projection = AV_SPHERICAL_CUBEMAP;
>  padding = avio_rb32(pb);
>  break;
> 

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 1/2] avcodec/vp568: Check that there is enough data for ff_vp56_init_range_decoder()

2017-03-07 Thread Michael Niedermayer
On Tue, Mar 07, 2017 at 07:09:38PM +0100, Michael Niedermayer wrote:
> Fixes: timeout in 730/clusterfuzz-testcase-5265113739165696 (part 1 of 2)
> 
> Found-by: continuous fuzzing process 
> https://github.com/google/oss-fuzz/tree/master/targets/ffmpeg
> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/vp5.c |  5 -
>  libavcodec/vp56.h|  2 +-
>  libavcodec/vp56rac.c |  5 -
>  libavcodec/vp6.c | 15 +++
>  libavcodec/vp8.c | 21 ++---
>  libavcodec/vp9.c |  9 +++--
>  6 files changed, 41 insertions(+), 16 deletions(-)

patchset was approved by ronald and
applied

thx

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Good people do not need laws to tell them to act responsibly, while bad
people will find a way around the laws. -- Plato


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 1/2] libx264: remove the "x264opts" AVOption

2017-03-07 Thread Michael Niedermayer
On Tue, Mar 07, 2017 at 10:08:59AM -0900, Lou Logan wrote:
> On Tue, 7 Mar 2017 04:24:23 +0100, Michael Niedermayer wrote:
> 
> > its a long time ago but i faintly remember that the option you are
> > about to remove worked while the one remaining had bugs
> 
> Can you provide a reproducible case? I will try as well.

the fast pskip i mentioned yesterday

./ffmpeg -y -i ~/videos/matrixbench_mpeg2.mpg -vcodec libx264 -x264-params 
fast-pskip -vframes 15 -an ref-p.nut
./ffmpeg -y -i ~/videos/matrixbench_mpeg2.mpg -vcodec libx264 -x264-params 
no-fast-pskip -vframes 15 -an ref-np.nut
./ffmpeg -y -i ~/videos/matrixbench_mpeg2.mpg -vcodec libx264 -x264opts 
no-fast-pskip -vframes 15 -an ref-no.nut
./ffmpeg -y -i ~/videos/matrixbench_mpeg2.mpg -vcodec libx264 -x264opts 
fast-pskip -vframes 15 -an ref-o.nut

ffd2c735cfb268d5f4291c2f6baab386  ref-o.nut
ffd2c735cfb268d5f4291c2f6baab386  ref-p.nut
f424075a964018aa83f98f2bf5ab2830  ref-no.nut
ffd2c735cfb268d5f4291c2f6baab386  ref-np.nu

it appears *fast-pskip has no effect in x264-params

i have no idea if thats the only issue

also to keep both x264-params and x264opts working you need just
1 line more than if you drop one. Its just one entry in the AVOption
array to pass both to the same implementation

> 
> > also this would break scripts
> 
> I've never liked this argument. Scripts could have been broken when
> we removed libfaac, libstagefright, libquvi, etc but I don't recall any
> users complaining.

> Scripts can be fixed: we shouldn't be beholden to
> alleged, random, moldy, old "scripts" in the ether, otherwise we'll

scripts as in examples all over the net, mailing list acrhives
blogs and so on. they can be updated but not by the people using
them and in many places also not by the authors who wrote them.

also testcase we have on trac can break when things get removed


> never clean up anything. If users want to write-it-and-forget-it they
> can just use a release branch.

cleanup is good and i support it, and theres a lot of code we have
that would benefit from cleanup.


> 
> There is much inertia facing attempts to tidy up the code, and I
> believe the strong expectation of encountering this often prevents
> people from even trying.

There is inertia facing removing API, removing features,
breaking users of FFmpeg, ...
cleanup is much more than that

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Asymptotically faster algorithms should always be preferred if you have
asymptotical amounts of data


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH] mov: Fix checking layout and loading padding for cubemaps

2017-03-07 Thread Vittorio Giovara
---
I missed this chunk of review in this version of the set,
sorry, here is the proper patch.
Vittorio

 libavformat/mov.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/libavformat/mov.c b/libavformat/mov.c
index cc098cd977..d5c3949050 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -4635,7 +4635,7 @@ static int mov_read_sv3d(MOVContext *c, AVIOContext *pb, 
MOVAtom atom)
 {
 AVStream *st;
 MOVStreamContext *sc;
-int size;
+int size, layout;
 int32_t yaw, pitch, roll;
 size_t l = 0, t = 0, r = 0, b = 0;
 size_t padding = 0;
@@ -4699,6 +4699,12 @@ static int mov_read_sv3d(MOVContext *c, AVIOContext *pb, 
MOVAtom atom)
 avio_skip(pb, 4); /*  version + flags */
 switch (tag) {
 case MKTAG('c','b','m','p'):
+layout = avio_rb32(pb);
+if (layout) {
+av_log(c->fc, AV_LOG_WARNING,
+   "Unsupported cubemap layout %d\n", layout);
+return 0;
+}
 projection = AV_SPHERICAL_CUBEMAP;
 padding = avio_rb32(pb);
 break;
-- 
2.12.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH] matroskadec: cosmetics: Rearrange checks for projection-depedendent properties

2017-03-07 Thread Vittorio Giovara
---
... and this chunk for matroska too.
Vittorio

 libavformat/matroskadec.c | 21 +
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index fdc3f268aa..fdb23ab05e 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -1930,9 +1930,7 @@ static int mkv_parse_video_projection(AVStream *st, const 
MatroskaTrack *track)
 
 switch (track->video.projection.type) {
 case MATROSKA_VIDEO_PROJECTION_TYPE_EQUIRECTANGULAR:
-if (track->video.projection.private.size == 0)
-projection = AV_SPHERICAL_EQUIRECTANGULAR;
-else if (track->video.projection.private.size == 20) {
+if (track->video.projection.private.size == 20) {
 t = bytestream2_get_be32();
 b = bytestream2_get_be32();
 l = bytestream2_get_be32();
@@ -1946,15 +1944,15 @@ static int mkv_parse_video_projection(AVStream *st, 
const MatroskaTrack *track)
l, t, r, b);
 return AVERROR_INVALIDDATA;
 }
-
-if (l || t || r || b)
-projection = AV_SPHERICAL_EQUIRECTANGULAR_TILE;
-else
-projection = AV_SPHERICAL_EQUIRECTANGULAR;
-} else {
+} else if (track->video.projection.private.size != 0) {
 av_log(NULL, AV_LOG_ERROR, "Unknown spherical metadata\n");
 return AVERROR_INVALIDDATA;
 }
+
+if (l || t || r || b)
+projection = AV_SPHERICAL_EQUIRECTANGULAR_TILE;
+else
+projection = AV_SPHERICAL_EQUIRECTANGULAR;
 break;
 case MATROSKA_VIDEO_PROJECTION_TYPE_CUBEMAP:
 if (track->video.projection.private.size < 4) {
@@ -1962,13 +1960,12 @@ static int mkv_parse_video_projection(AVStream *st, 
const MatroskaTrack *track)
 return AVERROR_INVALIDDATA;
 } else if (track->video.projection.private.size == 12) {
 uint32_t layout = bytestream2_get_be32();
-if (layout == 0) {
-projection = AV_SPHERICAL_CUBEMAP;
-} else {
+if (layout) {
 av_log(NULL, AV_LOG_WARNING,
"Unknown spherical cubemap layout %"PRIu32"\n", layout);
 return 0;
 }
+projection = AV_SPHERICAL_CUBEMAP;
 padding = bytestream2_get_be32();
 } else {
 av_log(NULL, AV_LOG_ERROR, "Unknown spherical metadata\n");
-- 
2.12.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] libavcodec/psd : add support for rgb/gray float

2017-03-07 Thread Martin Vignali
Hello

@Carl :
I make some tests in order to avoid temp buffer, but i have some strange
result, when i not use a temp buffer.


> Time to think about a float pixfmt? I find it weird that we support
> excessively obscure pixfmts like AV_PIX_FMT_BGR4_BYTE, but then have
> hacks like this one if the codec uses float data.
>
> (I'm just commenting, not arguing for/against your actual patch.)
>
I agree with your comment.
Float pixfmt is a more clean way.
it can also be useful for image processing with filters, and encoding float
data.

Any tips to add support for float pixfmts ?
I try to take a look to the source code, but it's not very easy to
understand.

Martin
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] libavcodec/exr : add support for uint32

2017-03-07 Thread Martin Vignali
Hello,

In attach patch for decoding uint32 channels in exr files.

Following previous comments, i doesn't use float/color transformation for
this kind of pixel data
in this new version.

Comments welcome

Martin Vignali
Jokyo Images


0001-libavcodec-exr-add-support-for-uint32.patch
Description: Binary data
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCHv3 3/3] mkv: Export bounds and padding from spherical metadata

2017-03-07 Thread James Almer
On 2/28/2017 3:06 PM, Vittorio Giovara wrote:
> On Tue, Feb 28, 2017 at 12:46 PM, Vittorio Giovara
>  wrote:
>>> 
>>> I think this'll look better as
>>>
>>>
>>> case MATROSKA_VIDEO_PROJECTION_TYPE_EQUIRECTANGULAR:
>>> projection = AV_SPHERICAL_EQUIRECTANGULAR;
>>>
>>> if (track->video.projection.private.size == 20) {
>>> [...]
>>> if (l || t || r || b)
>>> projection = AV_SPHERICAL_EQUIRECTANGULAR_TILE;
>>> } else if (track->video.projection.private.size != 0) {
>>> // return error
>>> }
>>
>> Sorry, I don't follow, what is your suggestion?
> 
> nevermind, i get it, and ok
> 
  case MATROSKA_VIDEO_PROJECTION_TYPE_CUBEMAP:
 -if (track->video.projection.private.size < 4)
 +if (track->video.projection.private.size < 4) {
 +av_log(NULL, AV_LOG_ERROR, "Missing projection private 
 properties\n");
 +return AVERROR_INVALIDDATA;
 +} else if (track->video.projection.private.size == 12) {
 +uint32_t layout = bytestream2_get_be32();
 +if (layout == 0) {
 +projection = AV_SPHERICAL_CUBEMAP;
 +} else {
 +av_log(NULL, AV_LOG_WARNING,
 +   "Unknown spherical cubemap layout %"PRIu32"\n", 
 layout);
 +return 0;
 +}
 +padding = bytestream2_get_be32();
>>>
>>> Nit: Maybe
>>>
>>>if (layout) {
>>>// return error
>>>}
>>>projection = AV_SPHERICAL_CUBEMAP;
>>>padding= bytestream2_get_be32();
> 
> ok sure

You pushed these two chunks without any of the cosmetic changes i
suggested. You did apply them on libav, though. Do you mind doing it
here as well, or should i?

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCHv3 2/3] mov: Export bounds and padding from spherical metadata

2017-03-07 Thread James Almer
On 2/28/2017 1:24 PM, Vittorio Giovara wrote:
> On Tue, Feb 28, 2017 at 10:58 AM, James Almer  wrote:
>> On 2/21/2017 8:07 PM, James Almer wrote:
>>> On 2/21/2017 7:35 PM, Vittorio Giovara wrote:
 Update the fate test as needed.
 ---
  libavformat/mov.c | 28 +++-
  tests/ref/fate/mov-spherical-mono |  6 +-
  2 files changed, 32 insertions(+), 2 deletions(-)

 diff --git a/libavformat/mov.c b/libavformat/mov.c
 index 7b0bbcc..d798336 100644
 --- a/libavformat/mov.c
 +++ b/libavformat/mov.c
 @@ -4637,6 +4637,8 @@ static int mov_read_sv3d(MOVContext *c, AVIOContext 
 *pb, MOVAtom atom)
  MOVStreamContext *sc;
  int size, version;
  int32_t yaw, pitch, roll;
 +size_t l, t, r, b;
 +size_t padding = 0;
  uint32_t tag;
  enum AVSphericalProjection projection;

 @@ -4716,9 +4718,25 @@ static int mov_read_sv3d(MOVContext *c, AVIOContext 
 *pb, MOVAtom atom)
  switch (tag) {
  case MKTAG('c','b','m','p'):
  projection = AV_SPHERICAL_CUBEMAP;
 +padding = avio_rb32(pb);
>>>
>>> Doesn't layout come first?
> 
> Ah, that's right, thanks

You pushed this patch without fixing it...

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCHv3 3/3] mkv: Export bounds and padding from spherical metadata

2017-03-07 Thread James Almer
On 3/7/2017 5:38 PM, Hendrik Leppkes wrote:
> On Tue, Mar 7, 2017 at 7:26 PM, wm4  wrote:
>> On Tue, 21 Feb 2017 17:35:53 -0500
>> Vittorio Giovara  wrote:
>>
>>> ---
>>>  libavformat/matroskadec.c  | 64 
>>> --
>>>  tests/ref/fate/matroska-spherical-mono |  6 +++-
>>>  2 files changed, 66 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
>>> index 7223e94..0a02237 100644
>>> --- a/libavformat/matroskadec.c
>>> +++ b/libavformat/matroskadec.c
>>> @@ -1913,16 +1913,65 @@ static int mkv_parse_video_projection(AVStream *st, 
>>> const MatroskaTrack *track)
>>>  AVSphericalMapping *spherical;
>>>  enum AVSphericalProjection projection;
>>>  size_t spherical_size;
>>> +size_t l, t, r, b;
>>> +size_t padding = 0;
>>>  int ret;
>>> +GetByteContext gb;
>>> +
>>> +bytestream2_init(, track->video.projection.private.data,
>>> + track->video.projection.private.size);
>>> +
>>> +if (bytestream2_get_byte() != 0) {
>>> +av_log(NULL, AV_LOG_WARNING, "Unknown spherical metadata\n");
>>> +return 0;
>>> +}
>>> +
>>> +bytestream2_skip(, 3); // flags
>>>
>>>  switch (track->video.projection.type) {
>>>  case MATROSKA_VIDEO_PROJECTION_TYPE_EQUIRECTANGULAR:
>>> -projection = AV_SPHERICAL_EQUIRECTANGULAR;
>>> +if (track->video.projection.private.size == 0)
>>> +projection = AV_SPHERICAL_EQUIRECTANGULAR;
>>> +else if (track->video.projection.private.size == 20) {
>>> +t = bytestream2_get_be32();
>>> +b = bytestream2_get_be32();
>>> +l = bytestream2_get_be32();
>>> +r = bytestream2_get_be32();
>>> +
>>> +if (b >= UINT_MAX - t || r >= UINT_MAX - l) {
>>> +av_log(NULL, AV_LOG_ERROR,
>>> +   "Invalid bounding rectangle coordinates "
>>> +   "%zu,%zu,%zu,%zu\n", l, t, r, b);
>>> +return AVERROR_INVALIDDATA;
>>> +}
>>> +
>>> +if (l || t || r || b)
>>> +projection = AV_SPHERICAL_EQUIRECTANGULAR_TILE;
>>> +else
>>> +projection = AV_SPHERICAL_EQUIRECTANGULAR;
>>> +} else {
>>> +av_log(NULL, AV_LOG_ERROR, "Unknown spherical metadata\n");
>>> +return AVERROR_INVALIDDATA;
>>> +}
>>>  break;
>>>  case MATROSKA_VIDEO_PROJECTION_TYPE_CUBEMAP:
>>> -if (track->video.projection.private.size < 4)
>>> +if (track->video.projection.private.size < 4) {
>>> +av_log(NULL, AV_LOG_ERROR, "Missing projection private 
>>> properties\n");
>>> +return AVERROR_INVALIDDATA;
>>> +} else if (track->video.projection.private.size == 12) {
>>> +uint32_t layout = bytestream2_get_be32();
>>> +if (layout == 0) {
>>> +projection = AV_SPHERICAL_CUBEMAP;
>>> +} else {
>>> +av_log(NULL, AV_LOG_WARNING,
>>> +   "Unknown spherical cubemap layout %"PRIu32"\n", 
>>> layout);
>>> +return 0;
>>> +}
>>> +padding = bytestream2_get_be32();
>>> +} else {
>>> +av_log(NULL, AV_LOG_ERROR, "Unknown spherical metadata\n");
>>>  return AVERROR_INVALIDDATA;
>>> -projection = AV_SPHERICAL_CUBEMAP;
>>> +}
>>>  break;
>>>  default:
>>>  return 0;
>>> @@ -1937,6 +1986,15 @@ static int mkv_parse_video_projection(AVStream *st, 
>>> const MatroskaTrack *track)
>>>  spherical->pitch = (int32_t)(track->video.projection.pitch * (1 << 
>>> 16));
>>>  spherical->roll  = (int32_t)(track->video.projection.roll  * (1 << 
>>> 16));
>>>
>>> +spherical->padding = padding;
>>> +
>>> +if (spherical->projection == AV_SPHERICAL_EQUIRECTANGULAR_TILE) {
>>> +spherical->bound_left   = l;
>>> +spherical->bound_top= t;
>>> +spherical->bound_right  = r;
>>> +spherical->bound_bottom = b;
>>> +}
>>> +
>>>  ret = av_stream_add_side_data(st, AV_PKT_DATA_SPHERICAL, (uint8_t 
>>> *)spherical,
>>>spherical_size);
>>>  if (ret < 0) {
>>> diff --git a/tests/ref/fate/matroska-spherical-mono 
>>> b/tests/ref/fate/matroska-spherical-mono
>>> index 8048aff..a70d879 100644
>>> --- a/tests/ref/fate/matroska-spherical-mono
>>> +++ b/tests/ref/fate/matroska-spherical-mono
>>> @@ -8,7 +8,11 @@ inverted=0
>>>  [SIDE_DATA]
>>>  side_data_type=Spherical Mapping
>>>  side_data_size=56
>>> -projection=equirectangular
>>> +projection=tiled equirectangular
>>> +bound_left=148
>>> +bound_top=73
>>> +bound_right=147
>>> +bound_bottom=72
>>>  yaw=45
>>>  pitch=30
>>>  roll=15
>>
>> Is it just me, or did this break FATE?
> 
> It broke on some systems because it  prints the side_data_size, 

Re: [FFmpeg-devel] [PATCHv3 3/3] mkv: Export bounds and padding from spherical metadata

2017-03-07 Thread Hendrik Leppkes
On Tue, Mar 7, 2017 at 7:26 PM, wm4  wrote:
> On Tue, 21 Feb 2017 17:35:53 -0500
> Vittorio Giovara  wrote:
>
>> ---
>>  libavformat/matroskadec.c  | 64 
>> --
>>  tests/ref/fate/matroska-spherical-mono |  6 +++-
>>  2 files changed, 66 insertions(+), 4 deletions(-)
>>
>> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
>> index 7223e94..0a02237 100644
>> --- a/libavformat/matroskadec.c
>> +++ b/libavformat/matroskadec.c
>> @@ -1913,16 +1913,65 @@ static int mkv_parse_video_projection(AVStream *st, 
>> const MatroskaTrack *track)
>>  AVSphericalMapping *spherical;
>>  enum AVSphericalProjection projection;
>>  size_t spherical_size;
>> +size_t l, t, r, b;
>> +size_t padding = 0;
>>  int ret;
>> +GetByteContext gb;
>> +
>> +bytestream2_init(, track->video.projection.private.data,
>> + track->video.projection.private.size);
>> +
>> +if (bytestream2_get_byte() != 0) {
>> +av_log(NULL, AV_LOG_WARNING, "Unknown spherical metadata\n");
>> +return 0;
>> +}
>> +
>> +bytestream2_skip(, 3); // flags
>>
>>  switch (track->video.projection.type) {
>>  case MATROSKA_VIDEO_PROJECTION_TYPE_EQUIRECTANGULAR:
>> -projection = AV_SPHERICAL_EQUIRECTANGULAR;
>> +if (track->video.projection.private.size == 0)
>> +projection = AV_SPHERICAL_EQUIRECTANGULAR;
>> +else if (track->video.projection.private.size == 20) {
>> +t = bytestream2_get_be32();
>> +b = bytestream2_get_be32();
>> +l = bytestream2_get_be32();
>> +r = bytestream2_get_be32();
>> +
>> +if (b >= UINT_MAX - t || r >= UINT_MAX - l) {
>> +av_log(NULL, AV_LOG_ERROR,
>> +   "Invalid bounding rectangle coordinates "
>> +   "%zu,%zu,%zu,%zu\n", l, t, r, b);
>> +return AVERROR_INVALIDDATA;
>> +}
>> +
>> +if (l || t || r || b)
>> +projection = AV_SPHERICAL_EQUIRECTANGULAR_TILE;
>> +else
>> +projection = AV_SPHERICAL_EQUIRECTANGULAR;
>> +} else {
>> +av_log(NULL, AV_LOG_ERROR, "Unknown spherical metadata\n");
>> +return AVERROR_INVALIDDATA;
>> +}
>>  break;
>>  case MATROSKA_VIDEO_PROJECTION_TYPE_CUBEMAP:
>> -if (track->video.projection.private.size < 4)
>> +if (track->video.projection.private.size < 4) {
>> +av_log(NULL, AV_LOG_ERROR, "Missing projection private 
>> properties\n");
>> +return AVERROR_INVALIDDATA;
>> +} else if (track->video.projection.private.size == 12) {
>> +uint32_t layout = bytestream2_get_be32();
>> +if (layout == 0) {
>> +projection = AV_SPHERICAL_CUBEMAP;
>> +} else {
>> +av_log(NULL, AV_LOG_WARNING,
>> +   "Unknown spherical cubemap layout %"PRIu32"\n", 
>> layout);
>> +return 0;
>> +}
>> +padding = bytestream2_get_be32();
>> +} else {
>> +av_log(NULL, AV_LOG_ERROR, "Unknown spherical metadata\n");
>>  return AVERROR_INVALIDDATA;
>> -projection = AV_SPHERICAL_CUBEMAP;
>> +}
>>  break;
>>  default:
>>  return 0;
>> @@ -1937,6 +1986,15 @@ static int mkv_parse_video_projection(AVStream *st, 
>> const MatroskaTrack *track)
>>  spherical->pitch = (int32_t)(track->video.projection.pitch * (1 << 16));
>>  spherical->roll  = (int32_t)(track->video.projection.roll  * (1 << 16));
>>
>> +spherical->padding = padding;
>> +
>> +if (spherical->projection == AV_SPHERICAL_EQUIRECTANGULAR_TILE) {
>> +spherical->bound_left   = l;
>> +spherical->bound_top= t;
>> +spherical->bound_right  = r;
>> +spherical->bound_bottom = b;
>> +}
>> +
>>  ret = av_stream_add_side_data(st, AV_PKT_DATA_SPHERICAL, (uint8_t 
>> *)spherical,
>>spherical_size);
>>  if (ret < 0) {
>> diff --git a/tests/ref/fate/matroska-spherical-mono 
>> b/tests/ref/fate/matroska-spherical-mono
>> index 8048aff..a70d879 100644
>> --- a/tests/ref/fate/matroska-spherical-mono
>> +++ b/tests/ref/fate/matroska-spherical-mono
>> @@ -8,7 +8,11 @@ inverted=0
>>  [SIDE_DATA]
>>  side_data_type=Spherical Mapping
>>  side_data_size=56
>> -projection=equirectangular
>> +projection=tiled equirectangular
>> +bound_left=148
>> +bound_top=73
>> +bound_right=147
>> +bound_bottom=72
>>  yaw=45
>>  pitch=30
>>  roll=15
>
> Is it just me, or did this break FATE?

It broke on some systems because it  prints the side_data_size, which
varies on some compilers/systems since the struct uses size_t
We could probably just remove side_data_size, its not very informative
and as seen here can even be compiler/platform 

Re: [FFmpeg-devel] [PATCH 1/2] libx264: remove the "x264opts" AVOption

2017-03-07 Thread Jan Ekstrom
On Tue, Mar 7, 2017 at 5:24 AM, Michael Niedermayer
 wrote:
> On Tue, Mar 07, 2017 at 12:20:20AM +0200, Jan Ekström wrote:
>> x264-params does the same thing and this leaves us with a single
>> option that is name-matched with x265-params available in the
>> libx265 wrapper.
>> ---
>>  libavcodec/libx264.c | 29 -
>>  1 file changed, 29 deletions(-)
>
> please wait with this
>

The patch set has [RFC] in its topic as I knew any sort of removal is
going to be a multi-step process. As I noted, I wasn't sure of the
process so I noted that in my cover letter.

> ...
> its a long time ago but i faintly remember that the option you are
> about to remove worked while the one remaining had bugs

The x264opt code seems to:

* Use custom dict parsing instead of av_dict_parse_string
* Try real hard to set you a "1" even if you set something else:

>  char param[256]={0}, val[256]={0};
>  if(sscanf(p, "%255[^:=]=%255[^:]", param, val) == 1){
>  OPT_STR(param, "1");
>  }else
>  OPT_STR(param, val);

* The OPT_STR macro tries to catch X264_PARAM_BAD_NAME, while
x264-params just outputs "Error parsing option '%s = %s'.\n" in all
failure cases of x264_param_parse.

The x264-params code seems to:

* Use av_dict_parse_string/av_dict_get

>  av_dict_parse_string(, x4->x264_params, "=", ":", 0)
>  en = av_dict_get(dict, "", en, AV_DICT_IGNORE_SUFFIX)

* Just pass key=value one by one into x264_param_parse

>  x264_param_parse(>params, en->key, en->value)


Personally I prefer the latter as it utilizes generic tools we have in
our libraries and seems to keep the idea closer to KISS. Catching
X264_PARAM_BAD_NAME might be a good addition to x264-params, but not
sure how much more helpful that would be in the end.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 1/2] libx264: remove the "x264opts" AVOption

2017-03-07 Thread Lou Logan
On Tue, 7 Mar 2017 04:24:23 +0100, Michael Niedermayer wrote:

> its a long time ago but i faintly remember that the option you are
> about to remove worked while the one remaining had bugs

Can you provide a reproducible case? I will try as well.

> also this would break scripts

I've never liked this argument. Scripts could have been broken when
we removed libfaac, libstagefright, libquvi, etc but I don't recall any
users complaining. Scripts can be fixed: we shouldn't be beholden to
alleged, random, moldy, old "scripts" in the ether, otherwise we'll
never clean up anything. If users want to write-it-and-forget-it they
can just use a release branch.

There is much inertia facing attempts to tidy up the code, and I
believe the strong expectation of encountering this often prevents
people from even trying.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCHv3 3/3] mkv: Export bounds and padding from spherical metadata

2017-03-07 Thread wm4
On Tue, 21 Feb 2017 17:35:53 -0500
Vittorio Giovara  wrote:

> ---
>  libavformat/matroskadec.c  | 64 
> --
>  tests/ref/fate/matroska-spherical-mono |  6 +++-
>  2 files changed, 66 insertions(+), 4 deletions(-)
> 
> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
> index 7223e94..0a02237 100644
> --- a/libavformat/matroskadec.c
> +++ b/libavformat/matroskadec.c
> @@ -1913,16 +1913,65 @@ static int mkv_parse_video_projection(AVStream *st, 
> const MatroskaTrack *track)
>  AVSphericalMapping *spherical;
>  enum AVSphericalProjection projection;
>  size_t spherical_size;
> +size_t l, t, r, b;
> +size_t padding = 0;
>  int ret;
> +GetByteContext gb;
> +
> +bytestream2_init(, track->video.projection.private.data,
> + track->video.projection.private.size);
> +
> +if (bytestream2_get_byte() != 0) {
> +av_log(NULL, AV_LOG_WARNING, "Unknown spherical metadata\n");
> +return 0;
> +}
> +
> +bytestream2_skip(, 3); // flags
>  
>  switch (track->video.projection.type) {
>  case MATROSKA_VIDEO_PROJECTION_TYPE_EQUIRECTANGULAR:
> -projection = AV_SPHERICAL_EQUIRECTANGULAR;
> +if (track->video.projection.private.size == 0)
> +projection = AV_SPHERICAL_EQUIRECTANGULAR;
> +else if (track->video.projection.private.size == 20) {
> +t = bytestream2_get_be32();
> +b = bytestream2_get_be32();
> +l = bytestream2_get_be32();
> +r = bytestream2_get_be32();
> +
> +if (b >= UINT_MAX - t || r >= UINT_MAX - l) {
> +av_log(NULL, AV_LOG_ERROR,
> +   "Invalid bounding rectangle coordinates "
> +   "%zu,%zu,%zu,%zu\n", l, t, r, b);
> +return AVERROR_INVALIDDATA;
> +}
> +
> +if (l || t || r || b)
> +projection = AV_SPHERICAL_EQUIRECTANGULAR_TILE;
> +else
> +projection = AV_SPHERICAL_EQUIRECTANGULAR;
> +} else {
> +av_log(NULL, AV_LOG_ERROR, "Unknown spherical metadata\n");
> +return AVERROR_INVALIDDATA;
> +}
>  break;
>  case MATROSKA_VIDEO_PROJECTION_TYPE_CUBEMAP:
> -if (track->video.projection.private.size < 4)
> +if (track->video.projection.private.size < 4) {
> +av_log(NULL, AV_LOG_ERROR, "Missing projection private 
> properties\n");
> +return AVERROR_INVALIDDATA;
> +} else if (track->video.projection.private.size == 12) {
> +uint32_t layout = bytestream2_get_be32();
> +if (layout == 0) {
> +projection = AV_SPHERICAL_CUBEMAP;
> +} else {
> +av_log(NULL, AV_LOG_WARNING,
> +   "Unknown spherical cubemap layout %"PRIu32"\n", 
> layout);
> +return 0;
> +}
> +padding = bytestream2_get_be32();
> +} else {
> +av_log(NULL, AV_LOG_ERROR, "Unknown spherical metadata\n");
>  return AVERROR_INVALIDDATA;
> -projection = AV_SPHERICAL_CUBEMAP;
> +}
>  break;
>  default:
>  return 0;
> @@ -1937,6 +1986,15 @@ static int mkv_parse_video_projection(AVStream *st, 
> const MatroskaTrack *track)
>  spherical->pitch = (int32_t)(track->video.projection.pitch * (1 << 16));
>  spherical->roll  = (int32_t)(track->video.projection.roll  * (1 << 16));
>  
> +spherical->padding = padding;
> +
> +if (spherical->projection == AV_SPHERICAL_EQUIRECTANGULAR_TILE) {
> +spherical->bound_left   = l;
> +spherical->bound_top= t;
> +spherical->bound_right  = r;
> +spherical->bound_bottom = b;
> +}
> +
>  ret = av_stream_add_side_data(st, AV_PKT_DATA_SPHERICAL, (uint8_t 
> *)spherical,
>spherical_size);
>  if (ret < 0) {
> diff --git a/tests/ref/fate/matroska-spherical-mono 
> b/tests/ref/fate/matroska-spherical-mono
> index 8048aff..a70d879 100644
> --- a/tests/ref/fate/matroska-spherical-mono
> +++ b/tests/ref/fate/matroska-spherical-mono
> @@ -8,7 +8,11 @@ inverted=0
>  [SIDE_DATA]
>  side_data_type=Spherical Mapping
>  side_data_size=56
> -projection=equirectangular
> +projection=tiled equirectangular
> +bound_left=148
> +bound_top=73
> +bound_right=147
> +bound_bottom=72
>  yaw=45
>  pitch=30
>  roll=15

Is it just me, or did this break FATE?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH 2/2] avcodec/vp8: Check for the bitstream end per MB in decode_mb_row_no_filter()

2017-03-07 Thread Michael Niedermayer
Fixes: timeout in 730/clusterfuzz-testcase-5265113739165696 (part 2 of 2)

Found-by: continuous fuzzing process 
https://github.com/google/oss-fuzz/tree/master/targets/ffmpeg
Signed-off-by: Michael Niedermayer 
---
 libavcodec/vp8.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/libavcodec/vp8.c b/libavcodec/vp8.c
index a3d057d62e..6759b310f0 100644
--- a/libavcodec/vp8.c
+++ b/libavcodec/vp8.c
@@ -2330,6 +2330,8 @@ static av_always_inline int 
decode_mb_row_no_filter(AVCodecContext *avctx, void
 s->mv_max.x = ((s->mb_width - 1) << 6) + MARGIN;
 
 for (mb_x = 0; mb_x < s->mb_width; mb_x++, mb_xy++, mb++) {
+if (c->end <= c->buffer && c->bits >= 0)
+return AVERROR_INVALIDDATA;
 // Wait for previous thread to read mb_x+2, and reach mb_y-1.
 if (prev_td != td) {
 if (threadnr != 0) {
-- 
2.11.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH 1/2] avcodec/vp568: Check that there is enough data for ff_vp56_init_range_decoder()

2017-03-07 Thread Michael Niedermayer
Fixes: timeout in 730/clusterfuzz-testcase-5265113739165696 (part 1 of 2)

Found-by: continuous fuzzing process 
https://github.com/google/oss-fuzz/tree/master/targets/ffmpeg
Signed-off-by: Michael Niedermayer 
---
 libavcodec/vp5.c |  5 -
 libavcodec/vp56.h|  2 +-
 libavcodec/vp56rac.c |  5 -
 libavcodec/vp6.c | 15 +++
 libavcodec/vp8.c | 21 ++---
 libavcodec/vp9.c |  9 +++--
 6 files changed, 41 insertions(+), 16 deletions(-)

diff --git a/libavcodec/vp5.c b/libavcodec/vp5.c
index 54db620bde..b5f06a0940 100644
--- a/libavcodec/vp5.c
+++ b/libavcodec/vp5.c
@@ -38,8 +38,11 @@ static int vp5_parse_header(VP56Context *s, const uint8_t 
*buf, int buf_size)
 {
 VP56RangeCoder *c = >c;
 int rows, cols;
+int ret;
 
-ff_vp56_init_range_decoder(>c, buf, buf_size);
+ret = ff_vp56_init_range_decoder(>c, buf, buf_size);
+if (ret < 0)
+return ret;
 s->frames[VP56_FRAME_CURRENT]->key_frame = !vp56_rac_get(c);
 vp56_rac_get(c);
 ff_vp56_init_dequant(s, vp56_rac_gets(c, 6));
diff --git a/libavcodec/vp56.h b/libavcodec/vp56.h
index e5c5bea963..c049399df8 100644
--- a/libavcodec/vp56.h
+++ b/libavcodec/vp56.h
@@ -224,7 +224,7 @@ int ff_vp56_decode_frame(AVCodecContext *avctx, void *data, 
int *got_frame,
  */
 
 extern const uint8_t ff_vp56_norm_shift[256];
-void ff_vp56_init_range_decoder(VP56RangeCoder *c, const uint8_t *buf, int 
buf_size);
+int ff_vp56_init_range_decoder(VP56RangeCoder *c, const uint8_t *buf, int 
buf_size);
 
 static av_always_inline unsigned int vp56_rac_renorm(VP56RangeCoder *c)
 {
diff --git a/libavcodec/vp56rac.c b/libavcodec/vp56rac.c
index 6061b7ee72..e70302bf85 100644
--- a/libavcodec/vp56rac.c
+++ b/libavcodec/vp56rac.c
@@ -37,11 +37,14 @@ const uint8_t ff_vp56_norm_shift[256]= {
  0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,
 };
 
-void ff_vp56_init_range_decoder(VP56RangeCoder *c, const uint8_t *buf, int 
buf_size)
+int ff_vp56_init_range_decoder(VP56RangeCoder *c, const uint8_t *buf, int 
buf_size)
 {
 c->high = 255;
 c->bits = -16;
 c->buffer = buf;
 c->end = buf + buf_size;
+if (buf_size < 1)
+return AVERROR_INVALIDDATA;
 c->code_word = bytestream_get_be24(>buffer);
+return 0;
 }
diff --git a/libavcodec/vp6.c b/libavcodec/vp6.c
index 662126ca70..f0e60a3822 100644
--- a/libavcodec/vp6.c
+++ b/libavcodec/vp6.c
@@ -52,6 +52,7 @@ static int vp6_parse_header(VP56Context *s, const uint8_t 
*buf, int buf_size)
 int sub_version;
 int rows, cols;
 int res = 0;
+int ret;
 int separated_coeff = buf[0] & 1;
 
 s->frames[VP56_FRAME_CURRENT]->key_frame = !(buf[0] & 0x80);
@@ -93,7 +94,7 @@ static int vp6_parse_header(VP56Context *s, const uint8_t 
*buf, int buf_size)
 s->avctx->coded_width  = 16 * cols;
 s->avctx->coded_height = 16 * rows;
 } else {
-int ret = ff_set_dimensions(s->avctx, 16 * cols, 16 * rows);
+ret = ff_set_dimensions(s->avctx, 16 * cols, 16 * rows);
 if (ret < 0)
 return ret;
 
@@ -105,7 +106,9 @@ static int vp6_parse_header(VP56Context *s, const uint8_t 
*buf, int buf_size)
 res = VP56_SIZE_CHANGE;
 }
 
-ff_vp56_init_range_decoder(c, buf+6, buf_size-6);
+ret = ff_vp56_init_range_decoder(c, buf+6, buf_size-6);
+if (ret < 0)
+return ret;
 vp56_rac_gets(c, 2);
 
 parse_filter_info = s->filter_header;
@@ -122,7 +125,9 @@ static int vp6_parse_header(VP56Context *s, const uint8_t 
*buf, int buf_size)
 buf += 2;
 buf_size -= 2;
 }
-ff_vp56_init_range_decoder(c, buf+1, buf_size-1);
+ret = ff_vp56_init_range_decoder(c, buf+1, buf_size-1);
+if (ret < 0)
+return ret;
 
 s->golden_frame = vp56_rac_get(c);
 if (s->filter_header) {
@@ -165,7 +170,9 @@ static int vp6_parse_header(VP56Context *s, const uint8_t 
*buf, int buf_size)
 s->parse_coeff = vp6_parse_coeff_huffman;
 init_get_bits(>gb, buf, buf_size<<3);
 } else {
-ff_vp56_init_range_decoder(>cc, buf, buf_size);
+ret = ff_vp56_init_range_decoder(>cc, buf, buf_size);
+if (ret < 0)
+return ret;
 s->ccp = >cc;
 }
 } else {
diff --git a/libavcodec/vp8.c b/libavcodec/vp8.c
index fb17ff114d..a3d057d62e 100644
--- a/libavcodec/vp8.c
+++ b/libavcodec/vp8.c
@@ -261,6 +261,7 @@ static int setup_partitions(VP8Context *s, const uint8_t 
*buf, int buf_size)
 {
 const uint8_t *sizes = buf;
 int i;
+int ret;
 
 s->num_coeff_partitions = 1 << vp8_rac_get_uint(>c, 2);
 
@@ -274,13 +275,13 @@ static int setup_partitions(VP8Context *s, const uint8_t 
*buf, int buf_size)
 if (buf_size - size < 0)
 return -1;
 
-

Re: [FFmpeg-devel] [PATCH] configure: convert MSVC ident to utf-8 if possible

2017-03-07 Thread Timo Rothenpieler

Am 01.03.2017 um 13:28 schrieb Timo Rothenpieler:

---
 configure | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index 0199fec..398d530 100755
--- a/configure
+++ b/configure
@@ -4095,7 +4095,11 @@ probe_cc(){
 disable stripping
 elif $_cc -nologo- 2>&1 | grep -q Microsoft; then
 _type=msvc
-_ident=$($_cc 2>&1 | head -n1)
+if command -v iconv >/dev/null 2>&1; then
+_ident=$($_cc 2>&1 | head -n1 | iconv -sc -f CP850 -t UTF-8)
+else
+_ident=$($_cc 2>&1 | head -n1)
+fi
 _DEPCMD='$(DEP$(1)) $(DEP$(1)FLAGS) $($(1)DEP_FLAGS) $< 2>&1 | awk '\''/including/ { sub(/^.*file: */, 
""); gsub(/\\/, "/"); if (!match($$0, / /)) print "$@:", $$0 }'\'' > $(@:.o=.d)'
 _DEPFLAGS='$(CPPFLAGS) $(CFLAGS) -showIncludes -Zs'
 _cflags_speed="-O2"



ping

Don't want to push this without at least a second pair of eyes to verify 
I didn't make any stupid mistakes or oversights regarding using iconv.

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 4/4] avcodec/vp8: remove redundant check

2017-03-07 Thread Michael Niedermayer
On Tue, Mar 07, 2017 at 09:20:09AM +0100, Paul B Mahol wrote:
> On 3/7/17, Michael Niedermayer  wrote:
> > Signed-off-by: Michael Niedermayer 
> > ---
> >  libavcodec/vp8.c | 2 --
> >  1 file changed, 2 deletions(-)
> >
> > diff --git a/libavcodec/vp8.c b/libavcodec/vp8.c
> > index 8039e7817e..a3d057d62e 100644
> > --- a/libavcodec/vp8.c
> > +++ b/libavcodec/vp8.c
> > @@ -2504,8 +2504,6 @@ int vp78_decode_mb_row_sliced(AVCodecContext *avctx,
> > void *tdata, int jobnr,
> >
> >  td->thread_nr = threadnr;
> >  for (mb_y = jobnr; mb_y < s->mb_height; mb_y += num_jobs) {
> > -if (mb_y >= s->mb_height)
> > -break;
> >  td->thread_mb_pos = mb_y << 16;
> >  ret = s->decode_mb_row_no_filter(avctx, tdata, jobnr, threadnr);
> >  if (ret < 0)
> > --
> > 2.11.0
> >
> > ___
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> 
> ok

applied
will be in my next push

thx

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If you fake or manipulate statistics in a paper in physics you will never
get a job again.
If you fake or manipulate statistics in a paper in medicin you will get
a job for life at the pharma industry.


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 3/4] avcodec/vp568: Check that there is enough data for ff_vp56_init_range_decoder()

2017-03-07 Thread Michael Niedermayer
On Tue, Mar 07, 2017 at 08:35:33AM -0500, Ronald S. Bultje wrote:
> Hi,
> 
> On Mon, Mar 6, 2017 at 8:41 PM, Michael Niedermayer 
> wrote:
> 
> > -void ff_vp56_init_range_decoder(VP56RangeCoder *c, const uint8_t *buf,
> > int buf_size)
> > +int ff_vp56_init_range_decoder(VP56RangeCoder *c, const uint8_t *buf,
> > int buf_size)
> >  {
> >  c->high = 255;
> >  c->bits = -16;
> >  c->buffer = buf;
> >  c->end = buf + buf_size;
> > +if (buf_size < 1)
> > +return AVERROR_INVALIDDATA;
> >  c->code_word = bytestream_get_be24(>buffer);
> > +return 0;
> >  }
> 
> 
> Isn't this AV_INPUT_BUFFER_PADDING_SIZE?

There was no out of array access, the issue was that the code spend
alot of time doing nothing as a result of 0 bytes input being turned
into 16bits of available data repeatly.

I intended to suggest to adjust the 16 to 8 on a 1 byte buf_size
subsequently

> 
> And this is inconsistent with the silent failure in renorm().

testing each use of renorm likely would cause a speed loss, such
a failure should be caught by the explicit end of bitstream checks
on the other loops

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I do not agree with what you have to say, but I'll defend to the death your
right to say it. -- Voltaire


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 1/4] avcodec/allcodecs: make avcodec_register_all thread safe

2017-03-07 Thread Michael Niedermayer
On Tue, Mar 07, 2017 at 03:04:14PM +0100, wm4 wrote:
> On Tue, 7 Mar 2017 14:37:27 +0100
> Michael Niedermayer  wrote:
> 
> > On Tue, Mar 07, 2017 at 07:44:43AM +0100, wm4 wrote:

[...]

> > The real problem behind all this is of course more complex and a
> > combination of many of our developers being rather aggressive and
> > having strong oppinions on code outside the area they actively maintain.
> 
> So devs who don't maintain a specific part of the code don't get to
> have a "strong" opinion? I guess only a weak opinion is wanted, at most?

Whats wanted is, what works. And with an increasing number of
developers with strong and different oppinions come increasing
conflicts. A possible solution would be strict rules on who has authority
over which part of the code or people being alot more tolerant than
they are. The end result either way would be that code like the
cinepack optimzations would be accepted but thats exactly what many
people dont want. And thats where the dilemma is and i have no awnser
or solution here. What i think though is
When people leave then "what was done" didnt work.
We can try to learn out of this or we can continue as we did previously.
And it all probably boils down to seeing other peoples needs and
accepting them vs. accepting to loose some of them.
But quite possibly there are solutions i fail to see ...


>
> If you argue this way, we'd have to put maintainership of central code
> under multiple developers or so.
> 
> > That drives some contributors away
> 
> Which ones?

> The Cinepak thing was clearly rejected, and the patch
> author couldn't deal with it. If that's all it takes to run away,

well, yes, but is that surprising?
Volunteers work in the environment where they are welcome and where
their contributions are welcome. If one community is rejecting a
sgnificant part or the work one cares about, moving to a different
community, field or area is the obvous option


> he probably wasn't meant to contribute to a big project with many
> developers anyway.

> (Funny how the Cinepak maintainer never even showed
> up - tells a lot about our MAINTAINERS file?)

would it help if he did ?
I think he couldnt have done much had he jumped into the discussion



[...]
> >
> > I would very much like to see some effort to add a real plugin
> > interface instead of the opposite direction
> 
> Go ahead and propose one? But if it forces too much private API to be
> exposed, then it'd make FFmpeg development harder, because we'd have to
> guarantee stability of those internal APIs until the next major bump,
> and it'd be near-useless for outside developers because they had to fix
> their plugins every major bump.
> 
> I think the thing that's wrong with FFmpeg API is that its APIs are not
> designed for long-time API and ABI stability. That would be a good
> place to start.
> 
> A realistic approach to supporting plugins would probably be to create
> a separate API, which translates the required calls to the internal
> API, but which is designed in a way that can be supported for a long
> time. Rather low-level (and low-feature) examples would include the
> ladspa and frei0r libavfilter wrappers.

Personally i would prefer internal codecs and plugins to use the same
API, but thats a detail. I am happy with any plugin API that works
and that people are happy with.

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I am the wisest man alive, for I know one thing, and that is that I know
nothing. -- Socrates


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avcodec/exr

2017-03-07 Thread Dzung Hoang
I uploaded rle_grayscale_noffset.exr using the videolan file uploader.
The rle_grayscale_nooffset.exr image was modified from 
https://trac.ffmpeg.org/raw-attachment/ticket/5621/rle_grayscale.exr by zeroing 
out the line offset table. The modified file is viewable with Adobe Photoshop 
but results in a blank image when processed by ffmpeg.
Let me know if you need additional test images.
Regards,- Dzung Hoang


  From: Paul B Mahol 
 To: dtho...@yahoo.com 
 Sent: Monday, March 6, 2017 11:37 AM
 Subject: Re: [FFmpeg-devel] [PATCH] avcodec/exr
   
On 3/6/17, Dzung Hoang  wrote:
> The EXR decoder cannot handle the image files included in NVidia's HDR
> SDK.
> After debugging, I found that the line offset table in the image files
> contained 0's. Other EXR decoders, including the official OpenEXR
> decoder,
> can detect and reconstruct missing line offset tables, so I added some
> code
> to do so.
> I filed a trac ticket for this issue here:
> https://trac.ffmpeg.org/ticket/6220.
>

 Do you have files with this issue?


   
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH v3 3/3] avcodec: estimate output bitrate for ffv1/huffyuv codecs

2017-03-07 Thread Tobias Rapp
Allows to get a more realistic total bitrate (and estimated file size)
in avi_write_header. Previously a static default value of 200k was
assumed.

Signed-off-by: Tobias Rapp 
---
 libavcodec/ffv1enc.c   | 3 +++
 libavcodec/huffyuvenc.c| 3 +++
 tests/ref/vsynth/vsynth1-ffv1  | 2 +-
 tests/ref/vsynth/vsynth1-ffv1-v0   | 2 +-
 tests/ref/vsynth/vsynth1-ffv1-v3-bgr0  | 2 +-
 tests/ref/vsynth/vsynth1-ffv1-v3-rgb48 | 2 +-
 tests/ref/vsynth/vsynth1-ffv1-v3-yuv420p   | 2 +-
 tests/ref/vsynth/vsynth1-ffv1-v3-yuv422p10 | 2 +-
 tests/ref/vsynth/vsynth1-ffv1-v3-yuv444p16 | 2 +-
 tests/ref/vsynth/vsynth1-ffvhuff   | 2 +-
 tests/ref/vsynth/vsynth1-ffvhuff420p12 | 2 +-
 tests/ref/vsynth/vsynth1-ffvhuff422p10left | 2 +-
 tests/ref/vsynth/vsynth1-ffvhuff444| 2 +-
 tests/ref/vsynth/vsynth1-ffvhuff444p16 | 2 +-
 tests/ref/vsynth/vsynth1-huffyuv   | 2 +-
 tests/ref/vsynth/vsynth1-huffyuvbgr24  | 2 +-
 tests/ref/vsynth/vsynth1-huffyuvbgra   | 2 +-
 tests/ref/vsynth/vsynth2-ffv1  | 2 +-
 tests/ref/vsynth/vsynth2-ffv1-v0   | 2 +-
 tests/ref/vsynth/vsynth2-ffv1-v3-bgr0  | 2 +-
 tests/ref/vsynth/vsynth2-ffv1-v3-rgb48 | 2 +-
 tests/ref/vsynth/vsynth2-ffv1-v3-yuv420p   | 2 +-
 tests/ref/vsynth/vsynth2-ffv1-v3-yuv422p10 | 2 +-
 tests/ref/vsynth/vsynth2-ffv1-v3-yuv444p16 | 2 +-
 tests/ref/vsynth/vsynth2-ffvhuff   | 2 +-
 tests/ref/vsynth/vsynth2-ffvhuff420p12 | 2 +-
 tests/ref/vsynth/vsynth2-ffvhuff422p10left | 2 +-
 tests/ref/vsynth/vsynth2-ffvhuff444| 2 +-
 tests/ref/vsynth/vsynth2-ffvhuff444p16 | 2 +-
 tests/ref/vsynth/vsynth2-huffyuv   | 2 +-
 tests/ref/vsynth/vsynth2-huffyuvbgr24  | 2 +-
 tests/ref/vsynth/vsynth2-huffyuvbgra   | 2 +-
 tests/ref/vsynth/vsynth3-ffv1  | 2 +-
 tests/ref/vsynth/vsynth3-ffv1-v0   | 2 +-
 tests/ref/vsynth/vsynth3-ffv1-v3-bgr0  | 2 +-
 tests/ref/vsynth/vsynth3-ffv1-v3-rgb48 | 2 +-
 tests/ref/vsynth/vsynth3-ffv1-v3-yuv420p   | 2 +-
 tests/ref/vsynth/vsynth3-ffv1-v3-yuv422p10 | 2 +-
 tests/ref/vsynth/vsynth3-ffv1-v3-yuv444p16 | 2 +-
 tests/ref/vsynth/vsynth3-ffvhuff   | 2 +-
 tests/ref/vsynth/vsynth3-ffvhuff420p12 | 2 +-
 tests/ref/vsynth/vsynth3-ffvhuff422p10left | 2 +-
 tests/ref/vsynth/vsynth3-ffvhuff444| 2 +-
 tests/ref/vsynth/vsynth3-ffvhuff444p16 | 2 +-
 tests/ref/vsynth/vsynth3-huffyuv   | 2 +-
 tests/ref/vsynth/vsynth3-huffyuvbgr24  | 2 +-
 tests/ref/vsynth/vsynth3-huffyuvbgra   | 2 +-
 tests/ref/vsynth/vsynth_lena-ffv1  | 2 +-
 tests/ref/vsynth/vsynth_lena-ffv1-v0   | 2 +-
 tests/ref/vsynth/vsynth_lena-ffv1-v3-bgr0  | 2 +-
 tests/ref/vsynth/vsynth_lena-ffv1-v3-rgb48 | 2 +-
 tests/ref/vsynth/vsynth_lena-ffv1-v3-yuv420p   | 2 +-
 tests/ref/vsynth/vsynth_lena-ffv1-v3-yuv422p10 | 2 +-
 tests/ref/vsynth/vsynth_lena-ffv1-v3-yuv444p16 | 2 +-
 tests/ref/vsynth/vsynth_lena-ffvhuff   | 2 +-
 tests/ref/vsynth/vsynth_lena-ffvhuff420p12 | 2 +-
 tests/ref/vsynth/vsynth_lena-ffvhuff422p10left | 2 +-
 tests/ref/vsynth/vsynth_lena-ffvhuff444| 2 +-
 tests/ref/vsynth/vsynth_lena-ffvhuff444p16 | 2 +-
 tests/ref/vsynth/vsynth_lena-huffyuv   | 2 +-
 tests/ref/vsynth/vsynth_lena-huffyuvbgr24  | 2 +-
 tests/ref/vsynth/vsynth_lena-huffyuvbgra   | 2 +-
 62 files changed, 66 insertions(+), 60 deletions(-)

diff --git a/libavcodec/ffv1enc.c b/libavcodec/ffv1enc.c
index 35f54c6..b57e8ba 100644
--- a/libavcodec/ffv1enc.c
+++ b/libavcodec/ffv1enc.c
@@ -894,6 +894,9 @@ slices_ok:
 }
 }
 
+// estimate compressed bitrate assuming 40% output size
+avctx->bit_rate = ff_guess_coded_bitrate(avctx) * 2 / 5;
+
 return 0;
 }
 
diff --git a/libavcodec/huffyuvenc.c b/libavcodec/huffyuvenc.c
index 89639b7..ddbaf14 100644
--- a/libavcodec/huffyuvenc.c
+++ b/libavcodec/huffyuvenc.c
@@ -446,6 +446,9 @@ FF_ENABLE_DEPRECATION_WARNINGS
 
 s->picture_number=0;
 
+// estimate compressed bitrate assuming 60% output size
+avctx->bit_rate = ff_guess_coded_bitrate(avctx) * 3 / 5;
+
 return 0;
 }
 static int encode_422_bitstream(HYuvContext *s, int offset, int count)
diff --git a/tests/ref/vsynth/vsynth1-ffv1 b/tests/ref/vsynth/vsynth1-ffv1
index 001f10a..64229db 100644
--- a/tests/ref/vsynth/vsynth1-ffv1
+++ b/tests/ref/vsynth/vsynth1-ffv1
@@ -1,4 +1,4 @@
-26b1296a0ef80a3b5c8b63cc57c52bc2 *tests/data/fate/vsynth1-ffv1.avi
+69e340b652a8f4327dd3206694038895 *tests/data/fate/vsynth1-ffv1.avi
 2691268 tests/data/fate/vsynth1-ffv1.avi
 c5ccac874dbf808e9088bc3107860042 *tests/data/fate/vsynth1-ffv1.out.rawvideo
 stddev:0.00 PSNR:999.99 MAXDIFF:0 bytes:  7603200/  7603200
diff --git 

[FFmpeg-devel] [PATCH v3 2/3] avcodec: estimate output bitrate for uncompressed video codecs

2017-03-07 Thread Tobias Rapp
Allows to get a more realistic total bitrate (and estimated file size)
in avi_write_header. Previously a static default value of 200k was
assumed.

Adds an internal helper function for bitrate guessing.

Signed-off-by: Tobias Rapp 
---
 libavcodec/internal.h|  6 ++
 libavcodec/r210enc.c | 15 +++
 libavcodec/rawenc.c  |  2 ++
 libavcodec/utils.c   | 21 +
 libavcodec/v210enc.c |  3 +++
 libavcodec/v308enc.c |  3 +++
 libavcodec/v408enc.c |  2 ++
 libavcodec/v410enc.c |  3 +++
 libavcodec/y41penc.c |  1 +
 tests/ref/fate/v410enc   |  2 +-
 tests/ref/vsynth/vsynth1-bpp1|  2 +-
 tests/ref/vsynth/vsynth1-bpp15   |  2 +-
 tests/ref/vsynth/vsynth1-r210|  2 +-
 tests/ref/vsynth/vsynth1-rgb |  2 +-
 tests/ref/vsynth/vsynth1-v210|  2 +-
 tests/ref/vsynth/vsynth1-v210-10 |  2 +-
 tests/ref/vsynth/vsynth1-v308|  2 +-
 tests/ref/vsynth/vsynth1-v408|  2 +-
 tests/ref/vsynth/vsynth1-y41p|  2 +-
 tests/ref/vsynth/vsynth1-yuv |  2 +-
 tests/ref/vsynth/vsynth2-bpp1|  2 +-
 tests/ref/vsynth/vsynth2-bpp15   |  2 +-
 tests/ref/vsynth/vsynth2-r210|  2 +-
 tests/ref/vsynth/vsynth2-rgb |  2 +-
 tests/ref/vsynth/vsynth2-v210|  2 +-
 tests/ref/vsynth/vsynth2-v210-10 |  2 +-
 tests/ref/vsynth/vsynth2-v308|  2 +-
 tests/ref/vsynth/vsynth2-v408|  2 +-
 tests/ref/vsynth/vsynth2-y41p|  2 +-
 tests/ref/vsynth/vsynth2-yuv |  2 +-
 tests/ref/vsynth/vsynth3-bpp1|  2 +-
 tests/ref/vsynth/vsynth3-bpp15   |  2 +-
 tests/ref/vsynth/vsynth3-r210|  2 +-
 tests/ref/vsynth/vsynth3-rgb |  2 +-
 tests/ref/vsynth/vsynth3-v210|  2 +-
 tests/ref/vsynth/vsynth3-v210-10 |  2 +-
 tests/ref/vsynth/vsynth3-v308|  2 +-
 tests/ref/vsynth/vsynth3-v408|  2 +-
 tests/ref/vsynth/vsynth3-yuv |  2 +-
 tests/ref/vsynth/vsynth_lena-bpp1|  2 +-
 tests/ref/vsynth/vsynth_lena-bpp15   |  2 +-
 tests/ref/vsynth/vsynth_lena-r210|  2 +-
 tests/ref/vsynth/vsynth_lena-rgb |  2 +-
 tests/ref/vsynth/vsynth_lena-v210|  2 +-
 tests/ref/vsynth/vsynth_lena-v210-10 |  2 +-
 tests/ref/vsynth/vsynth_lena-v308|  2 +-
 tests/ref/vsynth/vsynth_lena-v408|  2 +-
 tests/ref/vsynth/vsynth_lena-y41p|  2 +-
 tests/ref/vsynth/vsynth_lena-yuv |  2 +-
 49 files changed, 96 insertions(+), 40 deletions(-)

diff --git a/libavcodec/internal.h b/libavcodec/internal.h
index c92dba4..a63793b 100644
--- a/libavcodec/internal.h
+++ b/libavcodec/internal.h
@@ -361,4 +361,10 @@ int ff_side_data_set_encoder_stats(AVPacket *pkt, int 
quality, int64_t *error, i
 int ff_alloc_a53_sei(const AVFrame *frame, size_t prefix_len,
  void **data, size_t *sei_size);
 
+/**
+ * Get an estimated video bitrate based on frame size, frame rate and coded
+ * bits per pixel.
+ */
+int64_t ff_guess_coded_bitrate(AVCodecContext *avctx);
+
 #endif /* AVCODEC_INTERNAL_H */
diff --git a/libavcodec/r210enc.c b/libavcodec/r210enc.c
index 65b3c06..a55e543 100644
--- a/libavcodec/r210enc.c
+++ b/libavcodec/r210enc.c
@@ -24,6 +24,18 @@
 #include "internal.h"
 #include "bytestream.h"
 
+static av_cold int encode_init(AVCodecContext *avctx)
+{
+int aligned_width = FFALIGN(avctx->width,
+avctx->codec_id == AV_CODEC_ID_R10K ? 1 : 64);
+
+avctx->bits_per_coded_sample = 32;
+if (avctx->width > 0)
+avctx->bit_rate = ff_guess_coded_bitrate(avctx) * aligned_width / 
avctx->width;
+
+return 0;
+}
+
 static int encode_frame(AVCodecContext *avctx, AVPacket *pkt,
 const AVFrame *pic, int *got_packet)
 {
@@ -73,6 +85,7 @@ AVCodec ff_r210_encoder = {
 .long_name  = NULL_IF_CONFIG_SMALL("Uncompressed RGB 10-bit"),
 .type   = AVMEDIA_TYPE_VIDEO,
 .id = AV_CODEC_ID_R210,
+.init   = encode_init,
 .encode2= encode_frame,
 .pix_fmts   = (const enum AVPixelFormat[]) { AV_PIX_FMT_RGB48, 
AV_PIX_FMT_NONE },
 .capabilities   = AV_CODEC_CAP_INTRA_ONLY,
@@ -84,6 +97,7 @@ AVCodec ff_r10k_encoder = {
 .long_name  = NULL_IF_CONFIG_SMALL("AJA Kona 10-bit RGB Codec"),
 .type   = AVMEDIA_TYPE_VIDEO,
 .id = AV_CODEC_ID_R10K,
+.init   = encode_init,
 .encode2= encode_frame,
 .pix_fmts   = (const enum AVPixelFormat[]) { AV_PIX_FMT_RGB48, 
AV_PIX_FMT_NONE },
 .capabilities   = AV_CODEC_CAP_INTRA_ONLY,
@@ -95,6 +109,7 @@ AVCodec ff_avrp_encoder = {
 .long_name  = NULL_IF_CONFIG_SMALL("Avid 1:1 10-bit RGB Packer"),
 .type   = AVMEDIA_TYPE_VIDEO,
 .id = AV_CODEC_ID_AVRP,
+.init   = encode_init,
 .encode2= encode_frame,
 

[FFmpeg-devel] [PATCH v3 1/3] ffmpeg: set the encoding framerate when the output is CFR

2017-03-07 Thread Tobias Rapp
From: Anton Khirnov 

(cherry picked from Libav commit d10102d23c9467d4eb84f58e0cd12be284b982f6)

Signed-off-by: Tobias Rapp 
---
 ffmpeg.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/ffmpeg.c b/ffmpeg.c
index 79c91ff..4117b64 100644
--- a/ffmpeg.c
+++ b/ffmpeg.c
@@ -3309,6 +3309,8 @@ static int init_output_stream_encode(OutputStream *ost)
 enc_ctx->bits_per_raw_sample = FFMIN(dec_ctx->bits_per_raw_sample,
  
av_pix_fmt_desc_get(enc_ctx->pix_fmt)->comp[0].depth);
 
+enc_ctx->framerate = ost->frame_rate;
+
 ost->st->avg_frame_rate = ost->frame_rate;
 
 if (!dec_ctx ||
-- 
2.7.4


___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH v3 0/3] estimate output bitrate for some video codecs

2017-03-07 Thread Tobias Rapp
This patch-set tries to implement observations/suggestions from the discussions 
in

https://ffmpeg.org/pipermail/ffmpeg-devel/2017-January/205902.html
https://ffmpeg.org/pipermail/ffmpeg-devel/2017-January/206428.html

Changes since version 2:
 - replaced own patch with Libav commit for setting encoding framerate
 - avoid touching bit_rate_tolerance field
 - reorder patches and integrated helper function into (hopefully) less
   controversial uncompressed bitrate estimation patch

Anton Khirnov (1):
  ffmpeg: set the encoding framerate when the output is CFR

Tobias Rapp (2):
  avcodec: estimate output bitrate for uncompressed video codecs
  avcodec: estimate output bitrate for ffv1/huffyuv codecs

 ffmpeg.c   |  2 ++
 libavcodec/ffv1enc.c   |  3 +++
 libavcodec/huffyuvenc.c|  3 +++
 libavcodec/internal.h  |  6 ++
 libavcodec/r210enc.c   | 15 +++
 libavcodec/rawenc.c|  2 ++
 libavcodec/utils.c | 21 +
 libavcodec/v210enc.c   |  3 +++
 libavcodec/v308enc.c   |  3 +++
 libavcodec/v408enc.c   |  2 ++
 libavcodec/v410enc.c   |  3 +++
 libavcodec/y41penc.c   |  1 +
 tests/ref/fate/v410enc |  2 +-
 tests/ref/vsynth/vsynth1-bpp1  |  2 +-
 tests/ref/vsynth/vsynth1-bpp15 |  2 +-
 tests/ref/vsynth/vsynth1-ffv1  |  2 +-
 tests/ref/vsynth/vsynth1-ffv1-v0   |  2 +-
 tests/ref/vsynth/vsynth1-ffv1-v3-bgr0  |  2 +-
 tests/ref/vsynth/vsynth1-ffv1-v3-rgb48 |  2 +-
 tests/ref/vsynth/vsynth1-ffv1-v3-yuv420p   |  2 +-
 tests/ref/vsynth/vsynth1-ffv1-v3-yuv422p10 |  2 +-
 tests/ref/vsynth/vsynth1-ffv1-v3-yuv444p16 |  2 +-
 tests/ref/vsynth/vsynth1-ffvhuff   |  2 +-
 tests/ref/vsynth/vsynth1-ffvhuff420p12 |  2 +-
 tests/ref/vsynth/vsynth1-ffvhuff422p10left |  2 +-
 tests/ref/vsynth/vsynth1-ffvhuff444|  2 +-
 tests/ref/vsynth/vsynth1-ffvhuff444p16 |  2 +-
 tests/ref/vsynth/vsynth1-huffyuv   |  2 +-
 tests/ref/vsynth/vsynth1-huffyuvbgr24  |  2 +-
 tests/ref/vsynth/vsynth1-huffyuvbgra   |  2 +-
 tests/ref/vsynth/vsynth1-r210  |  2 +-
 tests/ref/vsynth/vsynth1-rgb   |  2 +-
 tests/ref/vsynth/vsynth1-v210  |  2 +-
 tests/ref/vsynth/vsynth1-v210-10   |  2 +-
 tests/ref/vsynth/vsynth1-v308  |  2 +-
 tests/ref/vsynth/vsynth1-v408  |  2 +-
 tests/ref/vsynth/vsynth1-y41p  |  2 +-
 tests/ref/vsynth/vsynth1-yuv   |  2 +-
 tests/ref/vsynth/vsynth2-bpp1  |  2 +-
 tests/ref/vsynth/vsynth2-bpp15 |  2 +-
 tests/ref/vsynth/vsynth2-ffv1  |  2 +-
 tests/ref/vsynth/vsynth2-ffv1-v0   |  2 +-
 tests/ref/vsynth/vsynth2-ffv1-v3-bgr0  |  2 +-
 tests/ref/vsynth/vsynth2-ffv1-v3-rgb48 |  2 +-
 tests/ref/vsynth/vsynth2-ffv1-v3-yuv420p   |  2 +-
 tests/ref/vsynth/vsynth2-ffv1-v3-yuv422p10 |  2 +-
 tests/ref/vsynth/vsynth2-ffv1-v3-yuv444p16 |  2 +-
 tests/ref/vsynth/vsynth2-ffvhuff   |  2 +-
 tests/ref/vsynth/vsynth2-ffvhuff420p12 |  2 +-
 tests/ref/vsynth/vsynth2-ffvhuff422p10left |  2 +-
 tests/ref/vsynth/vsynth2-ffvhuff444|  2 +-
 tests/ref/vsynth/vsynth2-ffvhuff444p16 |  2 +-
 tests/ref/vsynth/vsynth2-huffyuv   |  2 +-
 tests/ref/vsynth/vsynth2-huffyuvbgr24  |  2 +-
 tests/ref/vsynth/vsynth2-huffyuvbgra   |  2 +-
 tests/ref/vsynth/vsynth2-r210  |  2 +-
 tests/ref/vsynth/vsynth2-rgb   |  2 +-
 tests/ref/vsynth/vsynth2-v210  |  2 +-
 tests/ref/vsynth/vsynth2-v210-10   |  2 +-
 tests/ref/vsynth/vsynth2-v308  |  2 +-
 tests/ref/vsynth/vsynth2-v408  |  2 +-
 tests/ref/vsynth/vsynth2-y41p  |  2 +-
 tests/ref/vsynth/vsynth2-yuv   |  2 +-
 tests/ref/vsynth/vsynth3-bpp1  |  2 +-
 tests/ref/vsynth/vsynth3-bpp15 |  2 +-
 tests/ref/vsynth/vsynth3-ffv1  |  2 +-
 tests/ref/vsynth/vsynth3-ffv1-v0   |  2 +-
 tests/ref/vsynth/vsynth3-ffv1-v3-bgr0  |  2 +-
 tests/ref/vsynth/vsynth3-ffv1-v3-rgb48 |  2 +-
 tests/ref/vsynth/vsynth3-ffv1-v3-yuv420p   |  2 +-
 tests/ref/vsynth/vsynth3-ffv1-v3-yuv422p10 |  2 +-
 tests/ref/vsynth/vsynth3-ffv1-v3-yuv444p16 |  2 +-
 tests/ref/vsynth/vsynth3-ffvhuff   |  2 +-
 tests/ref/vsynth/vsynth3-ffvhuff420p12 |  2 +-
 tests/ref/vsynth/vsynth3-ffvhuff422p10left |  2 +-
 tests/ref/vsynth/vsynth3-ffvhuff444| 

Re: [FFmpeg-devel] [PATCH v2 2/3] avcodec: estimate output bitrate for ffv1/huffyuv codecs

2017-03-07 Thread Tobias Rapp

On 03.03.2017 16:14, Michael Niedermayer wrote:

On Mon, Feb 06, 2017 at 01:33:19PM +0100, Tobias Rapp wrote:

Allows to get a more realistic total bitrate (and estimated file size)
in avi_write_header. Previously a static default value of 200k was
assumed.

Signed-off-by: Tobias Rapp 
---
 [...]
diff --git a/libavcodec/ffv1enc.c b/libavcodec/ffv1enc.c
index 35f54c6..7138cc0 100644
--- a/libavcodec/ffv1enc.c
+++ b/libavcodec/ffv1enc.c
@@ -894,6 +894,10 @@ slices_ok:
 }
 }

+// estimate bitrate assuming 40% compression
+avctx->bit_rate = ff_guess_coded_bitrate(avctx) * 2 / 5;
+avctx->bit_rate_tolerance = avctx->bit_rate;
+
 return 0;
 }

diff --git a/libavcodec/huffyuvenc.c b/libavcodec/huffyuvenc.c
index 89639b7..8911408 100644
--- a/libavcodec/huffyuvenc.c
+++ b/libavcodec/huffyuvenc.c
@@ -446,6 +446,10 @@ FF_ENABLE_DEPRECATION_WARNINGS

 s->picture_number=0;

+// estimate bitrate assuming 60% compression
+avctx->bit_rate = ff_guess_coded_bitrate(avctx) * 3 / 5;
+avctx->bit_rate_tolerance = avctx->bit_rate;


these violate the API

See avcodec.h:
/**
 * number of bits the bitstream is allowed to diverge from the reference.
 *   the reference can be CBR (for CBR pass1) or VBR (for pass2)
 * - encoding: Set by user; unused for constant quantizer encoding.
 * - decoding: unused
 */
int bit_rate_tolerance;

also the compression rate depends on the material these numbers are
basically random


I agree that the bitrate can vary much depending on the type of video 
material (animation, film scanning, noise, etc) and my intent was to 
signal that with +/- bit_rate_tolerance. Anyway the 40%/60% compression 
numbers roughly match my experience with analogue ingest.


Will post version 3 of the patch-set without updates to bit_rate_tolerance.

Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH v2 1/4] avcodec/allcodecs: make avcodec_register_all thread safe

2017-03-07 Thread wm4
On Tue, 7 Mar 2017 11:09:19 -0300
James Almer  wrote:

> On 3/7/2017 11:05 AM, wm4 wrote:
> > On Tue, 7 Mar 2017 20:48:13 +0700
> > Muhammad Faiz  wrote:
> >   
> >> On Tue, Mar 7, 2017 at 4:17 PM, wm4  wrote:  
> >>> On Tue,  7 Mar 2017 16:01:58 +0700
> >>> Muhammad Faiz  wrote:
> >>>
>  use ff_thread_once
> 
>  Suggested-by: wm4 
>  Signed-off-by: Muhammad Faiz 
>  ---
>   libavcodec/allcodecs.c | 16 +---
>   1 file changed, 9 insertions(+), 7 deletions(-)
> 
>  diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
>  index eee322b..6ec7e79 100644
>  --- a/libavcodec/allcodecs.c
>  +++ b/libavcodec/allcodecs.c
>  @@ -25,6 +25,7 @@
>    */
> 
>   #include "config.h"
>  +#include "libavutil/thread.h"
>   #include "avcodec.h"
>   #include "version.h"
> 
>  @@ -58,14 +59,8 @@
>   av_register_codec_parser(_##x##_parser); 
>  \
>   }
> 
>  -void avcodec_register_all(void)
>  +static void register_all(void)
>   {
>  -static int initialized;
>  -
>  -if (initialized)
>  -return;
>  -initialized = 1;
>  -
>   /* hardware accelerators */
>   REGISTER_HWACCEL(H263_VAAPI,h263_vaapi);
>   REGISTER_HWACCEL(H263_VIDEOTOOLBOX, h263_videotoolbox);
>  @@ -717,3 +712,10 @@ void avcodec_register_all(void)
>   REGISTER_PARSER(VP9,vp9);
>   REGISTER_PARSER(XMA,xma);
>   }
>  +
>  +void avcodec_register_all(void)
>  +{
>  +static AVOnce control = AV_ONCE_INIT;
>  +
>  +ff_thread_once(, register_all);
>  +}
> >>>
> >>> Looks all good to me.
> >>>
> >>> Of course this doesn't change that the thread-safety of this whole
> >>> "list of components" code is still questionable, but it doesn't hurt to
> >>> do this either.
> >>
> >> Patchset applied.
> >>
> >> Thank's.  
> > 
> > I'm not the maintainer, though. Looks like you violated the development
> > rules. (Not that I blame you, it's just an observation.)  
> 
> There's no listed maintainer for all*.c files, so your review was enough
> for a simple patchset like this.

Hm, I guess with strict reading that's true... all is fine, then.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH v2 1/4] avcodec/allcodecs: make avcodec_register_all thread safe

2017-03-07 Thread James Almer
On 3/7/2017 11:05 AM, wm4 wrote:
> On Tue, 7 Mar 2017 20:48:13 +0700
> Muhammad Faiz  wrote:
> 
>> On Tue, Mar 7, 2017 at 4:17 PM, wm4  wrote:
>>> On Tue,  7 Mar 2017 16:01:58 +0700
>>> Muhammad Faiz  wrote:
>>>  
 use ff_thread_once

 Suggested-by: wm4 
 Signed-off-by: Muhammad Faiz 
 ---
  libavcodec/allcodecs.c | 16 +---
  1 file changed, 9 insertions(+), 7 deletions(-)

 diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
 index eee322b..6ec7e79 100644
 --- a/libavcodec/allcodecs.c
 +++ b/libavcodec/allcodecs.c
 @@ -25,6 +25,7 @@
   */

  #include "config.h"
 +#include "libavutil/thread.h"
  #include "avcodec.h"
  #include "version.h"

 @@ -58,14 +59,8 @@
  av_register_codec_parser(_##x##_parser); \
  }

 -void avcodec_register_all(void)
 +static void register_all(void)
  {
 -static int initialized;
 -
 -if (initialized)
 -return;
 -initialized = 1;
 -
  /* hardware accelerators */
  REGISTER_HWACCEL(H263_VAAPI,h263_vaapi);
  REGISTER_HWACCEL(H263_VIDEOTOOLBOX, h263_videotoolbox);
 @@ -717,3 +712,10 @@ void avcodec_register_all(void)
  REGISTER_PARSER(VP9,vp9);
  REGISTER_PARSER(XMA,xma);
  }
 +
 +void avcodec_register_all(void)
 +{
 +static AVOnce control = AV_ONCE_INIT;
 +
 +ff_thread_once(, register_all);
 +}  
>>>
>>> Looks all good to me.
>>>
>>> Of course this doesn't change that the thread-safety of this whole
>>> "list of components" code is still questionable, but it doesn't hurt to
>>> do this either.  
>>
>> Patchset applied.
>>
>> Thank's.
> 
> I'm not the maintainer, though. Looks like you violated the development
> rules. (Not that I blame you, it's just an observation.)

There's no listed maintainer for all*.c files, so your review was enough
for a simple patchset like this.

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH v2 1/4] avcodec/allcodecs: make avcodec_register_all thread safe

2017-03-07 Thread wm4
On Tue, 7 Mar 2017 20:48:13 +0700
Muhammad Faiz  wrote:

> On Tue, Mar 7, 2017 at 4:17 PM, wm4  wrote:
> > On Tue,  7 Mar 2017 16:01:58 +0700
> > Muhammad Faiz  wrote:
> >  
> >> use ff_thread_once
> >>
> >> Suggested-by: wm4 
> >> Signed-off-by: Muhammad Faiz 
> >> ---
> >>  libavcodec/allcodecs.c | 16 +---
> >>  1 file changed, 9 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
> >> index eee322b..6ec7e79 100644
> >> --- a/libavcodec/allcodecs.c
> >> +++ b/libavcodec/allcodecs.c
> >> @@ -25,6 +25,7 @@
> >>   */
> >>
> >>  #include "config.h"
> >> +#include "libavutil/thread.h"
> >>  #include "avcodec.h"
> >>  #include "version.h"
> >>
> >> @@ -58,14 +59,8 @@
> >>  av_register_codec_parser(_##x##_parser); \
> >>  }
> >>
> >> -void avcodec_register_all(void)
> >> +static void register_all(void)
> >>  {
> >> -static int initialized;
> >> -
> >> -if (initialized)
> >> -return;
> >> -initialized = 1;
> >> -
> >>  /* hardware accelerators */
> >>  REGISTER_HWACCEL(H263_VAAPI,h263_vaapi);
> >>  REGISTER_HWACCEL(H263_VIDEOTOOLBOX, h263_videotoolbox);
> >> @@ -717,3 +712,10 @@ void avcodec_register_all(void)
> >>  REGISTER_PARSER(VP9,vp9);
> >>  REGISTER_PARSER(XMA,xma);
> >>  }
> >> +
> >> +void avcodec_register_all(void)
> >> +{
> >> +static AVOnce control = AV_ONCE_INIT;
> >> +
> >> +ff_thread_once(, register_all);
> >> +}  
> >
> > Looks all good to me.
> >
> > Of course this doesn't change that the thread-safety of this whole
> > "list of components" code is still questionable, but it doesn't hurt to
> > do this either.  
> 
> Patchset applied.
> 
> Thank's.

I'm not the maintainer, though. Looks like you violated the development
rules. (Not that I blame you, it's just an observation.)
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 1/4] avcodec/allcodecs: make avcodec_register_all thread safe

2017-03-07 Thread wm4
On Tue, 7 Mar 2017 14:37:27 +0100
Michael Niedermayer  wrote:

> On Tue, Mar 07, 2017 at 07:44:43AM +0100, wm4 wrote:
> > On Tue,  7 Mar 2017 02:47:36 +0700
> > Muhammad Faiz  wrote:
> >   
> > > Signed-off-by: Muhammad Faiz 
> > > ---
> > >  libavcodec/allcodecs.c | 13 ++---
> > >  1 file changed, 10 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
> > > index eee322b..1d05fc1 100644
> > > --- a/libavcodec/allcodecs.c
> > > +++ b/libavcodec/allcodecs.c
> > > @@ -24,6 +24,8 @@
> > >   * Provide registration of all codecs, parsers and bitstream filters for 
> > > libavcodec.
> > >   */
> > >  
> > > +#include 
> > > +
> > >  #include "config.h"
> > >  #include "avcodec.h"
> > >  #include "version.h"
> > > @@ -60,11 +62,14 @@
> > >  
> > >  void avcodec_register_all(void)
> > >  {
> > > -static int initialized;
> > > +static atomic_int initialized;
> > > +static atomic_int ready;
> > >  
> > > -if (initialized)
> > > +if (atomic_exchange_explicit(, 1, memory_order_relaxed)) 
> > > {
> > > +while (!atomic_load_explicit(, memory_order_relaxed))
> > > +;
> > >  return;
> > > -initialized = 1;
> > > +}
> > >  
> > >  /* hardware accelerators */
> > >  REGISTER_HWACCEL(H263_VAAPI,h263_vaapi);
> > > @@ -716,4 +721,6 @@ void avcodec_register_all(void)
> > >  REGISTER_PARSER(VP8,vp8);
> > >  REGISTER_PARSER(VP9,vp9);
> > >  REGISTER_PARSER(XMA,xma);
> > > +
> > > +atomic_store_explicit(, 1, memory_order_relaxed);
> > >  }  
> >   
> 
> > avcodec_register() is already "safe" by attempting to do lock-free and
> > wait-free list insertion (not very convincing IMHO, but it's there).
> > Should that code be removed?  
> 
> that code is needed, otherwise avcodec_register() would not be thread
> safe

Sure, also list access wouldn't be synchronized in the first place.

> 
> > Does anyone know why avcodec_register() is
> > even still public API? The same affects some other things.  
> 
> It is public so that user written codecs can be registered

That's not possible. It's all private API, multitudes of complex,
unexposable private API.

> Ideally we would have a real plugin system and for example rune could
> maintain his cinepack decoder as a plugin in his github repo.

He can do that anyway, as a patch?

> Then he would likely not have left FFmpeg today.

> Removing avcodec_register() IMO is a step in the wrong direction as
> after that theres no way to register user written codecs and then the
> API also would no longer need to be unchanged and once the API changes
> frequently adding a real plugin interface becomes MUCH harder.

Wrong. It most likely would exist in a completely different form, and
API to support plugin registration could be added as needed. The
current avcodec_register() call is completely misleading, and implies
codecs can be added from the outside. But in fact, merely using this
function is an API usage error. Why is it public? It's an oxymoron.

> The real problem behind all this is of course more complex and a
> combination of many of our developers being rather aggressive and
> having strong oppinions on code outside the area they actively maintain.

So devs who don't maintain a specific part of the code don't get to
have a "strong" opinion? I guess only a weak opinion is wanted, at most?

If you argue this way, we'd have to put maintainership of central code
under multiple developers or so.

> That drives some contributors away

Which ones? The Cinepak thing was clearly rejected, and the patch
author couldn't deal with it. If that's all it takes to run away,
he probably wasn't meant to contribute to a big project with many
developers anyway. (Funny how the Cinepak maintainer never even showed
up - tells a lot about our MAINTAINERS file?)

> and now the lack of a plugin
> interface means they are driven over the ledge and a lost as
> contributors to FFmpeg.

FFmpeg never had a plugin interface.

I'm not even sure why you combine multiple completely separate issues in
your argumentation.

> 
> I would very much like to see some effort to add a real plugin
> interface instead of the opposite direction

Go ahead and propose one? But if it forces too much private API to be
exposed, then it'd make FFmpeg development harder, because we'd have to
guarantee stability of those internal APIs until the next major bump,
and it'd be near-useless for outside developers because they had to fix
their plugins every major bump.

I think the thing that's wrong with FFmpeg API is that its APIs are not
designed for long-time API and ABI stability. That would be a good
place to start.

A realistic approach to supporting plugins would probably be to create
a separate API, which translates the required calls to the internal
API, but which is designed in a way that 

Re: [FFmpeg-devel] [PATCH v2 1/4] avcodec/allcodecs: make avcodec_register_all thread safe

2017-03-07 Thread Muhammad Faiz
On Tue, Mar 7, 2017 at 4:17 PM, wm4  wrote:
> On Tue,  7 Mar 2017 16:01:58 +0700
> Muhammad Faiz  wrote:
>
>> use ff_thread_once
>>
>> Suggested-by: wm4 
>> Signed-off-by: Muhammad Faiz 
>> ---
>>  libavcodec/allcodecs.c | 16 +---
>>  1 file changed, 9 insertions(+), 7 deletions(-)
>>
>> diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
>> index eee322b..6ec7e79 100644
>> --- a/libavcodec/allcodecs.c
>> +++ b/libavcodec/allcodecs.c
>> @@ -25,6 +25,7 @@
>>   */
>>
>>  #include "config.h"
>> +#include "libavutil/thread.h"
>>  #include "avcodec.h"
>>  #include "version.h"
>>
>> @@ -58,14 +59,8 @@
>>  av_register_codec_parser(_##x##_parser); \
>>  }
>>
>> -void avcodec_register_all(void)
>> +static void register_all(void)
>>  {
>> -static int initialized;
>> -
>> -if (initialized)
>> -return;
>> -initialized = 1;
>> -
>>  /* hardware accelerators */
>>  REGISTER_HWACCEL(H263_VAAPI,h263_vaapi);
>>  REGISTER_HWACCEL(H263_VIDEOTOOLBOX, h263_videotoolbox);
>> @@ -717,3 +712,10 @@ void avcodec_register_all(void)
>>  REGISTER_PARSER(VP9,vp9);
>>  REGISTER_PARSER(XMA,xma);
>>  }
>> +
>> +void avcodec_register_all(void)
>> +{
>> +static AVOnce control = AV_ONCE_INIT;
>> +
>> +ff_thread_once(, register_all);
>> +}
>
> Looks all good to me.
>
> Of course this doesn't change that the thread-safety of this whole
> "list of components" code is still questionable, but it doesn't hurt to
> do this either.

Patchset applied.

Thank's.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 1/4] avcodec/allcodecs: make avcodec_register_all thread safe

2017-03-07 Thread Paul B Mahol
On 3/7/17, Michael Niedermayer  wrote:
> On Tue, Mar 07, 2017 at 07:44:43AM +0100, wm4 wrote:
>> On Tue,  7 Mar 2017 02:47:36 +0700
>> Muhammad Faiz  wrote:
>>
>> > Signed-off-by: Muhammad Faiz 
>> > ---
>> >  libavcodec/allcodecs.c | 13 ++---
>> >  1 file changed, 10 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
>> > index eee322b..1d05fc1 100644
>> > --- a/libavcodec/allcodecs.c
>> > +++ b/libavcodec/allcodecs.c
>> > @@ -24,6 +24,8 @@
>> >   * Provide registration of all codecs, parsers and bitstream filters
>> > for libavcodec.
>> >   */
>> >
>> > +#include 
>> > +
>> >  #include "config.h"
>> >  #include "avcodec.h"
>> >  #include "version.h"
>> > @@ -60,11 +62,14 @@
>> >
>> >  void avcodec_register_all(void)
>> >  {
>> > -static int initialized;
>> > +static atomic_int initialized;
>> > +static atomic_int ready;
>> >
>> > -if (initialized)
>> > +if (atomic_exchange_explicit(, 1,
>> > memory_order_relaxed)) {
>> > +while (!atomic_load_explicit(, memory_order_relaxed))
>> > +;
>> >  return;
>> > -initialized = 1;
>> > +}
>> >
>> >  /* hardware accelerators */
>> >  REGISTER_HWACCEL(H263_VAAPI,h263_vaapi);
>> > @@ -716,4 +721,6 @@ void avcodec_register_all(void)
>> >  REGISTER_PARSER(VP8,vp8);
>> >  REGISTER_PARSER(VP9,vp9);
>> >  REGISTER_PARSER(XMA,xma);
>> > +
>> > +atomic_store_explicit(, 1, memory_order_relaxed);
>> >  }
>>
>
>> avcodec_register() is already "safe" by attempting to do lock-free and
>> wait-free list insertion (not very convincing IMHO, but it's there).
>> Should that code be removed?
>
> that code is needed, otherwise avcodec_register() would not be thread
> safe
>
>
>> Does anyone know why avcodec_register() is
>> even still public API? The same affects some other things.
>
> It is public so that user written codecs can be registered
>
> Ideally we would have a real plugin system and for example rune could
> maintain his cinepack decoder as a plugin in his github repo.
> Then he would likely not have left FFmpeg today.
>
> Removing avcodec_register() IMO is a step in the wrong direction as
> after that theres no way to register user written codecs and then the
> API also would no longer need to be unchanged and once the API changes
> frequently adding a real plugin interface becomes MUCH harder.
>
> The real problem behind all this is of course more complex and a
> combination of many of our developers being rather aggressive and
> having strong oppinions on code outside the area they actively maintain.
> That drives some contributors away and now the lack of a plugin
> interface means they are driven over the ledge and a lost as
> contributors to FFmpeg.
>
> I would very much like to see some effort to add a real plugin
> interface instead of the opposite direction

In that case I will fork FFmpeg as AGPL only.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 3/4] avcodec/vp568: Check that there is enough data for ff_vp56_init_range_decoder()

2017-03-07 Thread Ronald S. Bultje
Hi,

On Mon, Mar 6, 2017 at 8:41 PM, Michael Niedermayer 
wrote:

> -void ff_vp56_init_range_decoder(VP56RangeCoder *c, const uint8_t *buf,
> int buf_size)
> +int ff_vp56_init_range_decoder(VP56RangeCoder *c, const uint8_t *buf,
> int buf_size)
>  {
>  c->high = 255;
>  c->bits = -16;
>  c->buffer = buf;
>  c->end = buf + buf_size;
> +if (buf_size < 1)
> +return AVERROR_INVALIDDATA;
>  c->code_word = bytestream_get_be24(>buffer);
> +return 0;
>  }


Isn't this AV_INPUT_BUFFER_PADDING_SIZE?

And this is inconsistent with the silent failure in renorm().

Ronald
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 1/4] avcodec/allcodecs: make avcodec_register_all thread safe

2017-03-07 Thread Michael Niedermayer
On Tue, Mar 07, 2017 at 07:44:43AM +0100, wm4 wrote:
> On Tue,  7 Mar 2017 02:47:36 +0700
> Muhammad Faiz  wrote:
> 
> > Signed-off-by: Muhammad Faiz 
> > ---
> >  libavcodec/allcodecs.c | 13 ++---
> >  1 file changed, 10 insertions(+), 3 deletions(-)
> > 
> > diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
> > index eee322b..1d05fc1 100644
> > --- a/libavcodec/allcodecs.c
> > +++ b/libavcodec/allcodecs.c
> > @@ -24,6 +24,8 @@
> >   * Provide registration of all codecs, parsers and bitstream filters for 
> > libavcodec.
> >   */
> >  
> > +#include 
> > +
> >  #include "config.h"
> >  #include "avcodec.h"
> >  #include "version.h"
> > @@ -60,11 +62,14 @@
> >  
> >  void avcodec_register_all(void)
> >  {
> > -static int initialized;
> > +static atomic_int initialized;
> > +static atomic_int ready;
> >  
> > -if (initialized)
> > +if (atomic_exchange_explicit(, 1, memory_order_relaxed)) {
> > +while (!atomic_load_explicit(, memory_order_relaxed))
> > +;
> >  return;
> > -initialized = 1;
> > +}
> >  
> >  /* hardware accelerators */
> >  REGISTER_HWACCEL(H263_VAAPI,h263_vaapi);
> > @@ -716,4 +721,6 @@ void avcodec_register_all(void)
> >  REGISTER_PARSER(VP8,vp8);
> >  REGISTER_PARSER(VP9,vp9);
> >  REGISTER_PARSER(XMA,xma);
> > +
> > +atomic_store_explicit(, 1, memory_order_relaxed);
> >  }
> 

> avcodec_register() is already "safe" by attempting to do lock-free and
> wait-free list insertion (not very convincing IMHO, but it's there).
> Should that code be removed?

that code is needed, otherwise avcodec_register() would not be thread
safe


> Does anyone know why avcodec_register() is
> even still public API? The same affects some other things.

It is public so that user written codecs can be registered

Ideally we would have a real plugin system and for example rune could
maintain his cinepack decoder as a plugin in his github repo.
Then he would likely not have left FFmpeg today.

Removing avcodec_register() IMO is a step in the wrong direction as
after that theres no way to register user written codecs and then the
API also would no longer need to be unchanged and once the API changes
frequently adding a real plugin interface becomes MUCH harder.

The real problem behind all this is of course more complex and a
combination of many of our developers being rather aggressive and
having strong oppinions on code outside the area they actively maintain.
That drives some contributors away and now the lack of a plugin
interface means they are driven over the ledge and a lost as
contributors to FFmpeg.

I would very much like to see some effort to add a real plugin
interface instead of the opposite direction


[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The educated differ from the uneducated as much as the living from the
dead. -- Aristotle 


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] Fix off-by-few crasher in ff_h2645_extract_rbsp function

2017-03-07 Thread Carl Eugen Hoyos
2017-03-07 12:30 GMT+01:00 Michał Krasowski :

>>> There are few things that are still not clear to me:
>>> * Why is the 32 bit padding used when the doc says that
>>>   64 bit offset may also be needed?
>>
>>I don't understand your question but you may want to
>>send an update for this sentence.
>
> I mean that the doc says:
>> * This is mainly needed because some optimized bitstream readers read
>> * 32 or 64 bit at once and could read over the end.
> So in case of reading 64 bits at once, may it be the case that 8 bytes padding
> is needed?

(No, 32 bytes are - curently - always needed, see Hendrik's email.)

The relevant line is the definition of AV_INPUT_BUFFER_PADDING_SIZE

Your code should look similar to:
uint8_t *ptr = av_malloc(buffer_size + AV_INPUT_BUFFER_PADDING_SIZE);
And a zero-initialization of the padding.

Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] Fix off-by-few crasher in ff_h2645_extract_rbsp function

2017-03-07 Thread Michael Niedermayer
On Tue, Mar 07, 2017 at 11:55:23AM +0100, Michał Krasowski wrote:
> @Michael Niedermayer
> I have read the documentation, namely
> 
> /**
>  * @ingroup lavc_decoding
>  * Required number of additionally allocated bytes at the end of the input
> bitstream for decoding.
>  * This is mainly needed because some optimized bitstream readers read
>  * 32 or 64 bit at once and could read over the end.
>  * Note: If the first 23 bits of the additional bytes are not 0, then
> damaged
>  * MPEG bitstreams could cause overread and segfault.
>  */
> #define AV_INPUT_BUFFER_PADDING_SIZE 32
> 
> and now it seems to me that you prefer speed (a.k.a. optimization)
> over having a self-contained functions.
> 
> There are few things that are still not clear to me:
> * Why is the 32 bit padding used when the doc says that
>   64 bit offset may also be needed?
> * Even if I extend my data buffer
>   to have those 4 bytes more, is there a risk that some functions
>   in ffmpeg will read out-of-bounds?

> * How to find such information without reading all bolts and nuts of ffmpeg
> source?

Where did you look?
Or said differently where would adding a pointer have helped you to
find this sooner ?
A patch adding such a pointer there is very likely welcome

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Freedom in capitalist society always remains about the same as it was in
ancient Greek republics: Freedom for slave owners. -- Vladimir Lenin


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] Fix off-by-few crasher in ff_h2645_extract_rbsp function

2017-03-07 Thread Michał Krasowski
>> This email may contain Opera confidential information
>
>Please remove this, Carl Eugen
Ah, yes, default footer, sorry.

>> There are few things that are still not clear to me:
>> * Why is the 32 bit padding used when the doc says that
>>   64 bit offset may also be needed?
>
>I don't understand your question but you may want to
>send an update for this sentence.

I mean that the doc says:
> * This is mainly needed because some optimized bitstream readers read
> * 32 or 64 bit at once and could read over the end.
So in case of reading 64 bits at once, may it be the case that 8 bytes padding
is needed?

On Tue, Mar 7, 2017 at 12:18 PM, Carl Eugen Hoyos  wrote:
>
> 2017-03-07 11:55 GMT+01:00 Michał Krasowski :
> > @Michael Niedermayer
> > I have read the documentation, namely
> >
> > /**
> >  * @ingroup lavc_decoding
> >  * Required number of additionally allocated bytes at the end of the input
> > bitstream for decoding.
> >  * This is mainly needed because some optimized bitstream readers read
> >  * 32 or 64 bit at once and could read over the end.
> >  * Note: If the first 23 bits of the additional bytes are not 0, then
> > damaged
> >  * MPEG bitstreams could cause overread and segfault.
> >  */
> > #define AV_INPUT_BUFFER_PADDING_SIZE 32
> >
> > and now it seems to me that you prefer speed (a.k.a. optimization)
> > over having a self-contained functions.
>
> Luckily:
> Video players would be unusable without optimizations.
>
> > There are few things that are still not clear to me:
> > * Why is the 32 bit padding used when the doc says that
> >   64 bit offset may also be needed?
>
> I don't understand your question but you may want to
> send an update for this sentence.
>
> > * Even if I extend my data buffer
> >   to have those 4 bytes more, is there a risk that some functions
> >   in ffmpeg will read out-of-bounds?
>
> Definitely: Bugs are possible.
>
> [...]
>
> > This email may contain Opera confidential information
>
> Please remove this, Carl Eugen
> ___
> 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 v2] avcodec/h264, videotoolbox: fix crash after VT decoder fails

2017-03-07 Thread wm4
On Thu, 2 Mar 2017 09:58:28 -0800
Aman Gupta  wrote:

> On Thu, Mar 2, 2017 at 1:34 AM, wm4  wrote:
> 
> > On Tue, 21 Feb 2017 13:40:08 -0800
> > Aman Gupta  wrote:
> >  
> > > On Tue, Feb 21, 2017 at 1:04 PM, Ronald S. Bultje 
> > > wrote:
> > >  
> > > > Hi,
> > > >
> > > > On Tue, Feb 21, 2017 at 1:48 PM, Aman Gupta  wrote:
> > > >  
> > > >> diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c
> > > >> index 41c0964392..a0ae632fed 100644
> > > >> --- a/libavcodec/h264dec.c
> > > >> +++ b/libavcodec/h264dec.c
> > > >> @@ -850,7 +850,12 @@ static int output_frame(H264Context *h, AVFrame
> > > >> *dst, H264Picture *srcp)
> > > >>  AVFrame *src = srcp->f;
> > > >>  const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(src->  
> > format);  
> > > >>  int i;
> > > >> -int ret = av_frame_ref(dst, src);
> > > >> +int ret;
> > > >> +
> > > >> +if (src->format == AV_PIX_FMT_VIDEOTOOLBOX && src->buf[0]->size  
> > == 1)  
> > > >> +return AVERROR_INVALIDDATA;
> > > >> +
> > > >> +ret = av_frame_ref(dst, src);
> > > >>  if (ret < 0)
> > > >>  return ret;  
> > > >
> > > >
> > > > This is a total hack :) Is there a way to hide this into VT-specific  
> > code  
> > > > outside h264*.[ch]?
> > > >  
> > >
> > > The way the VT hwaccel works is a total hack, as noted in my commit  
> > message.  
> > >
> > > AFAICT, given how the hwaccel APIs work, there's no way to do this  
> > outside  
> > > the h264 decoder. But I'm happy to try fixing this a different way if
> > > anyone has a suggestion.  
> >
> > So, should we push this?
> >  
> 
> I'm struggling to find a less hacky way to implement this, so my vote would
> be to move forward. Changing the error to AVERROR_EXTERNAL makes sense too.

Pushed, with error code changed and a minor version bump added.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] Fix off-by-few crasher in ff_h2645_extract_rbsp function

2017-03-07 Thread Carl Eugen Hoyos
2017-03-07 11:55 GMT+01:00 Michał Krasowski :
> @Michael Niedermayer
> I have read the documentation, namely
>
> /**
>  * @ingroup lavc_decoding
>  * Required number of additionally allocated bytes at the end of the input
> bitstream for decoding.
>  * This is mainly needed because some optimized bitstream readers read
>  * 32 or 64 bit at once and could read over the end.
>  * Note: If the first 23 bits of the additional bytes are not 0, then
> damaged
>  * MPEG bitstreams could cause overread and segfault.
>  */
> #define AV_INPUT_BUFFER_PADDING_SIZE 32
>
> and now it seems to me that you prefer speed (a.k.a. optimization)
> over having a self-contained functions.

Luckily:
Video players would be unusable without optimizations.

> There are few things that are still not clear to me:
> * Why is the 32 bit padding used when the doc says that
>   64 bit offset may also be needed?

I don't understand your question but you may want to
send an update for this sentence.

> * Even if I extend my data buffer
>   to have those 4 bytes more, is there a risk that some functions
>   in ffmpeg will read out-of-bounds?

Definitely: Bugs are possible.

[...]

> This email may contain Opera confidential information

Please remove this, Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] Fix off-by-few crasher in ff_h2645_extract_rbsp function

2017-03-07 Thread wm4
On Tue, 7 Mar 2017 11:55:23 +0100
Michał Krasowski  wrote:

> @Michael Niedermayer
> I have read the documentation, namely
> 
> /**
>  * @ingroup lavc_decoding
>  * Required number of additionally allocated bytes at the end of the input
> bitstream for decoding.
>  * This is mainly needed because some optimized bitstream readers read
>  * 32 or 64 bit at once and could read over the end.
>  * Note: If the first 23 bits of the additional bytes are not 0, then
> damaged
>  * MPEG bitstreams could cause overread and segfault.

I think the MPEG comment is misleading - this applies to all codecs.
Also, as someone said, the padding is in bytes, but these comments
above make it confusing by talking about bits.

>  */
> #define AV_INPUT_BUFFER_PADDING_SIZE 32
> 
> and now it seems to me that you prefer speed (a.k.a. optimization)
> over having a self-contained functions.

Unfortunately.

> There are few things that are still not clear to me:
> * Why is the 32 bit padding used when the doc says that
>   64 bit offset may also be needed?
> * Even if I extend my data buffer
>   to have those 4 bytes more, is there a risk that some functions
>   in ffmpeg will read out-of-bounds?
> * How to find such information without reading all bolts and nuts of ffmpeg
> source?

I sort of agree. Normally, the AVPacket functions guarantee the
padding. So you "only" need to be careful when you setup an AVPacket
manually.

I wonder if we could handle this transparently by checking the
AVPacket.buf allocation. We don't really support non-refcounted input
with the new decode/encode APIs anyway, so this would probably be
possible. Of course it'd mean that the data gets copied if the user
doesn't supply the padding.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] Fix off-by-few crasher in ff_h2645_extract_rbsp function

2017-03-07 Thread Hendrik Leppkes
On Tue, Mar 7, 2017 at 11:55 AM, Michał Krasowski  wrote:
> @Michael Niedermayer
> I have read the documentation, namely
>
> /**
>  * @ingroup lavc_decoding
>  * Required number of additionally allocated bytes at the end of the input
> bitstream for decoding.
>  * This is mainly needed because some optimized bitstream readers read
>  * 32 or 64 bit at once and could read over the end.
>  * Note: If the first 23 bits of the additional bytes are not 0, then
> damaged
>  * MPEG bitstreams could cause overread and segfault.
>  */
> #define AV_INPUT_BUFFER_PADDING_SIZE 32
>
> and now it seems to me that you prefer speed (a.k.a. optimization)
> over having a self-contained functions.
>
> There are few things that are still not clear to me:
> * Why is the 32 bit padding used when the doc says that
>   64 bit offset may also be needed?
> * Even if I extend my data buffer
>   to have those 4 bytes more, is there a risk that some functions
>   in ffmpeg will read out-of-bounds?

The padding is 32 *bytes*, which is plenty for all optimized bitstream
readers we use.
Just make sure those extra bytes are zeroed to avoid any issues.

> * How to find such information without reading all bolts and nuts of ffmpeg
> source?
>

You don't need to read everything, but the main header would
definitely be benefifical.
Feel free to recommend changes to the documentation if you think
something crucial could be clearer (ideally right in patch form!)

- Hendrik
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] Fix off-by-few crasher in ff_h2645_extract_rbsp function

2017-03-07 Thread Michał Krasowski
@Michael Niedermayer
I have read the documentation, namely

/**
 * @ingroup lavc_decoding
 * Required number of additionally allocated bytes at the end of the input
bitstream for decoding.
 * This is mainly needed because some optimized bitstream readers read
 * 32 or 64 bit at once and could read over the end.
 * Note: If the first 23 bits of the additional bytes are not 0, then
damaged
 * MPEG bitstreams could cause overread and segfault.
 */
#define AV_INPUT_BUFFER_PADDING_SIZE 32

and now it seems to me that you prefer speed (a.k.a. optimization)
over having a self-contained functions.

There are few things that are still not clear to me:
* Why is the 32 bit padding used when the doc says that
  64 bit offset may also be needed?
* Even if I extend my data buffer
  to have those 4 bytes more, is there a risk that some functions
  in ffmpeg will read out-of-bounds?
* How to find such information without reading all bolts and nuts of ffmpeg
source?


On Mon, Mar 6, 2017 at 8:53 PM, Michael Niedermayer 
wrote:

> On Mon, Mar 06, 2017 at 03:51:51PM +0100, Michał Krasowski wrote:
> > It seems that the loop tried to access the memory regions
> > beyond allocation, what caused crashes in not-so-rare cases, when
> > the memory read did not belong to current process.
> >
> > This change is fixing the out-of-bounds read problem.
> > Compiling this function with -fsanitize=address and running doesn't
> > result in sanitizer warning as before.
> > ---
> >  libavcodec/h2645_parse.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
>
> have you seen/read the documentation for AV_INPUT_BUFFER_PADDING_SIZE
> ?
>
> if not, that may be the cause of the issues you see
>
>
> [...]
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Avoid a single point of failure, be that a person or equipment.
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>


-- 
Regards,

Michał Krasowski
Software Developer

Opera TV
Pl. Teatralny 8, 50-051 Wrocław, Polska

This email may contain Opera confidential information and be protected by
privilege.  If you are not the intended recipient, any disclosure, copying,
or use is prohibited. Please notify Opera immediately if you have received
this message in error and delete all copies from your system.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] End of involvement (was: [PATCH] Cinepak: speed up decoding several-fold...)

2017-03-07 Thread u-9iep
Now it has been more than a month since the initial submission
of the Cinepak decoder speedup patch (not counting the proof of concept
posted two months ago).

Since then, more and more of the oftentimes ignorant discussion was
dedicated to the perceived design/development policy conformance (still
did not see any reference to a corresponding document :) with no outcome.

Given the apparent lack of the interest from the FFmpeg project in making
improvements to the lightweight fast codecs (the class Cinepak belongs to)
and my lack of appreciation for the conversational style on the list
I do not feel motivated to contribute to the project any longer.

My thanks for FFmpeg and for the enormous work on making it as useful
as it is go to Michael Niedermayer, who should be also credited for his
much appreciated support and work to allow the integration into FFmpeg
and the past improvements of Cinepak encoder and decoder.
(To be clear: the friendly support does not imply endorsement)

Thanks Michael.

Now I end my involvement with FFmpeg, good luck with the project.

If a casual reader interested in Cinepak would try the patches,
I recommend the series
  https://ffmpeg.org/pipermail/ffmpeg-devel/2017-February/207603.html
  https://ffmpeg.org/pipermail/ffmpeg-devel/2017-February/207597.html
including
  https://ffmpeg.org/pipermail/ffmpeg-devel/2017-February/207625.html

Regards,
Rune

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH v2 1/4] avcodec/allcodecs: make avcodec_register_all thread safe

2017-03-07 Thread wm4
On Tue,  7 Mar 2017 16:01:58 +0700
Muhammad Faiz  wrote:

> use ff_thread_once
> 
> Suggested-by: wm4 
> Signed-off-by: Muhammad Faiz 
> ---
>  libavcodec/allcodecs.c | 16 +---
>  1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
> index eee322b..6ec7e79 100644
> --- a/libavcodec/allcodecs.c
> +++ b/libavcodec/allcodecs.c
> @@ -25,6 +25,7 @@
>   */
>  
>  #include "config.h"
> +#include "libavutil/thread.h"
>  #include "avcodec.h"
>  #include "version.h"
>  
> @@ -58,14 +59,8 @@
>  av_register_codec_parser(_##x##_parser); \
>  }
>  
> -void avcodec_register_all(void)
> +static void register_all(void)
>  {
> -static int initialized;
> -
> -if (initialized)
> -return;
> -initialized = 1;
> -
>  /* hardware accelerators */
>  REGISTER_HWACCEL(H263_VAAPI,h263_vaapi);
>  REGISTER_HWACCEL(H263_VIDEOTOOLBOX, h263_videotoolbox);
> @@ -717,3 +712,10 @@ void avcodec_register_all(void)
>  REGISTER_PARSER(VP9,vp9);
>  REGISTER_PARSER(XMA,xma);
>  }
> +
> +void avcodec_register_all(void)
> +{
> +static AVOnce control = AV_ONCE_INIT;
> +
> +ff_thread_once(, register_all);
> +}

Looks all good to me.

Of course this doesn't change that the thread-safety of this whole
"list of components" code is still questionable, but it doesn't hurt to
do this either.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] swresample/resample: use uniform normalization

2017-03-07 Thread Muhammad Faiz
On Mon, Mar 6, 2017 at 5:06 PM, Muhammad Faiz  wrote:
> On Fri, Mar 3, 2017 at 3:58 PM, Muhammad Faiz  wrote:
>> On Wed, Mar 1, 2017 at 10:24 PM, Muhammad Faiz  wrote:
>>> this gives better frequency response
>>>
>>> update swresample fate and other fates
>>> that depend on resampling
>>>
>>> Signed-off-by: Muhammad Faiz 
>>> ---
>>>  libswresample/resample.c |  14 +--
>>>  tests/fate/libswresample.mak | 276 
>>> +--
>>>  tests/ref/acodec/roqaudio|   2 +-
>>>  tests/ref/acodec/s302m   |   6 +-
>>>  tests/ref/lavf/dv_fmt|   8 +-
>>>  tests/ref/lavf/gxf   |   8 +-
>>>  tests/ref/lavf/mxf   |  12 +-
>>>  7 files changed, 163 insertions(+), 163 deletions(-)
>>>
>>
>> Test case, from 44100 to 705600 (16*44100)
>> ffmpeg -filter_complex "aevalsrc='if(n-300,0,1)',
>> aresample=osr=705600:filter_size=16" -f f32le - | ./freq-resp >
>> freq-resp.txt
>>
>>
>> gnuplot result attached
>
> ping

I will push this tomorrow if no one object

thanks
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH v2 2/4] avfilter/allformats: make av_register_all thread safe

2017-03-07 Thread Muhammad Faiz
use ff_thread_once

Suggested-by: wm4 
Signed-off-by: Muhammad Faiz 
---
 libavformat/allformats.c | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/libavformat/allformats.c b/libavformat/allformats.c
index 35869e3..132e58b 100644
--- a/libavformat/allformats.c
+++ b/libavformat/allformats.c
@@ -19,6 +19,7 @@
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
  */
 
+#include "libavutil/thread.h"
 #include "avformat.h"
 #include "rtp.h"
 #include "rdt.h"
@@ -41,13 +42,8 @@
 
 #define REGISTER_MUXDEMUX(X, x) REGISTER_MUXER(X, x); REGISTER_DEMUXER(X, x)
 
-void av_register_all(void)
+static void register_all(void)
 {
-static int initialized;
-
-if (initialized)
-return;
-
 avcodec_register_all();
 
 /* (de)muxers */
@@ -383,6 +379,11 @@ void av_register_all(void)
 REGISTER_DEMUXER (LIBMODPLUG,   libmodplug);
 REGISTER_MUXDEMUX(LIBNUT,   libnut);
 REGISTER_DEMUXER (LIBOPENMPT,   libopenmpt);
+}
+
+void av_register_all(void)
+{
+AVOnce control = AV_ONCE_INIT;
 
-initialized = 1;
+ff_thread_once(, register_all);
 }
-- 
2.9.3

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH v2 4/4] avdevice/alldevices: make avdevice_register_all thread safe

2017-03-07 Thread Muhammad Faiz
use ff_thread_once

Suggested-by: wm4 
Signed-off-by: Muhammad Faiz 
---
 libavdevice/alldevices.c | 16 +---
 libavdevice/avdevice.h   |  1 -
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/libavdevice/alldevices.c b/libavdevice/alldevices.c
index a761be4..75f4ae0 100644
--- a/libavdevice/alldevices.c
+++ b/libavdevice/alldevices.c
@@ -19,6 +19,7 @@
  */
 
 #include "config.h"
+#include "libavutil/thread.h"
 #include "avdevice.h"
 
 #define REGISTER_OUTDEV(X, x)   \
@@ -37,14 +38,8 @@
 
 #define REGISTER_INOUTDEV(X, x) REGISTER_OUTDEV(X, x); REGISTER_INDEV(X, x)
 
-void avdevice_register_all(void)
+static void register_all(void)
 {
-static int initialized;
-
-if (initialized)
-return;
-initialized = 1;
-
 /* devices */
 REGISTER_INOUTDEV(ALSA, alsa);
 REGISTER_INDEV   (AVFOUNDATION, avfoundation);
@@ -76,3 +71,10 @@ void avdevice_register_all(void)
 REGISTER_INDEV   (LIBCDIO,  libcdio);
 REGISTER_INDEV   (LIBDC1394,libdc1394);
 }
+
+void avdevice_register_all(void)
+{
+AVOnce control = AV_ONCE_INIT;
+
+ff_thread_once(, register_all);
+}
diff --git a/libavdevice/avdevice.h b/libavdevice/avdevice.h
index 6153f2c..84f374a 100644
--- a/libavdevice/avdevice.h
+++ b/libavdevice/avdevice.h
@@ -67,7 +67,6 @@ const char *avdevice_license(void);
 
 /**
  * Initialize libavdevice and register all the input and output devices.
- * @warning This function is not thread safe.
  */
 void avdevice_register_all(void);
 
-- 
2.9.3

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH v2 3/4] avfilter/allfilters: make avfilter_register_all thread safe

2017-03-07 Thread Muhammad Faiz
use ff_thread_once

Suggested-by: wm4 
Signed-off-by: Muhammad Faiz 
---
 libavfilter/allfilters.c | 16 +---
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
index 15a74c4..df1af8d 100644
--- a/libavfilter/allfilters.c
+++ b/libavfilter/allfilters.c
@@ -19,6 +19,7 @@
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
  */
 
+#include "libavutil/thread.h"
 #include "avfilter.h"
 #include "config.h"
 #include "opencl_allkernels.h"
@@ -37,14 +38,8 @@
 avfilter_register(_##x); \
 }
 
-void avfilter_register_all(void)
+static void register_all(void)
 {
-static int initialized;
-
-if (initialized)
-return;
-initialized = 1;
-
 REGISTER_FILTER(ABENCH, abench, af);
 REGISTER_FILTER(ACOMPRESSOR,acompressor,af);
 REGISTER_FILTER(ACROSSFADE, acrossfade, af);
@@ -380,3 +375,10 @@ void avfilter_register_all(void)
 REGISTER_FILTER_UNCONDITIONAL(vf_fifo);
 ff_opencl_register_filter_kernel_code_all();
 }
+
+void avfilter_register_all(void)
+{
+AVOnce control = AV_ONCE_INIT;
+
+ff_thread_once(, register_all);
+}
-- 
2.9.3

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH v2 1/4] avcodec/allcodecs: make avcodec_register_all thread safe

2017-03-07 Thread Muhammad Faiz
use ff_thread_once

Suggested-by: wm4 
Signed-off-by: Muhammad Faiz 
---
 libavcodec/allcodecs.c | 16 +---
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
index eee322b..6ec7e79 100644
--- a/libavcodec/allcodecs.c
+++ b/libavcodec/allcodecs.c
@@ -25,6 +25,7 @@
  */
 
 #include "config.h"
+#include "libavutil/thread.h"
 #include "avcodec.h"
 #include "version.h"
 
@@ -58,14 +59,8 @@
 av_register_codec_parser(_##x##_parser); \
 }
 
-void avcodec_register_all(void)
+static void register_all(void)
 {
-static int initialized;
-
-if (initialized)
-return;
-initialized = 1;
-
 /* hardware accelerators */
 REGISTER_HWACCEL(H263_VAAPI,h263_vaapi);
 REGISTER_HWACCEL(H263_VIDEOTOOLBOX, h263_videotoolbox);
@@ -717,3 +712,10 @@ void avcodec_register_all(void)
 REGISTER_PARSER(VP9,vp9);
 REGISTER_PARSER(XMA,xma);
 }
+
+void avcodec_register_all(void)
+{
+static AVOnce control = AV_ONCE_INIT;
+
+ff_thread_once(, register_all);
+}
-- 
2.9.3

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH]lavc/xface: Reorder conditions to silence a gcc warning

2017-03-07 Thread Carl Eugen Hoyos
2017-02-26 11:07 GMT+01:00 Carl Eugen Hoyos :

> I believe attached patch does not change the logic of the conditions
> but silences a (9 times shown, long-time) gcc warning.

Ping.

Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH]lavc/avpacket: Make pkt parameter of av_packet_get_side_data() const

2017-03-07 Thread Carl Eugen Hoyos
2017-02-26 12:04 GMT+01:00 Carl Eugen Hoyos :
> 2017-02-26 11:51 GMT+01:00 Nicolas George :

>>> -uint8_t* av_packet_get_side_data(AVPacket *pkt, enum AVPacketSideDataType 
>>> type,
>>
>>> +uint8_t* av_packet_get_side_data(
>>> +#if FF_API_CONST_GET_SIDE_DATA
>>> +const
>>> +#endif
>>> + AVPacket *pkt, enum AVPacketSideDataType 
>>> type,
>>
>> I do not think we need the FF_API dance, since it is not an ABI break.
>
> I agree that there is no ABI break (but I suspect FF_API does not
> imply an ABI break).
>
>> I think is is not an API break either.
>
> I may misremember but I thought it's an API break for c++ users.
>
> Anyway, new patch attached.

Ping.
Should this not be changed, should one of the patches be applied?

Thank you, Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avutil/tests/lfg.c: added proper normality test

2017-03-07 Thread Carl Eugen Hoyos
2017-03-07 9:28 GMT+01:00 Thomas Turner :
> On Mar 6, 2017 11:47 PM, "Carl Eugen Hoyos"  wrote:
>
> 2017-03-07 6:36 GMT+01:00 Thomas Turner :
>> ./configure command worked without ccache:
>>
>> ./configure --enable-debug=3 --arch=x86_32 --target-os=linux
>> --extra-cflags=-m32 --extra-ldflags=-m32  --enable-cross-compile

Since we are discussing configure lines, I do:
$ ./configure --cc='gcc -m32'
It is shorter and I believe some things are tested in configure that
are skipped with --enable-cross-compile.

[...]

> Is ffmpeg is there a way to compile w/ a specific version of gcc, without
> having to downgrade?
>
> I've tried "./configure --cc='gcc-4.3.6' " which gives an error. However
> "./configure --cc='gcc' " doesn't give an error.

My local version of gcc-4.3.6 was compiled with "--prefix=/usr/local/gcc-4.3.6"
so I use the following configure line:
$ ./configure --cc='/usr/local/gcc-4.3.6/bin/gcc -m32'

Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH] avcodec: fix uninitialized variable read

2017-03-07 Thread wm4
This cna happen if the user tries to call the new decode API for
subtitles.

Fixes CID 1402071.
---
 libavcodec/utils.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index db3adb18d4..31586d51c5 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -2774,7 +2774,7 @@ void avsubtitle_free(AVSubtitle *sub)
 
 static int do_decode(AVCodecContext *avctx, AVPacket *pkt)
 {
-int got_frame;
+int got_frame = 0;
 int ret;
 
 av_assert0(!avctx->internal->buffer_frame->buf[0]);
-- 
2.11.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH]lsws/input: Do not define unused functions

2017-03-07 Thread Paul B Mahol
On 3/7/17, Carl Eugen Hoyos  wrote:
> Hi!
>
> Attached patch fixes a few warnings when compiling with newer gcc.
>
> Please comment, Carl Eugen
>

lgtm
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avutil/tests/lfg.c: added proper normality test

2017-03-07 Thread Thomas Turner
On Mar 6, 2017 11:47 PM, "Carl Eugen Hoyos"  wrote:

2017-03-07 6:36 GMT+01:00 Thomas Turner :
> ./configure command worked without ccache:
>
> ./configure --enable-debug=3 --arch=x86_32 --target-os=linux
> --extra-cflags=-m32 --extra-ldflags=-m32  --enable-cross-compile
>
> but after installing the following software packages w/ apt-get:
>
> apt-get install gcc-multilib g++-mult
>
> after recompiling and installing ffmpeg and running lfg selftest again,
the
> test still passes for x86_32 w/ gcc.
> Just to double check the results, i ran the lfg test in a virtual machine
> running 32 bit ubuntu and the test indeed passes.
>
> So i am unable to reproduce the error on x86_32.

The failure is reproducible with gcc 4.3.6 which we supported so far
(and several not-so-supported versions).

Carl Eugen

I see, well I'm running gcc 4.8.4.

Is ffmpeg is there a way to compile w/ a specific version of gcc, without
having to downgrade?

I've tried "./configure --cc='gcc-4.3.6' " which gives an error. However
"./configure --cc='gcc' " doesn't give an error.

___
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 4/4] avcodec/vp8: remove redundant check

2017-03-07 Thread Paul B Mahol
On 3/7/17, Michael Niedermayer  wrote:
> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/vp8.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/libavcodec/vp8.c b/libavcodec/vp8.c
> index 8039e7817e..a3d057d62e 100644
> --- a/libavcodec/vp8.c
> +++ b/libavcodec/vp8.c
> @@ -2504,8 +2504,6 @@ int vp78_decode_mb_row_sliced(AVCodecContext *avctx,
> void *tdata, int jobnr,
>
>  td->thread_nr = threadnr;
>  for (mb_y = jobnr; mb_y < s->mb_height; mb_y += num_jobs) {
> -if (mb_y >= s->mb_height)
> -break;
>  td->thread_mb_pos = mb_y << 16;
>  ret = s->decode_mb_row_no_filter(avctx, tdata, jobnr, threadnr);
>  if (ret < 0)
> --
> 2.11.0
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

ok
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel