Re: [FFmpeg-devel] [WIP][PATCH] Opus Piramid Vector Quantization Search in x86 SIMD asm
On 6/10/17, James Darnley wrote: > On 2017-06-09 13:41, Ivan Kalvachev wrote: >> On 6/9/17, Michael Niedermayer wrote: >>> seems this breaks build with mingw64, didnt investigate but it >>> fails with these errors: >>> >>> libavcodec/libavcodec.a(opus_pvq_search.o):src/libavcodec/x86/opus_pvq_search.asm:(.text+0x2d): >>> relocation truncated to fit: R_X86_64_32 against `const_align_abs_edge' >>> libavcodec/libavcodec.a(opus_pvq_search.o):src/libavcodec/x86/opus_pvq_search.asm:(.text+0x3fd): >>> relocation truncated to fit: R_X86_64_32 against `const_align_abs_edge' >>> libavcodec/libavcodec.a(opus_pvq_search.o):src/libavcodec/x86/opus_pvq_search.asm:(.text+0x7a1): >>> relocation truncated to fit: R_X86_64_32 against `const_align_abs_edge' >>> libavcodec/libavcodec.a(opus_pvq_search.o):src/libavcodec/x86/opus_pvq_search.asm:(.text+0xb48): >>> relocation truncated to fit: R_X86_64_32 against `const_align_abs_edge' >>> libavcodec/libavcodec.a(opus_pvq_search.o):src/libavcodec/x86/opus_pvq_search.asm:(.text+0x2d): >>> relocation truncated to fit: R_X86_64_32 against `const_align_abs_edge' >>> libavcodec/libavcodec.a(opus_pvq_search.o):src/libavcodec/x86/opus_pvq_search.asm:(.text+0x3fd): >>> relocation truncated to fit: R_X86_64_32 against `const_align_abs_edge' >>> libavcodec/libavcodec.a(opus_pvq_search.o):src/libavcodec/x86/opus_pvq_search.asm:(.text+0x7a1): >>> relocation truncated to fit: R_X86_64_32 against `const_align_abs_edge' >>> libavcodec/libavcodec.a(opus_pvq_search.o):src/libavcodec/x86/opus_pvq_search.asm:(.text+0xb48): >>> relocation truncated to fit: R_X86_64_32 against `const_align_abs_edge' >>> collect2: error: ld returned 1 exit status >>> collect2: error: ld returned 1 exit status >>> make: *** [ffmpeg_g.exe] Error 1 >>> make: *** Waiting for unfinished jobs >>> make: *** [ffprobe_g.exe] Error 1 >> >> >> const_*_edge is used on only one place is the code. >> Would you check if this patch fixes the issue. >> >> I expected that the addresses would be pre-calculated >> by n/yasm as one value and indexed >> relative to the section start. >> Instead it seems that each entry is represented with >> its own address and offset from it. >> Since the offset is negative it uses all 64 bits and >> it makes difference if it is truncated to 32 bits. >> >> Same issue could happen with clang tools. > > The problem is with the relative addressing. You need to load the real > address first before you can offset with another register at runtime. So > something like: > >> mov reg1, [read_only_const] lea ? >> mova mmreg, [reg1 + reg2] . OK, Getting mingw for my distro is problem and compiling one myself would take a bit more effort/time. So I'm posting a patch that "should" work. == --- a/libavcodec/x86/opus_pvq_search.asm +++ b/libavcodec/x86/opus_pvq_search.asm @@ -406,7 +406,7 @@ align 16 ; uint32 N - Number of vector elements. Must be 0 < N < 8192 ; %macro PVQ_FAST_SEARCH 0 -cglobal pvq_search,4,5,8, mmsize, inX, outY, K, N +cglobal pvq_search,4,6,8, mmsize, inX, outY, K, N %define tmpX rsp ;movsxdifnidn Nq, Nd @@ -419,7 +419,12 @@ cglobal pvq_search,4,5,8, mmsize, inX, outY, K, N add Nq, r4q ; Nq = align(Nq, mmsize) sub rsp, Nq; allocate tmpX[Nq] +%ifdef PIC +lea r5q, [const_align_abs_edge]; rip+const +movups m3, [r5q+r4q-mmsize] ; this is the bit mask for the padded read at the end of the input +%else movups m3, [const_align_abs_edge-mmsize+r4q] ; this is the bit mask for the padded read at the end of the input +%endif lea r4q, [Nq-mmsize] ; Nq is rounded up (aligned up) to mmsize, so r4q can't become negative here, unless N=0. movups m2, [inXq + r4q] == What I find surprising is that PIC is enabled only on Windows and does not seem to depend on CONFIG_PIC, so textrels are used all over assembly code. Do I miss something? Are there option(s) to signal/error when texrel is been used in code that should be pic ? ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [WIP][PATCH] Opus Piramid Vector Quantization Search in x86 SIMD asm
On Sun, Jun 11, 2017 at 11:34 AM, Ivan Kalvachev wrote: > On 6/10/17, James Darnley wrote: >> On 2017-06-09 13:41, Ivan Kalvachev wrote: >>> On 6/9/17, Michael Niedermayer wrote: seems this breaks build with mingw64, didnt investigate but it fails with these errors: libavcodec/libavcodec.a(opus_pvq_search.o):src/libavcodec/x86/opus_pvq_search.asm:(.text+0x2d): relocation truncated to fit: R_X86_64_32 against `const_align_abs_edge' libavcodec/libavcodec.a(opus_pvq_search.o):src/libavcodec/x86/opus_pvq_search.asm:(.text+0x3fd): relocation truncated to fit: R_X86_64_32 against `const_align_abs_edge' libavcodec/libavcodec.a(opus_pvq_search.o):src/libavcodec/x86/opus_pvq_search.asm:(.text+0x7a1): relocation truncated to fit: R_X86_64_32 against `const_align_abs_edge' libavcodec/libavcodec.a(opus_pvq_search.o):src/libavcodec/x86/opus_pvq_search.asm:(.text+0xb48): relocation truncated to fit: R_X86_64_32 against `const_align_abs_edge' libavcodec/libavcodec.a(opus_pvq_search.o):src/libavcodec/x86/opus_pvq_search.asm:(.text+0x2d): relocation truncated to fit: R_X86_64_32 against `const_align_abs_edge' libavcodec/libavcodec.a(opus_pvq_search.o):src/libavcodec/x86/opus_pvq_search.asm:(.text+0x3fd): relocation truncated to fit: R_X86_64_32 against `const_align_abs_edge' libavcodec/libavcodec.a(opus_pvq_search.o):src/libavcodec/x86/opus_pvq_search.asm:(.text+0x7a1): relocation truncated to fit: R_X86_64_32 against `const_align_abs_edge' libavcodec/libavcodec.a(opus_pvq_search.o):src/libavcodec/x86/opus_pvq_search.asm:(.text+0xb48): relocation truncated to fit: R_X86_64_32 against `const_align_abs_edge' collect2: error: ld returned 1 exit status collect2: error: ld returned 1 exit status make: *** [ffmpeg_g.exe] Error 1 make: *** Waiting for unfinished jobs make: *** [ffprobe_g.exe] Error 1 >>> >>> >>> const_*_edge is used on only one place is the code. >>> Would you check if this patch fixes the issue. >>> >>> I expected that the addresses would be pre-calculated >>> by n/yasm as one value and indexed >>> relative to the section start. >>> Instead it seems that each entry is represented with >>> its own address and offset from it. >>> Since the offset is negative it uses all 64 bits and >>> it makes difference if it is truncated to 32 bits. >>> >>> Same issue could happen with clang tools. >> >> The problem is with the relative addressing. You need to load the real >> address first before you can offset with another register at runtime. So >> something like: >> >>> mov reg1, [read_only_const] > lea ? >>> mova mmreg, [reg1 + reg2] > > . > OK, Getting mingw for my distro is problem > and compiling one myself would take a bit more effort/time. > So I'm posting a patch that "should" work. > == > --- a/libavcodec/x86/opus_pvq_search.asm > +++ b/libavcodec/x86/opus_pvq_search.asm > @@ -406,7 +406,7 @@ align 16 > ; uint32 N - Number of vector elements. Must be 0 < N < 8192 > ; > %macro PVQ_FAST_SEARCH 0 > -cglobal pvq_search,4,5,8, mmsize, inX, outY, K, N > +cglobal pvq_search,4,6,8, mmsize, inX, outY, K, N > %define tmpX rsp > > ;movsxdifnidn Nq, Nd > @@ -419,7 +419,12 @@ cglobal pvq_search,4,5,8, mmsize, inX, outY, K, N > add Nq, r4q ; Nq = align(Nq, mmsize) > sub rsp, Nq; allocate tmpX[Nq] > > +%ifdef PIC > +lea r5q, [const_align_abs_edge]; rip+const > +movups m3, [r5q+r4q-mmsize] ; this is > the bit mask for the padded read at the end of the input > +%else > movups m3, [const_align_abs_edge-mmsize+r4q] ; this is > the bit mask for the padded read at the end of the input > +%endif > > lea r4q, [Nq-mmsize] ; Nq is rounded up (aligned > up) to mmsize, so r4q can't become negative here, unless N=0. > movups m2, [inXq + r4q] > == > > What I find surprising is that PIC is enabled only on Windows and does not > seem > to depend on CONFIG_PIC, so textrels are used all over assembly code. > Do I miss something? > Win32 code is always PIC, independent of what ffmpeg configure thinks it should be. :) Using the PIC define seems to be the appropriate thing, thats what the other asm code does. - Hendrik ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [WIP][PATCH] Opus Piramid Vector Quantization Search in x86 SIMD asm
On 6/11/17, Hendrik Leppkes wrote: > On Sun, Jun 11, 2017 at 11:34 AM, Ivan Kalvachev > wrote: >> On 6/10/17, James Darnley wrote: >>> On 2017-06-09 13:41, Ivan Kalvachev wrote: On 6/9/17, Michael Niedermayer wrote: > seems this breaks build with mingw64, didnt investigate but it > fails with these errors: > > libavcodec/libavcodec.a(opus_pvq_search.o):src/libavcodec/x86/opus_pvq_search.asm:(.text+0x2d): > relocation truncated to fit: R_X86_64_32 against `const_align_abs_edge' > libavcodec/libavcodec.a(opus_pvq_search.o):src/libavcodec/x86/opus_pvq_search.asm:(.text+0x3fd): > relocation truncated to fit: R_X86_64_32 against `const_align_abs_edge' > libavcodec/libavcodec.a(opus_pvq_search.o):src/libavcodec/x86/opus_pvq_search.asm:(.text+0x7a1): > relocation truncated to fit: R_X86_64_32 against `const_align_abs_edge' > libavcodec/libavcodec.a(opus_pvq_search.o):src/libavcodec/x86/opus_pvq_search.asm:(.text+0xb48): > relocation truncated to fit: R_X86_64_32 against `const_align_abs_edge' > libavcodec/libavcodec.a(opus_pvq_search.o):src/libavcodec/x86/opus_pvq_search.asm:(.text+0x2d): > relocation truncated to fit: R_X86_64_32 against `const_align_abs_edge' > libavcodec/libavcodec.a(opus_pvq_search.o):src/libavcodec/x86/opus_pvq_search.asm:(.text+0x3fd): > relocation truncated to fit: R_X86_64_32 against `const_align_abs_edge' > libavcodec/libavcodec.a(opus_pvq_search.o):src/libavcodec/x86/opus_pvq_search.asm:(.text+0x7a1): > relocation truncated to fit: R_X86_64_32 against `const_align_abs_edge' > libavcodec/libavcodec.a(opus_pvq_search.o):src/libavcodec/x86/opus_pvq_search.asm:(.text+0xb48): > relocation truncated to fit: R_X86_64_32 against `const_align_abs_edge' > collect2: error: ld returned 1 exit status > collect2: error: ld returned 1 exit status > make: *** [ffmpeg_g.exe] Error 1 > make: *** Waiting for unfinished jobs > make: *** [ffprobe_g.exe] Error 1 const_*_edge is used on only one place is the code. Would you check if this patch fixes the issue. I expected that the addresses would be pre-calculated by n/yasm as one value and indexed relative to the section start. Instead it seems that each entry is represented with its own address and offset from it. Since the offset is negative it uses all 64 bits and it makes difference if it is truncated to 32 bits. Same issue could happen with clang tools. >>> >>> The problem is with the relative addressing. You need to load the real >>> address first before you can offset with another register at runtime. So >>> something like: >>> mov reg1, [read_only_const] >> lea ? mova mmreg, [reg1 + reg2] >> >> . >> OK, Getting mingw for my distro is problem >> and compiling one myself would take a bit more effort/time. >> So I'm posting a patch that "should" work. >> == >> --- a/libavcodec/x86/opus_pvq_search.asm >> +++ b/libavcodec/x86/opus_pvq_search.asm >> @@ -406,7 +406,7 @@ align 16 >> ; uint32 N - Number of vector elements. Must be 0 < N < 8192 >> ; >> %macro PVQ_FAST_SEARCH 0 >> -cglobal pvq_search,4,5,8, mmsize, inX, outY, K, N >> +cglobal pvq_search,4,6,8, mmsize, inX, outY, K, N >> %define tmpX rsp >> >> ;movsxdifnidn Nq, Nd >> @@ -419,7 +419,12 @@ cglobal pvq_search,4,5,8, mmsize, inX, outY, K, N >> add Nq, r4q ; Nq = align(Nq, mmsize) >> sub rsp, Nq; allocate tmpX[Nq] >> >> +%ifdef PIC >> +lea r5q, [const_align_abs_edge]; rip+const >> +movups m3, [r5q+r4q-mmsize] ; this is >> the bit mask for the padded read at the end of the input >> +%else >> movups m3, [const_align_abs_edge-mmsize+r4q] ; this is >> the bit mask for the padded read at the end of the input >> +%endif >> >> lea r4q, [Nq-mmsize] ; Nq is rounded up (aligned >> up) to mmsize, so r4q can't become negative here, unless N=0. >> movups m2, [inXq + r4q] >> == >> >> What I find surprising is that PIC is enabled only on Windows and does not >> seem >> to depend on CONFIG_PIC, so textrels are used all over assembly code. >> Do I miss something? >> > > Win32 code is always PIC, independent of what ffmpeg configure thinks > it should be. :) > Using the PIC define seems to be the appropriate thing, thats what the > other asm code does. > Yes, but my query is more about why CONFIG_PIC does not enable asm PIC too? After all gcc -fpic is supposed to generate code without texrel, isn't it? ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/fft_template: Fix multiple runtime error: signed integer overflow: -1943918714 - 1935113003 cannot be represented in type 'int'
On Fri, Jun 09, 2017 at 09:07:39PM -0400, Ronald S. Bultje wrote: > Hi, > > On Thu, Jun 8, 2017 at 8:57 PM, Michael Niedermayer > wrote: > > > On Thu, Jun 08, 2017 at 06:35:07PM -0400, Ronald S. Bultje wrote: > > > Hi, > > > > > > On Thu, Jun 8, 2017 at 6:10 PM, Michael Niedermayer > > > > > wrote: > > > > > > > Signed value in > > > > Unsigned > > > > INTeger type > > > > > > [..] > > > > Both SUINT and unsigned should produce identical binaries > > > > > > This seems to go against the rule that code should be as simple as > > possible. > > > > > > Unsigned is simpler than SUINT if the outcome is the same. > > > > You can simply add the part of my mail here as awnser that you snipped > > away: > > > > "But it makes the code hard to understand and maintain because these > > values are not positive integers but signed integers. Which for > > C standard compliance need to be stored in a unsigned type." > > > > A type that avoids the undefinedness of signed but is semantically > > signed is correct, unsigned is not. > > > > If understandable code and maintainable code has no value to you, > > you would favour using single letter variables exclusivly and would > > never use typedef. > > But you do not do that. > > > > I fail to understand why you insist on using unsigned in place of a > > more specific type, it is not the correct nor clean thing to do. > > > It's not just me, it appears to be most of us. Can't you just step back at > some point and be like "ok, I'll let the majority have their way"? I do not know what the majority prefers. What i see is that the people objecting are always the same 3-4 people. And very often they have no authorship or past activity in the code a patch is about. At least none i could find quickly. To me that makes these objections seem a bit out of place, more so because i just dont "get it" why people are against using a more specific type. The majority might prefer either way, i have no idea ... I very much want to choose the types and style the people activly working on some code prefer. But when i look at git shortlog of code and look at the copyright/author statments and look at MAINTAINERs and compare to the names in a thread i often find no real match. Also as i said, i belive a more specific type makes maintaince easier, thats why i want to use it in code i maintain. In code others maintain, if they see no value in it theres really alot less an argument to use it. with a specific type in this case here one can add a line and test if the FFT overflows and where it does so, if it ever produces a bad result for a real world file. With just unsigned theres no easy way to find a overflow, one has to painstakingly test this line by line by hand. If the people who intend to debug such issues see no value in a more specific type, no sense in using one. Iam fighting on this issue because i see this pushing FFmpeg into a direction where the code is harder to understand and harder to maintain and we already have many open bugs Do the people objecting to SUINT volunteer to do the extra maintaince work that may be caused by not using it ? or do they expect the existing maintainers not to use SUINT and then also do the extra work? its that 2nd case that offends me as iam one of the people who tries to maintain some of the code in question (not the fft of this patch here specifically) thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB I know you won't believe me, but the highest form of Human Excellence is to question oneself and others. -- Socrates signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/fft_template: Fix multiple runtime error: signed integer overflow: -1943918714 - 1935113003 cannot be represented in type 'int'
On Sun, 11 Jun 2017 03:58:30 +0300 Ivan Kalvachev wrote: > Of course, as FFmpeg developer, it is your right to initiate a vote > that would prevent Michael from trying to make FFmpeg more secure. > He has always complied with official decisions. Nothing but polemic nonsense intended to scare others into having your way. If our code is so tricky that nobody can understand it or the intention behind code (like types being simultaneously signed and unsigned), it won't have a positive influence on the security of the code. If you really want to make code more secure, you should probably think about making code _simpler_, nor more complex. > However this might turn into publicity nightmare, > as security community is known to overreact > on certain topics. That too. The security community in particular seems to be full of individuals who will gladly misrepresent risks to get attentions, and companies doing the same to sell their "security" products. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/fft_template: Fix multiple runtime error: signed integer overflow: -1943918714 - 1935113003 cannot be represented in type 'int'
On Sun, 11 Jun 2017 13:26:44 +0200 Michael Niedermayer wrote: > Iam fighting on this issue because i see this pushing FFmpeg into a > direction where the code is harder to understand and harder to maintain > and we already have many open bugs That's funny, because whenever I see something strange in e.g. libavformat/utils.c, I run git blame, and amazingly often you turn out as original author. Layering hack upon hack to fix specific issues. That might be because you're a very active developer, or because FFmpeg's development style forces anyone to do this "layering hacks upon hacks" to fix bugs. But still, I think that supports rather my point than yours. Also, none of these fuzz fixes close any open bugs as far as I'm aware. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Add support for RockChip Media Process Platform This adds hardware decoding for h264 / HEVC / VP8 using MPP Rockchip API. Will return frames holding a av_drmprime struct tha
On Sun, 11 Jun 2017 05:24:16 + "LongChair ." wrote: > From: LongChair > > --- > Changelog | 1 + > configure | 12 ++ > libavcodec/Makefile| 4 + > libavcodec/allcodecs.c | 6 + > libavcodec/drmprime.h | 17 ++ > libavcodec/rkmppdec.c | 522 > + > libavutil/pixdesc.c| 4 + > libavutil/pixfmt.h | 5 + > 8 files changed, 571 insertions(+) > create mode 100644 libavcodec/drmprime.h > create mode 100644 libavcodec/rkmppdec.c > I reviewed this before, so I have nothing more to say. You should fix your commit message, though (approx. 72 chars per line limit, concise summary/topic in the subject line, then empty line, then detailed description of the commit). ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/fft_template: Fix multiple runtime error: signed integer overflow: -1943918714 - 1935113003 cannot be represented in type 'int'
On 6/11/17, Michael Niedermayer wrote: > On Fri, Jun 09, 2017 at 09:07:39PM -0400, Ronald S. Bultje wrote: >> Hi, >> >> On Thu, Jun 8, 2017 at 8:57 PM, Michael Niedermayer >> >> wrote: >> >> > On Thu, Jun 08, 2017 at 06:35:07PM -0400, Ronald S. Bultje wrote: >> > > Hi, >> > > >> > > On Thu, Jun 8, 2017 at 6:10 PM, Michael Niedermayer >> > >> > > wrote: >> > > >> > > > Signed value in >> > > > Unsigned >> > > > INTeger type >> > > >> > > [..] >> > > > Both SUINT and unsigned should produce identical binaries >> > > >> > > This seems to go against the rule that code should be as simple as >> > possible. >> > > >> > > Unsigned is simpler than SUINT if the outcome is the same. >> > >> > You can simply add the part of my mail here as awnser that you snipped >> > away: >> > >> > "But it makes the code hard to understand and maintain because these >> > values are not positive integers but signed integers. Which for >> > C standard compliance need to be stored in a unsigned type." >> > >> > A type that avoids the undefinedness of signed but is semantically >> > signed is correct, unsigned is not. >> > >> > If understandable code and maintainable code has no value to you, >> > you would favour using single letter variables exclusivly and would >> > never use typedef. >> > But you do not do that. >> > >> > I fail to understand why you insist on using unsigned in place of a >> > more specific type, it is not the correct nor clean thing to do. >> >> >> It's not just me, it appears to be most of us. Can't you just step back >> at >> some point and be like "ok, I'll let the majority have their way"? > > I do not know what the majority prefers. What i see is that the > people objecting are always the same 3-4 people. And very often > they have no authorship or past activity in the code a patch is about. > At least none i could find quickly. How dare you speak like that about me? Do you think about yourself like holy cow in any aspect of FFmpeg, security or not. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 01/11] avfilter/unsharp: fix uninitialized pointer read
Fixes CID 1396855
---
libavfilter/unsharp_opencl.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/libavfilter/unsharp_opencl.c b/libavfilter/unsharp_opencl.c
index d84920c590..1545455846 100644
--- a/libavfilter/unsharp_opencl.c
+++ b/libavfilter/unsharp_opencl.c
@@ -43,7 +43,7 @@ static int compute_mask(int step, uint32_t *mask)
{
int i, z, ret = 0;
int counter_size = sizeof(uint32_t) * (2 * step + 1);
-uint32_t *temp1_counter, *temp2_counter, **counter;
+uint32_t *temp1_counter, *temp2_counter, **counter = NULL;
temp1_counter = av_mallocz(counter_size);
if (!temp1_counter) {
ret = AVERROR(ENOMEM);
@@ -80,7 +80,7 @@ static int compute_mask(int step, uint32_t *mask)
end:
av_freep(&temp1_counter);
av_freep(&temp2_counter);
-for (i = 0; i < 2 * step + 1; i++) {
+for (i = 0; counter && i < 2 * step + 1; i++) {
av_freep(&counter[i]);
}
av_freep(&counter);
--
2.13.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 03/11] libavutil/opencl: fix potentiall nul dereference
Fixes CID 1396840
---
libavutil/opencl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavutil/opencl.c b/libavutil/opencl.c
index af35770e06..202756516b 100644
--- a/libavutil/opencl.c
+++ b/libavutil/opencl.c
@@ -169,7 +169,7 @@ const char *av_opencl_errstr(cl_int status)
static void free_device_list(AVOpenCLDeviceList *device_list)
{
int i, j;
-if (!device_list)
+if (!device_list || !device_list->platform_node)
return;
for (i = 0; i < device_list->platform_num; i++) {
if (!device_list->platform_node[i])
--
2.13.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 02/11] avfilter/vf_scale_npp: fix out-of-bounds reads
Fixes CIDs 1396414 and 1396415
---
libavfilter/vf_scale_npp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavfilter/vf_scale_npp.c b/libavfilter/vf_scale_npp.c
index b5acce653b..c36772e800 100644
--- a/libavfilter/vf_scale_npp.c
+++ b/libavfilter/vf_scale_npp.c
@@ -400,7 +400,7 @@ static int nppscale_resize(AVFilterContext *ctx,
NPPScaleStageContext *stage,
NppStatus err;
int i;
-for (i = 0; i < FF_ARRAY_ELEMS(in->data) && in->data[i]; i++) {
+for (i = 0; i < FF_ARRAY_ELEMS(stage->planes_in) && i <
FF_ARRAY_ELEMS(in->data) && in->data[i]; i++) {
int iw = stage->planes_in[i].width;
int ih = stage->planes_in[i].height;
int ow = stage->planes_out[i].width;
--
2.13.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 08/11] avfilter/vf_signature: use av_strlcpy instead of strcpy
Fixes CID 1403236
---
libavfilter/vf_signature.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavfilter/vf_signature.c b/libavfilter/vf_signature.c
index f0078ba1a6..c20b0bfabb 100644
--- a/libavfilter/vf_signature.c
+++ b/libavfilter/vf_signature.c
@@ -576,7 +576,7 @@ static int export(AVFilterContext *ctx, StreamContext *sc,
int input)
/* error already handled */
av_assert0(av_get_frame_filename(filename, sizeof(filename),
sic->filename, input) == 0);
} else {
-strcpy(filename, sic->filename);
+av_strlcpy(filename, sic->filename, sizeof(filename));
}
if (sic->format == FORMAT_XML) {
return xml_export(ctx, sc, filename);
--
2.13.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 05/11] avformat/pcmdec: fix memory leak
Fixes CID 1396267 --- libavformat/pcmdec.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libavformat/pcmdec.c b/libavformat/pcmdec.c index 3c7e8ac84b..d0ceea6fa9 100644 --- a/libavformat/pcmdec.c +++ b/libavformat/pcmdec.c @@ -68,6 +68,7 @@ static int pcm_read_header(AVFormatContext *s) av_log(s, AV_LOG_ERROR, "Invalid sample_rate found in mime_type \"%s\"\n", mime_type); +av_freep(&mime_type); return AVERROR_INVALIDDATA; } st->codecpar->sample_rate = rate; @@ -75,6 +76,7 @@ static int pcm_read_header(AVFormatContext *s) st->codecpar->channels = channels; } } +av_freep(&mime_type); st->codecpar->bits_per_coded_sample = av_get_bits_per_sample(st->codecpar->codec_id); -- 2.13.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 09/11] avfilter/signature_lookup: fix potential uninitialized reads
Fixes CIDs 1403238 and 1403239
---
libavfilter/signature_lookup.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavfilter/signature_lookup.c b/libavfilter/signature_lookup.c
index 272c717c77..7de4a88ca2 100644
--- a/libavfilter/signature_lookup.c
+++ b/libavfilter/signature_lookup.c
@@ -421,7 +421,7 @@ static MatchingInfo evaluate_parameters(AVFilterContext
*ctx, SignatureContext *
int fcount = 0, goodfcount = 0, gooda = 0, goodb = 0;
double meandist, minmeandist = bestmatch.meandist;
int tolerancecount = 0;
-FineSignature *a, *b, *aprev, *bprev;
+FineSignature *a, *b, *aprev = NULL, *bprev = NULL;
int status = STATUS_NULL;
for (; infos != NULL; infos = infos->next) {
--
2.13.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 04/11] avformat/librtmp: check return value of setsockopt
Fixes CID 1396837
---
libavformat/librtmp.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/libavformat/librtmp.c b/libavformat/librtmp.c
index 146df660ac..f3cfa9a8e2 100644
--- a/libavformat/librtmp.c
+++ b/libavformat/librtmp.c
@@ -239,7 +239,10 @@ static int rtmp_open(URLContext *s, const char *uri, int
flags)
#if CONFIG_NETWORK
if (ctx->buffer_size >= 0 && (flags & AVIO_FLAG_WRITE)) {
int tmp = ctx->buffer_size;
-setsockopt(r->m_sb.sb_socket, SOL_SOCKET, SO_SNDBUF, &tmp,
sizeof(tmp));
+if (setsockopt(r->m_sb.sb_socket, SOL_SOCKET, SO_SNDBUF, &tmp,
sizeof(tmp))) {
+rc = AVERROR_EXTERNAL;
+goto fail;
+}
}
#endif
--
2.13.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 07/11] avfilter/vf_signature: fix memory leaks in error cases
Fixes CIDs 1403234 and 1403235
---
libavfilter/vf_signature.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/libavfilter/vf_signature.c b/libavfilter/vf_signature.c
index 06b1b910d4..f0078ba1a6 100644
--- a/libavfilter/vf_signature.c
+++ b/libavfilter/vf_signature.c
@@ -260,8 +260,10 @@ static int filter_frame(AVFilterLink *inlink, AVFrame
*picref)
if (!elemsignature)
return AVERROR(ENOMEM);
sortsignature = av_malloc_array(elemcat->elem_count, sizeof(int64_t));
-if (!sortsignature)
+if (!sortsignature) {
+av_freep(&elemsignature);
return AVERROR(ENOMEM);
+}
for (j = 0; j < elemcat->elem_count; j++) {
blocksum = 0;
@@ -508,6 +510,7 @@ static int binary_export(AVFilterContext *ctx,
StreamContext *sc, const char* fi
char buf[128];
av_strerror(err, buf, sizeof(buf));
av_log(ctx, AV_LOG_ERROR, "cannot open file %s: %s\n", filename, buf);
+av_freep(&buffer);
return err;
}
init_put_bits(&buf, buffer, len);
--
2.13.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 06/11] avcodec/vaapi_encode: fix potential uninitialized read
Fixes CID 1400440
---
libavcodec/vaapi_encode.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
index 7e9c00f51d..9336bbecd4 100644
--- a/libavcodec/vaapi_encode.c
+++ b/libavcodec/vaapi_encode.c
@@ -738,7 +738,7 @@ fail:
static int vaapi_encode_truncate_gop(AVCodecContext *avctx)
{
VAAPIEncodeContext *ctx = avctx->priv_data;
-VAAPIEncodePicture *pic, *last_pic, *next;
+VAAPIEncodePicture *pic, *last_pic = NULL, *next;
// Find the last picture we actually have input for.
for (pic = ctx->pic_start; pic; pic = pic->next) {
--
2.13.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 10/11] avcodec/h264_parser: zero-initialize H264PredWeightTable
Fixes CID 1404889
---
libavcodec/h264_parser.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavcodec/h264_parser.c b/libavcodec/h264_parser.c
index 2564c6c6c3..1a304f318f 100644
--- a/libavcodec/h264_parser.c
+++ b/libavcodec/h264_parser.c
@@ -155,7 +155,7 @@ found:
static int scan_mmco_reset(AVCodecParserContext *s, GetBitContext *gb,
void *logctx)
{
-H264PredWeightTable pwt;
+H264PredWeightTable pwt = { 0 };
int slice_type_nos = s->pict_type & 3;
H264ParseContext *p = s->priv_data;
int list_count, ref_count[2];
--
2.13.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 11/11] avcodec/dcaadpcm: fix unitialized read
Fixes CID 1409924
---
libavcodec/dcaadpcm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavcodec/dcaadpcm.c b/libavcodec/dcaadpcm.c
index 8742c7ccf6..aff737ddb6 100644
--- a/libavcodec/dcaadpcm.c
+++ b/libavcodec/dcaadpcm.c
@@ -80,7 +80,7 @@ static int64_t find_best_filter(const DCAADPCMEncContext *s,
const int32_t *in,
{
const premultiplied_coeffs *precalc_data = s->private_data;
int i, j, k = 0;
-int vq;
+int vq = -1;
int64_t err;
int64_t min_err = 1ll << 62;
int64_t corr[15];
--
2.13.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] hevc: Add support for alternative transfer characterics SEI
On 6/9/2017 6:35 PM, Vittorio Giovara wrote:
> Signed-off-by: Vittorio Giovara
> ---
> libavcodec/hevc_sei.c | 9 +
> libavcodec/hevc_sei.h | 7 +++
> libavcodec/hevcdec.c | 5 +
> 3 files changed, 21 insertions(+)
>
> diff --git a/libavcodec/hevc_sei.c b/libavcodec/hevc_sei.c
> index f0ad84f2f4..cd55d50212 100644
> --- a/libavcodec/hevc_sei.c
> +++ b/libavcodec/hevc_sei.c
> @@ -265,6 +265,13 @@ static int
> decode_nal_sei_active_parameter_sets(HEVCSEIContext *s, GetBitContext
> return 0;
> }
>
> +static int decode_nal_sei_alternative_transfer(HEVCSEIAlternativeTransfer
> *s, GetBitContext *gb)
> +{
> +s->present = 1;
> +s->preferred_transfer_characteristics = get_bits(gb, 8);
> +return 0;
> +}
> +
> static int decode_nal_sei_prefix(GetBitContext *gb, HEVCSEIContext *s, const
> HEVCParamSets *ps,
> int type, int size, void *logctx)
> {
> @@ -285,6 +292,8 @@ static int decode_nal_sei_prefix(GetBitContext *gb,
> HEVCSEIContext *s, const HEV
> return decode_nal_sei_active_parameter_sets(s, gb, logctx);
> case HEVC_SEI_TYPE_USER_DATA_REGISTERED_ITU_T_T35:
> return decode_nal_sei_user_data_registered_itu_t_t35(s, gb, size);
> +case HEVC_SEI_TYPE_ALTERNATIVE_TRANSFER_CHARACTERISTICS:
> +return decode_nal_sei_alternative_transfer(&s->alternative_transfer,
> gb);
> default:
> av_log(logctx, AV_LOG_DEBUG, "Skipped PREFIX SEI %d\n", type);
> skip_bits_long(gb, 8 * size);
> diff --git a/libavcodec/hevc_sei.h b/libavcodec/hevc_sei.h
> index 5645e4f5de..1b57ec 100644
> --- a/libavcodec/hevc_sei.h
> +++ b/libavcodec/hevc_sei.h
> @@ -56,6 +56,7 @@ typedef enum {
> HEVC_SEI_TYPE_REGION_REFRESH_INFO = 134,
> HEVC_SEI_TYPE_MASTERING_DISPLAY_INFO = 137,
> HEVC_SEI_TYPE_CONTENT_LIGHT_LEVEL_INFO = 144,
> +HEVC_SEI_TYPE_ALTERNATIVE_TRANSFER_CHARACTERISTICS = 147,
> } HEVC_SEI_Type;
>
> typedef struct HEVCSEIPictureHash {
> @@ -100,6 +101,11 @@ typedef struct HEVCSEIContentLight {
> uint16_t max_pic_average_light_level;
> } HEVCSEIContentLight;
>
> +typedef struct HEVCSEIAlternativeTransfer {
> +int present;
> +int preferred_transfer_characteristics;
> +} HEVCSEIAlternativeTransfer;
> +
> typedef struct HEVCSEIContext {
> HEVCSEIPictureHash picture_hash;
> HEVCSEIFramePacking frame_packing;
> @@ -109,6 +115,7 @@ typedef struct HEVCSEIContext {
> HEVCSEIMasteringDisplay mastering_display;
> HEVCSEIContentLight content_light;
> int active_seq_parameter_set_id;
> +HEVCSEIAlternativeTransfer alternative_transfer;
> } HEVCSEIContext;
>
> struct HEVCParamSets;
> diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c
> index 23a89345a8..955cc703cf 100644
> --- a/libavcodec/hevcdec.c
> +++ b/libavcodec/hevcdec.c
> @@ -2684,6 +2684,11 @@ static int set_side_data(HEVCContext *s)
> s->avctx->properties |= FF_CODEC_PROPERTY_CLOSED_CAPTIONS;
> }
>
> +if (s->sei.alternative_transfer.present &&
> +s->sei.alternative_transfer.preferred_transfer_characteristics !=
> AVCOL_SPC_UNSPECIFIED) {
Shouldn't this be AVCOL_TRC_UNSPECIFIED?
And validate it with av_color_transfer_name() instead, like it's done in
hevc_ps.c with the VUI value.
> +s->avctx->color_trc =
> s->sei.alternative_transfer.preferred_transfer_characteristics;
Did you make sure avctx->color_trc will not be overwritten by the VUI
value after this? As in, what's called first, set_side_data() or
export_stream_params()? And is it properly propagated in updates between
thread contexts?
> +}
> +
> return 0;
> }
>
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 10/11] avcodec/h264_parser: zero-initialize H264PredWeightTable
On 11/06/17 15:07, Timo Rothenpieler wrote:
> Fixes CID 1404889
> ---
> libavcodec/h264_parser.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavcodec/h264_parser.c b/libavcodec/h264_parser.c
> index 2564c6c6c3..1a304f318f 100644
> --- a/libavcodec/h264_parser.c
> +++ b/libavcodec/h264_parser.c
> @@ -155,7 +155,7 @@ found:
> static int scan_mmco_reset(AVCodecParserContext *s, GetBitContext *gb,
> void *logctx)
> {
> -H264PredWeightTable pwt;
> +H264PredWeightTable pwt = { 0 };
> int slice_type_nos = s->pict_type & 3;
> H264ParseContext *p = s->priv_data;
> int list_count, ref_count[2];
>
Seems dubious? That is not a small structure, and it's being used essentially
write-only here to skip over an unwanted part of the slice header - since it
will only ever write to a small proportion of the elements, initialising all of
them to zero feels like a waste.
(The only argument in Coverity seems to be that passing a pointer to an
uninitialised structure to an external function is bad - it hasn't actually
looked at the function to observe that it doesn't read anything currently in
the structure.)
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 10/11] avcodec/h264_parser: zero-initialize H264PredWeightTable
Seems dubious? That is not a small structure, and it's being used essentially
write-only here to skip over an unwanted part of the slice header - since it
will only ever write to a small proportion of the elements, initialising all of
them to zero feels like a waste.
(The only argument in Coverity seems to be that passing a pointer to an
uninitialised structure to an external function is bad - it hasn't actually
looked at the function to observe that it doesn't read anything currently in
the structure.)
It calls ff_h264_pred_weight_table in line 204, which it does analyze,
and which accesses pwt->chroma_log2_weight_denom, which was not
initialized before.
I don't think doing = { 0 }; is expensive. iirc all elements except for
the first one are zero-initialized already, even though it's
implementation-defined or even undefined.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] avcodec/htmlsubtitles: Replace very slow redundant sscanf() calls by cleaner and faster code
This reduces the worst case from O(n²) to O(n) time
Fixes Timeout
Fixes: 2127/clusterfuzz-testcase-minimized-6595787859427328
Signed-off-by: Michael Niedermayer
---
libavcodec/htmlsubtitles.c | 20 +++-
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/libavcodec/htmlsubtitles.c b/libavcodec/htmlsubtitles.c
index 16295daa0c..70311c66d5 100644
--- a/libavcodec/htmlsubtitles.c
+++ b/libavcodec/htmlsubtitles.c
@@ -56,6 +56,7 @@ int ff_htmlmarkup_to_ass(void *log_ctx, AVBPrint *dst, const
char *in)
char *param, buffer[128], tmp[128];
int len, tag_close, sptr = 1, line_start = 1, an = 0, end = 0;
SrtStack stack[16];
+int closing_brace_missing = 0;
stack[0].tag[0] = 0;
strcpy(stack[0].param[PARAM_SIZE], "{\\fs}");
@@ -83,11 +84,20 @@ int ff_htmlmarkup_to_ass(void *log_ctx, AVBPrint *dst,
const char *in)
and all microdvd like styles such as {Y:xxx} */
len = 0;
an += sscanf(in, "{\\an%*1u}%n", &len) >= 0 && len > 0;
-if ((an != 1 && (len = 0, sscanf(in, "{\\%*[^}]}%n", &len) >= 0 &&
len > 0)) ||
-(len = 0, sscanf(in, "{%*1[CcFfoPSsYy]:%*[^}]}%n", &len) >= 0
&& len > 0)) {
-in += len - 1;
-} else
-av_bprint_chars(dst, *in, 1);
+
+if (!closing_brace_missing) {
+if ( (an != 1 && in[1] == '\\')
+|| (in[1] && strchr("CcFfoPSsYy", in[1]) && in[2] == ':'))
{
+char *bracep = strchr(in+2, '}');
+if (bracep) {
+in = bracep;
+break;
+} else
+closing_brace_missing = 1;
+}
+}
+
+av_bprint_chars(dst, *in, 1);
break;
case '<':
tag_close = in[1] == '/';
--
2.13.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 10/11] avcodec/h264_parser: zero-initialize H264PredWeightTable
On 11/06/17 16:12, Timo Rothenpieler wrote:
>> Seems dubious? That is not a small structure, and it's being used
>> essentially write-only here to skip over an unwanted part of the slice
>> header - since it will only ever write to a small proportion of the
>> elements, initialising all of them to zero feels like a waste.
>>
>> (The only argument in Coverity seems to be that passing a pointer to an
>> uninitialised structure to an external function is bad - it hasn't actually
>> looked at the function to observe that it doesn't read anything currently in
>> the structure.)
>
> It calls ff_h264_pred_weight_table in line 204, which it does analyze, and
> which accesses pwt->chroma_log2_weight_denom, which was not initialized
> before.
I think those accesses are wrong and should be fixed - patch following.
(I could believe that the monochrome H.264 stream with prediction weights
required to make this actually error out with the current code is quite rare.)
> I don't think doing = { 0 }; is expensive. iirc all elements except for the
> first one are zero-initialized already, even though it's
> implementation-defined or even undefined.
It's all indeterminate - see C11 section 6.7.9. A compiler could in theory
choose to initialise it to zero or 42 or any other value, but there is no
reason for any particular choice so it won't bother.
- Mark
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] h264_parse: Do not touch chroma weights at all if stream is monochrome
Also return an error if the weight denominator is incorrect, rather
than overriding it with zero and continuing.
---
libavcodec/h264_parse.c | 24 ++--
1 file changed, 14 insertions(+), 10 deletions(-)
diff --git a/libavcodec/h264_parse.c b/libavcodec/h264_parse.c
index 3d20075f6a..96680640f5 100644
--- a/libavcodec/h264_parse.c
+++ b/libavcodec/h264_parse.c
@@ -34,17 +34,19 @@ int ff_h264_pred_weight_table(GetBitContext *gb, const SPS
*sps,
pwt->use_weight = 0;
pwt->use_weight_chroma = 0;
-pwt->luma_log2_weight_denom = get_ue_golomb(gb);
-if (sps->chroma_format_idc)
-pwt->chroma_log2_weight_denom = get_ue_golomb(gb);
+pwt->luma_log2_weight_denom = get_ue_golomb(gb);
if (pwt->luma_log2_weight_denom > 7U) {
av_log(logctx, AV_LOG_ERROR, "luma_log2_weight_denom %d is out of
range\n", pwt->luma_log2_weight_denom);
-pwt->luma_log2_weight_denom = 0;
+return AVERROR_INVALIDDATA;
}
-if (pwt->chroma_log2_weight_denom > 7U) {
-av_log(logctx, AV_LOG_ERROR, "chroma_log2_weight_denom %d is out of
range\n", pwt->chroma_log2_weight_denom);
-pwt->chroma_log2_weight_denom = 0;
+
+if (sps->chroma_format_idc) {
+pwt->chroma_log2_weight_denom = get_ue_golomb(gb);
+if (pwt->chroma_log2_weight_denom > 7U) {
+av_log(logctx, AV_LOG_ERROR, "chroma_log2_weight_denom %d is out
of range\n", pwt->chroma_log2_weight_denom);
+return AVERROR_INVALIDDATA;
+}
}
luma_def = 1 << pwt->luma_log2_weight_denom;
@@ -102,9 +104,11 @@ int ff_h264_pred_weight_table(GetBitContext *gb, const SPS
*sps,
if (picture_structure == PICT_FRAME) {
pwt->luma_weight[16 + 2 * i][list][0] = pwt->luma_weight[16 +
2 * i + 1][list][0] = pwt->luma_weight[i][list][0];
pwt->luma_weight[16 + 2 * i][list][1] = pwt->luma_weight[16 +
2 * i + 1][list][1] = pwt->luma_weight[i][list][1];
-for (j = 0; j < 2; j++) {
-pwt->chroma_weight[16 + 2 * i][list][j][0] =
pwt->chroma_weight[16 + 2 * i + 1][list][j][0] =
pwt->chroma_weight[i][list][j][0];
-pwt->chroma_weight[16 + 2 * i][list][j][1] =
pwt->chroma_weight[16 + 2 * i + 1][list][j][1] =
pwt->chroma_weight[i][list][j][1];
+if (sps->chroma_format_idc) {
+for (j = 0; j < 2; j++) {
+pwt->chroma_weight[16 + 2 * i][list][j][0] =
pwt->chroma_weight[16 + 2 * i + 1][list][j][0] =
pwt->chroma_weight[i][list][j][0];
+pwt->chroma_weight[16 + 2 * i][list][j][1] =
pwt->chroma_weight[16 + 2 * i + 1][list][j][1] =
pwt->chroma_weight[i][list][j][1];
+}
}
}
}
--
2.11.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 05/11] avformat/pcmdec: fix memory leak
On 6/11/17, Timo Rothenpieler wrote: > Fixes CID 1396267 > --- > libavformat/pcmdec.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/libavformat/pcmdec.c b/libavformat/pcmdec.c > index 3c7e8ac84b..d0ceea6fa9 100644 > --- a/libavformat/pcmdec.c > +++ b/libavformat/pcmdec.c > @@ -68,6 +68,7 @@ static int pcm_read_header(AVFormatContext *s) > av_log(s, AV_LOG_ERROR, > "Invalid sample_rate found in mime_type \"%s\"\n", > mime_type); > +av_freep(&mime_type); > return AVERROR_INVALIDDATA; > } > st->codecpar->sample_rate = rate; > @@ -75,6 +76,7 @@ static int pcm_read_header(AVFormatContext *s) > st->codecpar->channels = channels; > } > } > +av_freep(&mime_type); > > st->codecpar->bits_per_coded_sample = > av_get_bits_per_sample(st->codecpar->codec_id); > -- > 2.13.0 > > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > lgtm ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] avformat/libssh: check the user provided a password before trying to use it
Fixes ticket #6413
Signed-off-by: James Almer
---
The public key authentication also tries to use the password variable. I
don't know if NULL is valid in that case or not.
Perhaps for that one it would be better to replace the current usage of
legacy API instead.
libavformat/libssh.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavformat/libssh.c b/libavformat/libssh.c
index 49e92e7516..9e3d4da45e 100644
--- a/libavformat/libssh.c
+++ b/libavformat/libssh.c
@@ -103,7 +103,7 @@ static av_cold int libssh_authentication(LIBSSHContext
*libssh, const char *user
}
}
-if (!authorized && (auth_methods & SSH_AUTH_METHOD_PASSWORD)) {
+if (!authorized && password && (auth_methods & SSH_AUTH_METHOD_PASSWORD)) {
if (ssh_userauth_password(libssh->session, NULL, password) ==
SSH_AUTH_SUCCESS) {
av_log(libssh, AV_LOG_DEBUG, "Authentication successful with
password.\n");
authorized = 1;
--
2.13.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/mov: add support for reading VP Codec Configuration Box
On 6/3/2017 2:40 PM, James Almer wrote: > On 5/27/2017 7:00 PM, James Almer wrote: >> As defined in "VP Codec ISO Media File Format Binding v1.0" >> https://github.com/webmproject/vp9-dash/blob/master/VPCodecISOMediaFileFormatBinding.md >> >> Signed-off-by: James Almer >> --- >> libavformat/mov.c | 45 + >> 1 file changed, 45 insertions(+) > > Ping Pushed. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/fft_template: Fix multiple runtime error: signed integer overflow: -1943918714 - 1935113003 cannot be represented in type 'int'
On Sun, Jun 11, 2017 at 03:21:38PM +0200, Paul B Mahol wrote: > On 6/11/17, Michael Niedermayer wrote: > > On Fri, Jun 09, 2017 at 09:07:39PM -0400, Ronald S. Bultje wrote: > >> Hi, > >> > >> On Thu, Jun 8, 2017 at 8:57 PM, Michael Niedermayer > >> > >> wrote: > >> > >> > On Thu, Jun 08, 2017 at 06:35:07PM -0400, Ronald S. Bultje wrote: > >> > > Hi, > >> > > > >> > > On Thu, Jun 8, 2017 at 6:10 PM, Michael Niedermayer > >> > > >> > > wrote: > >> > > > >> > > > Signed value in > >> > > > Unsigned > >> > > > INTeger type > >> > > > >> > > [..] > >> > > > Both SUINT and unsigned should produce identical binaries > >> > > > >> > > This seems to go against the rule that code should be as simple as > >> > possible. > >> > > > >> > > Unsigned is simpler than SUINT if the outcome is the same. > >> > > >> > You can simply add the part of my mail here as awnser that you snipped > >> > away: > >> > > >> > "But it makes the code hard to understand and maintain because these > >> > values are not positive integers but signed integers. Which for > >> > C standard compliance need to be stored in a unsigned type." > >> > > >> > A type that avoids the undefinedness of signed but is semantically > >> > signed is correct, unsigned is not. > >> > > >> > If understandable code and maintainable code has no value to you, > >> > you would favour using single letter variables exclusivly and would > >> > never use typedef. > >> > But you do not do that. > >> > > >> > I fail to understand why you insist on using unsigned in place of a > >> > more specific type, it is not the correct nor clean thing to do. > >> > >> > >> It's not just me, it appears to be most of us. Can't you just step back > >> at > >> some point and be like "ok, I'll let the majority have their way"? > > > > I do not know what the majority prefers. What i see is that the > > people objecting are always the same 3-4 people. And very often > > they have no authorship or past activity in the code a patch is about. > > At least none i could find quickly. > > How dare you speak like that about me? About you ? It was not intended to be about you nor was it intended to insult anyone. Iam not sure why one would think otherwise. if i search now for onemda and mails matching SUINT there are only 5 matches, 2 or 3 of these have SUINT just in context IIUC. none is a reply to a patch from me in which you object due to SUINT. I did realize you arent in favor of the type but i didnt precive you as objecting to patches because of it. and I definitly tried to use unsigned instead of SUINT in the first place for code you wrote or maintain, if i made a mistake somewhere tell me and ill fix it. Also not complaining about it but people called the type a rootkit or part of a rootkit (thus kind of implying that i would add an exploit or rootkit), said i would ignore the majority and recommanded me to step back. (It wasnt you and it totally doesnt matter who did) but thats not exactly nice ... i should feel offended instead of you, no ? > > Do you think about yourself like holy cow in any aspect of FFmpeg, > security or not. no, certainly not. [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The educated differ from the uneducated as much as the living from the dead. -- Aristotle signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/fft_template: Fix multiple runtime error: signed integer overflow: -1943918714 - 1935113003 cannot be represented in type 'int'
On 6/11/2017 10:21 AM, Paul B Mahol wrote: > On 6/11/17, Michael Niedermayer wrote: >> On Fri, Jun 09, 2017 at 09:07:39PM -0400, Ronald S. Bultje wrote: >>> Hi, >>> >>> On Thu, Jun 8, 2017 at 8:57 PM, Michael Niedermayer >>> >>> wrote: >>> On Thu, Jun 08, 2017 at 06:35:07PM -0400, Ronald S. Bultje wrote: > Hi, > > On Thu, Jun 8, 2017 at 6:10 PM, Michael Niedermayer > wrote: > >> Signed value in >> Unsigned >> INTeger type > > [..] >> Both SUINT and unsigned should produce identical binaries > > This seems to go against the rule that code should be as simple as possible. > > Unsigned is simpler than SUINT if the outcome is the same. You can simply add the part of my mail here as awnser that you snipped away: "But it makes the code hard to understand and maintain because these values are not positive integers but signed integers. Which for C standard compliance need to be stored in a unsigned type." A type that avoids the undefinedness of signed but is semantically signed is correct, unsigned is not. If understandable code and maintainable code has no value to you, you would favour using single letter variables exclusivly and would never use typedef. But you do not do that. I fail to understand why you insist on using unsigned in place of a more specific type, it is not the correct nor clean thing to do. >>> >>> >>> It's not just me, it appears to be most of us. Can't you just step back >>> at >>> some point and be like "ok, I'll let the majority have their way"? >> >> I do not know what the majority prefers. What i see is that the >> people objecting are always the same 3-4 people. And very often >> they have no authorship or past activity in the code a patch is about. >> At least none i could find quickly. > > How dare you speak like that about me? > > Do you think about yourself like holy cow in any aspect of FFmpeg, > security or not. Please, can we all calm down? This is escalating way too much... ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [WIP][PATCH] Opus Piramid Vector Quantization Search in x86 SIMD asm
On 2017-06-11 11:34, Ivan Kalvachev wrote: > On 6/10/17, James Darnley wrote: >> On 2017-06-09 13:41, Ivan Kalvachev wrote: >>> >>> const_*_edge is used on only one place is the code. >>> Would you check if this patch fixes the issue. >>> >>> I expected that the addresses would be pre-calculated >>> by n/yasm as one value and indexed >>> relative to the section start. >>> Instead it seems that each entry is represented with >>> its own address and offset from it. >>> Since the offset is negative it uses all 64 bits and >>> it makes difference if it is truncated to 32 bits. >>> >>> Same issue could happen with clang tools. >> >> The problem is with the relative addressing. You need to load the real >> address first before you can offset with another register at runtime. So >> something like: >> >>> mov reg1, [read_only_const] > lea ? Yes, sorry about that. > == > --- a/libavcodec/x86/opus_pvq_search.asm > +++ b/libavcodec/x86/opus_pvq_search.asm > @@ -406,7 +406,7 @@ align 16 > ; uint32 N - Number of vector elements. Must be 0 < N < 8192 > ; > %macro PVQ_FAST_SEARCH 0 > -cglobal pvq_search,4,5,8, mmsize, inX, outY, K, N > +cglobal pvq_search,4,6,8, mmsize, inX, outY, K, N > %define tmpX rsp > > ;movsxdifnidn Nq, Nd > @@ -419,7 +419,12 @@ cglobal pvq_search,4,5,8, mmsize, inX, outY, K, N > add Nq, r4q ; Nq = align(Nq, mmsize) > sub rsp, Nq; allocate tmpX[Nq] > > +%ifdef PIC > +lea r5q, [const_align_abs_edge]; rip+const > +movups m3, [r5q+r4q-mmsize] ; this is the > bit mask for the padded read at the end of the input > +%else > movups m3, [const_align_abs_edge-mmsize+r4q] ; this is the > bit mask for the padded read at the end of the input > +%endif > > lea r4q, [Nq-mmsize] ; Nq is rounded up (aligned up) to > mmsize, so r4q can't become negative here, unless N=0. > movups m2, [inXq + r4q] > == FYI that diff works just fine, when I correct the line wrapping. I can compile and run on Win64. This is not a comment about the original patch. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 04/11] avformat/librtmp: check return value of setsockopt
2017-06-11 22:05 GMT+08:00 Timo Rothenpieler :
> Fixes CID 1396837
> ---
> libavformat/librtmp.c | 5 -
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/libavformat/librtmp.c b/libavformat/librtmp.c
> index 146df660ac..f3cfa9a8e2 100644
> --- a/libavformat/librtmp.c
> +++ b/libavformat/librtmp.c
> @@ -239,7 +239,10 @@ static int rtmp_open(URLContext *s, const char *uri, int
> flags)
> #if CONFIG_NETWORK
> if (ctx->buffer_size >= 0 && (flags & AVIO_FLAG_WRITE)) {
> int tmp = ctx->buffer_size;
> -setsockopt(r->m_sb.sb_socket, SOL_SOCKET, SO_SNDBUF, &tmp,
> sizeof(tmp));
> +if (setsockopt(r->m_sb.sb_socket, SOL_SOCKET, SO_SNDBUF, &tmp,
> sizeof(tmp))) {
> +rc = AVERROR_EXTERNAL;
> +goto fail;
> +}
> }
> #endif
LGTM
>
> --
> 2.13.0
>
> ___
> 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 1/2] libavfilter/scale: More descriptive in/ref/out logging
Hi Michael, Hoping to get your thoughts on the grepability issue (wrt my previous email). If it needs to be on a single line there's no reason the new format cannot be changed to do so (removing the \n and adding a separator, really). However I'm a big fan of it as-is (for both scale and scale2ref) and I can't find a case of my own where the regex I proposed would be troublesome. Thanks, Kevin On Tue, Jun 6, 2017 at 4:17 PM Kevin Mark wrote: > On Tue, Jun 6, 2017 at 11:49 AM, Michael Niedermayer > wrote: > > yes but its much harder to grep for as its not a single line anymore > > I agree that it's not going to be as pretty a regular expression to > grep through, as there is 33% more data, but it should still be doable > without too much effort. How important is it that we maintain "API" > compatibility on verbose CLI output? > > ffmpeg [...] scale2ref=0:0 [...] -v verbose - 2>&1 >/dev/null | grep -oP > 'regex' > > Where regex is: > > (in|out|ref) +w:(\d+) h:(\d+) fmt:(\w+) sar:(\d+)\/(\d+)(?: > flags:0x[[:xdigit:]]+)? > > Assuming GNU grep 2.25+, you'll get: > > in w:320 h:240 fmt:rgb24 sar:1/1 > ref w:640 h:360 fmt:rgb24 sar:1/1 > out w:640 h:360 fmt:rgb24 sar:3/4 flags:0x2 > > It also works with BSD grep 2.5.1-FreeBSD included in macOS if you use > the -E option instead of -P. These would be considered three separate > matches so if you're using a good regex engine it'd be pretty easy to > loop over each match, check the first group to determine if it's in, > ref, or out and act accordingly on the rest of the captured data. You > could also, if you wanted, assume that the first line is in and the > second line is out if you only have two matches (or lines even) and if > you have three matches/lines the first is in, second is ref, third is > out. If you needed it to work with less sophisticated engines it > shouldn't be too hard to dumb down the regex above. > > Live-ish example: https://regex101.com/r/wvHLpa/1 > > Is there a special property that makes single lines much easier to > grep? Something specific to bash? I wouldn't think bash would have any > problems looping over this by line. > > Thanks, > Kevin > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/2] libavfilter/scale2ref: Maintain main input's DAR
I've been using this patch for the past week now and I believe it's
good to go. Does someone want to take a second look before merging?
On Mon, Jun 5, 2017 at 6:55 AM, Kevin Mark wrote:
> The scale2ref filter will now maintain the DAR of the main input and
> not the DAR of the reference input. This previous behavior was deemed
> counterintuitive for most (all?) use-cases.
>
> Before:
> scale2ref=iw/4:ow/mdar
> in w:320 h:240 fmt:rgb24 sar:1/1
> ref w:640 h:360 fmt:rgb24 sar:1/1
> out w:160 h:120 fmt:rgb24 sar:4/3 flags:0x2
> SAR: ((120 * 640) / (160 * 360)) * (1 / 1) = 4 / 3
> DAR: (160 / 120) * (4 / 3) = 16 / 9
> (main out now same DAR as ref)
>
> Now:
> scale2ref=iw/4:ow/mdar
> in w:320 h:240 fmt:rgb24 sar:1/1
> ref w:640 h:360 fmt:rgb24 sar:1/1
> out w:160 h:120 fmt:rgb24 sar:1/1 flags:0x2
> SAR: ((120 * 320) / (160 * 240)) * (1 / 1) = 1 / 1
> DAR: (160 / 120) * (1 / 1) = 4 / 3
> (main out same DAR as main in)
>
> The scale2ref FATE test has also been updated.
>
> Signed-off-by: Kevin Mark
> ---
> libavfilter/vf_scale.c | 6 +++---
> tests/ref/fate/filter-scale2ref_keep_aspect | 2 +-
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c
> index 9232bc4439..5e55f9344b 100644
> --- a/libavfilter/vf_scale.c
> +++ b/libavfilter/vf_scale.c
> @@ -337,10 +337,10 @@ static int config_props(AVFilterLink *outlink)
> }
> }
>
> -if (inlink->sample_aspect_ratio.num){
> -outlink->sample_aspect_ratio = av_mul_q((AVRational){outlink->h *
> inlink->w, outlink->w * inlink->h}, inlink->sample_aspect_ratio);
> +if (inlink0->sample_aspect_ratio.num){
> +outlink->sample_aspect_ratio = av_mul_q((AVRational){outlink->h *
> inlink0->w, outlink->w * inlink0->h}, inlink0->sample_aspect_ratio);
> } else
> -outlink->sample_aspect_ratio = inlink->sample_aspect_ratio;
> +outlink->sample_aspect_ratio = inlink0->sample_aspect_ratio;
>
> if (ctx->filter == &ff_vf_scale2ref) {
> av_log(ctx, AV_LOG_VERBOSE, "in w:%d h:%d fmt:%s sar:%d/%d\n",
> diff --git a/tests/ref/fate/filter-scale2ref_keep_aspect
> b/tests/ref/fate/filter-scale2ref_keep_aspect
> index ca03277446..8dd0dbb13b 100644
> --- a/tests/ref/fate/filter-scale2ref_keep_aspect
> +++ b/tests/ref/fate/filter-scale2ref_keep_aspect
> @@ -5,7 +5,7 @@
> #media_type 0: video
> #codec_id 0: rawvideo
> #dimensions 0: 160x120
> -#sar 0: 4/3
> +#sar 0: 1/1
> #stream#, dts,pts, duration, size, hash
> 0, 0, 0,1,57600,
> 9a19c23dc3a557786840d0098606d5f1
> 0, 1, 1,1,57600,
> e6fbdabaf1bb0d28afc648ed4d27e9f0
> --
> 2.13.0
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] libavfilter/scale: Populate ow/oh when using 0 as w/h
I screwed up my git-send-email. Please ignore this patch as I already
submitted what should be an identical one on June 7th. My apologies.
On Mon, Jun 12, 2017 at 1:51 AM, Kevin Mark wrote:
> The input width and height is known at parse time so there's no
> reason ow/oh should not be usable when using 0 as the width or
> height expression.
>
> Previously in "scale=0:ow" ow would be set to "0" which works,
> conveniently, as "scale=0:0" is perfectly valid input but this breaks
> down when you do something like "scale=0:ow/4" which one could
> reasonably expect to work as well, but does not as ow is 0 not the
> real value.
>
> This change handles the 0 case for w/h immediately so the ow/oh
> variables work as expected. Consequently, the rest of the code does
> not need to handle 0 input. w/h will always be > 0 or < 0.
>
> Signed-off-by: Kevin Mark
> ---
> libavfilter/scale.c | 13 +
> 1 file changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/libavfilter/scale.c b/libavfilter/scale.c
> index 03745ddcb8..a6c32e3978 100644
> --- a/libavfilter/scale.c
> +++ b/libavfilter/scale.c
> @@ -158,19 +158,19 @@ int ff_scale_eval_dimensions(void *log_ctx,
> av_expr_parse_and_eval(&res, (expr = w_expr),
> names, var_values,
> NULL, NULL, NULL, NULL, NULL, 0, log_ctx);
> -eval_w = var_values[VAR_OUT_W] = var_values[VAR_OW] = res;
> +eval_w = var_values[VAR_OUT_W] = var_values[VAR_OW] = (int) res == 0 ?
> inlink->w : res;
>
> if ((ret = av_expr_parse_and_eval(&res, (expr = h_expr),
>names, var_values,
>NULL, NULL, NULL, NULL, NULL, 0,
> log_ctx)) < 0)
> goto fail;
> -eval_h = var_values[VAR_OUT_H] = var_values[VAR_OH] = res;
> +eval_h = var_values[VAR_OUT_H] = var_values[VAR_OH] = (int) res == 0 ?
> inlink->h : res;
> /* evaluate again the width, as it may depend on the output height */
> if ((ret = av_expr_parse_and_eval(&res, (expr = w_expr),
>names, var_values,
>NULL, NULL, NULL, NULL, NULL, 0,
> log_ctx)) < 0)
> goto fail;
> -eval_w = res;
> +eval_w = (int) res == 0 ? inlink->w : res;
>
> w = eval_w;
> h = eval_h;
> @@ -186,13 +186,10 @@ int ff_scale_eval_dimensions(void *log_ctx,
> factor_h = -h;
> }
>
> -if (w < 0 && h < 0)
> -eval_w = eval_h = 0;
> -
> -if (!(w = eval_w))
> +if (w < 0 && h < 0) {
> w = inlink->w;
> -if (!(h = eval_h))
> h = inlink->h;
> +}
>
> /* Make sure that the result is divisible by the factor we determined
> * earlier. If no factor was set, it is nothing will happen as the
> default
> --
> 2.13.1
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] doc/filters: Correct scale doc regarding w/h <= 0
According to libavfilter/scale.c, if the width and height are both
less than or equal to 0 then the input size is used for both
dimensions. It does not need to be -1. -1:-1 is the same as 0:0 which
is the same as -10:-42, etc.
if (w < 0 && h < 0)
eval_w = eval_h = 0;
The documentation for the zscale filter has also been updated since the
behavior is identical.
Signed-off-by: Kevin Mark
---
doc/filters.texi | 39 ---
1 file changed, 20 insertions(+), 19 deletions(-)
diff --git a/doc/filters.texi b/doc/filters.texi
index 65eef89d07..6f2cc89b1f 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -12125,17 +12125,18 @@ the complete list of scaler options.
Set the output video dimension expression. Default value is the input
dimension.
-If the value is 0, the input width is used for the output.
+If the @var{width} or @var{w} value is 0, the input width is used for
+the output. If the @var{height} or @var{h} value is 0, the input height
+is used for the output.
-If one of the values is -1, the scale filter will use a value that
-maintains the aspect ratio of the input image, calculated from the
-other specified dimension. If both of them are -1, the input size is
-used
+If one and only one of the values is -n with n >= 1, the scale filter
+will use a value that maintains the aspect ratio of the input image,
+calculated from the other specified dimension. After that it will,
+however, make sure that the calculated dimension is divisible by n and
+adjust the value if necessary.
-If one of the values is -n with n > 1, the scale filter will also use a value
-that maintains the aspect ratio of the input image, calculated from the other
-specified dimension. After that it will, however, make sure that the calculated
-dimension is divisible by n and adjust the value if necessary.
+If both values are -n with n >= 1, the behavior will be identical to
+both values being set to 0 as previously detailed.
See below for the list of accepted constants for use in the dimension
expression.
@@ -15268,18 +15269,18 @@ The filter accepts the following options.
Set the output video dimension expression. Default value is the input
dimension.
-If the @var{width} or @var{w} is 0, the input width is used for the output.
-If the @var{height} or @var{h} is 0, the input height is used for the output.
+If the @var{width} or @var{w} value is 0, the input width is used for
+the output. If the @var{height} or @var{h} value is 0, the input height
+is used for the output.
-If one of the values is -1, the zscale filter will use a value that
-maintains the aspect ratio of the input image, calculated from the
-other specified dimension. If both of them are -1, the input size is
-used
+If one and only one of the values is -n with n >= 1, the zscale filter
+will use a value that maintains the aspect ratio of the input image,
+calculated from the other specified dimension. After that it will,
+however, make sure that the calculated dimension is divisible by n and
+adjust the value if necessary.
-If one of the values is -n with n > 1, the zscale filter will also use a value
-that maintains the aspect ratio of the input image, calculated from the other
-specified dimension. After that it will, however, make sure that the calculated
-dimension is divisible by n and adjust the value if necessary.
+If both values are -n with n >= 1, the behavior will be identical to
+both values being set to 0 as previously detailed.
See below for the list of accepted constants for use in the dimension
expression.
--
2.13.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] libavfilter/scale: Populate ow/oh when using 0 as w/h
The input width and height is known at parse time so there's no
reason ow/oh should not be usable when using 0 as the width or
height expression.
Previously in "scale=0:ow" ow would be set to "0" which works,
conveniently, as "scale=0:0" is perfectly valid input but this breaks
down when you do something like "scale=0:ow/4" which one could
reasonably expect to work as well, but does not as ow is 0 not the
real value.
This change handles the 0 case for w/h immediately so the ow/oh
variables work as expected. Consequently, the rest of the code does
not need to handle 0 input. w/h will always be > 0 or < 0.
Signed-off-by: Kevin Mark
---
libavfilter/scale.c | 13 +
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/libavfilter/scale.c b/libavfilter/scale.c
index 03745ddcb8..a6c32e3978 100644
--- a/libavfilter/scale.c
+++ b/libavfilter/scale.c
@@ -158,19 +158,19 @@ int ff_scale_eval_dimensions(void *log_ctx,
av_expr_parse_and_eval(&res, (expr = w_expr),
names, var_values,
NULL, NULL, NULL, NULL, NULL, 0, log_ctx);
-eval_w = var_values[VAR_OUT_W] = var_values[VAR_OW] = res;
+eval_w = var_values[VAR_OUT_W] = var_values[VAR_OW] = (int) res == 0 ?
inlink->w : res;
if ((ret = av_expr_parse_and_eval(&res, (expr = h_expr),
names, var_values,
NULL, NULL, NULL, NULL, NULL, 0,
log_ctx)) < 0)
goto fail;
-eval_h = var_values[VAR_OUT_H] = var_values[VAR_OH] = res;
+eval_h = var_values[VAR_OUT_H] = var_values[VAR_OH] = (int) res == 0 ?
inlink->h : res;
/* evaluate again the width, as it may depend on the output height */
if ((ret = av_expr_parse_and_eval(&res, (expr = w_expr),
names, var_values,
NULL, NULL, NULL, NULL, NULL, 0,
log_ctx)) < 0)
goto fail;
-eval_w = res;
+eval_w = (int) res == 0 ? inlink->w : res;
w = eval_w;
h = eval_h;
@@ -186,13 +186,10 @@ int ff_scale_eval_dimensions(void *log_ctx,
factor_h = -h;
}
-if (w < 0 && h < 0)
-eval_w = eval_h = 0;
-
-if (!(w = eval_w))
+if (w < 0 && h < 0) {
w = inlink->w;
-if (!(h = eval_h))
h = inlink->h;
+}
/* Make sure that the result is divisible by the factor we determined
* earlier. If no factor was set, it is nothing will happen as the default
--
2.13.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] libavfilter/scale: Populate ow/oh when using 0 as w/h
Still interested in thoughts on this patch/discussion. Thanks, Kevin On Wed, Jun 7, 2017 at 3:54 AM, Kevin Mark wrote: > I also have to wonder if it would be advantageous to add the cast on > the right side as well. That way the var_values variables will have > the proper truncated values on future evaluations. Open to comments on > that. > > On Wed, Jun 7, 2017 at 3:45 AM, Kevin Mark wrote: >> -eval_w = var_values[VAR_OUT_W] = var_values[VAR_OW] = res; >> +eval_w = var_values[VAR_OUT_W] = var_values[VAR_OW] = (int) res == 0 ? >> inlink->w : res; > > to perhaps: > +eval_w = var_values[VAR_OUT_W] = var_values[VAR_OW] = (int) res > == 0 ? inlink->w : (int) res; > > Without that extra cast I assume the values in eval_w and > var_values[VAR_OUT_W], var_values[VAR_OW] could be different. I doubt > most users expect that those values could ever be non-integers which > has implications for how you're writing your expression. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
