Re: [FFmpeg-devel] [PATCH v2] avfilter/pthread: rewrite implementation

[Michael Niedermayer]

On Sat, Jul 08, 2017 at 09:06:05AM +0700, Muhammad Faiz wrote:
> On Sat, Jul 8, 2017 at 6:45 AM, Michael Niedermayer
>  wrote:
> > On Fri, Jul 07, 2017 at 09:04:37PM +0700, Muhammad Faiz wrote:
> >> Avoid pthread_cond_broadcast that wakes up all workers. Make each of them
> >> uses distict mutex/cond. Also let main thread help running jobs.
> >>
> >> Benchmark using afir with threads=5 and 4096 taps fir:
> >> channels=1:
> >> old:
> >> 1849650 decicycles in afir_execute,   2 runs,  0 skips
> >> 1525719 decicycles in afir_execute,1024 runs,  0 skips
> >> 1546032 decicycles in afir_execute,   16356 runs, 28 skips
> >> new:
> >> 1495525 decicycles in afir_execute,   2 runs,  0 skips
> >>  968897 decicycles in afir_execute,1024 runs,  0 skips
> >>  941286 decicycles in afir_execute,   16384 runs,  0 skips
> >>
> >> channels=2:
> >> old:
> >> 3135485 decicycles in afir_execute,   2 runs,  0 skips
> >> 1967158 decicycles in afir_execute,1024 runs,  0 skips
> >> 1802430 decicycles in afir_execute,   16364 runs, 20 skips
> >> new:
> >> 1864750 decicycles in afir_execute,   2 runs,  0 skips
> >> 1437792 decicycles in afir_execute,1024 runs,  0 skips
> >> 1183963 decicycles in afir_execute,   16382 runs,  2 skips
> >>
> >> channels=4:
> >> old:
> >> 4879925 decicycles in afir_execute,   2 runs,  0 skips
> >> 3557950 decicycles in afir_execute,1022 runs,  2 skips
> >> 3206843 decicycles in afir_execute,   16379 runs,  5 skips
> >> new:
> >> 2962320 decicycles in afir_execute,   2 runs,  0 skips
> >> 2450430 decicycles in afir_execute,1024 runs,  0 skips
> >> 2446219 decicycles in afir_execute,   16383 runs,  1 skips
> >>
> >> channels=8:
> >> old:
> >> 6032455 decicycles in afir_execute,   2 runs,  0 skips
> >> 4838614 decicycles in afir_execute,1023 runs,  1 skips
> >> 4720760 decicycles in afir_execute,   16369 runs, 15 skips
> >> new:
> >> 5228150 decicycles in afir_execute,   2 runs,  0 skips
> >> 4592129 decicycles in afir_execute,1023 runs,  1 skips
> >> 4469067 decicycles in afir_execute,   16383 runs,  1 skips
> >
> > this causes a strange change:
> >
> > ./ffmpeg -i ~/videos/matrixbench_mpeg2.mpg  -vcodec libxavs  -vf 
> > scale=80x60 -t 1 file3.nut
> >
> > results in different files before and after this patch. Neither plays
> > i suspect this is not a bug in the patch but something odd elsewhere
> > but i dont know
> >
> > -rw-r- 1 michael michael 57671 Jul  8 00:48 file3.nut
> > -rw-r- 1 michael michael 62162 Jul  8 00:48 file3p.nut
> >
> > framecrc difference of video with -vcodec copy -copyinkf -f framecrc
> >
> > --- a   2017-07-08 01:41:49.717555033 +0200
> > +++ b   2017-07-08 01:42:09.877555273 +0200
> > @@ -5,28 +5,28 @@
> >  #dimensions 0: 80x60
> >  #sar 0: 1/1
> >  0,  0,  0, 2048, 1860, 0xaa19412e, F=0x0
> > -0,   2048,   2048, 2048, 1261, 0xd0bd2d34, F=0x0
> > -0,   4096,   4096, 2048, 1261, 0x30083a11, F=0x0
> > -0,   6144,   6144, 2048, 1347, 0xfd5b5c17, F=0x0
> > -0,   8192,   8192, 2048,  933, 0x3e95a0aa, F=0x0
> > -0,  10240,  10240, 2048, 1299, 0x5fd141e1, F=0x0
> > -0,  12288,  12288, 2048, 1311, 0xcb90563e, F=0x0
> > -0,  14336,  14336, 2048, 1288, 0x3cee, F=0x0
> > -0,  16384,  16384, 2048, 1295, 0x68d34476, F=0x0
> > -0,  18432,  18432, 2048, 1397, 0xf0646699, F=0x0
> > -0,  20480,  20480, 2048, 1353, 0xbd0557f9, F=0x0
> > -0,  22528,  22528, 2048, 1358, 0x90095601, F=0x0
> > -0,  24576,  24576, 2048, 2004, 0x8de57d88, F=0x0
> > -0,  26624,  26624, 2048, 1477, 0x6c099b28, F=0x0
> > -0,  28672,  28672, 2048, 1515, 0x2fd78855, F=0x0
> > +0,   2048,   2048, 2048, 1827, 0x1a47f795, F=0x0
> > +0,   4096,   4096, 2048, 1379, 0x50435dbb, F=0x0
> > +0,   6144,   6144, 2048, 1912, 0x9b2529a8, F=0x0
> > +0,   8192,   8192, 2048, 1052, 0x6fe1ce3b, F=0x0
> > +0,  10240,  10240, 2048, 1862, 0x10e30eae, F=0x0
> > +0,  12288,  12288, 2048, 1432, 0x93858555, F=0x0
> > +0,  14336,  14336, 2048, 1850, 0xaf3b039d, F=0x0
> > +0,  16384,  16384, 2048, 1408, 0x049e668a, F=0x0
> > +0,  18432,  18432, 2048, 1956, 0x751c36c6, F=0x0
> > +0,  20480,  20480, 2048, 1465, 0xb6e58045, F=0x0
> > +0,  22528,  22528, 2048, 1916, 0x22dc1fe7, F=0x0
> > +0,  24576,  24576, 2048, 2038, 0x56548c7c, F=0x0
> > +0,  26624,  26624, 2048, 1490, 0x7e42a072, F=0x0
> > +0,  28672,  

Re: [FFmpeg-devel] [PATCH v2] avfilter/pthread: rewrite implementation

[Muhammad Faiz]

On Sat, Jul 8, 2017 at 6:45 AM, Michael Niedermayer
 wrote:
> On Fri, Jul 07, 2017 at 09:04:37PM +0700, Muhammad Faiz wrote:
>> Avoid pthread_cond_broadcast that wakes up all workers. Make each of them
>> uses distict mutex/cond. Also let main thread help running jobs.
>>
>> Benchmark using afir with threads=5 and 4096 taps fir:
>> channels=1:
>> old:
>> 1849650 decicycles in afir_execute,   2 runs,  0 skips
>> 1525719 decicycles in afir_execute,1024 runs,  0 skips
>> 1546032 decicycles in afir_execute,   16356 runs, 28 skips
>> new:
>> 1495525 decicycles in afir_execute,   2 runs,  0 skips
>>  968897 decicycles in afir_execute,1024 runs,  0 skips
>>  941286 decicycles in afir_execute,   16384 runs,  0 skips
>>
>> channels=2:
>> old:
>> 3135485 decicycles in afir_execute,   2 runs,  0 skips
>> 1967158 decicycles in afir_execute,1024 runs,  0 skips
>> 1802430 decicycles in afir_execute,   16364 runs, 20 skips
>> new:
>> 1864750 decicycles in afir_execute,   2 runs,  0 skips
>> 1437792 decicycles in afir_execute,1024 runs,  0 skips
>> 1183963 decicycles in afir_execute,   16382 runs,  2 skips
>>
>> channels=4:
>> old:
>> 4879925 decicycles in afir_execute,   2 runs,  0 skips
>> 3557950 decicycles in afir_execute,1022 runs,  2 skips
>> 3206843 decicycles in afir_execute,   16379 runs,  5 skips
>> new:
>> 2962320 decicycles in afir_execute,   2 runs,  0 skips
>> 2450430 decicycles in afir_execute,1024 runs,  0 skips
>> 2446219 decicycles in afir_execute,   16383 runs,  1 skips
>>
>> channels=8:
>> old:
>> 6032455 decicycles in afir_execute,   2 runs,  0 skips
>> 4838614 decicycles in afir_execute,1023 runs,  1 skips
>> 4720760 decicycles in afir_execute,   16369 runs, 15 skips
>> new:
>> 5228150 decicycles in afir_execute,   2 runs,  0 skips
>> 4592129 decicycles in afir_execute,1023 runs,  1 skips
>> 4469067 decicycles in afir_execute,   16383 runs,  1 skips
>
> this causes a strange change:
>
> ./ffmpeg -i ~/videos/matrixbench_mpeg2.mpg  -vcodec libxavs  -vf scale=80x60 
> -t 1 file3.nut
>
> results in different files before and after this patch. Neither plays
> i suspect this is not a bug in the patch but something odd elsewhere
> but i dont know
>
> -rw-r- 1 michael michael 57671 Jul  8 00:48 file3.nut
> -rw-r- 1 michael michael 62162 Jul  8 00:48 file3p.nut
>
> framecrc difference of video with -vcodec copy -copyinkf -f framecrc
>
> --- a   2017-07-08 01:41:49.717555033 +0200
> +++ b   2017-07-08 01:42:09.877555273 +0200
> @@ -5,28 +5,28 @@
>  #dimensions 0: 80x60
>  #sar 0: 1/1
>  0,  0,  0, 2048, 1860, 0xaa19412e, F=0x0
> -0,   2048,   2048, 2048, 1261, 0xd0bd2d34, F=0x0
> -0,   4096,   4096, 2048, 1261, 0x30083a11, F=0x0
> -0,   6144,   6144, 2048, 1347, 0xfd5b5c17, F=0x0
> -0,   8192,   8192, 2048,  933, 0x3e95a0aa, F=0x0
> -0,  10240,  10240, 2048, 1299, 0x5fd141e1, F=0x0
> -0,  12288,  12288, 2048, 1311, 0xcb90563e, F=0x0
> -0,  14336,  14336, 2048, 1288, 0x3cee, F=0x0
> -0,  16384,  16384, 2048, 1295, 0x68d34476, F=0x0
> -0,  18432,  18432, 2048, 1397, 0xf0646699, F=0x0
> -0,  20480,  20480, 2048, 1353, 0xbd0557f9, F=0x0
> -0,  22528,  22528, 2048, 1358, 0x90095601, F=0x0
> -0,  24576,  24576, 2048, 2004, 0x8de57d88, F=0x0
> -0,  26624,  26624, 2048, 1477, 0x6c099b28, F=0x0
> -0,  28672,  28672, 2048, 1515, 0x2fd78855, F=0x0
> +0,   2048,   2048, 2048, 1827, 0x1a47f795, F=0x0
> +0,   4096,   4096, 2048, 1379, 0x50435dbb, F=0x0
> +0,   6144,   6144, 2048, 1912, 0x9b2529a8, F=0x0
> +0,   8192,   8192, 2048, 1052, 0x6fe1ce3b, F=0x0
> +0,  10240,  10240, 2048, 1862, 0x10e30eae, F=0x0
> +0,  12288,  12288, 2048, 1432, 0x93858555, F=0x0
> +0,  14336,  14336, 2048, 1850, 0xaf3b039d, F=0x0
> +0,  16384,  16384, 2048, 1408, 0x049e668a, F=0x0
> +0,  18432,  18432, 2048, 1956, 0x751c36c6, F=0x0
> +0,  20480,  20480, 2048, 1465, 0xb6e58045, F=0x0
> +0,  22528,  22528, 2048, 1916, 0x22dc1fe7, F=0x0
> +0,  24576,  24576, 2048, 2038, 0x56548c7c, F=0x0
> +0,  26624,  26624, 2048, 1490, 0x7e42a072, F=0x0
> +0,  28672,  28672, 2048, 1521, 0x6e128b71, F=0x0
>  0,  30720,  30720, 2048, 1523, 0xa5819af8, F=0x0
>  0,  32768,  32768, 2048, 1528, 0x9898a156, F=0x0
> -0,  34816,  34816, 2048, 1601, 0x9873cdf4, F=0x0
> +0, 

Re: [FFmpeg-devel] [PATCH] libswresample: check input to swr_convert_frame for NULL

[Michael Niedermayer]

On Fri, Jul 07, 2017 at 10:54:47AM +0100, hexpointer wrote:
> When 'out' is an AVFrame that does not have buffers preallocated,
> swr_convert_frame tries to allocate buffers of the right size. However
> in calculating this size it failed to check for whether 'in' is NULL
> (requesting that swr's internal buffers are to be flushed).
> ---
>  libswresample/swresample_frame.c | 7 ---
>  1 file changed, 4 insertions(+), 3 deletions(-)

Is the author value as intended ?
"Author: hexpointer "
Theres no name

(it cannot be changed after pushing)

[...]
-- 
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 1/3] avcodec/get_bits: add cached bitstream reader

[Michael Niedermayer]

On Fri, Jul 07, 2017 at 08:48:46PM +0200, Paul B Mahol wrote:
> Signed-off-by: Paul B Mahol 
> ---
>  libavcodec/get_bits.h | 205 
> +++---
>  1 file changed, 196 insertions(+), 9 deletions(-)
> 
> diff --git a/libavcodec/get_bits.h b/libavcodec/get_bits.h
> index c530015..8a9021a 100644
> --- a/libavcodec/get_bits.h
> +++ b/libavcodec/get_bits.h
> @@ -1,5 +1,6 @@
>  /*
> - * copyright (c) 2004 Michael Niedermayer 
> + * Copyright (c) 2004 Michael Niedermayer 
> + * Copyright (c) 2016 Alexandra Hájková
>   *
>   * This file is part of FFmpeg.
>   *
> @@ -54,6 +55,10 @@
>  
>  typedef struct GetBitContext {
>  const uint8_t *buffer, *buffer_end;
> +#ifdef CACHED_BITSTREAM_READER
> +uint64_t cache;
> +unsigned bits_left;
> +#endif
>  int index;
>  int size_in_bits;
>  int size_in_bits_plus8;
> @@ -106,7 +111,9 @@ typedef struct GetBitContext {
>   * For examples see get_bits, show_bits, skip_bits, get_vlc.
>   */
>  
> -#ifdef LONG_BITSTREAM_READER
> +#ifdef CACHED_BITSTREAM_READER
> +#   define MIN_CACHE_BITS 64
> +#elif defined LONG_BITSTREAM_READER
>  #   define MIN_CACHE_BITS 32
>  #else
>  #   define MIN_CACHE_BITS 25
> @@ -198,7 +205,11 @@ typedef struct GetBitContext {
>  
>  static inline int get_bits_count(const GetBitContext *s)
>  {
> +#ifdef CACHED_BITSTREAM_READER
> +return s->index - s->bits_left;
> +#else
>  return s->index;
> +#endif
>  }
>  
>  static inline void skip_bits_long(GetBitContext *s, int n)
> @@ -210,6 +221,68 @@ static inline void skip_bits_long(GetBitContext *s, int 
> n)
>  #endif
>  }
>  
> +static inline void refill_32(GetBitContext *s)
> +{
> +#ifdef CACHED_BITSTREAM_READER

> +if (s->buffer + (s->index >> 3) >= s->buffer_end)
> +return;

should be under !UNCHECKED_BITSTREAM_READER

also this looks like it can result in intermediate invalid pointers
this should avoid that:
s->index >> 3 >= s->buffer_end - s->buffer
same issue in refill_64()

patch overall is nice

thanks

[...]
-- 
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

Re: [FFmpeg-devel] [PATCH v2] avfilter/pthread: rewrite implementation

[Michael Niedermayer]

On Fri, Jul 07, 2017 at 09:04:37PM +0700, Muhammad Faiz wrote:
> Avoid pthread_cond_broadcast that wakes up all workers. Make each of them
> uses distict mutex/cond. Also let main thread help running jobs.
> 
> Benchmark using afir with threads=5 and 4096 taps fir:
> channels=1:
> old:
> 1849650 decicycles in afir_execute,   2 runs,  0 skips
> 1525719 decicycles in afir_execute,1024 runs,  0 skips
> 1546032 decicycles in afir_execute,   16356 runs, 28 skips
> new:
> 1495525 decicycles in afir_execute,   2 runs,  0 skips
>  968897 decicycles in afir_execute,1024 runs,  0 skips
>  941286 decicycles in afir_execute,   16384 runs,  0 skips
> 
> channels=2:
> old:
> 3135485 decicycles in afir_execute,   2 runs,  0 skips
> 1967158 decicycles in afir_execute,1024 runs,  0 skips
> 1802430 decicycles in afir_execute,   16364 runs, 20 skips
> new:
> 1864750 decicycles in afir_execute,   2 runs,  0 skips
> 1437792 decicycles in afir_execute,1024 runs,  0 skips
> 1183963 decicycles in afir_execute,   16382 runs,  2 skips
> 
> channels=4:
> old:
> 4879925 decicycles in afir_execute,   2 runs,  0 skips
> 3557950 decicycles in afir_execute,1022 runs,  2 skips
> 3206843 decicycles in afir_execute,   16379 runs,  5 skips
> new:
> 2962320 decicycles in afir_execute,   2 runs,  0 skips
> 2450430 decicycles in afir_execute,1024 runs,  0 skips
> 2446219 decicycles in afir_execute,   16383 runs,  1 skips
> 
> channels=8:
> old:
> 6032455 decicycles in afir_execute,   2 runs,  0 skips
> 4838614 decicycles in afir_execute,1023 runs,  1 skips
> 4720760 decicycles in afir_execute,   16369 runs, 15 skips
> new:
> 5228150 decicycles in afir_execute,   2 runs,  0 skips
> 4592129 decicycles in afir_execute,1023 runs,  1 skips
> 4469067 decicycles in afir_execute,   16383 runs,  1 skips

this causes a strange change:

./ffmpeg -i ~/videos/matrixbench_mpeg2.mpg  -vcodec libxavs  -vf scale=80x60 -t 
1 file3.nut

results in different files before and after this patch. Neither plays
i suspect this is not a bug in the patch but something odd elsewhere
but i dont know

-rw-r- 1 michael michael 57671 Jul  8 00:48 file3.nut
-rw-r- 1 michael michael 62162 Jul  8 00:48 file3p.nut

framecrc difference of video with -vcodec copy -copyinkf -f framecrc

--- a   2017-07-08 01:41:49.717555033 +0200
+++ b   2017-07-08 01:42:09.877555273 +0200
@@ -5,28 +5,28 @@
 #dimensions 0: 80x60
 #sar 0: 1/1
 0,  0,  0, 2048, 1860, 0xaa19412e, F=0x0
-0,   2048,   2048, 2048, 1261, 0xd0bd2d34, F=0x0
-0,   4096,   4096, 2048, 1261, 0x30083a11, F=0x0
-0,   6144,   6144, 2048, 1347, 0xfd5b5c17, F=0x0
-0,   8192,   8192, 2048,  933, 0x3e95a0aa, F=0x0
-0,  10240,  10240, 2048, 1299, 0x5fd141e1, F=0x0
-0,  12288,  12288, 2048, 1311, 0xcb90563e, F=0x0
-0,  14336,  14336, 2048, 1288, 0x3cee, F=0x0
-0,  16384,  16384, 2048, 1295, 0x68d34476, F=0x0
-0,  18432,  18432, 2048, 1397, 0xf0646699, F=0x0
-0,  20480,  20480, 2048, 1353, 0xbd0557f9, F=0x0
-0,  22528,  22528, 2048, 1358, 0x90095601, F=0x0
-0,  24576,  24576, 2048, 2004, 0x8de57d88, F=0x0
-0,  26624,  26624, 2048, 1477, 0x6c099b28, F=0x0
-0,  28672,  28672, 2048, 1515, 0x2fd78855, F=0x0
+0,   2048,   2048, 2048, 1827, 0x1a47f795, F=0x0
+0,   4096,   4096, 2048, 1379, 0x50435dbb, F=0x0
+0,   6144,   6144, 2048, 1912, 0x9b2529a8, F=0x0
+0,   8192,   8192, 2048, 1052, 0x6fe1ce3b, F=0x0
+0,  10240,  10240, 2048, 1862, 0x10e30eae, F=0x0
+0,  12288,  12288, 2048, 1432, 0x93858555, F=0x0
+0,  14336,  14336, 2048, 1850, 0xaf3b039d, F=0x0
+0,  16384,  16384, 2048, 1408, 0x049e668a, F=0x0
+0,  18432,  18432, 2048, 1956, 0x751c36c6, F=0x0
+0,  20480,  20480, 2048, 1465, 0xb6e58045, F=0x0
+0,  22528,  22528, 2048, 1916, 0x22dc1fe7, F=0x0
+0,  24576,  24576, 2048, 2038, 0x56548c7c, F=0x0
+0,  26624,  26624, 2048, 1490, 0x7e42a072, F=0x0
+0,  28672,  28672, 2048, 1521, 0x6e128b71, F=0x0
 0,  30720,  30720, 2048, 1523, 0xa5819af8, F=0x0
 0,  32768,  32768, 2048, 1528, 0x9898a156, F=0x0
-0,  34816,  34816, 2048, 1601, 0x9873cdf4, F=0x0
+0,  34816,  34816, 2048, 1613, 0x5e97d399, F=0x0
 0,  36864,  36864, 2048, 1597, 0xf02ad0e6, F=0x0
-0,  38912,  38912, 2048, 1620, 0x4da2da72, F=0x0
-0,  40960,  40960, 

[FFmpeg-devel] [PATCHv2 2/2] avdevice/decklink_dec: add support for receiving op47 teletext

[Marton Balint]

v2:
- use uint16_t instead of int to store 10-bit ancillary data
- fix ancillary line numbers for 1080p
- some comments and clarifications as requested by Aaron Levinson

Signed-off-by: Marton Balint 
---
 doc/indevs.texi  |  20 ---
 libavdevice/decklink_dec.cpp | 127 +++
 2 files changed, 129 insertions(+), 18 deletions(-)

diff --git a/doc/indevs.texi b/doc/indevs.texi
index 330617a7c9..09e33216dc 100644
--- a/doc/indevs.texi
+++ b/doc/indevs.texi
@@ -245,13 +245,19 @@ of uyvy422. Not all Blackmagic devices support this 
option.
 
 @item teletext_lines
 If set to nonzero, an additional teletext stream will be captured from the
-vertical ancillary data. This option is a bitmask of the VBI lines checked,
-specifically lines 6 to 22, and lines 318 to 335. Line 6 is the LSB in the 
mask.
-Selected lines which do not contain teletext information will be ignored. You
-can use the special @option{all} constant to select all possible lines, or
-@option{standard} to skip lines 6, 318 and 319, which are not compatible with 
all
-receivers. Capturing teletext only works for SD PAL sources. To use this
-option, ffmpeg needs to be compiled with @code{--enable-libzvbi}.
+vertical ancillary data. Both SD PAL (576i) and HD (1080i or 1080p)
+sources are supported. In case of HD sources, OP47 packets are decoded.
+
+This option is a bitmask of the SD PAL VBI lines captured, specifically lines 6
+to 22, and lines 318 to 335. Line 6 is the LSB in the mask. Selected lines
+which do not contain teletext information will be ignored. You can use the
+special @option{all} constant to select all possible lines, or
+@option{standard} to skip lines 6, 318 and 319, which are not compatible with
+all receivers.
+
+For SD sources, ffmpeg needs to be compiled with @code{--enable-libzvbi}. For
+HD sources, on older (pre-4K) DeckLink card models you have to capture in 10
+bit mode.
 
 @item channels
 Defines number of audio channels to capture. Must be @samp{2}, @samp{8} or 
@samp{16}.
diff --git a/libavdevice/decklink_dec.cpp b/libavdevice/decklink_dec.cpp
index 6783a0ce77..8b5c1a20c1 100644
--- a/libavdevice/decklink_dec.cpp
+++ b/libavdevice/decklink_dec.cpp
@@ -36,6 +36,7 @@ extern "C" {
 #include "libavutil/imgutils.h"
 #include "libavutil/time.h"
 #include "libavutil/mathematics.h"
+#include "libavutil/reverse.h"
 #if CONFIG_LIBZVBI
 #include 
 #endif
@@ -44,7 +45,6 @@ extern "C" {
 #include "decklink_common.h"
 #include "decklink_dec.h"
 
-#if CONFIG_LIBZVBI
 static uint8_t calc_parity_and_line_offset(int line)
 {
 uint8_t ret = (line < 313) << 5;
@@ -63,6 +63,7 @@ static void fill_data_unit_head(int line, uint8_t *tgt)
 tgt[3] = 0xe4; // framing code
 }
 
+#if CONFIG_LIBZVBI
 static uint8_t* teletext_data_unit_from_vbi_data(int line, uint8_t *src, 
uint8_t *tgt, vbi_pixfmt fmt)
 {
 vbi_bit_slicer slicer;
@@ -95,6 +96,95 @@ static uint8_t* teletext_data_unit_from_vbi_data_10bit(int 
line, uint8_t *src, u
 }
 #endif
 
+static uint8_t* teletext_data_unit_from_op47_vbi_packet(int line, uint16_t 
*py, uint8_t *tgt)
+{
+int i;
+
+if (py[0] != 0x255 || py[1] != 0x255 || py[2] != 0x227)
+return tgt;
+
+fill_data_unit_head(line, tgt);
+
+py += 3;
+tgt += 4;
+
+for (i = 0; i < 42; i++)
+   *tgt++ = ff_reverse[py[i] & 255];
+
+return tgt;
+}
+
+static int linemask_matches(int line, int64_t mask)
+{
+int shift = -1;
+if (line >= 6 && line <= 22)
+shift = line - 6;
+if (line >= 318 && line <= 335)
+shift = line - 318 + 17;
+return shift >= 0 && ((1ULL << shift) & mask);
+}
+
+static uint8_t* teletext_data_unit_from_op47_data(uint16_t *py, uint16_t 
*pend, uint8_t *tgt, int64_t wanted_lines)
+{
+if (py < pend - 9) {
+if (py[0] == 0x151 && py[1] == 0x115 && py[3] == 0x102) {   // 
identifier, identifier, format code for WST teletext
+uint16_t *descriptors = py + 4;
+int i;
+py += 9;
+for (i = 0; i < 5 && py < pend - 45; i++, py += 45) {
+int line = (descriptors[i] & 31) + (!(descriptors[i] & 128)) * 
313;
+if (line && linemask_matches(line, wanted_lines))
+tgt = teletext_data_unit_from_op47_vbi_packet(line, py, 
tgt);
+}
+}
+}
+return tgt;
+}
+
+static uint8_t* teletext_data_unit_from_ancillary_packet(uint16_t *py, 
uint16_t *pend, uint8_t *tgt, int64_t wanted_lines, int allow_multipacket)
+{
+uint16_t did = py[0];   // 
data id
+uint16_t sdid = py[1];  // 
secondary data id
+uint16_t dc = py[2] & 255;  // 
data count
+py += 3;
+pend = FFMIN(pend, py + dc);
+if (did == 0x143 && sdid == 0x102) {// 
subtitle distribution packet
+tgt = 

Re: [FFmpeg-devel] [PATCH 1/1] This change adds an encoder for Camera metadata motion. This is a type of sensor data associated with video, such as GPS, acceleration, gyro, and camera orientation. It

[Louis O'Bryan]

It sounds like my code should be outside an encoder then. I think this
stems from my misunderstanding of how the API can be used. I will rewrite
my code that muxes streams to use an AVPacket directly instead.

On Fri, Jul 7, 2017 at 3:47 PM, Hendrik Leppkes  wrote:

> On Sat, Jul 8, 2017 at 12:24 AM, Louis O'Bryan
>  wrote:
> >
> > The use case is to write data to a new stream in the mp4 container. The
> > encoder isn't changing the data. This data would reside in the same mp4
> > container as video and audio streams. Are you suggesting there is a way I
> > can accomplish that task without creating this encoder (and if so, how)?
>
> You don't need an encoder to create AVPackets, you can just do that
> yourself, even more so if the encoder apparently doesn't do any
> encoding.
> So why does it need encoding in the first place? Why can't you just
> pass the finished data in an AVPacket to the mov muxer? Thats how data
> streams are supposed to work.
>
> >
> >> +static av_cold int encode_init(AVCodecContext *avctx) {
> >> > +// Use dummy values for the height and width.
> >> > +avctx->width = DUMMY_ENCODER_SIZE;
> >> > +avctx->height = DUMMY_ENCODER_SIZE;
> >> > +avctx->max_pixels = DUMMY_ENCODER_SIZE;
> >> What? This makes no sense.
> >
> > Using avcodec_encode_video2() seems to require that the width and height
> be
> > nonzero. What is the recommended way to avoid that?
> >
>
> Clearly you should not use a video encoder to encode something thats
> not a video. This entire approach is not acceptable.
> ___
> 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] [PATCHv2 1/2] avdevice/decklink_dec: add support for decoding teletext from 10bit ancillary data

[Marton Balint]

This also add supports for 4K DeckLink cards because they always output the
ancillary data in 10-bit.

v2:
- only try teletext decoding for 576i PAL mode
- some comments as requested by Aaron Levinson

Signed-off-by: Marton Balint 
---
 doc/indevs.texi  |  4 +--
 libavdevice/decklink_dec.cpp | 61 
 2 files changed, 47 insertions(+), 18 deletions(-)

diff --git a/doc/indevs.texi b/doc/indevs.texi
index 51c304f3ec..330617a7c9 100644
--- a/doc/indevs.texi
+++ b/doc/indevs.texi
@@ -250,8 +250,8 @@ specifically lines 6 to 22, and lines 318 to 335. Line 6 is 
the LSB in the mask.
 Selected lines which do not contain teletext information will be ignored. You
 can use the special @option{all} constant to select all possible lines, or
 @option{standard} to skip lines 6, 318 and 319, which are not compatible with 
all
-receivers. Capturing teletext only works for SD PAL sources in 8 bit mode.
-To use this option, ffmpeg needs to be compiled with @code{--enable-libzvbi}.
+receivers. Capturing teletext only works for SD PAL sources. To use this
+option, ffmpeg needs to be compiled with @code{--enable-libzvbi}.
 
 @item channels
 Defines number of audio channels to capture. Must be @samp{2}, @samp{8} or 
@samp{16}.
diff --git a/libavdevice/decklink_dec.cpp b/libavdevice/decklink_dec.cpp
index 39974e3ff4..6783a0ce77 100644
--- a/libavdevice/decklink_dec.cpp
+++ b/libavdevice/decklink_dec.cpp
@@ -30,6 +30,7 @@ extern "C" {
 extern "C" {
 #include "config.h"
 #include "libavformat/avformat.h"
+#include "libavutil/avassert.h"
 #include "libavutil/avutil.h"
 #include "libavutil/common.h"
 #include "libavutil/imgutils.h"
@@ -54,21 +55,43 @@ static uint8_t calc_parity_and_line_offset(int line)
 return ret;
 }
 
-int teletext_data_unit_from_vbi_data(int line, uint8_t *src, uint8_t *tgt)
+static void fill_data_unit_head(int line, uint8_t *tgt)
+{
+tgt[0] = 0x02; // data_unit_id
+tgt[1] = 0x2c; // data_unit_length
+tgt[2] = calc_parity_and_line_offset(line); // field_parity, line_offset
+tgt[3] = 0xe4; // framing code
+}
+
+static uint8_t* teletext_data_unit_from_vbi_data(int line, uint8_t *src, 
uint8_t *tgt, vbi_pixfmt fmt)
 {
 vbi_bit_slicer slicer;
 
-vbi_bit_slicer_init(, 720, 1350, 6937500, 6937500, 0x00e4, 
0x, 18, 6, 42 * 8, VBI_MODULATION_NRZ_MSB, VBI_PIXFMT_UYVY);
+vbi_bit_slicer_init(, 720, 1350, 6937500, 6937500, 0x00e4, 
0x, 18, 6, 42 * 8, VBI_MODULATION_NRZ_MSB, fmt);
 
 if (vbi_bit_slice(, src, tgt + 4) == FALSE)
-return -1;
+return tgt;
 
-tgt[0] = 0x02; // data_unit_id
-tgt[1] = 0x2c; // data_unit_length
-tgt[2] = calc_parity_and_line_offset(line); // field_parity, line_offset
-tgt[3] = 0xe4; // framing code
+fill_data_unit_head(line, tgt);
 
-return 0;
+return tgt + 46;
+}
+
+static uint8_t* teletext_data_unit_from_vbi_data_10bit(int line, uint8_t *src, 
uint8_t *tgt)
+{
+uint8_t y[720];
+uint8_t *py = y;
+uint8_t *pend = y + 720;
+/* The 10-bit VBI data is packed in V210, but libzvbi only supports 8-bit,
+ * so we extract the 8 MSBs of the luma component, that is enough for
+ * teletext bit slicing. */
+while (py < pend) {
+*py++ = (src[1] >> 4) + ((src[2] & 15) << 4);
+*py++ = (src[4] >> 2) + ((src[5] & 3 ) << 6);
+*py++ = (src[6] >> 6) + ((src[7] & 63) << 2);
+src += 8;
+}
+return teletext_data_unit_from_vbi_data(line, y, tgt, VBI_PIXFMT_YUV420);
 }
 #endif
 
@@ -359,7 +382,7 @@ HRESULT decklink_input_callback::VideoInputFrameArrived(
 //fprintf(stderr,"Video Frame size %d ts %d\n", pkt.size, pkt.pts);
 
 #if CONFIG_LIBZVBI
-if (!no_video && ctx->teletext_lines && videoFrame->GetPixelFormat() 
== bmdFormat8BitYUV && videoFrame->GetWidth() == 720) {
+if (!no_video && ctx->teletext_lines) {
 IDeckLinkVideoFrameAncillary *vanc;
 AVPacket txt_pkt;
 uint8_t txt_buf0[1611]; // max 35 * 46 bytes decoded teletext 
lines + 1 byte data_identifier
@@ -368,16 +391,22 @@ HRESULT decklink_input_callback::VideoInputFrameArrived(
 if (videoFrame->GetAncillaryData() == S_OK) {
 int i;
 int64_t line_mask = 1;
+BMDPixelFormat vanc_format = vanc->GetPixelFormat();
 txt_buf[0] = 0x10;// data_identifier - EBU_data
 txt_buf++;
-for (i = 6; i < 336; i++, line_mask <<= 1) {
-uint8_t *buf;
-if ((ctx->teletext_lines & line_mask) && 
vanc->GetBufferForVerticalBlankingLine(i, (void**)) == S_OK) {
-if (teletext_data_unit_from_vbi_data(i, buf, txt_buf) 
>= 0)
-txt_buf += 46;
+if (ctx->bmd_mode == bmdModePAL && (vanc_format == 
bmdFormat8BitYUV || vanc_format == bmdFormat10BitYUV)) {
+

Re: [FFmpeg-devel] [PATCH] movenc: write clap tag

[Derek Buitenhuis]

On 7/7/2017 10:13 PM, James Almer wrote:
> Isn't this necessary only for files with raw video? As is, this box
> would be written on any mov file with a video stream.

This was addressed a previous email:

http://lists.ffmpeg.org/pipermail/ffmpeg-devel/2017-July/213350.html

I guess the spec is up for interpretation.

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

Re: [FFmpeg-devel] [PATCH 1/1] This change adds an encoder for Camera metadata motion. This is a type of sensor data associated with video, such as GPS, acceleration, gyro, and camera orientation. It

[Hendrik Leppkes]

On Sat, Jul 8, 2017 at 12:24 AM, Louis O'Bryan
 wrote:
>
> The use case is to write data to a new stream in the mp4 container. The
> encoder isn't changing the data. This data would reside in the same mp4
> container as video and audio streams. Are you suggesting there is a way I
> can accomplish that task without creating this encoder (and if so, how)?

You don't need an encoder to create AVPackets, you can just do that
yourself, even more so if the encoder apparently doesn't do any
encoding.
So why does it need encoding in the first place? Why can't you just
pass the finished data in an AVPacket to the mov muxer? Thats how data
streams are supposed to work.

>
>> +static av_cold int encode_init(AVCodecContext *avctx) {
>> > +// Use dummy values for the height and width.
>> > +avctx->width = DUMMY_ENCODER_SIZE;
>> > +avctx->height = DUMMY_ENCODER_SIZE;
>> > +avctx->max_pixels = DUMMY_ENCODER_SIZE;
>> What? This makes no sense.
>
> Using avcodec_encode_video2() seems to require that the width and height be
> nonzero. What is the recommended way to avoid that?
>

Clearly you should not use a video encoder to encode something thats
not a video. This entire approach is not acceptable.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/1] This change adds an encoder for Camera metadata motion. This is a type of sensor data associated with video, such as GPS, acceleration, gyro, and camera orientation. It

[Louis O'Bryan]

Thanks for the feedback.

I don't think AVFrame.pkt_size should be used for any real purpose.
> It's a hack and doesn't really have anything to do with AVFrame data
> (the idea is that it corresponds to the source packet, but it's only a
> hint at best).

The AVFrame format is a raw byte stream with a packet type and variable
length depending on the data type. What is the best way to indicate the
data size, given that this isn't audio or video?

What is this encoder, which encodes nothing, supposed to be used for?

The use case is to write data to a new stream in the mp4 container. The
encoder isn't changing the data. This data would reside in the same mp4
container as video and audio streams. Are you suggesting there is a way I
can accomplish that task without creating this encoder (and if so, how)?

> +static av_cold int encode_init(AVCodecContext *avctx) {
> > +// Use dummy values for the height and width.
> > +avctx->width = DUMMY_ENCODER_SIZE;
> > +avctx->height = DUMMY_ENCODER_SIZE;
> > +avctx->max_pixels = DUMMY_ENCODER_SIZE;
> What? This makes no sense.

Using avcodec_encode_video2() seems to require that the width and height be
nonzero. What is the recommended way to avoid that?


On Fri, Jul 7, 2017 at 3:45 AM, wm4  wrote:

> On Thu,  6 Jul 2017 17:36:39 -0700
> "Louis O'Bryan"  wrote:
>
> > From: Louis O'Bryan 
> >
> > Signed-off-by: Louis O'Bryan 
> > ---
> >  Changelog   |   1 +
> >  doc/general.texi|   2 +
> >  libavcodec/Makefile |   1 +
> >  libavcodec/allcodecs.c  |   3 +
> >  libavcodec/avcodec.h|   1 +
> >  libavcodec/cammenc.c| 299 ++
> ++
> >  libavcodec/codec_desc.c |   6 +
> >  libavcodec/version.h|   4 +-
> >  libavformat/isom.c  |   6 +
> >  libavformat/movenc.c| 200 +---
> >  10 files changed, 431 insertions(+), 92 deletions(-)
> >  create mode 100644 libavcodec/cammenc.c
> >
> > diff --git a/Changelog b/Changelog
> > index a8726c6736..5f98385b53 100644
> > --- a/Changelog
> > +++ b/Changelog
> > @@ -2,6 +2,7 @@ Entries are sorted chronologically from oldest to
> youngest within each release,
> >  releases are sorted from youngest to oldest.
> >
> >  version :
> > +- Camera metadata motion encoder
> >  - deflicker video filter
> >  - doubleweave video filter
> >  - lumakey video filter
> > diff --git a/doc/general.texi b/doc/general.texi
> > index 8f582d586f..06996c81e8 100644
> > --- a/doc/general.texi
> > +++ b/doc/general.texi
> > @@ -912,6 +912,8 @@ following image formats are supported:
> >  @tab part of LCL, encoder experimental
> >  @item Zip Motion Blocks Video  @tab   X @tab  X
> >  @tab Encoder works only in PAL8.
> > +@item Camera metadata motion@tab X  @tab
> > +@tab Encoder for camera sensor data.
> >  @end multitable
> >
> >  @code{X} means that encoding (resp. decoding) is supported.
> > diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> > index b440a00746..306cc793ee 100644
> > --- a/libavcodec/Makefile
> > +++ b/libavcodec/Makefile
> > @@ -680,6 +680,7 @@ OBJS-$(CONFIG_ZLIB_DECODER)+= lcldec.o
> >  OBJS-$(CONFIG_ZLIB_ENCODER)+= lclenc.o
> >  OBJS-$(CONFIG_ZMBV_DECODER)+= zmbv.o
> >  OBJS-$(CONFIG_ZMBV_ENCODER)+= zmbvenc.o
> > +OBJS-$(CONFIG_CAMERA_MOTION_METADATA_ENCODER) += cammenc.o
> >
> >  # (AD)PCM decoders/encoders
> >  OBJS-$(CONFIG_PCM_ALAW_DECODER)   += pcm.o
> > diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
> > index 0243f47358..eff51f4042 100644
> > --- a/libavcodec/allcodecs.c
> > +++ b/libavcodec/allcodecs.c
> > @@ -651,6 +651,9 @@ static void register_all(void)
> >  REGISTER_DECODER(XBIN,  xbin);
> >  REGISTER_DECODER(IDF,   idf);
> >
> > +/* data */
> > +REGISTER_ENCODER(CAMERA_MOTION_METADATA, camera_motion_metadata);
> > +
> >  /* external libraries, that shouldn't be used by default if one of
> the
> >   * above is available */
> >  REGISTER_ENCDEC (LIBOPENH264,   libopenh264);
> > diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> > index b697afa0ae..622383f453 100644
> > --- a/libavcodec/avcodec.h
> > +++ b/libavcodec/avcodec.h
> > @@ -681,6 +681,7 @@ enum AVCodecID {
> >  AV_CODEC_ID_DVD_NAV,
> >  AV_CODEC_ID_TIMED_ID3,
> >  AV_CODEC_ID_BIN_DATA,
> > +AV_CODEC_ID_CAMERA_MOTION_METADATA,
> >
> >
> >  AV_CODEC_ID_PROBE = 0x19000, ///< codec_id is not known (like
> AV_CODEC_ID_NONE) but lavf should attempt to identify it
> > diff --git a/libavcodec/cammenc.c b/libavcodec/cammenc.c
> > new file mode 100644
> > index 00..d7592ab69d
> > --- /dev/null
> > +++ b/libavcodec/cammenc.c
> > @@ -0,0 +1,299 @@
> > +/*
> > + * Reference implementation for the CAMM Metadata encoder.
> > + * Encodes sensor data for 360-degree cameras such as
> > + * GPS, gyro, and 

Re: [FFmpeg-devel] [PATCH 1/1] This change adds an encoder for Camera metadata motion. This is a type of sensor data associated with video, such as GPS, acceleration, gyro, and camera orientation. It

[Reimar Döffinger]

I don't know what you are trying to achieve, but an encoder that just copies 
things through makes no sense.
If it's just about validating or logging some kind of parser or 
(bitstream-)filter probably makes more sense.
Even if doing it as encoder, why should it be a video encoder instead of 
something like raw (ok, not sure we support that) or subtitle?

> +// Sizes of each type of metadata.
> +static int metadata_type_sizes[] = {
> +  3 * sizeof(float),
> +  2 * sizeof(uint64_t),
> +  3 * sizeof(float),
> +  3 * sizeof(float),
> +  3 * sizeof(float),
> +  3 * sizeof(float),
> +  3 * sizeof(double) + sizeof(uint32_t) + 7 * sizeof(float),
> +  3 * sizeof(float)
> +};
> +
> +static int min_packet_size = sizeof(uint16_t) * 2;

These miss "const".
And as someone else mentioned, all the data reading needs to use special 
functions.
If it's still the same as when I was more active, that includes the float 
values.
Not all platforms use IEEE floats and there used to be platforms where float 
values had an endiannes different from the integers etc.
You should probably also consider if you validate against things like M_PI / 4 
how this deals with rounding errors.
If you had a spec, you would probably have the exact bit pattern of the minimum 
and maximum and you should then do a total order comparison against that, 
preferably without even converting to double.

> +static int validate_latitude(AVCodecContext *avctx, double latitude) {
> +if (latitude < -M_PI / 4 || latitude > M_PI / 4) {
> +av_log(avctx, AV_LOG_ERROR,
> +   "Latitude %f is not in bounds (%f, %f)\n",
> +   latitude, -M_PI / 4, M_PI / 4);
> +return 0;
> +}

This one in particular will allow all NaNs as valid, which I suspect is not the 
intention, and if it is should be made more explicit.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avformat: remove obsolete commented-out DEBUG define

[Michael Niedermayer]

On Fri, Jul 07, 2017 at 09:43:22AM +0200, Tobias Rapp wrote:
> Signed-off-by: Tobias Rapp 
> ---
>  libavformat/avienc.c  | 2 --
>  libavformat/segment.c | 2 --
>  2 files changed, 4 deletions(-)
> 
> diff --git a/libavformat/avienc.c b/libavformat/avienc.c
> index e8c0c71..da3d3de 100644
> --- a/libavformat/avienc.c
> +++ b/libavformat/avienc.c
> @@ -19,8 +19,6 @@
>   * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 
> USA
>   */
>  
> -//#define DEBUG
> -
>  #include 
>  
>  #include "avformat.h"

LGTM

thx

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

No human being will ever know the Truth, for even if they happen to say it
by chance, they would not even known they had done so. -- Xenophanes


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

Re: [FFmpeg-devel] [PATCH] pthread_frame: save the FF_DEBUG_THREADS option in FrameThreadContext.

[Ronald S. Bultje]

Hi,

On Fri, Jul 7, 2017 at 5:30 PM, Wan-Teh Chang 
wrote:

> Note: I suspect we can simply delete the following line from
> update_context_from_user() in libavcodec/pthread_frame.c:
>
> dst->debug= src->debug;
>
> That also fixes the tsan warning, but it'll take more time to
> investigate whether it is necessary to update the |debug| field from
> the user's AVCodecContext (src).
>
> That line in update_context_from_user() was added in the initial
> commit of libavcodec/pthread.c:
>
> http://git.videolan.org/?p=ffmpeg.git;a=commit;h=
> 37b00b47cbeecd66bb34c5c7c534d016d6e8da24
>
> Does any user actually modify avctx->debug after the avcodec_open2() call?


To sync values of debug between worker threads if the user dynamically
toggles bits in this flag.

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

Re: [FFmpeg-devel] [PATCH] pthread_frame: save the FF_DEBUG_THREADS option in FrameThreadContext.

[Wan-Teh Chang]

Note: I suspect we can simply delete the following line from
update_context_from_user() in libavcodec/pthread_frame.c:

dst->debug= src->debug;

That also fixes the tsan warning, but it'll take more time to
investigate whether it is necessary to update the |debug| field from
the user's AVCodecContext (src).

That line in update_context_from_user() was added in the initial
commit of libavcodec/pthread.c:

http://git.videolan.org/?p=ffmpeg.git;a=commit;h=37b00b47cbeecd66bb34c5c7c534d016d6e8da24

Does any user actually modify avctx->debug after the avcodec_open2() call?

Thanks,
Wan-Teh Chang
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 0/1] Add new encoder for camera sensor metadata

[Michael Niedermayer]

On Fri, Jul 07, 2017 at 02:25:20AM +0100, Derek Buitenhuis wrote:
> On 7/7/2017 1:36 AM, Louis O'Bryan wrote:
> > I am adding a new encoder for camera sensor metadata. This is an
> > implementation of a not-yet-published open standard for adding camera
> > sensor data to mp4 containers, including the GPS, acceleration, gyro,
> > and camera orientation.
> > After this change is submitted, I will be sending patches for a decoder
> > and usage samples under doc/examples for (de)muxing video/audio/camera 
> > sensor
> > data.
> 
> I think it's probably unlikely this will be pushed with no public 
> standard/draft,
> demuxer, or examples. I, for one, am not a fan of this approach. It feels like
> pushing a proprietary format with no known public samples into a FOSS 
> codebase.
> 
> If this is more of an RFC, though, then that works, IMO.

+1


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

If you drop bombs on a foreign country and kill a hundred thousand
innocent people, expect your government to call the consequence
"unprovoked inhuman terrorist attacks" and use it to justify dropping
more bombs and killing more people. The technology changed, the idea is old.


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

Re: [FFmpeg-devel] [PATCH 02/12] hdsenc: Add missing goto statement

[Michael Niedermayer]

On Thu, Jul 06, 2017 at 07:28:32PM +0100, Derek Buitenhuis wrote:
> Signed-off-by: Derek Buitenhuis 
> ---
>  libavformat/hdsenc.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/libavformat/hdsenc.c b/libavformat/hdsenc.c
> index 347df83a51..c362a75d8b 100644
> --- a/libavformat/hdsenc.c
> +++ b/libavformat/hdsenc.c
> @@ -421,6 +421,7 @@ static int hds_write_header(AVFormatContext *s)
>  av_log(s, AV_LOG_WARNING,
> "No video stream in output stream %d and no min frag 
> duration set\n", i);
>  ret = AVERROR(EINVAL);
> +goto fail;

If this is applied then the av_log level should be changed from
AV_LOG_WARNING to AV_LOG_ERROR

without checking history, the warning makes me think it was intended
to not fail

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

There will always be a question for which you do not know the correct answer.


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

Re: [FFmpeg-devel] [PATCH] pthread_frame: make accesses to debug field be protected by owner lock.

[Wan-Teh Chang]

Hi Ronald,

On Wed, Jul 5, 2017 at 7:24 PM, Ronald S. Bultje  wrote:
> Hi Wan-Teh,
>
> On Wed, Jul 5, 2017 at 8:08 PM, Wan-Teh Chang  wrote:
>>
>> Hi Ronald,
>>
>> On Wed, Jul 5, 2017 at 3:31 PM, Ronald S. Bultje 
>> wrote:
>> > Hi Wan-Teh,
>> >
>> > On Wed, Jul 5, 2017 at 6:12 PM, Wan-Teh Chang  wrote:
>> >>
>> >> Thank you for all the tsan warning fixes. In the meantime, it would be
>> >> good to revert 2e664b9c1e73c80aab91070c1eb7676f04bdd12d to avoid
>> >> confusion.
>> >
>> >
>> > Why? I believe it fixes a subset of the issue.
>>
>> Could you explain why it fixes a subset of the issue?
>
> From what I remember, I was (before this patch) semi-reliably able to
> reproduce the issue locally, and it went away after. I've observed the same
> thing in the relevant fate station, where before the patch, this warning
> occurred semi-reliably in 3-4 tests per run, and after the patch it
> sometimes occurs in 1 test per run. I understand this doesn't satisfy you
> but I currently don't want to dig through the code to figure out why I
> thought this was a good idea, I have other things that take priority.
> Reverting the patch will require me to dig through this code also, and I
> really don't see the gain.

In addition to avoiding confusion, reverting the patch will move the
av_log() statements outside the lock. Since I don't use those two
av_log() statements, I won't insist on reverting the patch.

> Some more thoughts:
> - I don't think we want to grab a second lock for something trivial like
> this (e.g. grabbing progress_mutex when copying the debug field in
> update_context_from_user);
> - making the debug flags field atomic will be difficult because it's public
> API, and I don't think we're ready to expose C11 types in our public API;
> - I suppose you could make a private (inside PerThreadContext, which allows
> it to be atomic) copy of debug intended for cross-threading purposes and use
> that here.

After studying this problem for two more days, I implemented a variant
of your last suggestion:

http://ffmpeg.org/pipermail/ffmpeg-devel/2017-July/213384.html

Let's continue the discussion in that email thread.

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

Re: [FFmpeg-devel] [PATCH] movenc: write clap tag

[James Almer]

On 7/7/2017 12:32 PM, Derek Buitenhuis wrote:
> On 7/7/2017 3:20 AM, Dave Rice wrote:
>> Resolves https://trac.ffmpeg.org/ticket/6145 and writes a clap atom that is 
>> coincident with the frames width and height.
>>
>>
>> From 23d80d0d47829fed61e817b1e7c3f6d420c9ab5c Mon Sep 17 00:00:00 2001
>> From: Dave Rice 
>> Date: Thu, 6 Jul 2017 21:12:38 -0400
>> Subject: [PATCH] movenc: write clap tag
>>
>> ---
>>  libavformat/movenc.c | 19 +++
>>  1 file changed, 19 insertions(+)
> 
> Whoever pushes, add the ticket number into the commit message.
> 
> [...]
> 
>> +avio_wb32(pb, 0); /* horizOff_N (= 0) */
>> +avio_wb32(pb, 1); /* horizOff_D (= 1) */
>> +avio_wb32(pb, 0); /* vertOff_N (= 0) */
>> +avio_wb32(pb, 1); /* vertOff_D (= 1) */
> 
> Near as I can tell, these related the active image area? Is it useful to ever 
> set
> this to anything but 0 (aka the whole image is active)? Was thinking of stuff 
> like
> analogue video, a la http://chromashift.org/aspectratio/. I assume the answer 
> is
> probably 'no'.
> 
> Patch itself LGTM.

Isn't this necessary only for files with raw video? As is, this box
would be written on any mov file with a video stream.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] Add A53 Closed Captions to MPEG header if they are available.

[John P Poet]

On Wed, Jul 5, 2017 at 3:30 PM Marton Balint  wrote:

>
> On Fri, 9 Jun 2017, Devin Heitmueller wrote:
>
> > Hello Marton,
> >
> > On Tue, Jun 6, 2017 at 5:45 PM, Marton Balint  wrote:
> >
> >> As far as I remember multiple side data of the same type is not
> something we
> >> wanted to support. Why do you need it? Can't a single
> AV_FRAME_DATA_A53_CC
> >> side data packet contain many CC entries?
> >
> > Could you please expand on where this was discussed?  Is there any
> > design documentation for side data infrastructure that's been
> > introduced into ffmpeg?  Is there some list of other known design
> > constraints/limitations?
> >
> > While I agree it would be great to simply say that you should never
> > have multiple side data items of the same type, I'm not sure how
> > realistic that is.  It would be helpful if I could better understand
> > the rationale in that thinking.
>
> I guess there are two reasons for prohibiting same type side data:
>
> - existing API implicitly gave the impression that is is not supported
>
> Av_frame_get_side_data gives you no possibility to retrieve multiple side
> data of the same type. API users might assumed that the reason for that is
> because it is not possible/permitted.
>
> - consistency with packet/stream side data types
>
> For stream side data is is already explicitly disallowed, API would be
> inconsistent if multiple side data entries of the same types were possible
> for one kind of side data (frame), but not for the other (stream).
>
> > I'm starting a rather large project which will likely stretch the
> > design for side data in order to support a variety of other ancillary
> > data protocols (e.g. SCTE-104, SMPTE 2038, etc), and it would be great
> > to better understand where the constraints are (so I can either plan
> > to address those issues, or if too significant then choose a different
> > multimedia framework to work with before making a significant
> > investment building out a bunch of features in ffmpeg).
>
> Yeah, you definitely have to think about this, depending on your actual
> goals libavcodec/format might not be the best choice for complex ancillary
> data handling.
>
> It seems multiple side data of the same type is not going to pick up
> support from other developers, so you have to come up with something else
> to move forward.
>

Hi Marton,

If I understand what you and others are saying, AVFrameSideDataType should
not be used to hold "MPEG user data", such as CEA-708, as described here
https://www.atsc.org/wp-content/uploads/2015/03/a_53-Part-4-2009.pdf in
section 6.2.

What I am unclear about, is the proper way to handle MPEG user data.  Are
you saying FFmpeg has no desire to handle this type of data, and that I
need to look to another project?  Or, are you just saying I need to define
a new structure (AVFrameUserDataType or AVFrameMPEGUserDataType) for this
type of data?

Thank you,

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

[FFmpeg-devel] [PATCH] pthread_frame: save the FF_DEBUG_THREADS option in FrameThreadContext.

[Wan-Teh Chang]

Add the debug_threads boolean field to FrameThreadContext. The
debug_threads field records whether the FF_DEBUG_THREADS bit is set in
the debug field of the avctx passed to ff_frame_thread_init(). The
debug_threads field is set when avcodec_open2() is called, never changes
thereafter, and applies to all the decoding threads owned by a
FrameThreadContext. The current code allows different decoding threads
to have different FF_DEBUG_THREADS options, but that does not seem
useful.

This fixes the tsan warning that
2e664b9c1e73c80aab91070c1eb7676f04bdd12d attempted to fix:

WARNING: ThreadSanitizer: data race (pid=452658)
  Write of size 4 at 0x7b640003f4fc by main thread (mutexes: write M248499):
#0 update_context_from_user [..]/libavcodec/pthread_frame.c:335:19 
(5ab42bb1a6f4b068d7863dabe9b2bacc+0xe73859)
[..]
  Previous read of size 4 at 0x7b640003f4fc by thread T130 (mutexes: write 
M248502, write M248500):
#0 ff_thread_await_progress [..]/libavcodec/pthread_frame.c:591:26 
(5ab42bb1a6f4b068d7863dabe9b2bacc+0xe749a1)

Signed-off-by: Wan-Teh Chang 
---
 libavcodec/pthread_frame.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c
index 363b139f71..eb4d2d9458 100644
--- a/libavcodec/pthread_frame.c
+++ b/libavcodec/pthread_frame.c
@@ -133,6 +133,7 @@ typedef struct FrameThreadContext {
 * Set for the first N packets, where N is 
the number of threads.
 * While it is set, 
ff_thread_en/decode_frame won't return any results.
 */
+int debug_threads; ///< Set if the FF_DEBUG_THREADS option is 
set.
 } FrameThreadContext;
 
 #define THREAD_SAFE_CALLBACKS(avctx) \
@@ -566,7 +567,7 @@ void ff_thread_report_progress(ThreadFrame *f, int n, int 
field)
 p = f->owner[field]->internal->thread_ctx;
 
 pthread_mutex_lock(>progress_mutex);
-if (f->owner[field]->debug_DEBUG_THREADS)
+if (p->parent->debug_threads)
 av_log(f->owner[field], AV_LOG_DEBUG,
"%p finished %d field %d\n", progress, n, field);
 
@@ -588,7 +589,7 @@ void ff_thread_await_progress(ThreadFrame *f, int n, int 
field)
 p = f->owner[field]->internal->thread_ctx;
 
 pthread_mutex_lock(>progress_mutex);
-if (f->owner[field]->debug_DEBUG_THREADS)
+if (p->parent->debug_threads)
 av_log(f->owner[field], AV_LOG_DEBUG,
"thread awaiting %d field %d from %p\n", n, field, progress);
 while (atomic_load_explicit([field], memory_order_relaxed) < n)
@@ -763,6 +764,7 @@ int ff_frame_thread_init(AVCodecContext *avctx)
 
 fctx->async_lock = 1;
 fctx->delaying = 1;
+fctx->debug_threads = (avctx->debug & FF_DEBUG_THREADS) != 0;
 
 for (i = 0; i < thread_count; i++) {
 AVCodecContext *copy = av_malloc(sizeof(AVCodecContext));
-- 
2.13.2.725.g09c95d1e9-goog

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

Re: [FFmpeg-devel] [PATCH 1/3] avcodec/get_bits: add cached bitstream reader

[Ronald S. Bultje]

Hi,

On Fri, Jul 7, 2017 at 4:43 PM, Paul B Mahol  wrote:

> On 7/7/17, Ronald S. Bultje  wrote:
> > (I'm assuming the low-level interface no longer works with the cached
> > reader, so can we prevent users from accessing these macros unless
> > cached=1?)
>
> They are not supposed to work, Do you mean to not define such macros
> if cached is defined?


Yes.

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

Re: [FFmpeg-devel] [PATCH 1/3] avcodec/get_bits: add cached bitstream reader

[Paul B Mahol]

On 7/7/17, Ronald S. Bultje  wrote:
> Hi,
>
> On Fri, Jul 7, 2017 at 2:48 PM, Paul B Mahol  wrote:
>
>>  typedef struct GetBitContext {
>>  const uint8_t *buffer, *buffer_end;
>> +#ifdef CACHED_BITSTREAM_READER
>> +uint64_t cache;
>> +unsigned bits_left;
>> +#endif
>>
>
> Can you post some stats (from relevant systems, ideally, e.g. 32-bit binary
> on x86, or 32-bit arm) on how a 32bit cache performs compared to a 64bit
> cache on systems with HAVE_FAST_64BIT=0?

I can only post results from 64bit x86 with gcc 6.2.0.

>
>
>> +static inline void refill_32(GetBitContext *s)
>>
> [..]
>
>> +#ifdef BITSTREAM_READER_LE
>> +s->cache   = (uint64_t)AV_RL32(s->buffer + (s->index >> 3)) <<
>> s->cache | s->cache;
>>
>
> As said on IRC: middle s->cache should be s->bits_left.
>
> Overall very nice improvement, I would in particular not be surprised if
> this is generally faster for almost all users, except those using the
> lower-level macros (things like SHOW_BITS() etc.) in the old interface. If
> that's true, it may be positive to enable this by default and disable only
> for those using the low-level interface.
>
> (I'm assuming the low-level interface no longer works with the cached
> reader, so can we prevent users from accessing these macros unless
> cached=1?)

They are not supposed to work, Do you mean to not define such macros
if cached is defined?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/3] avcodec/get_bits: add cached bitstream reader

[Ronald S. Bultje]

Hi,

On Fri, Jul 7, 2017 at 2:48 PM, Paul B Mahol  wrote:

>  typedef struct GetBitContext {
>  const uint8_t *buffer, *buffer_end;
> +#ifdef CACHED_BITSTREAM_READER
> +uint64_t cache;
> +unsigned bits_left;
> +#endif
>

Can you post some stats (from relevant systems, ideally, e.g. 32-bit binary
on x86, or 32-bit arm) on how a 32bit cache performs compared to a 64bit
cache on systems with HAVE_FAST_64BIT=0?


> +static inline void refill_32(GetBitContext *s)
>
[..]

> +#ifdef BITSTREAM_READER_LE
> +s->cache   = (uint64_t)AV_RL32(s->buffer + (s->index >> 3)) <<
> s->cache | s->cache;
>

As said on IRC: middle s->cache should be s->bits_left.

Overall very nice improvement, I would in particular not be surprised if
this is generally faster for almost all users, except those using the
lower-level macros (things like SHOW_BITS() etc.) in the old interface. If
that's true, it may be positive to enable this by default and disable only
for those using the low-level interface.

(I'm assuming the low-level interface no longer works with the cached
reader, so can we prevent users from accessing these macros unless
cached=1?)

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

Re: [FFmpeg-devel] [PATCH 2/3] avcodec/utvideodec: use cached bitstream reader

[Carl Eugen Hoyos]

> Am 07.07.2017 um 20:48 schrieb Paul B Mahol :
> 
> Signed-off-by: Paul B Mahol 
> ---
> libavcodec/utvideodec.c | 1 +
> 1 file changed, 1 insertion(+)
> 
> diff --git a/libavcodec/utvideodec.c b/libavcodec/utvideodec.c
> index 40c1277..c57d727 100644
> --- a/libavcodec/utvideodec.c
> +++ b/libavcodec/utvideodec.c
> @@ -27,6 +27,7 @@
> #include 
> #include 
> 
> +#define CACHED_BITSTREAM_READER 1

Please add a little information about the speed improvement to the commit 
message.

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

[FFmpeg-devel] [PATCH 3/3] doc/libav-merge: bitstream reader is now merged

[Paul B Mahol]

Signed-off-by: Paul B Mahol 
---
 doc/libav-merge.txt | 1 -
 1 file changed, 1 deletion(-)

diff --git a/doc/libav-merge.txt b/doc/libav-merge.txt
index 690c4ef..56d6109 100644
--- a/doc/libav-merge.txt
+++ b/doc/libav-merge.txt
@@ -96,7 +96,6 @@ Stuff that didn't reach the codebase:
   - e7078e842 hevcdsp: add x86 SIMD for MC
 - VAAPI VP8 decode hwaccel (currently under review: 
http://ffmpeg.org/pipermail/ffmpeg-devel/2017-February/thread.html#207348)
 - Removal of the custom atomic API (5cc0057f49, see 
http://ffmpeg.org/pipermail/ffmpeg-devel/2017-March/209003.html)
-- new bitstream reader (see 
http://ffmpeg.org/pipermail/ffmpeg-devel/2017-April/209609.html)
 - use of the bsf instead of our parser for vp9 superframes (see fa1749dd34)
 
 Collateral damage that needs work locally:
-- 
2.9.3

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

[FFmpeg-devel] [PATCH 2/3] avcodec/utvideodec: use cached bitstream reader

[Paul B Mahol]

Signed-off-by: Paul B Mahol 
---
 libavcodec/utvideodec.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libavcodec/utvideodec.c b/libavcodec/utvideodec.c
index 40c1277..c57d727 100644
--- a/libavcodec/utvideodec.c
+++ b/libavcodec/utvideodec.c
@@ -27,6 +27,7 @@
 #include 
 #include 
 
+#define CACHED_BITSTREAM_READER 1
 #define UNCHECKED_BITSTREAM_READER 1
 
 #include "libavutil/intreadwrite.h"
-- 
2.9.3

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

[FFmpeg-devel] [PATCH 1/3] avcodec/get_bits: add cached bitstream reader

[Paul B Mahol]

Signed-off-by: Paul B Mahol 
---
 libavcodec/get_bits.h | 205 +++---
 1 file changed, 196 insertions(+), 9 deletions(-)

diff --git a/libavcodec/get_bits.h b/libavcodec/get_bits.h
index c530015..8a9021a 100644
--- a/libavcodec/get_bits.h
+++ b/libavcodec/get_bits.h
@@ -1,5 +1,6 @@
 /*
- * copyright (c) 2004 Michael Niedermayer 
+ * Copyright (c) 2004 Michael Niedermayer 
+ * Copyright (c) 2016 Alexandra Hájková
  *
  * This file is part of FFmpeg.
  *
@@ -54,6 +55,10 @@
 
 typedef struct GetBitContext {
 const uint8_t *buffer, *buffer_end;
+#ifdef CACHED_BITSTREAM_READER
+uint64_t cache;
+unsigned bits_left;
+#endif
 int index;
 int size_in_bits;
 int size_in_bits_plus8;
@@ -106,7 +111,9 @@ typedef struct GetBitContext {
  * For examples see get_bits, show_bits, skip_bits, get_vlc.
  */
 
-#ifdef LONG_BITSTREAM_READER
+#ifdef CACHED_BITSTREAM_READER
+#   define MIN_CACHE_BITS 64
+#elif defined LONG_BITSTREAM_READER
 #   define MIN_CACHE_BITS 32
 #else
 #   define MIN_CACHE_BITS 25
@@ -198,7 +205,11 @@ typedef struct GetBitContext {
 
 static inline int get_bits_count(const GetBitContext *s)
 {
+#ifdef CACHED_BITSTREAM_READER
+return s->index - s->bits_left;
+#else
 return s->index;
+#endif
 }
 
 static inline void skip_bits_long(GetBitContext *s, int n)
@@ -210,6 +221,68 @@ static inline void skip_bits_long(GetBitContext *s, int n)
 #endif
 }
 
+static inline void refill_32(GetBitContext *s)
+{
+#ifdef CACHED_BITSTREAM_READER
+if (s->buffer + (s->index >> 3) >= s->buffer_end)
+return;
+
+#ifdef BITSTREAM_READER_LE
+s->cache   = (uint64_t)AV_RL32(s->buffer + (s->index >> 3)) << 
s->cache | s->cache;
+#else
+s->cache   = s->cache | (uint64_t)AV_RB32(s->buffer + (s->index >> 3)) 
<< (32 - s->bits_left);
+#endif
+s->index += 32;
+s->bits_left += 32;
+#endif
+}
+
+static inline void refill_64(GetBitContext *s)
+{
+#ifdef CACHED_BITSTREAM_READER
+#if !UNCHECKED_BITSTREAM_READER
+if (s->buffer + (s->index >> 3) >= s->buffer_end)
+return;
+#endif
+
+#ifdef BITSTREAM_READER_LE
+s->cache = AV_RL64(s->buffer + (s->index >> 3));
+#else
+s->cache = AV_RB64(s->buffer + (s->index >> 3));
+#endif
+s->index += 64;
+s->bits_left = 64;
+#endif
+}
+
+#ifdef CACHED_BITSTREAM_READER
+static inline uint64_t get_val(GetBitContext *s, unsigned n)
+{
+uint64_t ret;
+av_assert2(n>0 && n<=63);
+#ifdef BITSTREAM_READER_LE
+ret = s->cache & ((UINT64_C(1) << n) - 1);
+s->cache >>= n;
+#else
+ret = s->cache >> (64 - n);
+s->cache <<= n;
+#endif
+s->bits_left -= n;
+return ret;
+}
+#endif
+
+#ifdef CACHED_BITSTREAM_READER
+static inline unsigned show_val(const GetBitContext *s, unsigned n)
+{
+#ifdef BITSTREAM_READER_LE
+return s->cache & ((UINT64_C(1) << n) - 1);
+#else
+return s->cache >> (64 - n);
+#endif
+}
+#endif
+
 /**
  * Read MPEG-1 dc-style VLC (sign bit + mantissa with no MSB).
  * if MSB not set it is negative
@@ -243,30 +316,59 @@ static inline int get_xbits_le(GetBitContext *s, int n)
 return (zero_extend(sign ^ cache, n) ^ sign) - sign;
 }
 
-static inline int get_sbits(GetBitContext *s, int n)
+/**
+ * Read 1-25 bits.
+ */
+static inline unsigned int get_bits(GetBitContext *s, int n)
 {
+#ifdef CACHED_BITSTREAM_READER
+register int tmp = 0;
+#ifdef BITSTREAM_READER_LE
+uint64_t left = 0;
+#endif
+
+av_assert2(n>0 && n<=32);
+if (n > s->bits_left) {
+n -= s->bits_left;
+#ifdef BITSTREAM_READER_LE
+left = s->bits_left;
+#endif
+tmp = get_val(s, s->bits_left);
+refill_32(s);
+}
+
+#ifdef BITSTREAM_READER_LE
+tmp = get_val(s, n) << left | tmp;
+#else
+tmp = get_val(s, n) | tmp << n;
+#endif
+
+#else
 register int tmp;
 OPEN_READER(re, s);
 av_assert2(n>0 && n<=25);
 UPDATE_CACHE(re, s);
-tmp = SHOW_SBITS(re, s, n);
+tmp = SHOW_UBITS(re, s, n);
 LAST_SKIP_BITS(re, s, n);
 CLOSE_READER(re, s);
+#endif
 return tmp;
 }
 
-/**
- * Read 1-25 bits.
- */
-static inline unsigned int get_bits(GetBitContext *s, int n)
+static inline int get_sbits(GetBitContext *s, int n)
 {
 register int tmp;
+#ifdef CACHED_BITSTREAM_READER
+av_assert2(n>0 && n<=25);
+tmp = sign_extend(get_bits(s, n), n);
+#else
 OPEN_READER(re, s);
 av_assert2(n>0 && n<=25);
 UPDATE_CACHE(re, s);
-tmp = SHOW_UBITS(re, s, n);
+tmp = SHOW_SBITS(re, s, n);
 LAST_SKIP_BITS(re, s, n);
 CLOSE_READER(re, s);
+#endif
 return tmp;
 }
 
@@ -296,22 +398,65 @@ static inline unsigned int get_bits_le(GetBitContext *s, 
int n)
 static inline unsigned int show_bits(GetBitContext *s, int n)
 {
 register int tmp;
+#ifdef CACHED_BITSTREAM_READER
+if (n > s->bits_left)
+refill_32(s);
+
+tmp = show_val(s, n);
+#else
 OPEN_READER_NOSIZE(re, s);
 av_assert2(n>0 && n<=25);
 

Re: [FFmpeg-devel] [PATCH] movenc: write clap tag

[Derek Buitenhuis]

On 7/7/2017 4:32 PM, Derek Buitenhuis wrote:
> Patch itself LGTM.

Just realized, this will change a bunch of FATE tests, won't it?

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

Re: [FFmpeg-devel] [PATCH 02/12] hdsenc: Add missing goto statement

[Derek Buitenhuis]

On 7/6/2017 11:38 PM, Steven Liu wrote:
> Maybe there have no video stream, but have audio stream, so just give
> user a warning and continue?

Not an expert on the HDS code, but how can you fragment audio with no
min frag size set, and no video stream?

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

Re: [FFmpeg-devel] [PATCH] avformat: remove obsolete commented-out DEBUG define

[Derek Buitenhuis]

On 7/7/2017 8:43 AM, Tobias Rapp wrote:
> Signed-off-by: Tobias Rapp 
> ---
>  libavformat/avienc.c  | 2 --
>  libavformat/segment.c | 2 --
>  2 files changed, 4 deletions(-)

LGTM.

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

Re: [FFmpeg-devel] [PATCH] movenc: write clap tag

[Derek Buitenhuis]

On 7/7/2017 3:20 AM, Dave Rice wrote:
> Resolves https://trac.ffmpeg.org/ticket/6145 and writes a clap atom that is 
> coincident with the frames width and height.
> 
> 
> From 23d80d0d47829fed61e817b1e7c3f6d420c9ab5c Mon Sep 17 00:00:00 2001
> From: Dave Rice 
> Date: Thu, 6 Jul 2017 21:12:38 -0400
> Subject: [PATCH] movenc: write clap tag
> 
> ---
>  libavformat/movenc.c | 19 +++
>  1 file changed, 19 insertions(+)

Whoever pushes, add the ticket number into the commit message.

[...]

> +avio_wb32(pb, 0); /* horizOff_N (= 0) */
> +avio_wb32(pb, 1); /* horizOff_D (= 1) */
> +avio_wb32(pb, 0); /* vertOff_N (= 0) */
> +avio_wb32(pb, 1); /* vertOff_D (= 1) */

Near as I can tell, these related the active image area? Is it useful to ever 
set
this to anything but 0 (aka the whole image is active)? Was thinking of stuff 
like
analogue video, a la http://chromashift.org/aspectratio/. I assume the answer is
probably 'no'.

Patch itself LGTM.

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

Re: [FFmpeg-devel] [PATCH] avfilter/pthread: rewrite implementation

[Muhammad Faiz]

On Fri, Jul 7, 2017 at 4:26 PM, Clément Bœsch  wrote:
> On Fri, Jul 07, 2017 at 09:25:46AM +0700, Muhammad Faiz wrote:
>> Avoid pthread_cond_broadcast that wakes up all workers. Make each of them
>> uses distict mutex/cond.
>>
>
> Please make sure this doesn't make tsan go crazy.

Fixed. Post new patch.

>
> Also, the code is pretty much based on lavc/pthread_slice.c, so you may
> want to keep differences low by applying your changes in lavc as well. I'm
> sure people will be happy if libavcodec gets faster.

I'm currently looking at it.

>
> Interesting benchmarks btw :)
>
> --
> Clément B.
>
> ___
> 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 v2] avfilter/pthread: rewrite implementation

[Muhammad Faiz]

Avoid pthread_cond_broadcast that wakes up all workers. Make each of them
uses distict mutex/cond. Also let main thread help running jobs.

Benchmark using afir with threads=5 and 4096 taps fir:
channels=1:
old:
1849650 decicycles in afir_execute,   2 runs,  0 skips
1525719 decicycles in afir_execute,1024 runs,  0 skips
1546032 decicycles in afir_execute,   16356 runs, 28 skips
new:
1495525 decicycles in afir_execute,   2 runs,  0 skips
 968897 decicycles in afir_execute,1024 runs,  0 skips
 941286 decicycles in afir_execute,   16384 runs,  0 skips

channels=2:
old:
3135485 decicycles in afir_execute,   2 runs,  0 skips
1967158 decicycles in afir_execute,1024 runs,  0 skips
1802430 decicycles in afir_execute,   16364 runs, 20 skips
new:
1864750 decicycles in afir_execute,   2 runs,  0 skips
1437792 decicycles in afir_execute,1024 runs,  0 skips
1183963 decicycles in afir_execute,   16382 runs,  2 skips

channels=4:
old:
4879925 decicycles in afir_execute,   2 runs,  0 skips
3557950 decicycles in afir_execute,1022 runs,  2 skips
3206843 decicycles in afir_execute,   16379 runs,  5 skips
new:
2962320 decicycles in afir_execute,   2 runs,  0 skips
2450430 decicycles in afir_execute,1024 runs,  0 skips
2446219 decicycles in afir_execute,   16383 runs,  1 skips

channels=8:
old:
6032455 decicycles in afir_execute,   2 runs,  0 skips
4838614 decicycles in afir_execute,1023 runs,  1 skips
4720760 decicycles in afir_execute,   16369 runs, 15 skips
new:
5228150 decicycles in afir_execute,   2 runs,  0 skips
4592129 decicycles in afir_execute,1023 runs,  1 skips
4469067 decicycles in afir_execute,   16383 runs,  1 skips

Signed-off-by: Muhammad Faiz 
---
 libavfilter/pthread.c | 197 +++---
 1 file changed, 124 insertions(+), 73 deletions(-)

diff --git a/libavfilter/pthread.c b/libavfilter/pthread.c
index c7a0021..0ba7864 100644
--- a/libavfilter/pthread.c
+++ b/libavfilter/pthread.c
@@ -21,6 +21,7 @@
  * Libavfilter multithreading support
  */
 
+#include 
 #include "config.h"
 
 #include "libavutil/common.h"
@@ -32,61 +33,76 @@
 #include "internal.h"
 #include "thread.h"
 
+typedef struct WorkerContext WorkerContext;
+
 typedef struct ThreadContext {
 AVFilterGraph *graph;
 
-int nb_threads;
-pthread_t *workers;
+int nb_workers;
+WorkerContext *workers;
 avfilter_action_func *func;
 
 /* per-execute parameters */
 AVFilterContext *ctx;
 void *arg;
 int   *rets;
-int nb_jobs;
+unsigned nb_jobs;
 
-pthread_cond_t last_job_cond;
-pthread_cond_t current_job_cond;
-pthread_mutex_t current_job_lock;
-int current_job;
-unsigned int current_execute;
+pthread_mutex_t mutex_done;
+pthread_cond_t cond_done;
+atomic_uint current_job;
+atomic_uint nb_finished_jobs;
 int done;
+int worker_done;
 } ThreadContext;
 
-static void* attribute_align_arg worker(void *v)
+struct WorkerContext {
+ThreadContext   *ctx;
+pthread_t   thread;
+pthread_mutex_t mutex;
+pthread_cond_t  cond;
+int done;
+};
+
+static unsigned run_jobs(ThreadContext *c)
 {
-ThreadContext *c = v;
-int our_job  = c->nb_jobs;
-int nb_threads   = c->nb_threads;
-unsigned int last_execute = 0;
-int ret, self_id;
-
-pthread_mutex_lock(>current_job_lock);
-self_id = c->current_job++;
-
-for (;;) {
-while (our_job >= c->nb_jobs) {
-if (c->current_job == nb_threads + c->nb_jobs)
-pthread_cond_signal(>last_job_cond);
-
-while (last_execute == c->current_execute && !c->done)
-pthread_cond_wait(>current_job_cond, >current_job_lock);
-last_execute = c->current_execute;
-our_job = self_id;
-
-if (c->done) {
-pthread_mutex_unlock(>current_job_lock);
-return NULL;
-}
-}
-pthread_mutex_unlock(>current_job_lock);
+unsigned current_job, nb_finished_jobs = 0;
 
-ret = c->func(c->ctx, c->arg, our_job, c->nb_jobs);
+while (nb_finished_jobs != c->nb_jobs &&
+   (current_job = atomic_fetch_add_explicit(>current_job, 1, 
memory_order_acq_rel)) < c->nb_jobs) {
+int ret = c->func(c->ctx, c->arg, current_job, c->nb_jobs);
 if (c->rets)
-c->rets[our_job % c->nb_jobs] = ret;
+c->rets[current_job] = ret;
+nb_finished_jobs = atomic_fetch_add_explicit(>nb_finished_jobs, 1, 
memory_order_acq_rel) + 1;
+}
+
+return nb_finished_jobs;
+}
+
+static void* attribute_align_arg worker(void *v)
+{
+WorkerContext *w = v;
+ThreadContext *c = w->ctx;
+
+pthread_mutex_lock(>mutex);
+

Re: [FFmpeg-devel] [PATCH 1/1] This change adds an encoder for Camera metadata motion. This is a type of sensor data associated with video, such as GPS, acceleration, gyro, and camera orientation. It

[wm4]

On Thu,  6 Jul 2017 17:36:39 -0700
"Louis O'Bryan"  wrote:

> From: Louis O'Bryan 
> 
> Signed-off-by: Louis O'Bryan 
> ---
>  Changelog   |   1 +
>  doc/general.texi|   2 +
>  libavcodec/Makefile |   1 +
>  libavcodec/allcodecs.c  |   3 +
>  libavcodec/avcodec.h|   1 +
>  libavcodec/cammenc.c| 299 
> 
>  libavcodec/codec_desc.c |   6 +
>  libavcodec/version.h|   4 +-
>  libavformat/isom.c  |   6 +
>  libavformat/movenc.c| 200 +---
>  10 files changed, 431 insertions(+), 92 deletions(-)
>  create mode 100644 libavcodec/cammenc.c
> 
> diff --git a/Changelog b/Changelog
> index a8726c6736..5f98385b53 100644
> --- a/Changelog
> +++ b/Changelog
> @@ -2,6 +2,7 @@ Entries are sorted chronologically from oldest to youngest 
> within each release,
>  releases are sorted from youngest to oldest.
>  
>  version :
> +- Camera metadata motion encoder
>  - deflicker video filter
>  - doubleweave video filter
>  - lumakey video filter
> diff --git a/doc/general.texi b/doc/general.texi
> index 8f582d586f..06996c81e8 100644
> --- a/doc/general.texi
> +++ b/doc/general.texi
> @@ -912,6 +912,8 @@ following image formats are supported:
>  @tab part of LCL, encoder experimental
>  @item Zip Motion Blocks Video  @tab   X @tab  X
>  @tab Encoder works only in PAL8.
> +@item Camera metadata motion@tab X  @tab
> +@tab Encoder for camera sensor data.
>  @end multitable
>  
>  @code{X} means that encoding (resp. decoding) is supported.
> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> index b440a00746..306cc793ee 100644
> --- a/libavcodec/Makefile
> +++ b/libavcodec/Makefile
> @@ -680,6 +680,7 @@ OBJS-$(CONFIG_ZLIB_DECODER)+= lcldec.o
>  OBJS-$(CONFIG_ZLIB_ENCODER)+= lclenc.o
>  OBJS-$(CONFIG_ZMBV_DECODER)+= zmbv.o
>  OBJS-$(CONFIG_ZMBV_ENCODER)+= zmbvenc.o
> +OBJS-$(CONFIG_CAMERA_MOTION_METADATA_ENCODER) += cammenc.o
>  
>  # (AD)PCM decoders/encoders
>  OBJS-$(CONFIG_PCM_ALAW_DECODER)   += pcm.o
> diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
> index 0243f47358..eff51f4042 100644
> --- a/libavcodec/allcodecs.c
> +++ b/libavcodec/allcodecs.c
> @@ -651,6 +651,9 @@ static void register_all(void)
>  REGISTER_DECODER(XBIN,  xbin);
>  REGISTER_DECODER(IDF,   idf);
>  
> +/* data */
> +REGISTER_ENCODER(CAMERA_MOTION_METADATA, camera_motion_metadata);
> +
>  /* external libraries, that shouldn't be used by default if one of the
>   * above is available */
>  REGISTER_ENCDEC (LIBOPENH264,   libopenh264);
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index b697afa0ae..622383f453 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -681,6 +681,7 @@ enum AVCodecID {
>  AV_CODEC_ID_DVD_NAV,
>  AV_CODEC_ID_TIMED_ID3,
>  AV_CODEC_ID_BIN_DATA,
> +AV_CODEC_ID_CAMERA_MOTION_METADATA,
>  
>  
>  AV_CODEC_ID_PROBE = 0x19000, ///< codec_id is not known (like 
> AV_CODEC_ID_NONE) but lavf should attempt to identify it
> diff --git a/libavcodec/cammenc.c b/libavcodec/cammenc.c
> new file mode 100644
> index 00..d7592ab69d
> --- /dev/null
> +++ b/libavcodec/cammenc.c
> @@ -0,0 +1,299 @@
> +/*
> + * Reference implementation for the CAMM Metadata encoder.
> + * Encodes sensor data for 360-degree cameras such as
> + * GPS, gyro, and acceleration. This is stored in a track separate from video
> + * and audio.
> + *
> + * Copyright (c) 2017 Louis O'Bryan
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 
> USA
> + */
> +
> +/**
> + * CAMM Metadata encoder
> + * @author Louis O'Bryan
> + */
> +
> +#include 
> +#include 
> +#include "avcodec.h"
> +#include "internal.h"
> +#include "bytestream.h"
> +#include "libavutil/common.h"
> +#include "libavutil/opt.h"
> +
> +#define DUMMY_ENCODER_SIZE 1
> +
> +typedef struct CammContext {
> +AVCodecContext *avctx;
> +} CammContext;
> +
> +// Sizes of each type of metadata.
> +static int metadata_type_sizes[] = {
> +  3 * sizeof(float),
> +  2 * sizeof(uint64_t),
> +  3 * sizeof(float),
> +  3 * 

Re: [FFmpeg-devel] [PATCH 1/1] This change adds an encoder for Camera metadata motion. This is a type of sensor data associated with video, such as GPS, acceleration, gyro, and camera orientation. It

[Moritz Barsnick]

Just some small observations... Even if you postpone this patch,
perhaps it can help improve the next one.

On Thu, Jul 06, 2017 at 17:36:39 -0700, Louis O'Bryan wrote:

> @@ -2,6 +2,7 @@ Entries are sorted chronologically from oldest to youngest 
> within each release,
>  releases are sorted from youngest to oldest.
>  
>  version :
> +- Camera metadata motion encoder
>  - deflicker video filter

Did you read that sentence above? ;-) New entries at the bottom of the
"version " section please.

>  @item Zip Motion Blocks Video  @tab   X @tab  X
>  @tab Encoder works only in PAL8.
> +@item Camera metadata motion@tab X  @tab
> +@tab Encoder for camera sensor data.

This list is supposed to be alphabetical.

>  OBJS-$(CONFIG_ZLIB_ENCODER)+= lclenc.o
>  OBJS-$(CONFIG_ZMBV_DECODER)+= zmbv.o
>  OBJS-$(CONFIG_ZMBV_ENCODER)+= zmbvenc.o
> +OBJS-$(CONFIG_CAMERA_MOTION_METADATA_ENCODER) += cammenc.o

This also needs to be in alphabetical order.

> + * Reference implementation for the CAMM Metadata encoder.
> + * Encodes sensor data for 360-degree cameras such as
> + * GPS, gyro, and acceleration. This is stored in a track separate from video
> + * and audio.

I believe you're just supposed to have a short entry here, the more
detailed stuff goes into the section below the GPL boilerplate.

> +// Sizes of each type of metadata.
> +static int metadata_type_sizes[] = {
> +  3 * sizeof(float),
> +  2 * sizeof(uint64_t),
> +  3 * sizeof(float),
> +  3 * sizeof(float),
> +  3 * sizeof(float),
> +  3 * sizeof(float),
> +  3 * sizeof(double) + sizeof(uint32_t) + 7 * sizeof(float),
> +  3 * sizeof(float)
> +};

Doesn't this make the sizes platform dependant? (Unless all platforms
have standardized floats anyway, then I'm mistaken. I just don't know.)

> +av_log(avctx, AV_LOG_DEBUG,
> +   "pixel_exposure_time = %lu\n", pixel_exposure_time);

pixel_exposure_time is a uint64_t, so '%lu' is incorrect, it's
'%"PRIu64"'. (Elsewhere as well.) The  types have theitr own
format identifiers.

> +av_log(avctx, AV_LOG_DEBUG, "rolling_shutter_skew_time = %lu\n",
> +   rolling_shutter_skew_time);

Ditto.

> +if (gps_fix_type != 0 && gps_fix_type != 1 && gps_fix_type != 2) 
> {

You could do "gps_fix_type > 2", but since it's sort of an enum, I
guess this is fine.


> --- a/libavformat/movenc.c
> +++ b/libavformat/movenc.c

This does NOT belong into your patch! (Or only parts of it.) You copied
some modified code over a recent git checkout, thereby apparently
reverting some unrelated recent commits. I recommend you checkout/pull,
modify only you stuff, and then commit locally. That keeps your changes
constrained, even if the code changes upstream.

Please re-apply your changes to a recent checkout.

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

Re: [FFmpeg-devel] [PATCH] libavcodec/htmlsubtitles.c: make tags case-insensitive

[Clément Bœsch]

On Fri, Jun 16, 2017 at 08:26:37PM +0900, DongHoon Kang wrote:
> Signed-off-by: DongHoon Kang 
> ---
>  libavcodec/htmlsubtitles.c | 14 +++---
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 

Rebased, updated for recent  change, and applied.

Sorry for the delay, and thanks for the patch.

Regards,

[...]

-- 
Clément B.


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

[FFmpeg-devel] [PATCH] libswresample: check input to swr_convert_frame for NULL

[hexpointer]

When 'out' is an AVFrame that does not have buffers preallocated,
swr_convert_frame tries to allocate buffers of the right size. However
in calculating this size it failed to check for whether 'in' is NULL
(requesting that swr's internal buffers are to be flushed).
---
 libswresample/swresample_frame.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/libswresample/swresample_frame.c b/libswresample/swresample_frame.c
index 71d3ed711a..2853266d6c 100644
--- a/libswresample/swresample_frame.c
+++ b/libswresample/swresample_frame.c
@@ -139,9 +139,10 @@ int swr_convert_frame(SwrContext *s,
 
 if (out) {
 if (!out->linesize[0]) {
-out->nb_samples =   swr_get_delay(s, s->out_sample_rate)
-  + in->nb_samples*(int64_t)s->out_sample_rate / 
s->in_sample_rate
-  + 3;
+out->nb_samples = swr_get_delay(s, s->out_sample_rate) + 3;
+if (in) {
+out->nb_samples += in->nb_samples*(int64_t)s->out_sample_rate 
/ s->in_sample_rate;
+}
 if ((ret = av_frame_get_buffer(out, 0)) < 0) {
 if (setup)
 swr_close(s);
-- 
2.13.1

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

Re: [FFmpeg-devel] [PATCH] avfilter/pthread: rewrite implementation

[Clément Bœsch]

On Fri, Jul 07, 2017 at 09:25:46AM +0700, Muhammad Faiz wrote:
> Avoid pthread_cond_broadcast that wakes up all workers. Make each of them
> uses distict mutex/cond.
> 

Please make sure this doesn't make tsan go crazy.

Also, the code is pretty much based on lavc/pthread_slice.c, so you may
want to keep differences low by applying your changes in lavc as well. I'm
sure people will be happy if libavcodec gets faster.

Interesting benchmarks btw :)

-- 
Clément B.


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

Re: [FFmpeg-devel] [PATCH] avfilter/pthread: rewrite implementation

[Muhammad Faiz]

On Fri, Jul 7, 2017 at 2:45 PM, Paul B Mahol  wrote:
> On 7/7/17, Muhammad Faiz  wrote:
>> Avoid pthread_cond_broadcast that wakes up all workers. Make each of them
>> uses distict mutex/cond.
>>
>
> What about video filters?

./ffmpeg -filter_complex_threads 16 -filter_complex
"nullsrc=s=1920x1080, transpose=dir=clock" -f rawvideo -y /dev/null
old:
74242240 decicycles in vf_transpose_execute,   1 runs,  0 skips
69378780 decicycles in vf_transpose_execute,   2 runs,  0 skips
66627910 decicycles in vf_transpose_execute,   4 runs,  0 skips
69064686 decicycles in vf_transpose_execute,   8 runs,  0 skips
67557351 decicycles in vf_transpose_execute,  16 runs,  0 skips
65460021 decicycles in vf_transpose_execute,  32 runs,  0 skips
62830474 decicycles in vf_transpose_execute,  64 runs,  0 skips
65043030 decicycles in vf_transpose_execute, 128 runs,  0 skips
62733184 decicycles in vf_transpose_execute, 256 runs,  0 skips
61353307 decicycles in vf_transpose_execute, 512 runs,  0 skips
60308065 decicycles in vf_transpose_execute,1024 runs,  0 skips
60829621 decicycles in vf_transpose_execute,2048 runs,  0 skips
61178596 decicycles in vf_transpose_execute,4096 runs,  0 skips
60492125 decicycles in vf_transpose_execute,8192 runs,  0 skips
60141149 decicycles in vf_transpose_execute,   16384 runs,  0 skips
60567245 decicycles in vf_transpose_execute,   32768 runs,  0 skips

new:
53044380 decicycles in vf_transpose_execute,   1 runs,  0 skips
57543460 decicycles in vf_transpose_execute,   2 runs,  0 skips
59047585 decicycles in vf_transpose_execute,   4 runs,  0 skips
59139675 decicycles in vf_transpose_execute,   8 runs,  0 skips
62851108 decicycles in vf_transpose_execute,  16 runs,  0 skips
60888400 decicycles in vf_transpose_execute,  32 runs,  0 skips
59835808 decicycles in vf_transpose_execute,  64 runs,  0 skips
61603125 decicycles in vf_transpose_execute, 128 runs,  0 skips
60237035 decicycles in vf_transpose_execute, 256 runs,  0 skips
60044255 decicycles in vf_transpose_execute, 512 runs,  0 skips
59737751 decicycles in vf_transpose_execute,1024 runs,  0 skips
58839457 decicycles in vf_transpose_execute,2048 runs,  0 skips
56007488 decicycles in vf_transpose_execute,4096 runs,  0 skips
52096972 decicycles in vf_transpose_execute,8192 runs,  0 skips
50752821 decicycles in vf_transpose_execute,   16384 runs,  0 skips
50119401 decicycles in vf_transpose_execute,   32768 runs,  0 skips
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/2] pixdesc: Improve scoring for opaque/unknown pixel formats

[wm4]

On Thu, 6 Jul 2017 22:59:24 +0100
Mark Thompson  wrote:

> Hardware pixel formats do not tell you anything about their actual
> contents, but should still score higher than formats with completely
> unknown properties, which in turn should score higher than invalid
> formats.
> 
> Do not return an AVERROR code as a score.
> 
> Fixes a hang in libavfilter where format negotiation gets stuck in a
> loop because AV_PIX_FMT_NONE scores more highly than all other
> possibilities.
> ---
> The hang in libavfilter happens when trying to choose a pixfmt for output 
> from a list of software formats when a hardware format is the input (the 
> hwdownload filter does this).  The matching begins with AV_PIX_FMT_NONE as an 
> invalid value and compares against each possibility in turn, but 
> unfortunately it scores -1 when considered as a conversion from an opaque 
> hardware format, higher than the AVERROR(EINVAL) (== -22 on Linux) scored for 
> all of the real formats.  Hence the format selection code chooses 
> AV_PIX_FMT_NONE and thinks it is making forward progress, but actually hasn't 
> and therefore gets stuck in an infinite loop.
> 
> 
>  libavutil/pixdesc.c | 10 +++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/libavutil/pixdesc.c b/libavutil/pixdesc.c
> index 46a7eff06d..35b63f43c6 100644
> --- a/libavutil/pixdesc.c
> +++ b/libavutil/pixdesc.c
> @@ -2512,7 +2512,11 @@ static int get_pix_fmt_score(enum AVPixelFormat 
> dst_pix_fmt,
>  int score = INT_MAX - 1;
>  
>  if (dst_pix_fmt >= AV_PIX_FMT_NB || dst_pix_fmt <= AV_PIX_FMT_NONE)
> -return ~0;
> +return -2;
> +
> +if ((src_desc->flags & AV_PIX_FMT_FLAG_HWACCEL) ||
> +(dst_desc->flags & AV_PIX_FMT_FLAG_HWACCEL))
> +return 0;
>  
>  /* compute loss */
>  *lossp = loss = 0;
> @@ -2521,9 +2525,9 @@ static int get_pix_fmt_score(enum AVPixelFormat 
> dst_pix_fmt,
>  return INT_MAX;
>  
>  if ((ret = get_pix_fmt_depth(_min_depth, _max_depth, 
> src_pix_fmt)) < 0)
> -return ret;
> +return -1;
>  if ((ret = get_pix_fmt_depth(_min_depth, _max_depth, 
> dst_pix_fmt)) < 0)
> -return ret;
> +return -1;
>  
>  src_color = get_color_type(src_desc);
>  dst_color = get_color_type(dst_desc);

I don't think it makes any sense to prefer one hw format over an
another without additional information, but I guess that also means
returning a random one doesn't do any harms.

Wouldn't it be better to fix the lavfi code, though?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v2 1/2] lavu: Add DRM hwcontext

[wm4]

On Fri, 7 Jul 2017 08:51:51 +0800
Jun Zhao  wrote:

> On 2017/7/6 7:02, Mark Thompson wrote:

> > diff --git a/libavutil/hwcontext.h b/libavutil/hwcontext.h
> > index edf12cc631..fe7613b379 100644
> > --- a/libavutil/hwcontext.h
> > +++ b/libavutil/hwcontext.h
> > @@ -32,6 +32,7 @@ enum AVHWDeviceType {
> >  AV_HWDEVICE_TYPE_QSV,
> >  AV_HWDEVICE_TYPE_VIDEOTOOLBOX,
> >  AV_HWDEVICE_TYPE_NONE,
> > +AV_HWDEVICE_TYPE_DRM,
> >  };  
> 
> AV_HWDEVICE_TYPE_DRM = AV_HWDEVICE_TYPE_NONE + 1  ? 

Yes, for ABI reasons. AV_HWDEVICE_TYPE_NONE should really be 0 and on
the beginning of the enum, but we can fix that only on the next ABI
bump.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH v3] avcodec/rdft: remove sintable

[Muhammad Faiz]

It is redundant with costable. The first half of sintable is
identical with the second half of costable. The second half
of sintable is negative value of the first half of sintable.

The computation is changed to handle sign of sin values, in
C code and ARM assembly code.

Signed-off-by: Muhammad Faiz 
---
 libavcodec/Makefile|  3 +-
 libavcodec/arm/rdft_neon.S | 13 ++---
 libavcodec/rdft.c  | 68 --
 libavcodec/rdft.h  | 26 ++
 4 files changed, 36 insertions(+), 74 deletions(-)

diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index b440a00..59029a8 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -122,8 +122,7 @@ OBJS-$(CONFIG_QSV) += qsv.o
 OBJS-$(CONFIG_QSVDEC)  += qsvdec.o
 OBJS-$(CONFIG_QSVENC)  += qsvenc.o
 OBJS-$(CONFIG_RANGECODER)  += rangecoder.o
-RDFT-OBJS-$(CONFIG_HARDCODED_TABLES)   += sin_tables.o
-OBJS-$(CONFIG_RDFT)+= rdft.o $(RDFT-OBJS-yes)
+OBJS-$(CONFIG_RDFT)+= rdft.o
 OBJS-$(CONFIG_RV34DSP) += rv34dsp.o
 OBJS-$(CONFIG_SHARED)  += log2_tab.o reverse.o
 OBJS-$(CONFIG_SINEWIN) += sinewin.o sinewin_fixed.o
diff --git a/libavcodec/arm/rdft_neon.S b/libavcodec/arm/rdft_neon.S
index 781d976..eabb92b 100644
--- a/libavcodec/arm/rdft_neon.S
+++ b/libavcodec/arm/rdft_neon.S
@@ -30,18 +30,21 @@ function ff_rdft_calc_neon, export=1
 
 lslsr6,  r6,  #31
 bne 1f
-add r0,  r4,  #20
+add r0,  r4,  #24
 bl  X(ff_fft_permute_neon)
-add r0,  r4,  #20
+add r0,  r4,  #24
 mov r1,  r5
 bl  X(ff_fft_calc_neon)
 1:
 ldr r12, [r4, #0]   @ nbits
 mov r2,  #1
+ldr r8,  [r4, #20]  @ negative_sin
 lsl r12, r2,  r12
 add r0,  r5,  #8
+lsl r8,  r8,  #31
 add r1,  r5,  r12, lsl #2
 lsr r12, r12, #2
+vdup.32 d26, r8
 ldr r2,  [r4, #12]  @ tcos
 sub r12, r12, #2
 ldr r3,  [r4, #16]  @ tsin
@@ -55,6 +58,7 @@ function ff_rdft_calc_neon, export=1
 vld1.32 {d5}, [r3,:64]! @ tsin[i]
 vmov.f32d18, #0.5   @ k1
 vdup.32 d19, r6
+veord5,  d26, d5
 pld [r0, #32]
 veord19, d18, d19   @ k2
 vmov.i32d16, #0
@@ -90,6 +94,7 @@ function ff_rdft_calc_neon, export=1
 vld1.32 {d5}, [r3,:64]! @  tsin[i]
 veord24, d22, d17   @  ev.re,-ev.im
 vrev64.32   d3,  d23@  od.re, od.im
+veord5, d26, d5
 pld [r2, #32]
 veord2,  d3,  d16   @ -od.re, od.im
 pld [r3, #32]
@@ -140,10 +145,10 @@ function ff_rdft_calc_neon, export=1
 
 vmul.f32d22, d22, d18
 vst1.32 {d22},[r5,:64]
-add r0,  r4,  #20
+add r0,  r4,  #24
 mov r1,  r5
 bl  X(ff_fft_permute_neon)
-add r0,  r4,  #20
+add r0,  r4,  #24
 mov r1,  r5
 pop {r4-r8,lr}
 b   X(ff_fft_calc_neon)
diff --git a/libavcodec/rdft.c b/libavcodec/rdft.c
index c318aa8..194e0bc 100644
--- a/libavcodec/rdft.c
+++ b/libavcodec/rdft.c
@@ -28,28 +28,6 @@
  * (Inverse) Real Discrete Fourier Transforms.
  */
 
-/* sin(2*pi*x/n) for 0<=xhttp://www.engineeringproductivitytools.com/stuff/T0001/PT10.HTM
@@ -73,20 +51,29 @@ static void rdft_calc_c(RDFTContext *s, FFTSample *data)
 ev.re = data[0];
 data[0] = ev.re+data[1];
 data[1] = ev.re-data[1];
-for (i = 1; i < (n>>2); i++) {
-i1 = 2*i;
-i2 = n-i1;
-/* Separate even and odd FFTs */
-ev.re =  k1*(data[i1  ]+data[i2  ]);
-od.im = -k2*(data[i1  ]-data[i2  ]);
-ev.im =  k1*(data[i1+1]-data[i2+1]);
-od.re =  k2*(data[i1+1]+data[i2+1]);
-/* Apply twiddle factors to the odd FFT and add to the even FFT */
-data[i1  ] =  ev.re + od.re*tcos[i] - od.im*tsin[i];
-data[i1+1] =  ev.im + od.im*tcos[i] + od.re*tsin[i];
-data[i2  ] =  ev.re - od.re*tcos[i] + od.im*tsin[i];
-data[i2+1] = -ev.im + od.im*tcos[i] + od.re*tsin[i];
+
+#define RDFT_UNMANGLE(sign0, sign1) \
+for (i = 1; i < (n>>2); i++) {  \
+i1 = 2*i; 

Re: [FFmpeg-devel] [PATCH v2] avcodec/rdft: remove sintable

[Muhammad Faiz]

On Thu, Jul 6, 2017 at 4:18 PM, Muhammad Faiz  wrote:
> It is redundant with costable. The first half of sintable is
> identical with the second half of costable. The second half
> of sintable is negative value of the first half of sintable.
>
> The computation is changed to handle sign of sin values.
>
> Signed-off-by: Muhammad Faiz 
> ---
>  libavcodec/Makefile|  3 +-
>  libavcodec/arm/rdft_neon.S | 20 --
>  libavcodec/rdft.c  | 68 
> --
>  libavcodec/rdft.h  | 26 ++
>  4 files changed, 39 insertions(+), 78 deletions(-)
>
> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> index b440a00..59029a8 100644
> --- a/libavcodec/Makefile
> +++ b/libavcodec/Makefile
> @@ -122,8 +122,7 @@ OBJS-$(CONFIG_QSV) += qsv.o
>  OBJS-$(CONFIG_QSVDEC)  += qsvdec.o
>  OBJS-$(CONFIG_QSVENC)  += qsvenc.o
>  OBJS-$(CONFIG_RANGECODER)  += rangecoder.o
> -RDFT-OBJS-$(CONFIG_HARDCODED_TABLES)   += sin_tables.o
> -OBJS-$(CONFIG_RDFT)+= rdft.o $(RDFT-OBJS-yes)
> +OBJS-$(CONFIG_RDFT)+= rdft.o
>  OBJS-$(CONFIG_RV34DSP) += rv34dsp.o
>  OBJS-$(CONFIG_SHARED)  += log2_tab.o reverse.o
>  OBJS-$(CONFIG_SINEWIN) += sinewin.o sinewin_fixed.o
> diff --git a/libavcodec/arm/rdft_neon.S b/libavcodec/arm/rdft_neon.S
> index 781d976..3bea8b4 100644
> --- a/libavcodec/arm/rdft_neon.S
> +++ b/libavcodec/arm/rdft_neon.S
> @@ -22,7 +22,7 @@
>  #include "libavutil/arm/asm.S"
>
>  function ff_rdft_calc_neon, export=1
> -push{r4-r8,lr}
> +push{r4-r9,lr}
>
>  ldr r6,  [r0, #4]   @ inverse
>  mov r4,  r0
> @@ -30,9 +30,9 @@ function ff_rdft_calc_neon, export=1
>
>  lslsr6,  r6,  #31
>  bne 1f
> -add r0,  r4,  #20
> +add r0,  r4,  #24
>  bl  X(ff_fft_permute_neon)
> -add r0,  r4,  #20
> +add r0,  r4,  #24
>  mov r1,  r5
>  bl  X(ff_fft_calc_neon)
>  1:
> @@ -46,8 +46,10 @@ function ff_rdft_calc_neon, export=1
>  sub r12, r12, #2
>  ldr r3,  [r4, #16]  @ tsin
>  mov r7,  r0
> +ldr r9,  [r4, #20]  @ negative_sin
>  sub r1,  r1,  #8
>  mov lr,  r1
> +lsl r9,  r9,  #31
>  mov r8,  #-8
>  vld1.32 {d0}, [r0,:64]! @ d1[0,1]
>  vld1.32 {d1}, [r1,:64], r8  @ d2[0,1]
> @@ -61,8 +63,10 @@ function ff_rdft_calc_neon, export=1
>  vmov.i32d17, #1<<31
>  pld [r1, #-32]
>  vtrn.32 d16, d17
> +vdup.32 d16, r9
>  pld [r2, #32]
> -vrev64.32   d16, d16@ d16=1,0 d17=0,1
> +veord17, d16, d17
> +vrev64.32   d16, d17@ negative_sin ? d16=0,1 
> d17=1,0 : d16=1,0 d17=0,1
>  pld [r3, #32]
>  2:
>  veorq1,  q0,  q8@ -d1[0],d1[1], d2[0],-d2[1]
> @@ -136,15 +140,15 @@ function ff_rdft_calc_neon, export=1
>
>  cmp r6,  #0
>  it  eq
> -popeq   {r4-r8,pc}
> +popeq   {r4-r9,pc}
>
>  vmul.f32d22, d22, d18
>  vst1.32 {d22},[r5,:64]
> -add r0,  r4,  #20
> +add r0,  r4,  #24
>  mov r1,  r5
>  bl  X(ff_fft_permute_neon)
> -add r0,  r4,  #20
> +add r0,  r4,  #24
>  mov r1,  r5
> -pop {r4-r8,lr}
> +pop {r4-r9,lr}
>  b   X(ff_fft_calc_neon)
>  endfunc
> diff --git a/libavcodec/rdft.c b/libavcodec/rdft.c
> index c318aa8..194e0bc 100644
> --- a/libavcodec/rdft.c
> +++ b/libavcodec/rdft.c
> @@ -28,28 +28,6 @@
>   * (Inverse) Real Discrete Fourier Transforms.
>   */
>
> -/* sin(2*pi*x/n) for 0<=x -#if !CONFIG_HARDCODED_TABLES
> -SINTABLE(16);
> -SINTABLE(32);
> -SINTABLE(64);
> -SINTABLE(128);
> -SINTABLE(256);
> -SINTABLE(512);
> -SINTABLE(1024);
> -SINTABLE(2048);
> -SINTABLE(4096);
> -SINTABLE(8192);
> -SINTABLE(16384);
> -SINTABLE(32768);
> -SINTABLE(65536);
> -#endif
> -static SINTABLE_CONST FFTSample * const ff_sin_tabs[] = {
> -NULL, NULL, NULL, NULL,
> -ff_sin_16, ff_sin_32, ff_sin_64, ff_sin_128, ff_sin_256, ff_sin_512, 
> ff_sin_1024,
> -ff_sin_2048, ff_sin_4096, ff_sin_8192, ff_sin_16384, ff_sin_32768, 
> ff_sin_65536,
> -};
> -
>  /** Map one real FFT into two parallel real even and odd FFTs. Then 
> 

Re: [FFmpeg-devel] [PATCH] avfilter/pthread: rewrite implementation

[Paul B Mahol]

On 7/7/17, Muhammad Faiz  wrote:
> Avoid pthread_cond_broadcast that wakes up all workers. Make each of them
> uses distict mutex/cond.
>

What about video filters?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] avformat: remove obsolete commented-out DEBUG define

[Tobias Rapp]

Signed-off-by: Tobias Rapp 
---
 libavformat/avienc.c  | 2 --
 libavformat/segment.c | 2 --
 2 files changed, 4 deletions(-)

diff --git a/libavformat/avienc.c b/libavformat/avienc.c
index e8c0c71..da3d3de 100644
--- a/libavformat/avienc.c
+++ b/libavformat/avienc.c
@@ -19,8 +19,6 @@
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
  */
 
-//#define DEBUG
-
 #include 
 
 #include "avformat.h"
diff --git a/libavformat/segment.c b/libavformat/segment.c
index 8ec3653..0e8bcdd 100644
--- a/libavformat/segment.c
+++ b/libavformat/segment.c
@@ -24,8 +24,6 @@
  * @url{http://tools.ietf.org/id/draft-pantos-http-live-streaming}
  */
 
-/* #define DEBUG */
-
 #include 
 #include 
 
-- 
2.7.4


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

Re: [FFmpeg-devel] [PATCH] avformat/avio: Remove no-op code in url_find_protocol().

[Muhammad Faiz]

On Fri, Jul 7, 2017 at 11:27 AM, Wan-Teh Chang
 wrote:
> Hi Muhammad,
>
> On Thu, Jul 6, 2017 at 7:56 PM, Muhammad Faiz  wrote:
>> On Fri, Jul 7, 2017 at 9:18 AM, Wan-Teh Chang
>>  wrote:
>>> In url_find_protocol(), proto_str is either "file" or a string
>>> consisting of only the characters in URL_SCHEME_CHARS, which does not
>>> include ','. Therefore the strchr(proto_str, ',') call always returns
>>> NULL.
>>>
>>> Note: The code was added in commit
>>> 6161c41817f6e53abb3021d67ca0f19def682718.
>>>
>>> Signed-off-by: Wan-Teh Chang 
>>> ---
>>>  libavformat/avio.c | 2 --
>>>  1 file changed, 2 deletions(-)
>>>
>>> diff --git a/libavformat/avio.c b/libavformat/avio.c
>>> index 1e79c9dd5c..64248e098b 100644
>>> --- a/libavformat/avio.c
>>> +++ b/libavformat/avio.c
>>> @@ -263,8 +263,6 @@ static const struct URLProtocol 
>>> *url_find_protocol(const char *filename)
>>>  av_strlcpy(proto_str, filename,
>>> FFMIN(proto_len + 1, sizeof(proto_str)));
>>>
>>> -if ((ptr = strchr(proto_str, ',')))
>>> -*ptr = '\0';
>>
>> What about handling "subfile," ?
>
> I don't know what url_find_protocol() is intended to do, but I can
> show you what it actually does.
>
> Here is the relevant code in libavformat/avio.c:
>
> ==
> #define URL_SCHEME_CHARS\
> "abcdefghijklmnopqrstuvwxyz"\
> "ABCDEFGHIJKLMNOPQRSTUVWXYZ"\
> "0123456789+-."
>
> static const struct URLProtocol *url_find_protocol(const char *filename)
> {
> const URLProtocol **protocols;
> char proto_str[128], proto_nested[128], *ptr;
> size_t proto_len = strspn(filename, URL_SCHEME_CHARS);
> int i;
>
> if (filename[proto_len] != ':' &&
> (strncmp(filename, "subfile,", 8) || !strchr(filename +
> proto_len + 1, ':')) ||
> is_dos_path(filename))
> strcpy(proto_str, "file");
> else
> av_strlcpy(proto_str, filename,
>FFMIN(proto_len + 1, sizeof(proto_str)));
>
> if ((ptr = strchr(proto_str, ',')))
> *ptr = '\0';
> ==
>
> Since I don't know how "subfile," should be handled by
> url_find_protocol(), I ran the following three test inputs in the
> debugger:
>
> If |filename| is "subfile,", then proto_len is 7 and the if branch
> copies "file" into proto_str.
>
> If |filename| is "subfile,abcdefg", then proto_len is 7 and the if
> branch copies "file" into proto_str.
>
> If |filename| is "subfile,abcdefg:hijk", then proto_len is 7 and the
> else branch copies "subfile" into proto_str.

Oh, I see. I was wrong.

>
> Is this the expected behavior?

I don't know. However it is the previous behavior, so LGTM.

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