Re: [FFmpeg-devel] [PATCH] This fixes ISO date formatissue when manifest created by this muxer is not playable in most players. This ensures compatibility with dash standard. Tested on many players (d
On 5/2/2017 2:29 PM, wm4 wrote: On Tue, 2 May 2017 14:17:33 -0700 Aaron Levinsonwrote: On 5/1/2017 11:06 PM, MFojtak wrote: Currently this muxer does not work at all. I don't know if 000Z would make it compatible with more player as I don't know any. However, adding Z makes it compatible with most popular ones like dash.js and shaka. Having this muxer working properly would bring more attention to it and maybe in the future somebody tests it with more players. But for now I suggest to roll out the change and "unblock" this muxer for some real wold use. Also, it would be great to make this muxer codec and container agnostic as it works with h264 and mp4 only. But again, nobody would bother if the muxer doesn't work at all with browsers. I think your original patch is fine, and I only offered the possibility that adding ".000Z" might be even better than just "Z". I don't have push privileges, so I can't commit your patch, but hopefully someone else will do so. Before someone pushes it, please fix the commit message. MFojtak: you can make it more likely that this patch will be pushed sooner by altering the commit message (shortening it--plenty of examples on the e-mail list) and then putting any additional information later. Something like: " avformat/dashenc: Adjusts ISO date formatting for improved compatibility with most players Adjusts ISO date formatting " Aaron Levinson ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/hlsenc: hold old key info when append list
2017-05-05 12:29 GMT+08:00 Aaron Levinson: > On 5/4/2017 9:15 PM, Steven Liu wrote: > >> 2017-05-03 9:49 GMT+08:00 Aaron Levinson : >> >> On 4/27/2017 7:21 PM, Steven Liu wrote: >>> >>> 2017-04-26 7:30 GMT+08:00 Steven Liu : fix ticket id: #6353 > > Signed-off-by: Steven Liu > --- > libavformat/hlsenc.c | 24 > 1 file changed, 24 insertions(+) > > diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c > index 27c8e3355d..3ec0f330fb 100644 > --- a/libavformat/hlsenc.c > +++ b/libavformat/hlsenc.c > @@ -810,6 +810,7 @@ static int parse_playlist(AVFormatContext *s, const > char *url) > int64_t new_start_pos; > char line[1024]; > const char *ptr; > +const char *end; > > if ((ret = ffio_open_whitelist(, url, AVIO_FLAG_READ, > >interrupt_callback, NULL, > @@ -842,6 +843,29 @@ static int parse_playlist(AVFormatContext *s, > const > char *url) > } else if (av_strstart(line, "#EXTINF:", )) { > is_segment = 1; > hls->duration = atof(ptr); > +} else if (av_stristart(line, "#EXT-X-KEY:", )) { > +ptr = av_stristr(line, "URI=\""); > +if (ptr) { > +ptr += strlen("URI=\""); > +end = av_stristr(ptr, ","); > +if (end) { > +av_strlcpy(hls->key_uri, ptr, end - ptr); > +} else { > +av_strlcpy(hls->key_uri, ptr, > sizeof(hls->key_uri)); > > I know that this patch has already been applied (although it didn't get >>> any reviews on the ML), but I think that there are some issues with it. >>> First, it seems that both av_strlcpy() cases above will copy the >>> terminating '\"' character into hls->key_uri. Perhaps that won't cause >>> an >>> issue in practice, but it is likely not the intended result. Second, >>> with >>> both av_strlcpy() calls that use a length of end - ptr, this could in >>> theory result in a buffer overrun depending on how long the URI is, since >>> the destination buffers have a fixed size. This issue is prevented in >>> the >>> second call to av_strlcpy(), since it uses sizeof(hls->key_uri), but it >>> is >>> a potential issue with the first calls (note that this comment applies to >>> the IV=0x code below). If you want to use av_strlcpy(), either make sure >>> that end - ptr is less than or equal to sizeof(hls->key_uri) or do >>> something like *end = 0 first and then use av_strlcpy(hls->key_uri, ptr, >>> sizeof(hls->key_uri)). >>> >>> In addition, based on the EXT-X-KEY example at >>> https://developer.apple.com/library/content/technotes/tn2288/_index.html >>> , it would appear that an EXT-X-KEY declaration may span multiple lines. >>> Your solution will not work properly in this case. >>> >>> Hi Aaron, >> I think the libavformat/hls.c maybe have the same problem, because >> both of hlsenc and hls use read_chomp_line to read one line, >> that need check the '\' tail a line, and read the next line into a >> buffer, that maybe better, is that right? >> > > Yes, I was thinking the same thing, that read_chomp_line() needs to be > enhanced to deal with declarations that span multiple lines. That probably > belongs in a separate patch though, even if it is only relevant for > EXT-X-KEY. Yes, I think the better way is move them to a public space for the hlsenc and hls. > > > Aaron Levinson > > ___ > 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] avformat/hlsenc: hold old key info when append list
On 5/4/2017 9:15 PM, Steven Liu wrote: 2017-05-03 9:49 GMT+08:00 Aaron Levinson: On 4/27/2017 7:21 PM, Steven Liu wrote: 2017-04-26 7:30 GMT+08:00 Steven Liu : fix ticket id: #6353 Signed-off-by: Steven Liu --- libavformat/hlsenc.c | 24 1 file changed, 24 insertions(+) diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c index 27c8e3355d..3ec0f330fb 100644 --- a/libavformat/hlsenc.c +++ b/libavformat/hlsenc.c @@ -810,6 +810,7 @@ static int parse_playlist(AVFormatContext *s, const char *url) int64_t new_start_pos; char line[1024]; const char *ptr; +const char *end; if ((ret = ffio_open_whitelist(, url, AVIO_FLAG_READ, >interrupt_callback, NULL, @@ -842,6 +843,29 @@ static int parse_playlist(AVFormatContext *s, const char *url) } else if (av_strstart(line, "#EXTINF:", )) { is_segment = 1; hls->duration = atof(ptr); +} else if (av_stristart(line, "#EXT-X-KEY:", )) { +ptr = av_stristr(line, "URI=\""); +if (ptr) { +ptr += strlen("URI=\""); +end = av_stristr(ptr, ","); +if (end) { +av_strlcpy(hls->key_uri, ptr, end - ptr); +} else { +av_strlcpy(hls->key_uri, ptr, sizeof(hls->key_uri)); I know that this patch has already been applied (although it didn't get any reviews on the ML), but I think that there are some issues with it. First, it seems that both av_strlcpy() cases above will copy the terminating '\"' character into hls->key_uri. Perhaps that won't cause an issue in practice, but it is likely not the intended result. Second, with both av_strlcpy() calls that use a length of end - ptr, this could in theory result in a buffer overrun depending on how long the URI is, since the destination buffers have a fixed size. This issue is prevented in the second call to av_strlcpy(), since it uses sizeof(hls->key_uri), but it is a potential issue with the first calls (note that this comment applies to the IV=0x code below). If you want to use av_strlcpy(), either make sure that end - ptr is less than or equal to sizeof(hls->key_uri) or do something like *end = 0 first and then use av_strlcpy(hls->key_uri, ptr, sizeof(hls->key_uri)). In addition, based on the EXT-X-KEY example at https://developer.apple.com/library/content/technotes/tn2288/_index.html , it would appear that an EXT-X-KEY declaration may span multiple lines. Your solution will not work properly in this case. Hi Aaron, I think the libavformat/hls.c maybe have the same problem, because both of hlsenc and hls use read_chomp_line to read one line, that need check the '\' tail a line, and read the next line into a buffer, that maybe better, is that right? Yes, I was thinking the same thing, that read_chomp_line() needs to be enhanced to deal with declarations that span multiple lines. That probably belongs in a separate patch though, even if it is only relevant for EXT-X-KEY. Aaron Levinson ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/hlsenc: hold old key info when append list
2017-05-03 9:49 GMT+08:00 Aaron Levinson: > On 4/27/2017 7:21 PM, Steven Liu wrote: > >> 2017-04-26 7:30 GMT+08:00 Steven Liu : >> >> fix ticket id: #6353 >>> >>> Signed-off-by: Steven Liu >>> --- >>> libavformat/hlsenc.c | 24 >>> 1 file changed, 24 insertions(+) >>> >>> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c >>> index 27c8e3355d..3ec0f330fb 100644 >>> --- a/libavformat/hlsenc.c >>> +++ b/libavformat/hlsenc.c >>> @@ -810,6 +810,7 @@ static int parse_playlist(AVFormatContext *s, const >>> char *url) >>> int64_t new_start_pos; >>> char line[1024]; >>> const char *ptr; >>> +const char *end; >>> >>> if ((ret = ffio_open_whitelist(, url, AVIO_FLAG_READ, >>> >interrupt_callback, NULL, >>> @@ -842,6 +843,29 @@ static int parse_playlist(AVFormatContext *s, const >>> char *url) >>> } else if (av_strstart(line, "#EXTINF:", )) { >>> is_segment = 1; >>> hls->duration = atof(ptr); >>> +} else if (av_stristart(line, "#EXT-X-KEY:", )) { >>> +ptr = av_stristr(line, "URI=\""); >>> +if (ptr) { >>> +ptr += strlen("URI=\""); >>> +end = av_stristr(ptr, ","); >>> +if (end) { >>> +av_strlcpy(hls->key_uri, ptr, end - ptr); >>> +} else { >>> +av_strlcpy(hls->key_uri, ptr, sizeof(hls->key_uri)); >>> >> > I know that this patch has already been applied (although it didn't get > any reviews on the ML), but I think that there are some issues with it. > First, it seems that both av_strlcpy() cases above will copy the > terminating '\"' character into hls->key_uri. Perhaps that won't cause an > issue in practice, but it is likely not the intended result. Second, with > both av_strlcpy() calls that use a length of end - ptr, this could in > theory result in a buffer overrun depending on how long the URI is, since > the destination buffers have a fixed size. This issue is prevented in the > second call to av_strlcpy(), since it uses sizeof(hls->key_uri), but it is > a potential issue with the first calls (note that this comment applies to > the IV=0x code below). If you want to use av_strlcpy(), either make sure > that end - ptr is less than or equal to sizeof(hls->key_uri) or do > something like *end = 0 first and then use av_strlcpy(hls->key_uri, ptr, > sizeof(hls->key_uri)). > > In addition, based on the EXT-X-KEY example at > https://developer.apple.com/library/content/technotes/tn2288/_index.html > , it would appear that an EXT-X-KEY declaration may span multiple lines. > Your solution will not work properly in this case. > Hi Aaron, I think the libavformat/hls.c maybe have the same problem, because both of hlsenc and hls use read_chomp_line to read one line, that need check the '\' tail a line, and read the next line into a buffer, that maybe better, is that right? > > Aaron Levinson > > +} >>> +} >>> + >>> +ptr = av_stristr(line, "IV=0x"); >>> +if (ptr) { >>> +ptr += strlen("IV=0x"); >>> +end = av_stristr(ptr, ","); >>> +if (end) { >>> +av_strlcpy(hls->iv_string, ptr, end - ptr); >>> +} else { >>> +av_strlcpy(hls->iv_string, ptr, >>> sizeof(hls->iv_string)); >>> +} >>> +} >>> + >>> } else if (av_strstart(line, "#", NULL)) { >>> continue; >>> } else if (line[0]) { >>> -- >>> 2.11.0 (Apple Git-81) >>> >>> >>> Applied! >> >> >> Thanks >> > ___ > 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] fate/exr : add test for Y, B44A negative, datawindow != display window
On Mon, May 01, 2017 at 02:31:28PM +0200, Martin Vignali wrote: > Hello, > > In attach a patch to add fate tests for exr > > samples can be found here > https://we.tl/ItuIX0BMfk uploaded [..] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Everything should be made as simple as possible, but not simpler. -- Albert Einstein signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] libavformat/hls: Observe Set-Cookie headers
On 2017-05-02 09:04 PM, wm4 wrote: On Tue, 2 May 2017 20:47:06 -0400 Micah Galiziawrote: Signed-off-by: Micah Galizia --- libavformat/hls.c | 12 +--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/libavformat/hls.c b/libavformat/hls.c index bac53a4350..643d50e1da 100644 --- a/libavformat/hls.c +++ b/libavformat/hls.c @@ -630,8 +630,14 @@ static int open_url(AVFormatContext *s, AVIOContext **pb, const char *url, ret = s->io_open(s, pb, url, AVIO_FLAG_READ, ); if (ret >= 0) { // update cookies on http response with setcookies. -void *u = (s->flags & AVFMT_FLAG_CUSTOM_IO) ? NULL : s->pb; -update_options(>cookies, "cookies", u); +char *new_cookies = NULL; + +av_opt_get(*pb, "cookies", AV_OPT_SEARCH_CHILDREN, (uint8_t**)_cookies); +if (new_cookies) { +av_free(c->cookies); +c->cookies = new_cookies; +} + av_dict_set(, "cookies", c->cookies, 0); } @@ -1608,7 +1614,7 @@ static int hls_close(AVFormatContext *s) static int hls_read_header(AVFormatContext *s) { -void *u = (s->flags & AVFMT_FLAG_CUSTOM_IO) ? NULL : s->pb; +void *u = s->pb; HLSContext *c = s->priv_data; int ret = 0, i; int highest_cur_seq_no = 0; Not comfortable with this without knowing what the purpose of this CUSTOM_IO check was. Sorry, I misunderstood a prior email. Regarding why we check the custom IO flags, I was able to track down the following: https://lists.ffmpeg.org/pipermail/ffmpeg-trac/2013-February/013020.html for the hls_read_header. It was originally added in commit db6e2e848b21d988fb108995387a8c7836da4dc7 back in 2013. After reading that, I think the problem it was trying to solve was about treating pb->opaque as a urlcontext -- but the code no longer does that, unless I've misunderstood all the casting in av_opt_get. What I believe it does now is call av_opt_get(*pb...), which eventually winds up in av_opt_find2 where it pulls options out the AVClass (av_class) contained in AVIOContext and starts reading its options. It doesn't seem to be touching opaque field in AVIOContext anymore. Given this, I _think_ what I did is still ok. However, I can also change the patch to leave hls_read_header alone and add an if (s->flags ^ AVFMT_FLAG_CUSTOM_IO) before getting the new_cookies. This, effectively, fixes the bug without disregarding the check against AVFMT_FLAG_CUSTOM_IO -- best of both worlds and a smaller patch. If nobody can confirm my analysis that opaque is now being left alone (thus, no longer requiring the check against the custom IO flag) then I'll just put the smaller patch up. Thanks, ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/2] avcodec/avpacket: Use av_copy_packet_side_data() in av_packet_copy_props()
On 5/4/2017 6:59 PM, wm4 wrote: > On Thu, 4 May 2017 12:51:14 +0200 > Michael Niedermayerwrote: > >> On Wed, May 03, 2017 at 11:27:53PM -0300, James Almer wrote: >>> On 5/3/2017 11:00 PM, Michael Niedermayer wrote: On Wed, May 03, 2017 at 03:28:53PM +0200, Hendrik Leppkes wrote: > On Wed, May 3, 2017 at 3:23 PM, Michael Niedermayer > wrote: >> On Wed, May 03, 2017 at 12:54:53PM +0200, Hendrik Leppkes wrote: >>> On Wed, May 3, 2017 at 11:50 AM, Michael Niedermayer >>> wrote: On Wed, May 03, 2017 at 11:37:35AM +0200, wm4 wrote: > On Wed, 3 May 2017 11:29:04 +0200 > Michael Niedermayer wrote: > >> On Wed, May 03, 2017 at 05:29:07AM +0200, wm4 wrote: >>> On Wed, 3 May 2017 05:21:50 +0200 >>> Michael Niedermayer wrote: >>> Fixes timeout Fixes: 1293/clusterfuzz-testcase-minimized-6054752074858496 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/targets/ffmpeg Signed-off-by: Michael Niedermayer --- libavcodec/avpacket.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c index 4bf830bb8a..ff7ee730a4 100644 --- a/libavcodec/avpacket.c +++ b/libavcodec/avpacket.c @@ -557,6 +557,9 @@ FF_ENABLE_DEPRECATION_WARNINGS dst->flags= src->flags; dst->stream_index = src->stream_index; +if (!dst->side_data_elems); +return av_copy_packet_side_data(dst, src); + for (i = 0; i < src->side_data_elems; i++) { enum AVPacketSideDataType type = src->side_data[i].type; int size = src->side_data[i].size; >>> >>> This doesn't look right... >> >> already fixed the ; locally >> >> >> [...] > > I didn't see that, I was referring to the fact that you call > av_copy_packet_side_data(), and only sometimes (at least by > intention). > That requires at least an explanation in the commit message. av_packet_copy_props() would add side data to the destination packet it doesnt replace previously existing side data except in case of error. I dont know if that is intended but i didnt want to change it as that would be unrelated to this patch >>> >>> This behavior seems odd at best, so maybe we should just change that >>> and make the behavior more logical, and fix your issue at the same >>> time? >> >> That can be done and makes alot of sense after the patch. >> >> we need to fix this issue in our releases too >> a simple bugfix and a seperate behavior change afterwards allows us >> to simply backport the bugfix from master. >> > > If anything your "bugfix" here is a performance improvement, and I > don't think that warrants backporting either way. Before the patch the sample file takes 51seconds to decode, after it, it takes 203 milliseconds. The difference is caused by only 2 calls to the copy code blocking a machine for 51 seconds for something that decodes in 203ms otherwise, is IMO worth backporting a fix for. We can add a bound to the number of side data elements if theres consens about doing that and doing it in releases. still, no code should call realloc() in a loop when it can do one (re)allocation call at the top. >>> >>> First we need to figure out if these side data copy functions are meant >>> to append the source packet's side data to the dest's, or if they should >>> replace it. That is, we need to see if these functions are meant to >>> assume the dest packet is empty or not. Because judging by every other >>> field copied, i guess it's implied the dest packet is expected to be empty. >>> It's worth nothing that av_stream_add_side_data() overwrites existing >>> side data if one of the same type already exists, unlike >>> av_packet_add_side_data(). >> >> There are 2 Issues >> >> A. Is that there is a bug in master and the releases that allows one >>to craft a input which will get you stuck in a realloc loop a long >>time >> B. The API is not well documented about what should happen if the >>destination packet isnt empty on a side data copy >> >> If we fix A and then fix B, then we can backport A without B. >> >> If we fix B and then fix A then A will likely depend on B and backporting >> just the fix without
Re: [FFmpeg-devel] [RFC]lavu/timecode: Increase AV_TIMECODE_STR_SIZE.
On 5/4/2017 4:27 PM, Carl Eugen Hoyos wrote: Hi! Attached patch is one possibility to fix the following warning with gcc 7: libavutil/timecode.c: In function ‘av_timecode_make_string’: libavutil/timecode.c:103:60: warning: ‘%02d’ directive output may be truncated writing between 2 and 10 bytes into a region of size between 0 and 7 [-Wformat-truncation=] snprintf(buf, AV_TIMECODE_STR_SIZE, "%s%02d:%02d:%02d%c%02d", ^~~~ libavutil/timecode.c:103:41: note: directive argument in the range [0, 2147483647] snprintf(buf, AV_TIMECODE_STR_SIZE, "%s%02d:%02d:%02d%c%02d", ^~~~ libavutil/timecode.c:103:5: note: ‘snprintf’ output between 12 and 32 bytes into a destination of size 16 snprintf(buf, AV_TIMECODE_STR_SIZE, "%s%02d:%02d:%02d%c%02d", ^ neg ? "-" : "", ~~~ hh, mm, ss, drop ? ';' : ':', ff); ~ Several similar warnings are printed, an alternative is to disable the warning. The warning seemed wrong on first sight but may actually be correct, we accept invalid fps for timecode. Regarding the change in your patch: -#define AV_TIMECODE_STR_SIZE 16 +#define AV_TIMECODE_STR_SIZE 23 It seems like 23 characters wouldn't be sufficient based on the warning: "output between 12 and 32 bytes into a destination of size 16". I would guess that you would need at least 32 characters and perhaps one more (?) for the terminating null character to avoid that warning. Even if in practice, it will never use 32 characters, its not a big deal to have a little waste here. The buffer size should match the format specification. An alternative would be to use "%02hd" or "%02hhd", but doing so would require casts or using different types for hh, mm, etc. Aaron Levinson ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [RFC]lavu/opt: Use && instead of * in boolean expression
On 5/4/2017 4:32 PM, Carl Eugen Hoyos wrote: > Hi! > > It may be better to disable the warning. > > Carl Eugen > > -num = den ? num * intnum / den : (num * intnum ? INFINITY : NAN); > +num = den ? num * intnum / den : (num && intnum ? INFINITY : NAN); In order to preserve the original logic, why not do the following: +num = den ? num * intnum / den : (((num * intnum) != 0) ? INFINITY : NAN); Aaron Levinson ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] ffmpeg: check for unconnected outputs
2017-05-03 1:06 GMT+02:00 wm4: > Fixes e.g.: > > ffmpeg -f lavfi -i testsrc -f lavfi -i testsrc -filter_complex > "[0:v][1:v]psnr[out]" -f null none I believe you forgot to fix the commit message. Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] avcodec/wavpack: Fix signed integer overflow: 1285114081 * 2 cannot be represented in type 'int'
Fixes: 945/clusterfuzz-testcase-6037937588273152 Fixes: integer overflow Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/targets/ffmpeg Signed-off-by: Michael Niedermayer--- libavcodec/wavpack.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libavcodec/wavpack.c b/libavcodec/wavpack.c index bc4402f638..d2ba97ee2c 100644 --- a/libavcodec/wavpack.c +++ b/libavcodec/wavpack.c @@ -240,7 +240,7 @@ static int wv_get_value(WavpackFrameContext *ctx, GetBitContext *gb, if (get_bits_left(gb) <= 0) goto error; } else { -int mid = (base * 2 + add + 1) >> 1; +int mid = (base * 2U + add + 1) >> 1; while (add > c->error_limit) { if (get_bits_left(gb) <= 0) goto error; @@ -249,7 +249,7 @@ static int wv_get_value(WavpackFrameContext *ctx, GetBitContext *gb, base = mid; } else add = mid - base - 1; -mid = (base * 2 + add + 1) >> 1; +mid = (base * 2U + add + 1) >> 1; } ret = mid; } -- 2.11.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [RFC]lavu/opt: Use && instead of * in boolean expression
Hi! It may be better to disable the warning. Carl Eugen From ab94367f502ab00f643a78608593eb9522e5c3be Mon Sep 17 00:00:00 2001 From: Carl Eugen HoyosDate: Fri, 5 May 2017 01:30:19 +0200 Subject: [PATCH] lavu/opt: Use "&&" instead of "*" in boolean expression. Fixes the following warning: libavutil/opt.c:101:47: warning: '*' in boolean context, suggest '&&' instead --- libavutil/opt.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavutil/opt.c b/libavutil/opt.c index 6f87078..df88663 100644 --- a/libavutil/opt.c +++ b/libavutil/opt.c @@ -98,7 +98,7 @@ static int write_number(void *obj, const AVOption *o, void *dst, double num, int { if (o->type != AV_OPT_TYPE_FLAGS && (!den || o->max * den < num * intnum || o->min * den > num * intnum)) { -num = den ? num * intnum / den : (num * intnum ? INFINITY : NAN); +num = den ? num * intnum / den : (num && intnum ? INFINITY : NAN); av_log(obj, AV_LOG_ERROR, "Value %f for parameter '%s' out of range [%g - %g]\n", num, o->name, o->min, o->max); return AVERROR(ERANGE); -- 1.7.10.4 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [RFC]lavu/timecode: Increase AV_TIMECODE_STR_SIZE.
Hi! Attached patch is one possibility to fix the following warning with gcc 7: libavutil/timecode.c: In function ‘av_timecode_make_string’: libavutil/timecode.c:103:60: warning: ‘%02d’ directive output may be truncated writing between 2 and 10 bytes into a region of size between 0 and 7 [-Wformat-truncation=] snprintf(buf, AV_TIMECODE_STR_SIZE, "%s%02d:%02d:%02d%c%02d", ^~~~ libavutil/timecode.c:103:41: note: directive argument in the range [0, 2147483647] snprintf(buf, AV_TIMECODE_STR_SIZE, "%s%02d:%02d:%02d%c%02d", ^~~~ libavutil/timecode.c:103:5: note: ‘snprintf’ output between 12 and 32 bytes into a destination of size 16 snprintf(buf, AV_TIMECODE_STR_SIZE, "%s%02d:%02d:%02d%c%02d", ^ neg ? "-" : "", ~~~ hh, mm, ss, drop ? ';' : ':', ff); ~ Several similar warnings are printed, an alternative is to disable the warning. The warning seemed wrong on first sight but may actually be correct, we accept invalid fps for timecode. Carl Eugen From 8906d9b41b5576ddc0bc5f79f1192001825b89c7 Mon Sep 17 00:00:00 2001 From: Carl Eugen HoyosDate: Fri, 5 May 2017 01:23:24 +0200 Subject: [PATCH] lavu/timecode: Increase AV_TIMECODE_STR_SIZE. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes the following warning: libavutil/timecode.c:103:60: warning: â%02dâ directive output may be truncated writing between 2 and 10 bytes into a region of size between 0 and 7 --- libavutil/timecode.h |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavutil/timecode.h b/libavutil/timecode.h index 56e3975..37c1361 100644 --- a/libavutil/timecode.h +++ b/libavutil/timecode.h @@ -30,7 +30,7 @@ #include #include "rational.h" -#define AV_TIMECODE_STR_SIZE 16 +#define AV_TIMECODE_STR_SIZE 23 enum AVTimecodeFlag { AV_TIMECODE_FLAG_DROPFRAME = 1<<0, ///< timecode is drop frame -- 1.7.10.4 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/libmp3lame: properly handle unaligned frame data
On Fri, 5 May 2017 00:05:35 +0200 Nicolas Georgewrote: > Le quintidi 15 floréal, an CCXXV, Nicolas George a écrit : > > Absolutely not, these change were there for a reason, that reason is > > still valid. > > Also, the bug is not in libavfilter, so reverting changes in libavfilter > makes no sense. Yes, it does, because it triggers the crash. It amazes me that you don't think crashing bugs should be fixed quickly. Nobody cares if your code is awesome and perfect if it trigger crashes. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/libmp3lame: properly handle unaligned frame data
Le quintidi 15 floréal, an CCXXV, Nicolas George a écrit : > Absolutely not, these change were there for a reason, that reason is > still valid. Also, the bug is not in libavfilter, so reverting changes in libavfilter makes no sense. Regards, -- Nicolas George signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/2] avcodec/avpacket: Use av_copy_packet_side_data() in av_packet_copy_props()
On Thu, 4 May 2017 12:51:14 +0200 Michael Niedermayerwrote: > On Wed, May 03, 2017 at 11:27:53PM -0300, James Almer wrote: > > On 5/3/2017 11:00 PM, Michael Niedermayer wrote: > > > On Wed, May 03, 2017 at 03:28:53PM +0200, Hendrik Leppkes wrote: > > >> On Wed, May 3, 2017 at 3:23 PM, Michael Niedermayer > > >> wrote: > > >>> On Wed, May 03, 2017 at 12:54:53PM +0200, Hendrik Leppkes wrote: > > On Wed, May 3, 2017 at 11:50 AM, Michael Niedermayer > > wrote: > > > On Wed, May 03, 2017 at 11:37:35AM +0200, wm4 wrote: > > >> On Wed, 3 May 2017 11:29:04 +0200 > > >> Michael Niedermayer wrote: > > >> > > >>> On Wed, May 03, 2017 at 05:29:07AM +0200, wm4 wrote: > > On Wed, 3 May 2017 05:21:50 +0200 > > Michael Niedermayer wrote: > > > > > Fixes timeout > > > Fixes: 1293/clusterfuzz-testcase-minimized-6054752074858496 > > > > > > Found-by: continuous fuzzing process > > > https://github.com/google/oss-fuzz/tree/master/targets/ffmpeg > > > Signed-off-by: Michael Niedermayer > > > --- > > > libavcodec/avpacket.c | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c > > > index 4bf830bb8a..ff7ee730a4 100644 > > > --- a/libavcodec/avpacket.c > > > +++ b/libavcodec/avpacket.c > > > @@ -557,6 +557,9 @@ FF_ENABLE_DEPRECATION_WARNINGS > > > dst->flags= src->flags; > > > dst->stream_index = src->stream_index; > > > > > > +if (!dst->side_data_elems); > > > +return av_copy_packet_side_data(dst, src); > > > + > > > for (i = 0; i < src->side_data_elems; i++) { > > > enum AVPacketSideDataType type = src->side_data[i].type; > > > int size = src->side_data[i].size; > > > > This doesn't look right... > > >>> > > >>> already fixed the ; locally > > >>> > > >>> > > >>> [...] > > >> > > >> I didn't see that, I was referring to the fact that you call > > >> av_copy_packet_side_data(), and only sometimes (at least by > > >> intention). > > >> That requires at least an explanation in the commit message. > > > > > > av_packet_copy_props() would add side data to the destination packet > > > it doesnt replace previously existing side data except in case of > > > error. > > > I dont know if that is intended but i didnt want to change it as that > > > would be unrelated to this patch > > > > > > > This behavior seems odd at best, so maybe we should just change that > > and make the behavior more logical, and fix your issue at the same > > time? > > >>> > > >>> That can be done and makes alot of sense after the patch. > > >>> > > >>> we need to fix this issue in our releases too > > >>> a simple bugfix and a seperate behavior change afterwards allows us > > >>> to simply backport the bugfix from master. > > >>> > > >> > > >> If anything your "bugfix" here is a performance improvement, and I > > >> don't think that warrants backporting either way. > > > > > > Before the patch the sample file takes > > > 51seconds to decode, after it, it takes 203 milliseconds. > > > The difference is caused by only 2 calls to the copy code > > > > > > blocking a machine for 51 seconds for something that decodes in 203ms > > > otherwise, is IMO worth backporting a fix for. > > > > > > We can add a bound to the number of side data elements if theres > > > consens about doing that and doing it in releases. > > > still, no code should call realloc() in a loop when it can do one > > > (re)allocation call at the top. > > > > First we need to figure out if these side data copy functions are meant > > to append the source packet's side data to the dest's, or if they should > > replace it. That is, we need to see if these functions are meant to > > assume the dest packet is empty or not. Because judging by every other > > field copied, i guess it's implied the dest packet is expected to be empty. > > It's worth nothing that av_stream_add_side_data() overwrites existing > > side data if one of the same type already exists, unlike > > av_packet_add_side_data(). > > There are 2 Issues > > A. Is that there is a bug in master and the releases that allows one >to craft a input which will get you stuck in a realloc loop a long >time > B. The API is not well documented about what should happen if the >destination packet isnt empty on a side data copy > > If we fix A and then fix B, then we can backport A without B. > > If we fix B and then fix A then A will likely depend on B
Re: [FFmpeg-devel] [PATCH] avcodec/libmp3lame: properly handle unaligned frame data
On Thu, 4 May 2017 22:24:15 +0200 Nicolas Georgewrote: > Le quintidi 15 floréal, an CCXXV, Kyle Swanson a écrit : > > I believe this is still broken on git master and is present on release > > Well, nobody fixed it. > > > 3.3. If a proper fix is going to take time, > > There is no reason it should, the issue is rather simple. > > > the patches that caused > > this should probably just be reverted for the time being, IMHO. > > Absolutely not, these change were there for a reason, that reason is > still valid. So I guess our users are supposed to tolerate those crashes? Revert seems like the right thing to do. Your changes can be reapplied once the other issues are fixed. Or you come up with a fix now. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]configure: Fix libopus detection
2017-05-04 5:28 GMT+02:00 James Almer: > On 5/3/2017 4:24 AM, Carl Eugen Hoyos wrote: >> 2017-03-30 0:47 GMT+02:00 Carl Eugen Hoyos : >> >>> Attached patch fixes a compilation error here. >> >> If nobody wants to work on this issue, I'll commit >> this patch in a few days. >> >> Carl Eugen > > I have pushed a variation of my previous suggestion. I can confirm that this fixes my original issue (that libopus detection succeeds but compilation fails) - thank you! Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]compat/strtod: Add missing const qualifiers
2017-05-02 22:29 GMT+02:00 Aaron Levinson: > On 5/1/2017 1:51 AM, Carl Eugen Hoyos wrote: >> >> Hi! >> >> Even without the casts, the patch reduces the number of warnings shown >> when compiling compat/strtod from seven to three. >> >> Please comment, Carl Eugen > > LGTM Patch applied. Thank you, Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] cuvid: support AVCodecContext.hw_device_ctx API
--- a/libavcodec/cuvid.c +++ b/libavcodec/cuvid.c @@ -802,9 +802,17 @@ static av_cold int cuvid_decode_init(AVCodecContext *avctx) goto error; } } else { -ret = av_hwdevice_ctx_create(>hwdevice, AV_HWDEVICE_TYPE_CUDA, ctx->cu_gpu, NULL, 0); -if (ret < 0) -goto error; +if (avctx->hw_device_ctx) { +ctx->hwdevice = av_buffer_ref(avctx->hw_device_ctx); +if (!ctx->hwdevice) { +ret = AVERROR(ENOMEM); +goto error; +} +} else { +ret = av_hwdevice_ctx_create(>hwdevice, AV_HWDEVICE_TYPE_CUDA, ctx->cu_gpu, NULL, 0); +if (ret < 0) +goto error; +} ctx->hwframe = av_hwframe_ctx_alloc(ctx->hwdevice); if (!ctx->hwframe) { Simple enough, LGTM ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/libmp3lame: properly handle unaligned frame data
Le quintidi 15 floréal, an CCXXV, Kyle Swanson a écrit : > I believe this is still broken on git master and is present on release Well, nobody fixed it. > 3.3. If a proper fix is going to take time, There is no reason it should, the issue is rather simple. > the patches that caused > this should probably just be reverted for the time being, IMHO. Absolutely not, these change were there for a reason, that reason is still valid. Regards, -- Nicolas George 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/libmp3lame: properly handle unaligned frame data
Hi, On Tue, May 2, 2017 at 12:18 PM, wm4wrote: > On Tue, 2 May 2017 16:16:35 +0200 > Nicolas George wrote: > >> Le duodi 12 floréal, an CCXXV, Paul B Mahol a écrit : >> > This is all one big mess. >> >> It is, but I will not take responsibility when it is not mine. >> >> Libavfilter was in need of a serious overhaul. I do not see you denying >> it, and you suffered from the limitations it was causing as much as >> anybody else. I have undertaken that overhaul, and the help I received >> from other developers in that project has been scarce, to say the least. > > That doesn't free you from the responsibility to keep things in a > reasonably working state. > >> I do not complain, I can manage on my own; it will take longer, but I am >> in no rush. But I will not let the complaints of the inspectors of >> finished work trouble me. > > I can understand that attitude, but if a fix takes "longer" and > meanwhile certain filters or encoders crash left and right in git > master and even in the latest FFmpeg release, there's a need to act > quickly, even if it means reverting certain patches. I believe this is still broken on git master and is present on release 3.3. If a proper fix is going to take time, the patches that caused this should probably just be reverted for the time being, IMHO. > I don't want to blame anyone (and I ask you not to blame others either, > like you did above), but please fix/avoid crashing bugs? > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel Thanks, Kyle ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] cuvid: support AVCodecContext.hw_device_ctx API
On 2017-05-02 16:56, wm4 wrote: This is a newer API that is intended for decoders like the cuvid wrapper. Until now, the wrapper required to set an awkward "incomplete" hw_frames_ctx to set the device. Now the device can be set directly, and the user can get AV_PIX_FMT_CUDA output for a specific device simply by setting hw_device_ctx. This still does a dummy ff_get_format() call at init time, and should be fully backward compatible. --- Not sure how to test correctly - it worked with mpv even when I accidentally didn't use the correct VO device. --- libavcodec/cuvid.c | 14 +++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/libavcodec/cuvid.c b/libavcodec/cuvid.c index 288083423e..3453003965 100644 --- a/libavcodec/cuvid.c +++ b/libavcodec/cuvid.c @@ -802,9 +802,17 @@ static av_cold int cuvid_decode_init(AVCodecContext *avctx) goto error; } } else { -ret = av_hwdevice_ctx_create(>hwdevice, AV_HWDEVICE_TYPE_CUDA, ctx->cu_gpu, NULL, 0); -if (ret < 0) -goto error; +if (avctx->hw_device_ctx) { +ctx->hwdevice = av_buffer_ref(avctx->hw_device_ctx); +if (!ctx->hwdevice) { +ret = AVERROR(ENOMEM); +goto error; +} +} else { +ret = av_hwdevice_ctx_create(>hwdevice, AV_HWDEVICE_TYPE_CUDA, ctx->cu_gpu, NULL, 0); +if (ret < 0) +goto error; +} ctx->hwframe = av_hwframe_ctx_alloc(ctx->hwdevice); if (!ctx->hwframe) { Makes sense to me. Ship it. --phil ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/wavpack: Fix invalid shift and integer overflow
On Fri, Apr 07, 2017 at 03:38:12AM +0200, Michael Niedermayer wrote: > Fixes: 940/clusterfuzz-testcase-5200378381467648 > > Found-by: continuous fuzzing process > https://github.com/google/oss-fuzz/tree/master/targets/ffmpeg > Signed-off-by: Michael Niedermayer> --- > libavcodec/wavpack.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) applied [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB I have often repented speaking, but never of holding my tongue. -- Xenocrates signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] fate/exr : add test for Y, B44A negative, datawindow != display window
2017-05-01 14:31 GMT+02:00 Martin Vignali: > Hello, > > In attach a patch to add fate tests for exr > > samples can be found here > https://we.tl/ItuIX0BMfk > > and need to be put inside fate-suite/exr > > can be test with make fate-exr SAMPLES=fate-suite/ > > Theses tests increase coverage of exr.c : > > - rgb_b44a_half_negative_4x4 : test negative half value inside B44A bloc > (the entire picture is one b44A bloc) > > - y_tile_zip_half_12x8 and y_scanline_zip_half_12x8 : > test the luma only mode in tile and scanline > > - rgb_scanline_half_piz_dw_t08 : it's one of the official sample for > testing datawindow/display window (https://github.com/openexr/ > openexr-images/tree/master/DisplayWindow) > Test when data window != display window, but data are entirely inside the > display window (the rest of the picture is fill with black) > > > Martin > > Ping for upload Martin ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/flicvideo: Check for chunk overread
On Tue, May 02, 2017 at 12:54:34AM +0200, Michael Niedermayer wrote: > Fixes integer overflow > Fixes: 1292/clusterfuzz-testcase-minimized-5795512143839232 > > Signed-off-by: Michael Niedermayer> --- > libavcodec/flicvideo.c | 20 +++- > 1 file changed, 19 insertions(+), 1 deletion(-) applied [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Republics decline into democracies and democracies degenerate into despotisms. -- 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 2/2] libavcodec/mpeg4videodec: Convert sprite_offset to 64bit
On Wed, May 03, 2017 at 05:21:51AM +0200, Michael Niedermayer wrote: > This avoids intermediates from overflowing (the final values are checked) > Fixes: runtime error: signed integer overflow: -167712 + -2147352576 cannot > be represented in type 'int' > > Fixes: 1298/clusterfuzz-testcase-minimized-5955580877340672 > > Found-by: continuous fuzzing process > https://github.com/google/oss-fuzz/tree/master/targets/ffmpeg > Signed-off-by: Michael Niedermayer> --- > libavcodec/mpeg4videodec.c | 102 > ++--- > 1 file changed, 50 insertions(+), 52 deletions(-) applied [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Observe your enemies, for they first find out your faults. -- Antisthenes signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] fixes the issue https://trac.ffmpeg.org/ticket/6338
thank for you pointing it out. I'll go through the docs. and for this patch do I have to redo as per norms, or is it accepted? regards, Vineet On Thu, May 4, 2017 at 8:04 PM, Moritz Barsnickwrote: > On Thu, May 04, 2017 at 08:04:21 +0530, Vineet Goel wrote: > > sorry about that. I am not proficient with git and development. > > There are a lot of useful hints here: > https://ffmpeg.org/developer.html > Actually pretty much a must-read before submitting patches. ;-) > > > And I am attaching the patch file as is generated by "git format-patch > > origin/master" command > > Fine. > > If you look at how other contributions to git look (it's worth taking > the time!), you will see that the first line of your commit message > should probably read: > > lavf/http: fix cookies with proxy > > followed by an empty line, and then something like > " > Fixes #6338. > " > > Sorry for nitpicking and not actually reviewing the change, ;-) > Moritz > ___ > 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] avcodec: Avoid splitting side data repeatedly
On 5/4/2017 12:15 PM, Michael Niedermayer wrote: > Fixes Timeout > Fixes: 508/clusterfuzz-testcase-6245747678773248 > > Found-by: continuous fuzzing process > https://github.com/google/oss-fuzz/tree/master/targets/ffmpeg > Signed-off-by: Michael Niedermayer> --- > libavcodec/avpacket.c | 24 > libavcodec/decode.c | 9 +++-- > libavcodec/internal.h | 4 > 3 files changed, 35 insertions(+), 2 deletions(-) > > diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c > index d97400eaef..de12a1f1ce 100644 > --- a/libavcodec/avpacket.c > +++ b/libavcodec/avpacket.c > @@ -463,6 +463,30 @@ int av_packet_split_side_data(AVPacket *pkt){ > return 0; > } > > +int ff_packet_split_and_drop_side_data(AVPacket *pkt){ > +if (!pkt->side_data_elems && pkt->size >12 && AV_RB64(pkt->data + > pkt->size - 8) == FF_MERGE_MARKER){ > +int i; > +unsigned int size; > +uint8_t *p; > + > +p = pkt->data + pkt->size - 8 - 5; > +for (i=1; ; i++){ > +size = AV_RB32(p); > +if (size>INT_MAX - 5 || p - pkt->data < size) > +return 0; > +if (p[4]&128) > +break; > +if (p - pkt->data < size + 5) > +return 0; > +p-= size+5; > +} > +pkt->size = p - pkt->data - size; > +av_assert0(pkt->size >= 0); > +return 1; > +} > +return 0; > +} > + > #endif > > uint8_t *av_packet_pack_dictionary(AVDictionary *dict, int *size) > diff --git a/libavcodec/decode.c b/libavcodec/decode.c > index 708071fd07..43ba04550a 100644 > --- a/libavcodec/decode.c > +++ b/libavcodec/decode.c > @@ -392,7 +392,9 @@ static int decode_simple_internal(AVCodecContext *avctx, > AVFrame *frame) > tmp = *pkt; > #if FF_API_MERGE_SD > FF_DISABLE_DEPRECATION_WARNINGS > -did_split = av_packet_split_side_data(); > +did_split = avci->compat_decode_partial_size ? > +ff_packet_split_and_drop_side_data() : > +av_packet_split_side_data(); Eh, this is all going away as soon as we bump major and will not make it to any upcoming release, so guess it's fine for now. > > if (did_split) { > ret = extract_packet_props(avctx->internal, ); > @@ -961,6 +963,7 @@ int avcodec_decode_subtitle2(AVCodecContext *avctx, > AVSubtitle *sub, > AVPacket *avpkt) > { > int i, ret = 0; > +AVCodecInternal *avci = avctx->internal; > > if (!avpkt->data && avpkt->size) { > av_log(avctx, AV_LOG_ERROR, "invalid packet: NULL data, size != > 0\n"); > @@ -981,7 +984,9 @@ int avcodec_decode_subtitle2(AVCodecContext *avctx, > AVSubtitle *sub, > AVPacket tmp = *avpkt; > #if FF_API_MERGE_SD > FF_DISABLE_DEPRECATION_WARNINGS > -int did_split = av_packet_split_side_data(); > +int did_split = avci->compat_decode_partial_size ? > +ff_packet_split_and_drop_side_data() : > +av_packet_split_side_data(); > //apply_param_change(avctx, ); > > if (did_split) { > diff --git a/libavcodec/internal.h b/libavcodec/internal.h > index e30d4aa73d..f3d10a14f1 100644 > --- a/libavcodec/internal.h > +++ b/libavcodec/internal.h > @@ -356,6 +356,10 @@ int ff_set_sar(AVCodecContext *avctx, AVRational sar); > int ff_side_data_update_matrix_encoding(AVFrame *frame, > enum AVMatrixEncoding > matrix_encoding); > > +#if FF_API_MERGE_SD_API > +int ff_packet_split_and_drop_side_data(AVPacket *pkt); > +#endif This function will not be used by anything after the ABI part of this functionality is dropped, so maybe make it depend on FF_API_MERGE_SD instead, here and in avpacket.c > + > /** > * Select the (possibly hardware accelerated) pixel format. > * This is a wrapper around AVCodecContext.get_format() and should be used > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] avcodec: Avoid splitting side data repeatedly
Fixes Timeout Fixes: 508/clusterfuzz-testcase-6245747678773248 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/targets/ffmpeg Signed-off-by: Michael Niedermayer--- libavcodec/avpacket.c | 24 libavcodec/decode.c | 9 +++-- libavcodec/internal.h | 4 3 files changed, 35 insertions(+), 2 deletions(-) diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c index d97400eaef..de12a1f1ce 100644 --- a/libavcodec/avpacket.c +++ b/libavcodec/avpacket.c @@ -463,6 +463,30 @@ int av_packet_split_side_data(AVPacket *pkt){ return 0; } +int ff_packet_split_and_drop_side_data(AVPacket *pkt){ +if (!pkt->side_data_elems && pkt->size >12 && AV_RB64(pkt->data + pkt->size - 8) == FF_MERGE_MARKER){ +int i; +unsigned int size; +uint8_t *p; + +p = pkt->data + pkt->size - 8 - 5; +for (i=1; ; i++){ +size = AV_RB32(p); +if (size>INT_MAX - 5 || p - pkt->data < size) +return 0; +if (p[4]&128) +break; +if (p - pkt->data < size + 5) +return 0; +p-= size+5; +} +pkt->size = p - pkt->data - size; +av_assert0(pkt->size >= 0); +return 1; +} +return 0; +} + #endif uint8_t *av_packet_pack_dictionary(AVDictionary *dict, int *size) diff --git a/libavcodec/decode.c b/libavcodec/decode.c index 708071fd07..43ba04550a 100644 --- a/libavcodec/decode.c +++ b/libavcodec/decode.c @@ -392,7 +392,9 @@ static int decode_simple_internal(AVCodecContext *avctx, AVFrame *frame) tmp = *pkt; #if FF_API_MERGE_SD FF_DISABLE_DEPRECATION_WARNINGS -did_split = av_packet_split_side_data(); +did_split = avci->compat_decode_partial_size ? +ff_packet_split_and_drop_side_data() : +av_packet_split_side_data(); if (did_split) { ret = extract_packet_props(avctx->internal, ); @@ -961,6 +963,7 @@ int avcodec_decode_subtitle2(AVCodecContext *avctx, AVSubtitle *sub, AVPacket *avpkt) { int i, ret = 0; +AVCodecInternal *avci = avctx->internal; if (!avpkt->data && avpkt->size) { av_log(avctx, AV_LOG_ERROR, "invalid packet: NULL data, size != 0\n"); @@ -981,7 +984,9 @@ int avcodec_decode_subtitle2(AVCodecContext *avctx, AVSubtitle *sub, AVPacket tmp = *avpkt; #if FF_API_MERGE_SD FF_DISABLE_DEPRECATION_WARNINGS -int did_split = av_packet_split_side_data(); +int did_split = avci->compat_decode_partial_size ? +ff_packet_split_and_drop_side_data() : +av_packet_split_side_data(); //apply_param_change(avctx, ); if (did_split) { diff --git a/libavcodec/internal.h b/libavcodec/internal.h index e30d4aa73d..f3d10a14f1 100644 --- a/libavcodec/internal.h +++ b/libavcodec/internal.h @@ -356,6 +356,10 @@ int ff_set_sar(AVCodecContext *avctx, AVRational sar); int ff_side_data_update_matrix_encoding(AVFrame *frame, enum AVMatrixEncoding matrix_encoding); +#if FF_API_MERGE_SD_API +int ff_packet_split_and_drop_side_data(AVPacket *pkt); +#endif + /** * Select the (possibly hardware accelerated) pixel format. * This is a wrapper around AVCodecContext.get_format() and should be used -- 2.11.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/2] avcodec/avpacket: Use av_copy_packet_side_data() in av_packet_copy_props()
On 5/4/2017 7:51 AM, Michael Niedermayer wrote: > On Wed, May 03, 2017 at 11:27:53PM -0300, James Almer wrote: >> On 5/3/2017 11:00 PM, Michael Niedermayer wrote: >>> On Wed, May 03, 2017 at 03:28:53PM +0200, Hendrik Leppkes wrote: On Wed, May 3, 2017 at 3:23 PM, Michael Niedermayerwrote: > On Wed, May 03, 2017 at 12:54:53PM +0200, Hendrik Leppkes wrote: >> On Wed, May 3, 2017 at 11:50 AM, Michael Niedermayer >> wrote: >>> On Wed, May 03, 2017 at 11:37:35AM +0200, wm4 wrote: On Wed, 3 May 2017 11:29:04 +0200 Michael Niedermayer wrote: > On Wed, May 03, 2017 at 05:29:07AM +0200, wm4 wrote: >> On Wed, 3 May 2017 05:21:50 +0200 >> Michael Niedermayer wrote: >> >>> Fixes timeout >>> Fixes: 1293/clusterfuzz-testcase-minimized-6054752074858496 >>> >>> Found-by: continuous fuzzing process >>> https://github.com/google/oss-fuzz/tree/master/targets/ffmpeg >>> Signed-off-by: Michael Niedermayer >>> --- >>> libavcodec/avpacket.c | 3 +++ >>> 1 file changed, 3 insertions(+) >>> >>> diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c >>> index 4bf830bb8a..ff7ee730a4 100644 >>> --- a/libavcodec/avpacket.c >>> +++ b/libavcodec/avpacket.c >>> @@ -557,6 +557,9 @@ FF_ENABLE_DEPRECATION_WARNINGS >>> dst->flags= src->flags; >>> dst->stream_index = src->stream_index; >>> >>> +if (!dst->side_data_elems); >>> +return av_copy_packet_side_data(dst, src); >>> + >>> for (i = 0; i < src->side_data_elems; i++) { >>> enum AVPacketSideDataType type = src->side_data[i].type; >>> int size = src->side_data[i].size; >> >> This doesn't look right... > > already fixed the ; locally > > > [...] I didn't see that, I was referring to the fact that you call av_copy_packet_side_data(), and only sometimes (at least by intention). That requires at least an explanation in the commit message. >>> >>> av_packet_copy_props() would add side data to the destination packet >>> it doesnt replace previously existing side data except in case of >>> error. >>> I dont know if that is intended but i didnt want to change it as that >>> would be unrelated to this patch >>> >> >> This behavior seems odd at best, so maybe we should just change that >> and make the behavior more logical, and fix your issue at the same >> time? > > That can be done and makes alot of sense after the patch. > > we need to fix this issue in our releases too > a simple bugfix and a seperate behavior change afterwards allows us > to simply backport the bugfix from master. > If anything your "bugfix" here is a performance improvement, and I don't think that warrants backporting either way. >>> >>> Before the patch the sample file takes >>> 51seconds to decode, after it, it takes 203 milliseconds. >>> The difference is caused by only 2 calls to the copy code >>> >>> blocking a machine for 51 seconds for something that decodes in 203ms >>> otherwise, is IMO worth backporting a fix for. >>> >>> We can add a bound to the number of side data elements if theres >>> consens about doing that and doing it in releases. >>> still, no code should call realloc() in a loop when it can do one >>> (re)allocation call at the top. >> >> First we need to figure out if these side data copy functions are meant >> to append the source packet's side data to the dest's, or if they should >> replace it. That is, we need to see if these functions are meant to >> assume the dest packet is empty or not. Because judging by every other >> field copied, i guess it's implied the dest packet is expected to be empty. >> It's worth nothing that av_stream_add_side_data() overwrites existing >> side data if one of the same type already exists, unlike >> av_packet_add_side_data(). > > There are 2 Issues > > A. Is that there is a bug in master and the releases that allows one >to craft a input which will get you stuck in a realloc loop a long >time > B. The API is not well documented about what should happen if the >destination packet isnt empty on a side data copy > > If we fix A and then fix B, then we can backport A without B. > > If we fix B and then fix A then A will likely depend on B and backporting > just the fix without the API change could be hard. > Is there a consensus about backporting a change to this API documentation > and implementation aka "B" ? > > A can be fixed by the proposed patch, by limiting the side data count > or by first
Re: [FFmpeg-devel] [PATCH] fixes the issue https://trac.ffmpeg.org/ticket/6338
On Thu, May 04, 2017 at 08:04:21 +0530, Vineet Goel wrote: > sorry about that. I am not proficient with git and development. There are a lot of useful hints here: https://ffmpeg.org/developer.html Actually pretty much a must-read before submitting patches. ;-) > And I am attaching the patch file as is generated by "git format-patch > origin/master" command Fine. If you look at how other contributions to git look (it's worth taking the time!), you will see that the first line of your commit message should probably read: lavf/http: fix cookies with proxy followed by an empty line, and then something like " Fixes #6338. " Sorry for nitpicking and not actually reviewing the change, ;-) Moritz ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] Multipage tiff
On Thu, May 04, 2017 at 13:31:16 +0200, Olivier Galibert wrote: > Is multipage tiff supported? Information on the web is contradictory. This is a question for ffmpeg-user, not for ffmpeg-devel. Have you tried? I did, and it seems that ffmpeg sees only the first image. I always considered multi-page tiff somewhat like multi-page PDF - which gives a document viewer such as evince a good reason to support them. ;-) Patch (and trac ticket) welcome. ;-) Moritz ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] tests/fate/fifo-muxer: update fifo-muxer dependencies
Fixes fate when configured with --disable-network. --- tests/fate/fifo-muxer.mak | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/fate/fifo-muxer.mak b/tests/fate/fifo-muxer.mak index ef8b3b3..9c13954 100644 --- a/tests/fate/fifo-muxer.mak +++ b/tests/fate/fifo-muxer.mak @@ -13,7 +13,7 @@ FATE_SAMPLES_FIFO_MUXER-$(call ALLYES, FIFO_MUXER, WAV_DEMUXER) += fate-fifo-mux fate-fifo-muxer-tst: libavformat/tests/fifo_muxer$(EXESUF) fate-fifo-muxer-tst: CMD = run libavformat/tests/fifo_muxer$(EXESUF) -FATE_FIFO_MUXER-$(CONFIG_FIFO_MUXER) += fate-fifo-muxer-tst +FATE_FIFO_MUXER-$(call ALLYES, FIFO_MUXER NETWORK) += fate-fifo-muxer-tst FATE_SAMPLES_FFMPEG += $(FATE_SAMPLES_FIFO_MUXER-yes) FATE_FFMPEG += $(FATE_FIFO_MUXER-yes) -- 2.7.4 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/2] avcodec/avpacket: Use av_copy_packet_side_data() in av_packet_copy_props()
On Wed, May 03, 2017 at 05:21:50AM +0200, Michael Niedermayer wrote: > Fixes timeout > Fixes: 1293/clusterfuzz-testcase-minimized-6054752074858496 This patch also fixes 1309/clusterfuzz-testcase-minimized-5754803370065920 [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Many that live deserve death. And some that die deserve life. Can you give it to them? Then do not be too eager to deal out death in judgement. For even the very wise cannot see all ends. -- Gandalf signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] Multipage tiff
Hi, Is multipage tiff supported? Information on the web is contradictory. Best, OG. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/2] avcodec/avpacket: Use av_copy_packet_side_data() in av_packet_copy_props()
On Wed, May 03, 2017 at 11:27:53PM -0300, James Almer wrote: > On 5/3/2017 11:00 PM, Michael Niedermayer wrote: > > On Wed, May 03, 2017 at 03:28:53PM +0200, Hendrik Leppkes wrote: > >> On Wed, May 3, 2017 at 3:23 PM, Michael Niedermayer > >>wrote: > >>> On Wed, May 03, 2017 at 12:54:53PM +0200, Hendrik Leppkes wrote: > On Wed, May 3, 2017 at 11:50 AM, Michael Niedermayer > wrote: > > On Wed, May 03, 2017 at 11:37:35AM +0200, wm4 wrote: > >> On Wed, 3 May 2017 11:29:04 +0200 > >> Michael Niedermayer wrote: > >> > >>> On Wed, May 03, 2017 at 05:29:07AM +0200, wm4 wrote: > On Wed, 3 May 2017 05:21:50 +0200 > Michael Niedermayer wrote: > > > Fixes timeout > > Fixes: 1293/clusterfuzz-testcase-minimized-6054752074858496 > > > > Found-by: continuous fuzzing process > > https://github.com/google/oss-fuzz/tree/master/targets/ffmpeg > > Signed-off-by: Michael Niedermayer > > --- > > libavcodec/avpacket.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c > > index 4bf830bb8a..ff7ee730a4 100644 > > --- a/libavcodec/avpacket.c > > +++ b/libavcodec/avpacket.c > > @@ -557,6 +557,9 @@ FF_ENABLE_DEPRECATION_WARNINGS > > dst->flags= src->flags; > > dst->stream_index = src->stream_index; > > > > +if (!dst->side_data_elems); > > +return av_copy_packet_side_data(dst, src); > > + > > for (i = 0; i < src->side_data_elems; i++) { > > enum AVPacketSideDataType type = src->side_data[i].type; > > int size = src->side_data[i].size; > > This doesn't look right... > >>> > >>> already fixed the ; locally > >>> > >>> > >>> [...] > >> > >> I didn't see that, I was referring to the fact that you call > >> av_copy_packet_side_data(), and only sometimes (at least by intention). > >> That requires at least an explanation in the commit message. > > > > av_packet_copy_props() would add side data to the destination packet > > it doesnt replace previously existing side data except in case of > > error. > > I dont know if that is intended but i didnt want to change it as that > > would be unrelated to this patch > > > > This behavior seems odd at best, so maybe we should just change that > and make the behavior more logical, and fix your issue at the same > time? > >>> > >>> That can be done and makes alot of sense after the patch. > >>> > >>> we need to fix this issue in our releases too > >>> a simple bugfix and a seperate behavior change afterwards allows us > >>> to simply backport the bugfix from master. > >>> > >> > >> If anything your "bugfix" here is a performance improvement, and I > >> don't think that warrants backporting either way. > > > > Before the patch the sample file takes > > 51seconds to decode, after it, it takes 203 milliseconds. > > The difference is caused by only 2 calls to the copy code > > > > blocking a machine for 51 seconds for something that decodes in 203ms > > otherwise, is IMO worth backporting a fix for. > > > > We can add a bound to the number of side data elements if theres > > consens about doing that and doing it in releases. > > still, no code should call realloc() in a loop when it can do one > > (re)allocation call at the top. > > First we need to figure out if these side data copy functions are meant > to append the source packet's side data to the dest's, or if they should > replace it. That is, we need to see if these functions are meant to > assume the dest packet is empty or not. Because judging by every other > field copied, i guess it's implied the dest packet is expected to be empty. > It's worth nothing that av_stream_add_side_data() overwrites existing > side data if one of the same type already exists, unlike > av_packet_add_side_data(). There are 2 Issues A. Is that there is a bug in master and the releases that allows one to craft a input which will get you stuck in a realloc loop a long time B. The API is not well documented about what should happen if the destination packet isnt empty on a side data copy If we fix A and then fix B, then we can backport A without B. If we fix B and then fix A then A will likely depend on B and backporting just the fix without the API change could be hard. Is there a consensus about backporting a change to this API documentation and implementation aka "B" ? A can be fixed by the proposed patch, by limiting the side data count or by first defining the API more clearly aka B and then fixing it along the lines of the clear