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

2017-05-04 Thread Aaron Levinson

On 5/2/2017 2:29 PM, wm4 wrote:

On Tue, 2 May 2017 14:17:33 -0700
Aaron Levinson  wrote:


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-04 Thread Steven Liu
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

2017-05-04 Thread 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.


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-04 Thread Steven Liu
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

2017-05-04 Thread Michael Niedermayer
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

2017-05-04 Thread Micah Galizia

On 2017-05-02 09:04 PM, wm4 wrote:

On Tue,  2 May 2017 20:47:06 -0400
Micah Galizia  wrote:


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()

2017-05-04 Thread James Almer
On 5/4/2017 6:59 PM, wm4 wrote:
> On Thu, 4 May 2017 12:51:14 +0200
> 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 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.

2017-05-04 Thread Aaron Levinson

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

2017-05-04 Thread Aaron Levinson
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-04 Thread Carl Eugen Hoyos
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'

2017-05-04 Thread Michael Niedermayer
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

2017-05-04 Thread Carl Eugen Hoyos
Hi!

It may be better to disable the warning.

Carl Eugen
From ab94367f502ab00f643a78608593eb9522e5c3be Mon Sep 17 00:00:00 2001
From: Carl Eugen Hoyos 
Date: 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.

2017-05-04 Thread Carl Eugen Hoyos
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 Hoyos 
Date: 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

2017-05-04 Thread wm4
On Fri, 5 May 2017 00:05:35 +0200
Nicolas George  wrote:

> 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

2017-05-04 Thread Nicolas George
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()

2017-05-04 Thread wm4
On Thu, 4 May 2017 12:51:14 +0200
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 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

2017-05-04 Thread wm4
On Thu, 4 May 2017 22:24:15 +0200
Nicolas George  wrote:

> 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 Thread Carl Eugen Hoyos
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-04 Thread Carl Eugen Hoyos
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

2017-05-04 Thread Timo Rothenpieler

--- 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

2017-05-04 Thread Nicolas George
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

2017-05-04 Thread Kyle Swanson
Hi,

On Tue, May 2, 2017 at 12:18 PM, wm4  wrote:
> 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

2017-05-04 Thread Philip Langdale

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

2017-05-04 Thread Michael Niedermayer
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-04 Thread Martin Vignali
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

2017-05-04 Thread Michael Niedermayer
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

2017-05-04 Thread Michael Niedermayer
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

2017-05-04 Thread Vineet Goel
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 Barsnick  wrote:

> 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

2017-05-04 Thread James Almer
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

2017-05-04 Thread Michael Niedermayer
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()

2017-05-04 Thread James Almer
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 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 

Re: [FFmpeg-devel] [PATCH] fixes the issue https://trac.ffmpeg.org/ticket/6338

2017-05-04 Thread Moritz Barsnick
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

2017-05-04 Thread Moritz Barsnick
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

2017-05-04 Thread Tobias Rapp
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()

2017-05-04 Thread Michael Niedermayer
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

2017-05-04 Thread Olivier Galibert
  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()

2017-05-04 Thread Michael Niedermayer
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