[FFmpeg-devel] [PATCH]lavc/exr: Ignore long names flag
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
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 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 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
--- 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
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
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
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
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
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
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
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
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
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
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
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
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 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
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
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.
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
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.
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
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()
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
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.
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.
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.
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
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_*
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
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
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
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.
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
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.
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
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