[FFmpeg-devel] [PATCH]lavc/exr: Ignore long names flag

2018-01-30 Thread Carl Eugen Hoyos
Hi!

Attached patch fixes ticket #6994, unknown tag names are ignored by FFmpeg.

Please comment, Carl Eugen
From 19f1896fc182f2bc8d1c715883ab40aaf17fc828 Mon Sep 17 00:00:00 2001
From: Carl Eugen Hoyos 
Date: Tue, 30 Jan 2018 10:24:08 +0100
Subject: [PATCH] lavc/exr: Ignore long names flag.

The decoder only reads known names, others are skipped.

Fixes ticket #6994.
---
 libavcodec/exr.c |7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/libavcodec/exr.c b/libavcodec/exr.c
index 454dc74..802618a 100644
--- a/libavcodec/exr.c
+++ b/libavcodec/exr.c
@@ -1349,11 +1349,8 @@ static int decode_header(EXRContext *s, AVFrame *frame)
 
 flags = bytestream2_get_le24(&s->gb);
 
-if (flags == 0x00)
-s->is_tile = 0;
-else if (flags & 0x02)
-s->is_tile = 1;
-else{
+s->is_tile = !!(flags & 0x02);
+if (flags & ~0x06) {
 avpriv_report_missing_feature(s->avctx, "flags %d", flags);
 return AVERROR_PATCHWELCOME;
 }
-- 
1.7.10.4

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


Re: [FFmpeg-devel] [PATCH]lavc/exr: Ignore long names flag

2018-01-30 Thread Paul B Mahol
On 1/30/18, Carl Eugen Hoyos  wrote:
> Hi!
>
> Attached patch fixes ticket #6994, unknown tag names are ignored by FFmpeg.
>
> Please comment, Carl Eugen
>

Really? Are you sure this does not cause not decoding previously
decodable files?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH]lavc/exr: Ignore long names flag

2018-01-30 Thread Carl Eugen Hoyos
2018-01-30 10:30 GMT+01:00 Paul B Mahol :
> On 1/30/18, Carl Eugen Hoyos  wrote:
>>
>> Attached patch fixes ticket #6994, unknown tag names are
>> ignored by FFmpeg.
>
> Really? Are you sure this does not cause not decoding previously
> decodable files?

Could you point me to a sample?

Which previously working flags would be broken now?

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


Re: [FFmpeg-devel] [PATCH] libavfilter: constify filter list

2018-01-30 Thread Carl Eugen Hoyos
2018-01-30 8:24 GMT+01:00 Muhammad Faiz :

> +UNCONDITIONAL_FILTER_TABLE="
> +abuffer asrc
> +buffer  vsrc
> +abuffersink asink
> +buffersink  vsink
> +afifo   af
> +fifovf

Why are these unconditional now?

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


[FFmpeg-devel] [PATCH] Remove battleforthenet widget

2018-01-30 Thread Ricardo Constantino
---
 src/template_head2 | 23 ---
 1 file changed, 23 deletions(-)

diff --git a/src/template_head2 b/src/template_head2
index 71daf07..a0b11ab 100644
--- a/src/template_head2
+++ b/src/template_head2
@@ -3,29 +3,6 @@
   
   
 

Re: [FFmpeg-devel] [PATCH] Remove battleforthenet widget

2018-01-30 Thread wm4
On Tue, 30 Jan 2018 11:05:29 +
Ricardo Constantino  wrote:

> ---
>  src/template_head2 | 23 ---
>  1 file changed, 23 deletions(-)
> 
> diff --git a/src/template_head2 b/src/template_head2
> index 71daf07..a0b11ab 100644
> --- a/src/template_head2
> +++ b/src/template_head2
> @@ -3,29 +3,6 @@
>
>
>  

Re: [FFmpeg-devel] [PATCH] Remove battleforthenet widget

2018-01-30 Thread Michael Niedermayer
On Tue, Jan 30, 2018 at 11:05:29AM +, Ricardo Constantino wrote:
> ---
>  src/template_head2 | 23 ---
>  1 file changed, 23 deletions(-)

applied

i dont understand why this did not disable itself at the end of the campaign.
It should have disabled itself

Thanks

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

Old school: Use the lowest level language in which you can solve the problem
conveniently.
New school: Use the highest level language in which the latest supercomputer
can solve the problem without the user falling asleep waiting.


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


Re: [FFmpeg-devel] [PATCH]lavc/exr: Ignore long names flag

2018-01-30 Thread Paul B Mahol
On 1/30/18, Carl Eugen Hoyos  wrote:
> 2018-01-30 10:30 GMT+01:00 Paul B Mahol :
>> On 1/30/18, Carl Eugen Hoyos  wrote:
>>>
>>> Attached patch fixes ticket #6994, unknown tag names are
>>> ignored by FFmpeg.
>>
>> Really? Are you sure this does not cause not decoding previously
>> decodable files?
>
> Could you point me to a sample?
>
> Which previously working flags would be broken now?

I'm afraid I can not share such files.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH] avformat/opensrt: add Haivision Open SRT protocol

2018-01-30 Thread Nablet Developer
protocol requires libsrt (https://github.com/Haivision/srt) to be
installed

Signed-off-by: Nablet Developer 
---
 MAINTAINERS |   1 +
 configure   |   9 +
 doc/protocols.texi  | 116 +
 libavformat/Makefile|   1 +
 libavformat/opensrt.c   | 621 
 libavformat/protocols.c |   1 +
 6 files changed, 749 insertions(+)
 create mode 100644 libavformat/opensrt.c

diff --git a/MAINTAINERS b/MAINTAINERS
index ba7a728..0317f24 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -498,6 +498,7 @@ Protocols:
   http.cRonald S. Bultje
   libssh.c  Lukasz Marek
   mms*.cRonald S. Bultje
+  opensrt.c Nablet Developer
   udp.c Luca Abeni
   icecast.c Marvin Scholz
 
diff --git a/configure b/configure
index fcfa7aa..57705ee 100755
--- a/configure
+++ b/configure
@@ -294,6 +294,7 @@ External library support:
   --enable-opengl  enable OpenGL rendering [no]
   --enable-openssl enable openssl, needed for https support
if gnutls or libtls is not used [no]
+  --enable-opensrt enable Haivision Open SRT protocol [no]
   --disable-sndio  disable sndio support [autodetect]
   --disable-schannel   disable SChannel SSP, needed for TLS support on
Windows if openssl and gnutls are not used 
[autodetect]
@@ -1641,6 +1642,7 @@ EXTERNAL_LIBRARY_LIST="
 mediacodec
 openal
 opengl
+opensrt
 "
 
 HWACCEL_AUTODETECT_LIBRARY_LIST="
@@ -3148,6 +3150,8 @@ libssh_protocol_deps="libssh"
 libtls_conflict="openssl gnutls"
 mmsh_protocol_select="http_protocol"
 mmst_protocol_select="network"
+opensrt_protocol_select="network"
+opensrt_protocol_deps="opensrt"
 rtmp_protocol_conflict="librtmp_protocol"
 rtmp_protocol_select="tcp_protocol"
 rtmp_protocol_suggest="zlib"
@@ -5986,6 +5990,7 @@ enabled omx   && require_header OMX_Core.h
 enabled omx_rpi   && { check_header OMX_Core.h ||
{ ! enabled cross_compile && add_cflags 
-isystem/opt/vc/include/IL && check_header OMX_Core.h ; } ||
die "ERROR: OpenMAX IL headers not found"; } && 
enable omx
+enabled opensrt   && require_pkg_config libsrt "srt >= 1.2.0" 
srt/srt.h srt_socket
 enabled openssl   && { check_pkg_config openssl openssl openssl/ssl.h 
OPENSSL_init_ssl ||
check_pkg_config openssl openssl openssl/ssl.h 
SSL_library_init ||
check_lib openssl openssl/ssl.h 
SSL_library_init -lssl -lcrypto ||
@@ -6036,6 +6041,10 @@ if enabled decklink; then
 esac
 fi
 
+if enabled opensrt; then
+opensrt_protocol_extralibs="$opensrt_protocol_extralibs -lsrt"
+fi
+
 enabled securetransport &&
 check_func SecIdentityCreate "-Wl,-framework,CoreFoundation 
-Wl,-framework,Security" &&
 check_lib securetransport "Security/SecureTransport.h Security/Security.h" 
"SSLCreateContext" "-Wl,-framework,CoreFoundation -Wl,-framework,Security" ||
diff --git a/doc/protocols.texi b/doc/protocols.texi
index 98deb73..2e5e630 100644
--- a/doc/protocols.texi
+++ b/doc/protocols.texi
@@ -755,6 +755,122 @@ Set the workgroup used for making connections. By default 
workgroup is not speci
 
 For more information see: @url{http://www.samba.org/}.
 
+@section srt
+
+Haivision Secure Reliable Transport Protocol via libsrt.
+
+The required syntax for a SRT url is:
+@example
+srt://@var{hostname}:@var{port}[?@var{options}]
+@end example
+
+@var{options} contains a list of &-separated options of the form
+@var{key}=@var{val}.
+
+This protocol accepts the following options.
+
+@table @option
+@item conntimeo
+Connection timeout. SRT cannot connect for RTT > 1500 msec
+(2 handshake exchanges) with the default connect timeout of 3 seconds. This 
option
+applies to the caller and rendezvous connection modes. The connect timeout is 
10 times
+the value set for the rendezvous mode (which can be used as a workaround for 
this
+connection problem with earlier versions).
+
+@item fc=@var{bytes}
+Flight Flag Size (Window Size), in bytes. FC is actually an internal parameter 
and
+you should set it to not less than @option{recv_buffer_size} and @option{mss}.
+The default value is relatively large, therefore unless you set a very large
+receiver buffer, you do not need to change this option. Default value is 25600.
+
+@item inputbw=@var{bytes/seconds}
+Sender nominal input rate, in bytes per seconds. Used along with 
@option{oheadbw},
+when @option{maxbw} is set to relative (0), to calculate maximum sending rate 
when
+recovery packets are sent along with main media stream:
+@option{inputbw} * (100 + @option{oheadbw}) / 100
+if @option{inputbw} is not set while @option{maxbw} is set to relative (0), 
the actual
+ctual i

[FFmpeg-devel] [PATCH] id3v2: fix unsynchronization

2018-01-30 Thread wm4
The ID3v2 "unsynchronization scheme" requires replacing any 0xFF 0x00
sequences with 0xFF. This has to be done on every byte of the source
data, while the current code skipped a byte after a replacement. This
meant 0xFF 0x00 0xFF 00 was translated to 0xFF 0xFF 0x00 instead of 0xFF
0xFF. It feels a bit messy to do this correctly with the avio use. But
fortunately, this translation can be done in-place, so we can just do it
in memory.

Inspired by what taglib does.
---
Sample (which had corrupted cover art, displays fine with the fix):
https://0x0.st/sbQ9.bin (unfortunately a bit too large for FATE)
---
 libavformat/id3v2.c | 26 ++
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/libavformat/id3v2.c b/libavformat/id3v2.c
index b80178d67a..f7de26a1d8 100644
--- a/libavformat/id3v2.c
+++ b/libavformat/id3v2.c
@@ -976,19 +976,21 @@ static void id3v2_parse(AVIOContext *pb, AVDictionary 
**metadata,
 }
 }
 if (unsync || tunsync) {
-int64_t end = avio_tell(pb) + tlen;
-uint8_t *b;
-
-b = buffer;
-while (avio_tell(pb) < end && b - buffer < tlen && 
!pb->eof_reached) {
-*b++ = avio_r8(pb);
-if (*(b - 1) == 0xff && avio_tell(pb) < end - 1 &&
-b - buffer < tlen &&
-!pb->eof_reached ) {
-uint8_t val = avio_r8(pb);
-*b++ = val ? val : avio_r8(pb);
-}
+uint8_t *b = buffer;
+uint8_t *t = buffer;
+uint8_t *end = t + tlen;
+
+if (avio_read(pb, buffer, tlen) != tlen) {
+av_log(s, AV_LOG_ERROR, "Failed to read tag data\n");
+goto seek;
 }
+
+while (t != end) {
+*b++ = *t++;
+if (t != end && t[-1] == 0xff && !t[0])
+t++;
+}
+
 ffio_init_context(&pb_local, buffer, b - buffer, 0, NULL, 
NULL, NULL,
   NULL);
 tlen = b - buffer;
-- 
2.15.1

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


Re: [FFmpeg-devel] [PATCH] id3v2: fix unsynchronization

2018-01-30 Thread wm4
On Tue, 30 Jan 2018 13:43:25 +0100
wm4  wrote:

> The ID3v2 "unsynchronization scheme" requires replacing any 0xFF 0x00
> sequences with 0xFF. This has to be done on every byte of the source
> data, while the current code skipped a byte after a replacement. This
> meant 0xFF 0x00 0xFF 00 was translated to 0xFF 0xFF 0x00 instead of 0xFF
> 0xFF. It feels a bit messy to do this correctly with the avio use. But
> fortunately, this translation can be done in-place, so we can just do it
> in memory.
> 
> Inspired by what taglib does.
> ---
> Sample (which had corrupted cover art, displays fine with the fix):
> https://0x0.st/sbQ9.bin (unfortunately a bit too large for FATE)
> ---
>  libavformat/id3v2.c | 26 ++
>  1 file changed, 14 insertions(+), 12 deletions(-)
> 
> diff --git a/libavformat/id3v2.c b/libavformat/id3v2.c
> index b80178d67a..f7de26a1d8 100644
> --- a/libavformat/id3v2.c
> +++ b/libavformat/id3v2.c
> @@ -976,19 +976,21 @@ static void id3v2_parse(AVIOContext *pb, AVDictionary 
> **metadata,
>  }
>  }
>  if (unsync || tunsync) {
> -int64_t end = avio_tell(pb) + tlen;
> -uint8_t *b;
> -
> -b = buffer;
> -while (avio_tell(pb) < end && b - buffer < tlen && 
> !pb->eof_reached) {
> -*b++ = avio_r8(pb);
> -if (*(b - 1) == 0xff && avio_tell(pb) < end - 1 &&
> -b - buffer < tlen &&
> -!pb->eof_reached ) {
> -uint8_t val = avio_r8(pb);
> -*b++ = val ? val : avio_r8(pb);
> -}
> +uint8_t *b = buffer;
> +uint8_t *t = buffer;
> +uint8_t *end = t + tlen;
> +
> +if (avio_read(pb, buffer, tlen) != tlen) {
> +av_log(s, AV_LOG_ERROR, "Failed to read tag data\n");
> +goto seek;
>  }
> +
> +while (t != end) {
> +*b++ = *t++;
> +if (t != end && t[-1] == 0xff && !t[0])
> +t++;
> +}
> +
>  ffio_init_context(&pb_local, buffer, b - buffer, 0, NULL, 
> NULL, NULL,
>NULL);
>  tlen = b - buffer;

Also I just saw that 9ae80e6a9cefcab61e867256ba19ef78a4bfe0cb seems to
claim that the current code is correct (and my commit essentially
reverts it). Taglib decodes the tag correctly, though.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] libavfilter: constify filter list

2018-01-30 Thread Michael Niedermayer
On Tue, Jan 30, 2018 at 02:24:12PM +0700, Muhammad Faiz wrote:
> Move REGISTER_FILTER to FILTER_TABLE in configure.
> Auto generate filter extern and filter table.
> Sort filter table, use bsearch on avfilter_get_by_name.
> Define next pointer at filter extern, no need to initialize
> next pointer at run time, so AVFilter can be set to const.

> Make avfilter_register always return error.

That breaks API
Its also a step away from supporting plugins.
Why plugins matter ? Because having plugin
support is a big advantage, it allows a much wider
community to work on, write and maintain filters.

With plugins, people can write filters that are
written in languages other than C. Or filters which
some developer in FFmpeg doesnt want. Or they can be
maintained externally by people who just do not like us.
Or by people who perfer a FOSS license different from
LGPL/GPL/BSD. Iam sure others can come up with more
reasons ...

Of course avfilter_register() isnt enough for plugins
but it or something equivalent is needed for plugins.

So i would prefer if avfilter_register() stays supported
indefinitly or in case a different system is written for
plugins then until that system is in place.

That is just a preferrance, not an objection to the patch.


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

Many things microsoft did are stupid, but not doing something just because
microsoft did it is even more stupid. If everything ms did were stupid they
would be bankrupt already.


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


Re: [FFmpeg-devel] [PATCH] libavfilter: constify filter list

2018-01-30 Thread wm4
On Tue, 30 Jan 2018 13:50:37 +0100
Michael Niedermayer  wrote:

> On Tue, Jan 30, 2018 at 02:24:12PM +0700, Muhammad Faiz wrote:
> > Move REGISTER_FILTER to FILTER_TABLE in configure.
> > Auto generate filter extern and filter table.
> > Sort filter table, use bsearch on avfilter_get_by_name.
> > Define next pointer at filter extern, no need to initialize
> > next pointer at run time, so AVFilter can be set to const.  
> 
> > Make avfilter_register always return error.  
> 
> That breaks API
> Its also a step away from supporting plugins.
> Why plugins matter ? Because having plugin
> support is a big advantage, it allows a much wider
> community to work on, write and maintain filters.
> 
> With plugins, people can write filters that are
> written in languages other than C. Or filters which
> some developer in FFmpeg doesnt want. Or they can be
> maintained externally by people who just do not like us.
> Or by people who perfer a FOSS license different from
> LGPL/GPL/BSD. Iam sure others can come up with more
> reasons ...
> 
> Of course avfilter_register() isnt enough for plugins
> but it or something equivalent is needed for plugins.
> 
> So i would prefer if avfilter_register() stays supported
> indefinitly or in case a different system is written for
> plugins then until that system is in place.



> That is just a preferrance, not an objection to the patch.

It certainly sounds like one.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] libavfilter: constify filter list

2018-01-30 Thread Muhammad Faiz
On Tue, Jan 30, 2018 at 4:47 PM, Carl Eugen Hoyos  wrote:
> 2018-01-30 8:24 GMT+01:00 Muhammad Faiz :
>
>> +UNCONDITIONAL_FILTER_TABLE="
>> +abuffer asrc
>> +buffer  vsrc
>> +abuffersink asink
>> +buffersink  vsink
>> +afifo   af
>> +fifovf
>
> Why are these unconditional now?

They were previously unconditional by REGISTER_FILTER_UNCONDITIONAL().
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] libavfilter: constify filter list

2018-01-30 Thread Muhammad Faiz
On Tue, Jan 30, 2018 at 2:58 PM, wm4  wrote:
> On Tue, 30 Jan 2018 14:24:12 +0700
> Muhammad Faiz  wrote:
>
>> Move REGISTER_FILTER to FILTER_TABLE in configure.
>> Auto generate filter extern and filter table.
>> Sort filter table, use bsearch on avfilter_get_by_name.
>> Define next pointer at filter extern, no need to initialize
>> next pointer at run time, so AVFilter can be set to const.
>> Make avfilter_register always return error.
>> Target checkasm now depends on EXTRALIBS-avformat.
>>
>> Signed-off-by: Muhammad Faiz 
>> ---
>
> Pretty nice. Should we still add a new filter listing API that uses the
> same idiom as the BSF API? (Although you generate the .next links at
> configure time it might still be nice to eventually get rid of that.)

IMHO, it is uglier (i.e. by requiring opaque pointer). I don't know
other people's opinion. Anyway, it can be added later.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] libavfilter: constify filter list

2018-01-30 Thread Muhammad Faiz
On Tue, Jan 30, 2018 at 7:50 PM, Michael Niedermayer
 wrote:
> On Tue, Jan 30, 2018 at 02:24:12PM +0700, Muhammad Faiz wrote:
>> Move REGISTER_FILTER to FILTER_TABLE in configure.
>> Auto generate filter extern and filter table.
>> Sort filter table, use bsearch on avfilter_get_by_name.
>> Define next pointer at filter extern, no need to initialize
>> next pointer at run time, so AVFilter can be set to const.
>
>> Make avfilter_register always return error.
>
> That breaks API

No. Because with current API, it is impossible to implement external filter.


> Its also a step away from supporting plugins.
> Why plugins matter ? Because having plugin
> support is a big advantage, it allows a much wider
> community to work on, write and maintain filters.
>
> With plugins, people can write filters that are
> written in languages other than C. Or filters which
> some developer in FFmpeg doesnt want. Or they can be
> maintained externally by people who just do not like us.
> Or by people who perfer a FOSS license different from
> LGPL/GPL/BSD. Iam sure others can come up with more
> reasons ...
>
> Of course avfilter_register() isnt enough for plugins
> but it or something equivalent is needed for plugins.
>
> So i would prefer if avfilter_register() stays supported
> indefinitly or in case a different system is written for
> plugins then until that system is in place.

It just returns error and logs error message that currently external
filter is unsupported. If someone wants to implement support for
external filter, it can be easily added later using separate
list/table.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] libavfilter: constify filter list

2018-01-30 Thread wm4
On Tue, 30 Jan 2018 20:16:50 +0700
Muhammad Faiz  wrote:

> On Tue, Jan 30, 2018 at 2:58 PM, wm4  wrote:
> > On Tue, 30 Jan 2018 14:24:12 +0700
> > Muhammad Faiz  wrote:
> >  
> >> Move REGISTER_FILTER to FILTER_TABLE in configure.
> >> Auto generate filter extern and filter table.
> >> Sort filter table, use bsearch on avfilter_get_by_name.
> >> Define next pointer at filter extern, no need to initialize
> >> next pointer at run time, so AVFilter can be set to const.
> >> Make avfilter_register always return error.
> >> Target checkasm now depends on EXTRALIBS-avformat.
> >>
> >> Signed-off-by: Muhammad Faiz 
> >> ---  
> >
> > Pretty nice. Should we still add a new filter listing API that uses the
> > same idiom as the BSF API? (Although you generate the .next links at
> > configure time it might still be nice to eventually get rid of that.)  
> 
> IMHO, it is uglier (i.e. by requiring opaque pointer). I don't know
> other people's opinion. Anyway, it can be added later.

Yes, because it uses an opaque pointer, it could use the linked list,
without changing any of the existing code. I think a simple filter array
would be preferable in the end, which would require an iteration
function which can pass an index to the user.

(Although I agree that this opaque thing is uglier than the current API
or an API which just takes an index.)
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH]lavc/exr: Ignore long names flag

2018-01-30 Thread Carl Eugen Hoyos
2018-01-30 13:16 GMT+01:00 Paul B Mahol :
> On 1/30/18, Carl Eugen Hoyos  wrote:
>> 2018-01-30 10:30 GMT+01:00 Paul B Mahol :
>>> On 1/30/18, Carl Eugen Hoyos  wrote:

 Attached patch fixes ticket #6994, unknown tag names are
 ignored by FFmpeg.
>>>
>>> Really? Are you sure this does not cause not decoding previously
>>> decodable files?
>>
>> Could you point me to a sample?
>>
>> Which previously working flags would be broken now?
>
> I'm afraid I can not share such files.

Which flags are set in these files?
How can I produce such a file?

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


Re: [FFmpeg-devel] [PATCH] libavfilter: constify filter list

2018-01-30 Thread Mark Thompson
On 30/01/18 07:24, Muhammad Faiz wrote:
> Move REGISTER_FILTER to FILTER_TABLE in configure.
> Auto generate filter extern and filter table.
> Sort filter table, use bsearch on avfilter_get_by_name.
> Define next pointer at filter extern, no need to initialize
> next pointer at run time, so AVFilter can be set to const.
> Make avfilter_register always return error.
> Target checkasm now depends on EXTRALIBS-avformat.
> 
> Signed-off-by: Muhammad Faiz 
> ---

I like the idea of this, but I'm not sure about some of the implementation 
details.

Have you considered dropping the "next" links entirely and having just the 
array of pointers instead?  I feel like they don't really add anything useful 
once we are in this form, and result in more boilerplate on every filter and 
some tricky generation code.

avfilter_next() would be a bit slower (since it would have to search the array, 
and therefore have runtime linear in the number of filters), but 
avfilter_get_by_name() is a lot faster because of the binary search (and is the 
only common use of it) so I don't think that complaint would be a strong one.

>  Makefile   |   4 +-
>  configure  | 440 
> -
> ...
>  tests/checkasm/Makefile|   2 +-
>  303 files changed, 1229 insertions(+), 796 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 9defddebfd..f607579369 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -56,6 +56,7 @@ tools/uncoded_frame$(EXESUF): ELIBS = $(FF_EXTRALIBS)
>  tools/target_dec_%_fuzzer$(EXESUF): $(FF_DEP_LIBS)
>  
>  CONFIGURABLE_COMPONENTS =   \
> +$(SRC_PATH)/configure   \
>  $(wildcard $(FFLIBS:%=$(SRC_PATH)/lib%/all*.c)) \
>  $(SRC_PATH)/libavcodec/bitstream_filters.c  \
>  $(SRC_PATH)/libavformat/protocols.c \
> @@ -142,7 +143,8 @@ distclean:: clean
>   $(RM) .version avversion.h config.asm config.h mapfile  \
>   ffbuild/.config ffbuild/config.* libavutil/avconfig.h \
>   version.h libavutil/ffversion.h libavcodec/codec_names.h \
> - libavcodec/bsf_list.c libavformat/protocol_list.c
> + libavcodec/bsf_list.c libavformat/protocol_list.c \
> + libavfilter/filter_list.h libavfilter/filter_list.c
>  ifeq ($(SRC_LINK),src)
>   $(RM) src
>  endif
> diff --git a/configure b/configure
> index fcfa7aa442..3261f5fd1a 100755
> --- a/configure
> +++ b/configure
> @@ -3177,6 +3177,381 @@ unix_protocol_deps="sys_un_h"
>  unix_protocol_select="network"
>  
>  # filters
> +FILTER_TABLE="
> +abench  af
> +acompressor af
> +acontrast   af
> ...
> +spectrumsynth   vaf
> +amovie  avsrc
> +movie   avsrc
> +"
> +
> +UNCONDITIONAL_FILTER_TABLE="
> +abuffer asrc
> +buffer  vsrc
> +abuffersink asink
> +buffersink  vsink
> +afifo   af
> +fifovf
> +"
> +

I don't really like having this table in configure.  Since you're generating 
the filter_list.h header with the external definitions from it anyway, why not 
write that and use it as the source rather than having the table here?

>  afftfilt_filter_deps="avcodec"
>  afftfilt_filter_select="fft"
>  afir_filter_deps="avcodec"
> @@ -3530,7 +3905,18 @@ MUXER_LIST=$(find_thingsmuxer_MUX 
> libavformat/allformats.c)
>  DEMUXER_LIST=$(find_things  demuxer  DEMUXlibavformat/allformats.c)
>  OUTDEV_LIST=$(find_things   outdev   OUTDEV   libavdevice/alldevices.c)
>  INDEV_LIST=$(find_thingsindev_IN  libavdevice/alldevices.c)
> -FILTER_LIST=$(find_things   filter   FILTER   libavfilter/allfilters.c)
> +
> +extract_list_from_table(){
> +cols=$1
> +suffix=$2
> +shift 2
> +while test -n "$1"; do
> +echo "${1}${suffix}"
> +shift $cols
> +done
> +}
> +
> +FILTER_LIST=$(extract_list_from_table 2 _filter $FILTER_TABLE)
>  
>  find_things_extern(){
>  thing=$1
> @@ -7030,6 +7416,58 @@ print_enabled_components(){
>  print_enabled_components libavcodec/bsf_list.c AVBitStreamFilter 
> bitstream_filters $BSF_LIST
>  print_enabled_components libavformat/protocol_list.c URLProtocol 
> url_protocols $PROTOCOL_LIST
>  
> +# filters
> +extract_enabled_filter(){
> +while test -n "$1"; do
> +if enabled "${1}_filter"; then
> +echo "$1 $2"
> +fi
> +shift 2
> +done
> +}
> +
> +extract_sorted_filter(){
> +while test -n "$1"; do
> +echo "$1 $2"
> +shift 2
> +done | sort
> +}
> +
> +print_filter_extern(){
> +while test -n "$1"; do
> +echo "extern const AVFilter ff_${2}_${1};"
> +if test -n "$3"; then
> +echo "#define ff_next_${2}_${1} &ff_${4}_${3}"
> +else
> +echo "#define ff_next_${2}_${1} NULL"
> +fi
> +   

Re: [FFmpeg-devel] [PATCH] libavfilter: constify filter list

2018-01-30 Thread wm4
On Tue, 30 Jan 2018 14:09:43 +
Mark Thompson  wrote:

> On 30/01/18 07:24, Muhammad Faiz wrote:
> > Move REGISTER_FILTER to FILTER_TABLE in configure.
> > Auto generate filter extern and filter table.
> > Sort filter table, use bsearch on avfilter_get_by_name.
> > Define next pointer at filter extern, no need to initialize
> > next pointer at run time, so AVFilter can be set to const.
> > Make avfilter_register always return error.
> > Target checkasm now depends on EXTRALIBS-avformat.
> > 
> > Signed-off-by: Muhammad Faiz 
> > ---  
> 
> I like the idea of this, but I'm not sure about some of the implementation 
> details.
> 
> Have you considered dropping the "next" links entirely and having just the 
> array of pointers instead?  I feel like they don't really add anything useful 
> once we are in this form, and result in more boilerplate on every filter and 
> some tricky generation code.
> 
> avfilter_next() would be a bit slower (since it would have to search the 
> array, and therefore have runtime linear in the number of filters), but 
> avfilter_get_by_name() is a lot faster because of the binary search (and is 
> the only common use of it) so I don't think that complaint would be a strong 
> one.

I think we considered that for libavcodec, but discarded the idea
because ffmpeg.c's use was causing noticable performance problems? (I
don't remember the details.) Also wouldn't the runtime be quadratic
rather than linear when iterating filters?

> > +const AVFilter *avfilter_next(const AVFilter *prev)
> > +{
> > +CHECK_VALIDITY();  
> 
> Calling avfilter_next() without having called avfilter_register_all() 
> violates the API, though?

Before this change it just returned NULL, now it returns the full list.
I think that was the intent and I don't think it's a problem, besides
keeping the old behavior would require mutable state.


> (Or is there an intent to deprecate avfilter_register_all() immediately after 
> this?)

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


Re: [FFmpeg-devel] [mp3] Skip APE tags when parsing mp3 packets.

2018-01-30 Thread James Almer
On 1/30/2018 2:45 AM, wm4 wrote:
> On Tue, 30 Jan 2018 02:24:29 +0100
> Michael Niedermayer  wrote:
> 
>> On Mon, Jan 29, 2018 at 03:13:54PM -0800, Dale Curtis wrote:
>>> Otherwise the decoder will throw "Missing header" errors when the
>>> packets are sent for decoding.  
>>
>>>  mpegaudio_parser.c |7 +++
>>>  1 file changed, 7 insertions(+)
>>> 2628fa8480b15237a528e94b1689da7321ce9440  skip-ape-tags.patch
>>> From 05f5e837862a958fb8791668ab3cdf8fc7766fe5 Mon Sep 17 00:00:00 2001
>>> From: Dale Curtis 
>>> Date: Mon, 29 Jan 2018 15:10:26 -0800
>>> Subject: [PATCH] [mp3] Skip APE tags when parsing mp3 packets.
>>>
>>> Otherwise the decoder will throw "Missing header" errors when the
>>> packets are sent for decoding.
>>> ---
>>>  libavcodec/mpegaudio_parser.c | 7 +++
>>>  1 file changed, 7 insertions(+)
>>>
>>> diff --git a/libavcodec/mpegaudio_parser.c b/libavcodec/mpegaudio_parser.c
>>> index 8c39825792..244281b56f 100644
>>> --- a/libavcodec/mpegaudio_parser.c
>>> +++ b/libavcodec/mpegaudio_parser.c
>>> @@ -23,6 +23,7 @@
>>>  #include "parser.h"
>>>  #include "mpegaudiodecheader.h"
>>>  #include "libavutil/common.h"
>>> +#include "libavformat/apetag.h" // for APE tag.
>>>  #include "libavformat/id3v1.h" // for ID3v1_TAG_SIZE
>>>  
>>>  typedef struct MpegAudioParseContext {
>>> @@ -120,6 +121,12 @@ static int mpegaudio_parse(AVCodecParserContext *s1,
>>>  return next;
>>>  }
>>>  
>>> +if (flush && buf_size >= APE_TAG_FOOTER_BYTES && memcmp(buf, 
>>> APE_TAG_PREAMBLE, 8) == 0) {
>>> +*poutbuf = NULL;
>>> +*poutbuf_size = 0;
>>> +return next;
>>> +}  
>>
>> This doesnt feel right
>>
>> Parsers should not discard data
>>
>> a bistream filter could discard data, so could a demuxer if thats how the
>> format should be interpreted. Or the decoder could simply detect this case
>> and not print an error/warning
> 
> This should obviously be done by the demuxer, unless I'm missing some
> other use cases. Should still be OK to skip in the parser. Tags have no
> business in a packet stream (they're not supposed to be there)

I recently changed the raw aac demuxer to stop propagating "junk" at the
beginning and end of the stream (things like id3 and ape tags) by
discarding anything that's not a complete frame, but the result was that
it kinda broke some fully playable files that had one or two damaged
frames. It was meant for codec copy cases, since the aac decoder dealt
with it just fine.

Skipping junk until a sync word is found then combining data until you
get a full frame is currently a valid usage for parsers (see mlp
parser). But i think the plan was to have them only analyze headers to
set relevant parameters in both frames and codec context, and leave the
task to produce full frames to bitstream filters.

, but
> given how broken mp3 data usually is, it's probably nice if the parser
> can filter them out. (Wouldn't be surprised if you find tags in muxed
> tracks in mkv or mp4...)

I've seen files with internal ffmpeg markers in the wild. I have no idea
how they even made it there, but some users seem to find ways to mux the
craziest shit :p
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] libavfilter: constify filter list

2018-01-30 Thread Mark Thompson
On 30/01/18 14:20, wm4 wrote:
> On Tue, 30 Jan 2018 14:09:43 +
> Mark Thompson  wrote:
> 
>> On 30/01/18 07:24, Muhammad Faiz wrote:
>>> Move REGISTER_FILTER to FILTER_TABLE in configure.
>>> Auto generate filter extern and filter table.
>>> Sort filter table, use bsearch on avfilter_get_by_name.
>>> Define next pointer at filter extern, no need to initialize
>>> next pointer at run time, so AVFilter can be set to const.
>>> Make avfilter_register always return error.
>>> Target checkasm now depends on EXTRALIBS-avformat.
>>>
>>> Signed-off-by: Muhammad Faiz 
>>> ---  
>>
>> I like the idea of this, but I'm not sure about some of the implementation 
>> details.
>>
>> Have you considered dropping the "next" links entirely and having just the 
>> array of pointers instead?  I feel like they don't really add anything 
>> useful once we are in this form, and result in more boilerplate on every 
>> filter and some tricky generation code.
>>
>> avfilter_next() would be a bit slower (since it would have to search the 
>> array, and therefore have runtime linear in the number of filters), but 
>> avfilter_get_by_name() is a lot faster because of the binary search (and is 
>> the only common use of it) so I don't think that complaint would be a strong 
>> one.
> 
> I think we considered that for libavcodec, but discarded the idea
> because ffmpeg.c's use was causing noticable performance problems? (I
> don't remember the details.) Also wouldn't the runtime be quadratic
> rather than linear when iterating filters?

Linear per-call, quadratic to iterate over all.

Do you have any reference for that discussion so we can compare?  The only call 
to avfilter_next() in ffmpeg.c is for showing the filter list (-filters), which 
I don't think is a performance-sensitive option.

Also, avfilter_next() can be made faster (possibly, constants will be larger) 
by using the binary search in the list since you know the name of the current 
filter (it's in the pointer you are given).

>>> +const AVFilter *avfilter_next(const AVFilter *prev)
>>> +{
>>> +CHECK_VALIDITY();  
>>
>> Calling avfilter_next() without having called avfilter_register_all() 
>> violates the API, though?
> 
> Before this change it just returned NULL, now it returns the full list.
> I think that was the intent and I don't think it's a problem, besides
> keeping the old behavior would require mutable state.

I was thinking of just not having the validity check there.  Maybe it doesn't 
matter.

>> (Or is there an intent to deprecate avfilter_register_all() immediately 
>> after this?)
> 
> Hopefully.

Sounds good :)

Thanks,

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


Re: [FFmpeg-devel] [mp3] Skip APE tags when parsing mp3 packets.

2018-01-30 Thread wm4
On Tue, 30 Jan 2018 11:30:43 -0300
James Almer  wrote:

> On 1/30/2018 2:45 AM, wm4 wrote:
> > On Tue, 30 Jan 2018 02:24:29 +0100
> > Michael Niedermayer  wrote:
> >   
> >> On Mon, Jan 29, 2018 at 03:13:54PM -0800, Dale Curtis wrote:  
> >>> Otherwise the decoder will throw "Missing header" errors when the
> >>> packets are sent for decoding.
> >>  
> >>>  mpegaudio_parser.c |7 +++
> >>>  1 file changed, 7 insertions(+)
> >>> 2628fa8480b15237a528e94b1689da7321ce9440  skip-ape-tags.patch
> >>> From 05f5e837862a958fb8791668ab3cdf8fc7766fe5 Mon Sep 17 00:00:00 2001
> >>> From: Dale Curtis 
> >>> Date: Mon, 29 Jan 2018 15:10:26 -0800
> >>> Subject: [PATCH] [mp3] Skip APE tags when parsing mp3 packets.
> >>>
> >>> Otherwise the decoder will throw "Missing header" errors when the
> >>> packets are sent for decoding.
> >>> ---
> >>>  libavcodec/mpegaudio_parser.c | 7 +++
> >>>  1 file changed, 7 insertions(+)
> >>>
> >>> diff --git a/libavcodec/mpegaudio_parser.c b/libavcodec/mpegaudio_parser.c
> >>> index 8c39825792..244281b56f 100644
> >>> --- a/libavcodec/mpegaudio_parser.c
> >>> +++ b/libavcodec/mpegaudio_parser.c
> >>> @@ -23,6 +23,7 @@
> >>>  #include "parser.h"
> >>>  #include "mpegaudiodecheader.h"
> >>>  #include "libavutil/common.h"
> >>> +#include "libavformat/apetag.h" // for APE tag.
> >>>  #include "libavformat/id3v1.h" // for ID3v1_TAG_SIZE
> >>>  
> >>>  typedef struct MpegAudioParseContext {
> >>> @@ -120,6 +121,12 @@ static int mpegaudio_parse(AVCodecParserContext *s1,
> >>>  return next;
> >>>  }
> >>>  
> >>> +if (flush && buf_size >= APE_TAG_FOOTER_BYTES && memcmp(buf, 
> >>> APE_TAG_PREAMBLE, 8) == 0) {
> >>> +*poutbuf = NULL;
> >>> +*poutbuf_size = 0;
> >>> +return next;
> >>> +}
> >>
> >> This doesnt feel right
> >>
> >> Parsers should not discard data
> >>
> >> a bistream filter could discard data, so could a demuxer if thats how the
> >> format should be interpreted. Or the decoder could simply detect this case
> >> and not print an error/warning  
> > 
> > This should obviously be done by the demuxer, unless I'm missing some
> > other use cases. Should still be OK to skip in the parser. Tags have no
> > business in a packet stream (they're not supposed to be there)  
> 
> I recently changed the raw aac demuxer to stop propagating "junk" at the
> beginning and end of the stream (things like id3 and ape tags) by
> discarding anything that's not a complete frame, but the result was that
> it kinda broke some fully playable files that had one or two damaged
> frames. It was meant for codec copy cases, since the aac decoder dealt
> with it just fine.
> 
> Skipping junk until a sync word is found then combining data until you
> get a full frame is currently a valid usage for parsers (see mlp
> parser). But i think the plan was to have them only analyze headers to
> set relevant parameters in both frames and codec context, and leave the
> task to produce full frames to bitstream filters.

In theory we could have a flag that controls this (I thought there
actually was such a flag, but didn't find anything). If we ever find
that discarding data is a problem, maybe such a flag could be added.

Also I still kind of hope someone will make a new parser API, since the
current one is clunky with its AVCodecContext use and its inability to
return errors.

Anyway, my point was that raw demuxers should strip tags like id3v2 and
APE before we add tag stripping to every parser that's used for raw
demuxing. (Still would be good to get info on what use case the patch
fixes.)

> , but
> > given how broken mp3 data usually is, it's probably nice if the parser
> > can filter them out. (Wouldn't be surprised if you find tags in muxed
> > tracks in mkv or mp4...)  
> 
> I've seen files with internal ffmpeg markers in the wild. I have no idea
> how they even made it there, but some users seem to find ways to mux the
> craziest shit :p

The wonderful merged side data stuff?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avcodec/hevc_parser: use ff_hevc_decode_extradata() to parse extradata

2018-01-30 Thread James Almer
On 1/24/2018 12:48 PM, James Almer wrote:
> On 1/20/2018 3:00 PM, James Almer wrote:
>> On 1/20/2018 1:12 AM, James Almer wrote:
>>> Signed-off-by: James Almer 
>>> ---
>>>  libavcodec/hevc_parser.c | 21 +
>>>  1 file changed, 9 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/libavcodec/hevc_parser.c b/libavcodec/hevc_parser.c
>>> index ff7e8a49d6..a3a9098c7c 100644
>>> --- a/libavcodec/hevc_parser.c
>>> +++ b/libavcodec/hevc_parser.c
>>> @@ -24,6 +24,7 @@
>>>  
>>>  #include "golomb.h"
>>>  #include "hevc.h"
>>> +#include "hevc_parse.h"
>>>  #include "hevc_ps.h"
>>>  #include "hevc_sei.h"
>>>  #include "h2645_parse.h"
>>> @@ -43,6 +44,8 @@ typedef struct HEVCParserContext {
>>>  HEVCSEI sei;
>>>  SliceHeader sh;
>>>  
>>> +int is_avc;
>>> +int nal_length_size;
>>>  int parsed_extradata;
>>>  
>>>  int poc;
>>> @@ -181,7 +184,6 @@ static int parse_nal_units(AVCodecParserContext *s, 
>>> const uint8_t *buf,
>>>  HEVCParserContext *ctx = s->priv_data;
>>>  HEVCParamSets *ps = &ctx->ps;
>>>  HEVCSEI *sei = &ctx->sei;
>>> -int is_global = buf == avctx->extradata;
>>>  int ret, i;
>>>  
>>>  /* set some sane default values */
>>> @@ -191,8 +193,8 @@ static int parse_nal_units(AVCodecParserContext *s, 
>>> const uint8_t *buf,
>>>  
>>>  ff_hevc_reset_sei(sei);
>>>  
>>> -ret = ff_h2645_packet_split(&ctx->pkt, buf, buf_size, avctx, 0, 0,
>>> -AV_CODEC_ID_HEVC, 1);
>>> +ret = ff_h2645_packet_split(&ctx->pkt, buf, buf_size, avctx, 
>>> ctx->is_avc,
>>> +ctx->nal_length_size, AV_CODEC_ID_HEVC, 1);
>>>  if (ret < 0)
>>>  return ret;
>>>  
>>> @@ -230,12 +232,6 @@ static int parse_nal_units(AVCodecParserContext *s, 
>>> const uint8_t *buf,
>>>  case HEVC_NAL_RADL_R:
>>>  case HEVC_NAL_RASL_N:
>>>  case HEVC_NAL_RASL_R:
>>> -
>>> -if (is_global) {
>>> -av_log(avctx, AV_LOG_ERROR, "Invalid NAL unit: %d\n", 
>>> nal->type);
>>> -return AVERROR_INVALIDDATA;
>>> -}
>>> -
>>>  ret = hevc_parse_slice_header(s, nal, avctx);
>>>  if (ret)
>>>  return ret;
>>> @@ -243,8 +239,7 @@ static int parse_nal_units(AVCodecParserContext *s, 
>>> const uint8_t *buf,
>>>  }
>>>  }
>>>  /* didn't find a picture! */
>>> -if (!is_global)
>>> -av_log(avctx, AV_LOG_ERROR, "missing picture in access unit\n");
>>> +av_log(avctx, AV_LOG_ERROR, "missing picture in access unit\n");
>>>  return -1;
>>>  }
>>>  
>>> @@ -301,7 +296,9 @@ static int hevc_parse(AVCodecParserContext *s, 
>>> AVCodecContext *avctx,
>>>  ParseContext *pc = &ctx->pc;
>>>  
>>>  if (avctx->extradata && !ctx->parsed_extradata) {
>>> -parse_nal_units(s, avctx->extradata, avctx->extradata_size, avctx);
>>> +ff_hevc_decode_extradata(buf, buf_size, &ctx->ps, &ctx->sei, 
>>> &ctx->is_avc,
>>
>> Changed buf and buf_size locally to avctx->extradata and
>> avctx->extradata_size.
>>
>>> + &ctx->nal_length_size, 
>>> avctx->err_recognition,
>>> + 1, avctx);
>>>  ctx->parsed_extradata = 1;
>>>  }
>>>  
> 
> Ping.

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


Re: [FFmpeg-devel] [PATCH 4/5] avcodec/mediacodecdec: use ff_hevc_uninit_parameter_sets()

2018-01-30 Thread James Almer
On 1/20/2018 8:02 PM, James Almer wrote:
> On 1/20/2018 8:00 PM, wm4 wrote:
>> On Sat, 20 Jan 2018 18:12:52 -0300
>> James Almer  wrote:
>>
>>> Signed-off-by: James Almer 
>>> ---
>>>  libavcodec/mediacodecdec.c | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/libavcodec/mediacodecdec.c b/libavcodec/mediacodecdec.c
>>> index 6c5d3ddd79..b360e7a7f1 100644
>>> --- a/libavcodec/mediacodecdec.c
>>> +++ b/libavcodec/mediacodecdec.c
>>> @@ -258,6 +258,8 @@ static int hevc_set_extradata(AVCodecContext *avctx, 
>>> FFAMediaFormat *format)
>>>  }
>>>  
>>>  done:
>>> +ff_hevc_uninit_parameter_sets(&ps);
>>> +
>>>  av_freep(&vps_data);
>>>  av_freep(&sps_data);
>>>  av_freep(&pps_data);
>>
>> Did this leak before? Or is it only needed to make it work with
>> patch 5/5?
> 
> It most likely leaked before. Notice how hevc parser and decoder both
> free the buffers, and even mediacodecdec here does it with h264.
> 
> I can't test this module in any case, so someone else will have to
> confirm this.

Pushed. It's pretty clear there are memleaks without it.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] libavfilter: constify filter list

2018-01-30 Thread Muhammad Faiz
On Tue, Jan 30, 2018 at 9:09 PM, Mark Thompson  wrote:
> On 30/01/18 07:24, Muhammad Faiz wrote:
>> Move REGISTER_FILTER to FILTER_TABLE in configure.
>> Auto generate filter extern and filter table.
>> Sort filter table, use bsearch on avfilter_get_by_name.
>> Define next pointer at filter extern, no need to initialize
>> next pointer at run time, so AVFilter can be set to const.
>> Make avfilter_register always return error.
>> Target checkasm now depends on EXTRALIBS-avformat.
>>
>> Signed-off-by: Muhammad Faiz 
>> ---
>
> I like the idea of this, but I'm not sure about some of the implementation 
> details.
>
> Have you considered dropping the "next" links entirely and having just the 
> array of pointers instead?  I feel like they don't really add anything useful 
> once we are in this form, and result in more boilerplate on every filter and 
> some tricky generation code.
>
> avfilter_next() would be a bit slower (since it would have to search the 
> array, and therefore have runtime linear in the number of filters), but 
> avfilter_get_by_name() is a lot faster because of the binary search (and is 
> the only common use of it) so I don't think that complaint would be a strong 
> one.

Making avfilter_next() slower (even if it is rarely used) isn't good, I think.


>
>>  Makefile   |   4 +-
>>  configure  | 440 
>> -
>> ...
>>  tests/checkasm/Makefile|   2 +-
>>  303 files changed, 1229 insertions(+), 796 deletions(-)
>>
>> diff --git a/Makefile b/Makefile
>> index 9defddebfd..f607579369 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -56,6 +56,7 @@ tools/uncoded_frame$(EXESUF): ELIBS = $(FF_EXTRALIBS)
>>  tools/target_dec_%_fuzzer$(EXESUF): $(FF_DEP_LIBS)
>>
>>  CONFIGURABLE_COMPONENTS =   \
>> +$(SRC_PATH)/configure   \
>>  $(wildcard $(FFLIBS:%=$(SRC_PATH)/lib%/all*.c)) \
>>  $(SRC_PATH)/libavcodec/bitstream_filters.c  \
>>  $(SRC_PATH)/libavformat/protocols.c \
>> @@ -142,7 +143,8 @@ distclean:: clean
>>   $(RM) .version avversion.h config.asm config.h mapfile  \
>>   ffbuild/.config ffbuild/config.* libavutil/avconfig.h \
>>   version.h libavutil/ffversion.h libavcodec/codec_names.h \
>> - libavcodec/bsf_list.c libavformat/protocol_list.c
>> + libavcodec/bsf_list.c libavformat/protocol_list.c \
>> + libavfilter/filter_list.h libavfilter/filter_list.c
>>  ifeq ($(SRC_LINK),src)
>>   $(RM) src
>>  endif
>> diff --git a/configure b/configure
>> index fcfa7aa442..3261f5fd1a 100755
>> --- a/configure
>> +++ b/configure
>> @@ -3177,6 +3177,381 @@ unix_protocol_deps="sys_un_h"
>>  unix_protocol_select="network"
>>
>>  # filters
>> +FILTER_TABLE="
>> +abench  af
>> +acompressor af
>> +acontrast   af
>> ...
>> +spectrumsynth   vaf
>> +amovie  avsrc
>> +movie   avsrc
>> +"
>> +
>> +UNCONDITIONAL_FILTER_TABLE="
>> +abuffer asrc
>> +buffer  vsrc
>> +abuffersink asink
>> +buffersink  vsink
>> +afifo   af
>> +fifovf
>> +"
>> +
>
> I don't really like having this table in configure.  Since you're generating 
> the filter_list.h header with the external definitions from it anyway, why 
> not write that and use it as the source rather than having the table here?

Imho, parsing source code and then generating source code is
pointless, it is redundant. Previously, it was just parsing source
code without generating source code. And now it is just generating
source code without parsing source code. There are no duplicates here.

Also storing filter table in configure is more consistent, I think.
Because filter dependencies are in configure.


>
>>  afftfilt_filter_deps="avcodec"
>>  afftfilt_filter_select="fft"
>>  afir_filter_deps="avcodec"
>> @@ -3530,7 +3905,18 @@ MUXER_LIST=$(find_thingsmuxer_MUX 
>> libavformat/allformats.c)
>>  DEMUXER_LIST=$(find_things  demuxer  DEMUXlibavformat/allformats.c)
>>  OUTDEV_LIST=$(find_things   outdev   OUTDEV   libavdevice/alldevices.c)
>>  INDEV_LIST=$(find_thingsindev_IN  libavdevice/alldevices.c)
>> -FILTER_LIST=$(find_things   filter   FILTER   libavfilter/allfilters.c)
>> +
>> +extract_list_from_table(){
>> +cols=$1
>> +suffix=$2
>> +shift 2
>> +while test -n "$1"; do
>> +echo "${1}${suffix}"
>> +shift $cols
>> +done
>> +}
>> +
>> +FILTER_LIST=$(extract_list_from_table 2 _filter $FILTER_TABLE)
>>
>>  find_things_extern(){
>>  thing=$1
>> @@ -7030,6 +7416,58 @@ print_enabled_components(){
>>  print_enabled_components libavcodec/bsf_list.c AVBitStreamFilter 
>> bitstream_filters $BSF_LIST
>>  print_enabled_components libavformat/protocol_list.c URLProtocol 
>> url_pro

Re: [FFmpeg-devel] [mp3] Skip APE tags when parsing mp3 packets.

2018-01-30 Thread Michael Niedermayer
On Tue, Jan 30, 2018 at 11:30:43AM -0300, James Almer wrote:
> On 1/30/2018 2:45 AM, wm4 wrote:
> > On Tue, 30 Jan 2018 02:24:29 +0100
> > Michael Niedermayer  wrote:
> > 
> >> On Mon, Jan 29, 2018 at 03:13:54PM -0800, Dale Curtis wrote:
> >>> Otherwise the decoder will throw "Missing header" errors when the
> >>> packets are sent for decoding.  
> >>
> >>>  mpegaudio_parser.c |7 +++
> >>>  1 file changed, 7 insertions(+)
> >>> 2628fa8480b15237a528e94b1689da7321ce9440  skip-ape-tags.patch
> >>> From 05f5e837862a958fb8791668ab3cdf8fc7766fe5 Mon Sep 17 00:00:00 2001
> >>> From: Dale Curtis 
> >>> Date: Mon, 29 Jan 2018 15:10:26 -0800
> >>> Subject: [PATCH] [mp3] Skip APE tags when parsing mp3 packets.
> >>>
> >>> Otherwise the decoder will throw "Missing header" errors when the
> >>> packets are sent for decoding.
> >>> ---
> >>>  libavcodec/mpegaudio_parser.c | 7 +++
> >>>  1 file changed, 7 insertions(+)
> >>>
> >>> diff --git a/libavcodec/mpegaudio_parser.c b/libavcodec/mpegaudio_parser.c
> >>> index 8c39825792..244281b56f 100644
> >>> --- a/libavcodec/mpegaudio_parser.c
> >>> +++ b/libavcodec/mpegaudio_parser.c
> >>> @@ -23,6 +23,7 @@
> >>>  #include "parser.h"
> >>>  #include "mpegaudiodecheader.h"
> >>>  #include "libavutil/common.h"
> >>> +#include "libavformat/apetag.h" // for APE tag.
> >>>  #include "libavformat/id3v1.h" // for ID3v1_TAG_SIZE
> >>>  
> >>>  typedef struct MpegAudioParseContext {
> >>> @@ -120,6 +121,12 @@ static int mpegaudio_parse(AVCodecParserContext *s1,
> >>>  return next;
> >>>  }
> >>>  
> >>> +if (flush && buf_size >= APE_TAG_FOOTER_BYTES && memcmp(buf, 
> >>> APE_TAG_PREAMBLE, 8) == 0) {
> >>> +*poutbuf = NULL;
> >>> +*poutbuf_size = 0;
> >>> +return next;
> >>> +}  
> >>
> >> This doesnt feel right
> >>
> >> Parsers should not discard data
> >>
> >> a bistream filter could discard data, so could a demuxer if thats how the
> >> format should be interpreted. Or the decoder could simply detect this case
> >> and not print an error/warning
> > 
> > This should obviously be done by the demuxer, unless I'm missing some
> > other use cases. Should still be OK to skip in the parser. Tags have no
> > business in a packet stream (they're not supposed to be there)
> 
> I recently changed the raw aac demuxer to stop propagating "junk" at the
> beginning and end of the stream (things like id3 and ape tags) by
> discarding anything that's not a complete frame, but the result was that
> it kinda broke some fully playable files that had one or two damaged
> frames. It was meant for codec copy cases, since the aac decoder dealt
> with it just fine.
> 

> Skipping junk until a sync word is found then combining data until you
> get a full frame is currently a valid usage for parsers (see mlp
> parser).

I do disagree about this
Parsers where supposed to split data in frames but not discard any data. Yes 
we do have some code that breaks that rule and some of it like the one you
quoted earlier written by me (iam not really proud of that one but lucky its
very limited to what it can discard and where)

But the ideal behavior was to never discard data, include it in the next
frame, the previous or a seperate frame but not to discard.

Parsers are "mandatory" for some demuxers and if they discard data then that
data is lost and the user app would not even know that there was something that
was discarded. A decoder may very well be able to recover something from the
data.



> But i think the plan was to have them only analyze headers to
> set relevant parameters in both frames and codec context, and leave the
> task to produce full frames to bitstream filters.

This is a good idea but i dont think thats a plan that was discussed or
agreed on for FFmpeg. Design changes like this should be discussed on
ffmpeg-devel. FFmpegs design should be directed by the FFmpeg community

Thanks

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

The real ebay dictionary, page 2
"100% positive feedback" - "All either got their money back or didnt complain"
"Best seller ever, very honest" - "Seller refunded buyer after failed scam"


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


Re: [FFmpeg-devel] [mp3] Skip APE tags when parsing mp3 packets.

2018-01-30 Thread Dale Curtis
On Tue, Jan 30, 2018 at 6:44 AM, wm4  wrote:

> In theory we could have a flag that controls this (I thought there
> actually was such a flag, but didn't find anything). If we ever find
> that discarding data is a problem, maybe such a flag could be added.
>
> Also I still kind of hope someone will make a new parser API, since the
> current one is clunky with its AVCodecContext use and its inability to
> return errors.
>
>
+1 to both of these points. To add in my 2 cents, consumption of metadata
(APE, ID3, etc) packets seems like something that should be part of the
core avformat routines. I.e. they should transparently feed into the
AVFormatContext metadata structure. Chrome doesn't use them, so we're fine
with discarding them like I did in my patch, but forwarding them to clients
which have been told these are packets for codec X when in reality they are
metadata packets feels unfortunate.

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


Re: [FFmpeg-devel] [PATCH 1/3] avformat/mov: Increase support for common encryption.

2018-01-30 Thread Jacob Trimble
On Wed, Jan 24, 2018 at 5:46 PM, Michael Niedermayer
 wrote:
> On Wed, Jan 24, 2018 at 11:43:26AM -0800, Jacob Trimble wrote:
>> On Mon, Jan 22, 2018 at 7:38 PM, Michael Niedermayer
>>  wrote
>> > [...]
>> >> This removes support for saio/saiz atoms, but it was incorrect before.
>> >> A follow-up change will add correct support for those.
>> >
>> > This removal should be done by a seperate patch if it is done.
>> > diff has matched up the removed function with a added one making this
>> > hard to read as is
>> >
>>
>> The problem is that the old code used the saiz atoms to parse the senc
>> atom.  I split the patch up for readability, but the two patches need
>> to be applied at the same time (or squashed) since the first breaks
>> encrypted content.  But I can squash them again if it is preferable to
>> not have a commit that intentionally breaks things.
>
> I didnt investigate this deeply so there is likely a better option that
> i miss but you could just remove the functions which become unused in a
> subsequent patch to prevent diff from messing the line matching up totally
>

Done.

>
>>
>> >
>> >>
>> >> Signed-off-by: Jacob Trimble 
>> >> ---
>> >>  libavformat/isom.h |  20 +-
>> >>  libavformat/mov.c  | 432 
>> >> ++---
>> >>  tests/fate/mov.mak |   8 +
>> >>  tests/ref/fate/mov-frag-encrypted  |  57 +
>> >>  tests/ref/fate/mov-tenc-only-encrypted |  57 +
>> >>  5 files changed, 422 insertions(+), 152 deletions(-)
>> >>  create mode 100644 tests/ref/fate/mov-frag-encrypted
>> >>  create mode 100644 tests/ref/fate/mov-tenc-only-encrypted
>> >
>> > This depends on other patches you posted, this should be mentioned or
>> > all patches should be in the same patchset in order
>> >
>>
>> This depends on
>> http://ffmpeg.org/pipermail/ffmpeg-devel/2018-January/223754.html and
>> the recently pushed change to libavutil/aes_ctr.  Should I add
>> something to the commit message or is that enough?
>
> If you post a new version, then there should be a mail or comment explaining
> any dependancies on yet to be applied patches.
> It should not be in the commit messages or commited changes ideally
> This way people trying to test code dont need to guess what they need
> to apply first before a patchset
>
>
> [...]
>> >> +static int get_current_encryption_info(MOVContext *c, MOVEncryptionIndex 
>> >> **encryption_index, MOVStreamContext **sc)
>> >>  {
>> >> +MOVFragmentStreamInfo *frag_stream_info;
>> >>  AVStream *st;
>> >> -MOVStreamContext *sc;
>> >> -size_t auxiliary_info_size;
>> >> +int i;
>> >>
>> >> -if (c->decryption_key_len == 0 || c->fc->nb_streams < 1)
>> >> -return 0;
>> >> +frag_stream_info = get_current_frag_stream_info(&c->frag_index);
>> >> +if (frag_stream_info) {
>> >> +for (i = 0; i < c->fc->nb_streams; i++) {
>> >> +if (c->fc->streams[i]->id == frag_stream_info->id) {
>> >> +  st = c->fc->streams[i];
>> >> +  break;
>> >> +}
>> >> +}
>> >
>> > the indention is inconsistent here
>> >
>>
>> No it's not, it looks like it because the diff looks odd.  If you
>> apply the patch, the indentation in this method is consistent.
>
> Indention depth is 4 in mov*.c
> the hunk seems to add lines with a depth of 2
> I would be surprised if this is not in the file after applying the patch
>
> personally i dont care about the depth that much but i know many other people
> care so this needs to be fixed before this can be applied

Didn't see that.  Fixed and did a grep for incorrect indentations.

>
> [...]
>
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Let us carefully observe those good qualities wherein our enemies excel us
> and endeavor to excel them, by avoiding what is faulty, and imitating what
> is excellent in them. -- Plutarch
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
From 18807b5c77e65db5f641b7dbfd4a539c3d7bd5c7 Mon Sep 17 00:00:00 2001
From: Jacob Trimble 
Date: Wed, 6 Dec 2017 16:17:54 -0800
Subject: [PATCH] avformat/mov: Increase support for common encryption.

- Parse schm atom to get different encryption schemes.
- Allow senc atom to appear in track fragments.
- Allow 16-byte IVs.
- Allow constant IVs (specified in tenc).
- Allow only tenc to specify encryption (i.e. no senc/saiz/saio).
- Use sample descriptor to detect clear fragments.

This doesn't support:
- Different sample descriptor holding different encryption info.
  - Only first sample descriptor can be encrypted.
- Encrypted sample groups (i.e. seig).
- Non-'cenc' encryption scheme when using -decryption_key.

Signed-off-by: Jacob Trimble 
---
 libavformat/isom.h |  14 ++
 libavformat/mov.c  | 396 ++---
 tests/fate/mo

Re: [FFmpeg-devel] [PATCH] libavfilter: constify filter list

2018-01-30 Thread Michael Niedermayer
On Tue, Jan 30, 2018 at 08:27:27PM +0700, Muhammad Faiz wrote:
> On Tue, Jan 30, 2018 at 7:50 PM, Michael Niedermayer
>  wrote:
> > On Tue, Jan 30, 2018 at 02:24:12PM +0700, Muhammad Faiz wrote:
> >> Move REGISTER_FILTER to FILTER_TABLE in configure.
> >> Auto generate filter extern and filter table.
> >> Sort filter table, use bsearch on avfilter_get_by_name.
> >> Define next pointer at filter extern, no need to initialize
> >> next pointer at run time, so AVFilter can be set to const.
> >
> >> Make avfilter_register always return error.
> >
> > That breaks API
> 
> No. Because with current API, it is impossible to implement external filter.
> 
> 
> > Its also a step away from supporting plugins.
> > Why plugins matter ? Because having plugin
> > support is a big advantage, it allows a much wider
> > community to work on, write and maintain filters.
> >
> > With plugins, people can write filters that are
> > written in languages other than C. Or filters which
> > some developer in FFmpeg doesnt want. Or they can be
> > maintained externally by people who just do not like us.
> > Or by people who perfer a FOSS license different from
> > LGPL/GPL/BSD. Iam sure others can come up with more
> > reasons ...
> >
> > Of course avfilter_register() isnt enough for plugins
> > but it or something equivalent is needed for plugins.
> >
> > So i would prefer if avfilter_register() stays supported
> > indefinitly or in case a different system is written for
> > plugins then until that system is in place.
> 
> It just returns error and logs error message that currently external
> filter is unsupported. If someone wants to implement support for
> external filter, it can be easily added later using separate
> list/table.

Iam not sure "easily" is true

We started out with a fully public API that allowed external filters.
and little by little its removed or made private.
each individual change may be easy to undo but as a whole moving
libavfilter back to having a public API is not that easy anymore

IIRC the arguments to make things private where that people wanted to
improve the API. But from the idea and impression of a plan like:
1. make it private
2. design and implement better API
3. make it public again

Somehow now people lost sight and interrest in 3. and even 2. is becoming
less interresting. But the problem is really without 2 + 3 actually happening
doing 1 seems like not a good idea at all.

To me it seems even mentioning external filters and public API makes some
people angry.
But really that has to be the goal at some point. To again have a public API

Is the plan to have 2 seperrate lists at that point ?
one static for internal filters and one dynamic for externally registered
ones ?

Iam not objecting to the patch, theres nothing i have that uses the call
but iam a bit concerned about interrest_to_remove > interrest_in_public_api

thanks

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

It is what and why we do it that matters, not just one of them.


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


Re: [FFmpeg-devel] [PATCH 1/2] avcodec/hevc_ps: Check log2_sao_offset_scale_*

2018-01-30 Thread Michael Niedermayer
On Wed, Jan 24, 2018 at 04:34:49AM +0100, Michael Niedermayer wrote:
> Fixes: 4868/clusterfuzz-testcase-minimized-6236542906400768
> Fixes: runtime error: shift exponent 126 is too large for 32-bit type 'int'
> 
> Found-by: continuous fuzzing process 
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/hevc_ps.c | 11 +++
>  1 file changed, 11 insertions(+)

applied without the error message


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

Concerning the gods, I have no means of knowing whether they exist or not
or of what sort they may be, because of the obscurity of the subject, and
the brevity of human life -- Protagoras


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


Re: [FFmpeg-devel] [PATCH 1/2] avcodec/dirac_dwt: Fix several integer overflows

2018-01-30 Thread Michael Niedermayer
On Fri, Jan 26, 2018 at 02:56:29AM +0100, Michael Niedermayer wrote:
> Fixes: runtime error: signed integer overflow: -2146071175 + -268479557 
> cannot be represented in type 'int'
> Fixes: 5237/clusterfuzz-testcase-minimized-4569895275593728
> 
> Found-by: continuous fuzzing process 
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/dirac_dwt.h  | 4 ++--
>  libavcodec/dirac_dwt_template.c | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)

patchset applied

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

If you think the mosad wants you dead since a long time then you are either
wrong or dead since a long time.


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


Re: [FFmpeg-devel] [PATCH] id3v2: fix unsynchronization

2018-01-30 Thread Richard Shaffer
LGTM, for whatever my opinion is worth.

Also easier to follow than the old code.

-Richard

On Tue, Jan 30, 2018 at 4:47 AM, wm4  wrote:
> On Tue, 30 Jan 2018 13:43:25 +0100
> wm4  wrote:
>
>> The ID3v2 "unsynchronization scheme" requires replacing any 0xFF 0x00
>> sequences with 0xFF. This has to be done on every byte of the source
>> data, while the current code skipped a byte after a replacement. This
>> meant 0xFF 0x00 0xFF 00 was translated to 0xFF 0xFF 0x00 instead of 0xFF
>> 0xFF. It feels a bit messy to do this correctly with the avio use. But
>> fortunately, this translation can be done in-place, so we can just do it
>> in memory.
>>
>> Inspired by what taglib does.
>> ---
>> Sample (which had corrupted cover art, displays fine with the fix):
>> https://0x0.st/sbQ9.bin (unfortunately a bit too large for FATE)
>> ---
>>  libavformat/id3v2.c | 26 ++
>>  1 file changed, 14 insertions(+), 12 deletions(-)
>>
>> diff --git a/libavformat/id3v2.c b/libavformat/id3v2.c
>> index b80178d67a..f7de26a1d8 100644
>> --- a/libavformat/id3v2.c
>> +++ b/libavformat/id3v2.c
>> @@ -976,19 +976,21 @@ static void id3v2_parse(AVIOContext *pb, AVDictionary 
>> **metadata,
>>  }
>>  }
>>  if (unsync || tunsync) {
>> -int64_t end = avio_tell(pb) + tlen;
>> -uint8_t *b;
>> -
>> -b = buffer;
>> -while (avio_tell(pb) < end && b - buffer < tlen && 
>> !pb->eof_reached) {
>> -*b++ = avio_r8(pb);
>> -if (*(b - 1) == 0xff && avio_tell(pb) < end - 1 &&
>> -b - buffer < tlen &&
>> -!pb->eof_reached ) {
>> -uint8_t val = avio_r8(pb);
>> -*b++ = val ? val : avio_r8(pb);
>> -}
>> +uint8_t *b = buffer;
>> +uint8_t *t = buffer;
>> +uint8_t *end = t + tlen;
>> +
>> +if (avio_read(pb, buffer, tlen) != tlen) {
>> +av_log(s, AV_LOG_ERROR, "Failed to read tag data\n");
>> +goto seek;
>>  }
>> +
>> +while (t != end) {
>> +*b++ = *t++;
>> +if (t != end && t[-1] == 0xff && !t[0])
>> +t++;
>> +}
>> +
>>  ffio_init_context(&pb_local, buffer, b - buffer, 0, NULL, 
>> NULL, NULL,
>>NULL);
>>  tlen = b - buffer;
>
> Also I just saw that 9ae80e6a9cefcab61e867256ba19ef78a4bfe0cb seems to
> claim that the current code is correct (and my commit essentially
> reverts it). Taglib decodes the tag correctly, though.
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] libavfilter: constify filter list

2018-01-30 Thread Mark Thompson
On 30/01/18 18:06, Muhammad Faiz wrote:
> On Tue, Jan 30, 2018 at 9:09 PM, Mark Thompson  wrote:
>> On 30/01/18 07:24, Muhammad Faiz wrote:
>>> Move REGISTER_FILTER to FILTER_TABLE in configure.
>>> Auto generate filter extern and filter table.
>>> Sort filter table, use bsearch on avfilter_get_by_name.
>>> Define next pointer at filter extern, no need to initialize
>>> next pointer at run time, so AVFilter can be set to const.
>>> Make avfilter_register always return error.
>>> Target checkasm now depends on EXTRALIBS-avformat.
>>>
>>> Signed-off-by: Muhammad Faiz 
>>> ---
>>
>> I like the idea of this, but I'm not sure about some of the implementation 
>> details.
>>
>> Have you considered dropping the "next" links entirely and having just the 
>> array of pointers instead?  I feel like they don't really add anything 
>> useful once we are in this form, and result in more boilerplate on every 
>> filter and some tricky generation code.
>>
>> avfilter_next() would be a bit slower (since it would have to search the 
>> array, and therefore have runtime linear in the number of filters), but 
>> avfilter_get_by_name() is a lot faster because of the binary search (and is 
>> the only common use of it) so I don't think that complaint would be a strong 
>> one.
> 
> Making avfilter_next() slower (even if it is rarely used) isn't good, I think.

I think the slowdown is irrelevant if it is rarely used.

On the other side, you get rid of a field in AVFilter and avoid having to put 
some pointless boilerplate in a lot of places.

Related: the one remaining use of avfilter_next() inside lavfi, in 
filter_child_class_next(), should also be replaced with array operations.  (I 
can easily do that if you don't want to bother as part of this patch.)

>>
>>>  Makefile   |   4 +-
>>>  configure  | 440 
>>> -
>>> ...
>>>  tests/checkasm/Makefile|   2 +-
>>>  303 files changed, 1229 insertions(+), 796 deletions(-)
>>>
>>> diff --git a/Makefile b/Makefile
>>> index 9defddebfd..f607579369 100644
>>> --- a/Makefile
>>> +++ b/Makefile
>>> @@ -56,6 +56,7 @@ tools/uncoded_frame$(EXESUF): ELIBS = $(FF_EXTRALIBS)
>>>  tools/target_dec_%_fuzzer$(EXESUF): $(FF_DEP_LIBS)
>>>
>>>  CONFIGURABLE_COMPONENTS =   \
>>> +$(SRC_PATH)/configure   \
>>>  $(wildcard $(FFLIBS:%=$(SRC_PATH)/lib%/all*.c)) \
>>>  $(SRC_PATH)/libavcodec/bitstream_filters.c  \
>>>  $(SRC_PATH)/libavformat/protocols.c \
>>> @@ -142,7 +143,8 @@ distclean:: clean
>>>   $(RM) .version avversion.h config.asm config.h mapfile  \
>>>   ffbuild/.config ffbuild/config.* libavutil/avconfig.h \
>>>   version.h libavutil/ffversion.h libavcodec/codec_names.h \
>>> - libavcodec/bsf_list.c libavformat/protocol_list.c
>>> + libavcodec/bsf_list.c libavformat/protocol_list.c \
>>> + libavfilter/filter_list.h libavfilter/filter_list.c
>>>  ifeq ($(SRC_LINK),src)
>>>   $(RM) src
>>>  endif
>>> diff --git a/configure b/configure
>>> index fcfa7aa442..3261f5fd1a 100755
>>> --- a/configure
>>> +++ b/configure
>>> @@ -3177,6 +3177,381 @@ unix_protocol_deps="sys_un_h"
>>>  unix_protocol_select="network"
>>>
>>>  # filters
>>> +FILTER_TABLE="
>>> +abench  af
>>> +acompressor af
>>> +acontrast   af
>>> ...
>>> +spectrumsynth   vaf
>>> +amovie  avsrc
>>> +movie   avsrc
>>> +"
>>> +
>>> +UNCONDITIONAL_FILTER_TABLE="
>>> +abuffer asrc
>>> +buffer  vsrc
>>> +abuffersink asink
>>> +buffersink  vsink
>>> +afifo   af
>>> +fifovf
>>> +"
>>> +
>>
>> I don't really like having this table in configure.  Since you're generating 
>> the filter_list.h header with the external definitions from it anyway, why 
>> not write that and use it as the source rather than having the table here?
> 
> Imho, parsing source code and then generating source code is
> pointless, it is redundant. Previously, it was just parsing source
> code without generating source code. And now it is just generating
> source code without parsing source code. There are no duplicates here.

This is parsing a huge variable in configure and generating two different files 
from it.  Parsing one file and generating one file seems clearer, though I 
guess that's mostly subjective.

> Also storing filter table in configure is more consistent, I think.
> Because filter dependencies are in configure.

Storing it in configure feels less consistent to me - no other components lists 
like this are in configure, they are all in their own files (either as macros 
the allfoo.c files like the one you are removing or as external declarations).  
Also configure is already inconveniently huge, and this is adding many mor

Re: [FFmpeg-devel] [FFmpeg-cvslog] avcodec/mpegaudio_parser: Skip APE tags when parsing mp3 packets.

2018-01-30 Thread Alexander Strasser
On 2018-01-30 04:29 +, Dale Curtis wrote:
> ffmpeg | branch: master | Dale Curtis  | Mon Jan 29 
> 15:10:26 2018 -0800| [42323c3e3a600288e4bf1cefe952486ffc29d280] | committer: 
> Michael Niedermayer
> 
> avcodec/mpegaudio_parser: Skip APE tags when parsing mp3 packets.
> 
> Otherwise the decoder will throw "Missing header" errors when the
> packets are sent for decoding.
> 
> This is similar to 89a420b71b5.
> 
> Signed-off-by: Michael Niedermayer 
> 
> > http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=42323c3e3a600288e4bf1cefe952486ffc29d280
> ---
> 
>  libavcodec/mpegaudio_parser.c | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/libavcodec/mpegaudio_parser.c b/libavcodec/mpegaudio_parser.c
> index 8c39825792..244281b56f 100644
> --- a/libavcodec/mpegaudio_parser.c
> +++ b/libavcodec/mpegaudio_parser.c
> @@ -23,6 +23,7 @@
>  #include "parser.h"
>  #include "mpegaudiodecheader.h"
>  #include "libavutil/common.h"
> +#include "libavformat/apetag.h" // for APE tag.
>  #include "libavformat/id3v1.h" // for ID3v1_TAG_SIZE
>  
>  typedef struct MpegAudioParseContext {
> @@ -120,6 +121,12 @@ static int mpegaudio_parse(AVCodecParserContext *s1,
>  return next;
>  }
>  

> +if (flush && buf_size >= APE_TAG_FOOTER_BYTES && memcmp(buf, 
> APE_TAG_PREAMBLE, 8) == 0) {

I wonder if it would have been better to write out the preamble as
literal ("APETAGEX"). It just feels a bit wrong to have the data
described in one place (apetag.h) and the length in another.

For this instance it isn't important at all and I would think
APE_TAG_PREAMBLE is very unlikely to ever change.


  Alexander

> +*poutbuf = NULL;
> +*poutbuf_size = 0;
> +return next;
> +}
> +
>  *poutbuf = buf;
>  *poutbuf_size = buf_size;
>  return next;
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH]: Change Stack Frame Limit in Cuda Context

2018-01-30 Thread Ben Chang
On Fri, Jan 26, 2018 at 3:10 PM, Mark Thompson  wrote:

> On 26/01/18 20:51, Ben Chang wrote:
> > On Fri, Jan 26, 2018 at 3:32 AM, Mark Thompson  wrote:
> >
> >> On 26/01/18 09:06, Ben Chang wrote:
> >>> Thanks for the review Mark.
> >>>
>
> To clarify, since it is less clear now with the trimmed context: my two
> comments about this change are completely independent.  (Given that, maybe
> it should be split into two parts - one for hwcontext and one for nvenc?)

Sorry for the delay in reply Mark; been caught up by something else.

>


> This part is about the change to NVENC:
>
> >>>  There are some cuda kernels in the driver that may be invoked
> depending
> >> on
> >>> the nvenc operations specified in the commandline. My observation from
> >>> looking at the nvcc statistics is that most stack frame size for these
> >> cuda
> >>> kernels are 0 (highest observed was 120 bytes).
> >>
> >> Right, that makes sense.  If Nvidia is happy that this will always work
> in
> >> drivers compatible with this API version (including any future ones)
> then
> >> sure.
> >>
> > I am not saying this should be the "permanent" value for stack frame size
> > per GPU thread. However, at this moment (looking at existing cuda kernels
> > that devs have control over), I do not see this reduction as an issue.
>
> I think you should be confident that the chosen value here will last well
> into the future for NVENC use.  Consider that this will end up in releases
> - if a future Nvidia driver update happens to need a larger stack then all
> previous releases and binaries will stop working for all users.
>
>
> This part is about the change to the hwcontext device creation:
>
>  This is technically a user-visible change, since it will apply to all
> >> user
>  programs run on the CUDA context created here as well as those inside
>  ffmpeg.  I'm not sure how many people actually use that, though, so
> >> maybe
>  it won't affect anyone.
> 
> >>> In ffmpeg, I see vf_thumbnail_cuda and vf_scale_cuda available (not
> sure
> >> if
> >>> there is more, but these two should not be affected by this reduction).
> >>> User can always raise the stack limit size if their own custom kernel
> >>> require higher stack frame size.
> >>
> >> I don't mean filters inside ffmpeg, I mean a user program which probably
> >> uses NVDEC and/or NVENC (and possibly other things) from libavcodec but
> >> then does its own CUDA processing with the same context.  This is
> silently
> >> changing the setup underneath it, and 128 feels like a very small
> number.
> >>
> > Yes, this is really a trade off between reducing memory usage (since
> there
> > are numerous complaints of high memory usage preventing having more
> ffmpeg
> > instances) and user convenience (custom cuda implementation may be
> > impacted). My thought (which can be wrong) is that users who implement
> > their own cuda kernel may have better knowledge about cuda (eg. how much
> > stack frame size their kernel needs or use cuda debugger to find out what
> > issue they may have). The size of the kernels are really implementation
> > dependent (eg, allocating arrays in stacks or heap, recursions, how much
> > register spills, etc) so stack frame sizes may vary widely. The default,
> > 1024 bytes, may not be enough at times and user will need to adjust the
> > stack limit accordingly anyway.
>
> Note that since you are changing a library the users in this context are
> all ffmpeg library users.  So, it means any program or library which uses
> ffmpeg, and transitively anyone who uses them.  The end-user need not be
> informed about CUDA at all.
>
> (What you've said also makes it sound like it can change by compiler
> version, but I guess such changes should be small.)
>
> 
>  If the stack limit is violated, what happens?  Will that be undefined
>  behaviour with random effects (crash / incorrect results), or is it
> >> likely
>  to be caught at program compile/load-time?
> 
> >>> Stack will likely overflow and kernel will terminate (though I have yet
> >>> encounter this before).
> >>
> >> As long as the user gets a clear message that a stack overflow has
> >> occurred so that they can realise that they need to raise the value
> then it
> >> should be fine.
> >
> > I believe you will see stack overflow if attached to cuda debugger. But
> the
> > default error may just be kernel launch error/failure. This goes back to
> my
> > opinion that cuda developer should figure this out relatively easy if
> they
> > want to customize the cuda part of their program.
>
> Ok, sure.  It's possibly unfortunate if a user who knows nothing about
> CUDA rebuilds a program from source with newer ffmpeg libraries including
> this change because something could stop working in an opaque way, but if
> it errors out then it won't silently give the wrong answer and should be
> diagnosable by the original developer.

I have asked my colleagues about the effects of cuda kernel's stack

[FFmpeg-devel] [PATCH] hwcontext_vaapi: add iHD open source driver.

2018-01-30 Thread Jun Zhao
Now FFmpeg-VAAPI work with iHD open source driver
(https://github.com/intel/media-driver) initiatory.
From 069611945880643aad6ffb5ea70b732f40bbb510 Mon Sep 17 00:00:00 2001
From: Jun Zhao 
Date: Wed, 31 Jan 2018 08:21:30 +0800
Subject: [PATCH] hwcontext_vaapi: add iHD open source driver.

Signed-off-by: Jun Zhao 
---
 libavutil/hwcontext_vaapi.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/libavutil/hwcontext_vaapi.c b/libavutil/hwcontext_vaapi.c
index 29698d1b27..34c5a2c264 100644
--- a/libavutil/hwcontext_vaapi.c
+++ b/libavutil/hwcontext_vaapi.c
@@ -285,6 +285,10 @@ static const struct {
 "ubit",
 AV_VAAPI_DRIVER_QUIRK_ATTRIB_MEMTYPE,
 },
+{
+"Intel iHD open source driver",
+"Intel iHD driver",
+},
 {
 "VDPAU wrapper",
 "Splitted-Desktop Systems VDPAU backend for VA-API",
-- 
2.14.1

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


[FFmpeg-devel] [PATCH] avcodec/wavpack: Fix integer overflow in FFABS

2018-01-30 Thread Michael Niedermayer
Fixes: negation of -2147483648 cannot be represented in type 'int'; cast to an 
unsigned type to negate this value to itself
Fixes: 5396/clusterfuzz-testcase-minimized-655829281536

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

diff --git a/libavcodec/wavpack.c b/libavcodec/wavpack.c
index d5e1e07b74..0e40b29879 100644
--- a/libavcodec/wavpack.c
+++ b/libavcodec/wavpack.c
@@ -480,7 +480,7 @@ static inline int wv_unpack_stereo(WavpackFrameContext *s, 
GetBitContext *gb,
 }
 
 if (type == AV_SAMPLE_FMT_S16P) {
-if (FFABS(L) + (unsigned)FFABS(R) > (1<<19)) {
+if (FFABS((int64_t)L) + FFABS((int64_t)R) > (1<<19)) {
 av_log(s->avctx, AV_LOG_ERROR, "sample %d %d too large\n", L, 
R);
 return AVERROR_INVALIDDATA;
 }
-- 
2.16.1

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