Re: [FFmpeg-devel] [PATCH 3/4] avformat/mov: add support for reading Mastering Display Metadata Box

2017-05-26 Thread James Almer
On 5/26/2017 8:05 PM, Michael Niedermayer wrote:
> On Wed, May 17, 2017 at 09:49:40PM -0300, James Almer wrote:
>> As defined in "VP Codec ISO Media File Format Binding v1.0"
>> https://github.com/webmproject/vp9-dash/blob/master/VPCodecISOMediaFileFormatBinding.md
>>
>> Partially based on Matroska decoder code.
>>
>> Signed-off-by: James Almer 
>> ---
>>  libavformat/isom.h |  2 ++
>>  libavformat/mov.c  | 65 
>> ++
>>  2 files changed, 67 insertions(+)
>>
>> diff --git a/libavformat/isom.h b/libavformat/isom.h
>> index d9956cf63a..426f732247 100644
>> --- a/libavformat/isom.h
>> +++ b/libavformat/isom.h
>> @@ -27,6 +27,7 @@
>>  #include 
>>  #include 
>>  
>> +#include "libavutil/mastering_display_metadata.h"
>>  #include "libavutil/spherical.h"
>>  #include "libavutil/stereo3d.h"
>>  
>> @@ -194,6 +195,7 @@ typedef struct MOVStreamContext {
>>  AVStereo3D *stereo3d;
>>  AVSphericalMapping *spherical;
>>  size_t spherical_size;
>> +AVMasteringDisplayMetadata *mastering;
>>  
>>  uint32_t format;
>>  
>> diff --git a/libavformat/mov.c b/libavformat/mov.c
>> index afef53b79a..0b5fd849f3 100644
>> --- a/libavformat/mov.c
>> +++ b/libavformat/mov.c
>> @@ -4612,6 +4612,60 @@ static int mov_read_tmcd(MOVContext *c, AVIOContext 
>> *pb, MOVAtom atom)
>>  return 0;
>>  }
>>  
>> +static int mov_read_smdm(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>> +{
>> +MOVStreamContext *sc;
>> +const int chroma_den = 5;
>> +const int luma_den = 1;
>> +int version;
>> +
>> +if (c->fc->nb_streams < 1)
>> +return AVERROR_INVALIDDATA;
>> +
>> +sc = c->fc->streams[c->fc->nb_streams - 1]->priv_data;
>> +
>> +if (atom.size < 5) {
>> +av_log(c->fc, AV_LOG_ERROR, "Empty Mastering Display Metadata 
>> box\n");
>> +return AVERROR_INVALIDDATA;
>> +}
>> +
>> +version = avio_r8(pb);
>> +if (version) {
>> +av_log(c->fc, AV_LOG_WARNING, "Unsupported Mastering Display 
>> Metadata box version %d\n", version);
>> +return 0;
>> +}
>> +avio_skip(pb, 3); /* flags */
>> +
>> +sc->mastering = av_mastering_display_metadata_alloc();
>> +if (!sc->mastering)
>> +return AVERROR(ENOMEM);
>> +
> 
>> +sc->mastering->display_primaries[0][0] =
>> +av_make_q(lrint(((double)avio_rb16(pb) / (1 << 16)) * chroma_den), 
>> chroma_den);
> 
> this is not optimal, precission wise
> av_d2q() should produce closer rationals
> alternativly av_reduce() can be used directly
> 
> but iam not sure why a fixed chroma_den and luma_den is fixed
> maybe iam missing something

Does

for (i = 0; i < 3; i++)
  for (j = 0; j < 2; j++)
av_reduce(>mastering->display_primaries[i][j].num,
  >mastering->display_primaries[i][j].den,
  lrint(((double)avio_rb16(pb) / (1 << 16)) * chroma_den),
  chroma_den, chroma_den);
for (i = 0; i < 2; i++)
  av_reduce(>mastering->white_point[i].num,
>mastering->white_point[i].den,
lrint(((double)avio_rb16(pb) / (1 << 16)) * chroma_den),
chroma_den, chroma_den);
av_reduce(>mastering->max_luminance.num,
  >mastering->max_luminance.den,
  lrint(((double)avio_rb32(pb) / (1 << 8)) * luma_den),
  luma_den, INT_MAX);
av_reduce(>mastering->min_luminance.num,
  >mastering->min_luminance.den,
  lrint(((double)avio_rb32(pb) / (1 << 14)) * luma_den),
  luma_den, INT_MAX);

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


[FFmpeg-devel] [PATCH]libavformat/http: fix http error eof

2017-05-26 Thread raymond zheng
Hi:
 I find an issue about http. I don't use chunked, so s->chunksize will
be set as UINT64_MAX when http open, but because of "if (s->chunksize > 0)
s->chunksize -= len;" then chunksize will not be UINT64_MAX.

If ffurl_read return to 0, s->off < target_end, http_buf_read will
return to 0, then this will lead to eof, so this is incorrect, and
http_buf_read should return to AVERROR(EIO).

the bug reproduce step:

1. An exception occurred in the  CDN edge server, that will lead to close
the http connection.

2. Because http is disconnected, so ffurl_read will return 0

3. Avformat will consider I/O is eof

4. Right now http is actually disconnect abnormally, it should return to
ERROR, rather than return to EOF.


this problem was caused by commit: 2a05c8f813de6f2278827734bf8102291e7484aa


0001-libavformat-http-return-EIO-when-ffurl_read-return-0.patch
Description: Binary data
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avcodec/fft_template: Fix multiple runtime error: signed integer overflow: -1943918714 - 1935113003 cannot be represented in type 'int'

2017-05-26 Thread Michael Niedermayer
On Fri, May 26, 2017 at 07:06:44PM -0400, Ronald S. Bultje wrote:
> Hi,
> 
> On Fri, May 26, 2017 at 6:34 PM, Michael Niedermayer  > wrote:
> 
> > On Fri, May 26, 2017 at 06:07:35PM -0400, Ronald S. Bultje wrote:
> > > Hi,
> > >
> > > On Fri, May 26, 2017 at 5:50 PM, Michael Niedermayer
> >  > > > wrote:
> > >
> > > > On Fri, May 26, 2017 at 11:18:12PM +0200, Hendrik Leppkes wrote:
> > > > > On Fri, May 26, 2017 at 11:11 PM, Michael Niedermayer
> > > > >  wrote:
> > > > > > On Fri, May 26, 2017 at 03:20:14PM +0100, Rostislav Pehlivanov
> > wrote:
> > > > > >> On 26 May 2017 at 12:21, wm4  wrote:
> > > > > >>
> > > > > >> > On Thu, 25 May 2017 16:10:49 +0200
> > > > > >> > Michael Niedermayer  wrote:
> > > > > >> >
> > > > > >> > > Fixes: 1735/clusterfuzz-testcase-minimized-5350472347025408
> > > > > >> > >
> > > > > >> > > Found-by: continuous fuzzing process
> > > > https://github.com/google/oss-
> > > > > >> > fuzz/tree/master/projects/ffmpeg
> > > > > >> > > Signed-off-by: Michael Niedermayer 
> > > > > >> > > ---
> > > > > >> > >  libavcodec/fft_template.c | 50 +++---
> > > > > >> > -
> > > > > >> > >  1 file changed, 25 insertions(+), 25 deletions(-)
> > > > > >> > >
> > > > > >> > > diff --git a/libavcodec/fft_template.c
> > b/libavcodec/fft_template.c
> > > > > >> > > index 480557f49f..e3a37e5d69 100644
> > > > > >> > > --- a/libavcodec/fft_template.c
> > > > > >> > > +++ b/libavcodec/fft_template.c
> > > > > >> > > @@ -249,7 +249,7 @@ static void fft_calc_c(FFTContext *s,
> > > > FFTComplex *z)
> > > > > >> > {
> > > > > >> > >
> > > > > >> > >  int nbits, i, n, num_transforms, offset, step;
> > > > > >> > >  int n4, n2, n34;
> > > > > >> > > -FFTSample tmp1, tmp2, tmp3, tmp4, tmp5, tmp6, tmp7, tmp8;
> > > > > >> > > +SUINT tmp1, tmp2, tmp3, tmp4, tmp5, tmp6, tmp7, tmp8;
> > > > > >> >
> > > > > >> > I want this SUINT thing gone, not have more of it.
> > > > > >> > ___
> > > > > >> > ffmpeg-devel mailing list
> > > > > >> > ffmpeg-devel@ffmpeg.org
> > > > > >> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > > > > >> >
> > > > > >>
> > > > > >> I agree, especially here.
> > > > > >
> > > > > >> Overflows should be left to happen in transforms if the input is
> > > > corrupt.
> > > > > >
> > > > > > signed int overflow is not allowed in C, if that is what you meant.
> > > > > >
> > > > > >
> > > > >
> > > > > Its "undefined", which means what the result will be is not defined -
> > > > > which in such a DSP function is irrelevant, if all it causes is
> > > > > corrupt output ... from corrupt input.
> > > >
> > > > no, this is not correct.
> > > > undefined behavior does not mean the effect will be limited to
> > > > the output.
> > > > It could cause the process to hard lockup, trigger an exception or
> > > > set a flag causing errors in later unrelated code.
> > >
> > >
> >
> > > We've discussed this before, if you believe this to be exploitable, why
> > > allow it in our repository at all? I know of no other project that even
> > > remotely allows such ridiculous things. Please limit exploitable code to
> > > your personal tree, ffmpeg is not a rootkit.
> >
> > please calm down, you make all kinds of statments and implications in
> > the text above which are not true.
> >
> > This specific code in git triggers undefined behavior, the patch fixes
> > this undefined behavior.
> > If that is exploitable (which i neither claim it is nor that it isnt)
> > its a issue that exists before the patch but not afterwards.
> 
> 

> *unsigned* removes the exploit. SUINT keeps it, and is therefore part of a
> rootkit.

SUINT is defined to unsigned, if unsigned removes the issue
so does SUINT.

If an attacker can convince his victim to define or redefine a symbol
during build then SUINT is neither needed nor anything an attacker
would choose.

redefining SUINT makes sense to developers though to test for
arithmetic overflow. It also keeps a clear seperation between

unsigned values in unsigned types and
signed values in unsigned types  aka "SUINT"


>
> Or there is no exploit, in which case SUINT is not necessary.
>
> ??
>
> Ronald
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

There will always be a question for which you do not know the correct answer.


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-26 Thread Micah Galizia
On Sat, May 20, 2017 at 9:36 PM, Micah Galizia  wrote:
> On 2017-05-17 05:23 AM, wm4 wrote:
>>
>> On Sat, 6 May 2017 14:28:10 -0400
>> Micah Galizia  wrote:
>>
>>> On 2017-05-05 09:28 PM, wm4 wrote:

 On Fri,  5 May 2017 20:55:05 -0400
 Micah Galizia  wrote:

>
> Signed-off-by: Micah Galizia 
> ---
>libavformat/hls.c | 12 ++--
>1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/libavformat/hls.c b/libavformat/hls.c
> index bac53a4350..bda9abecfa 100644
> --- a/libavformat/hls.c
> +++ b/libavformat/hls.c
> @@ -630,8 +630,16 @@ 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;
> +
> +if (s->flags ^ AVFMT_FLAG_CUSTOM_IO)
> +av_opt_get(*pb, "cookies", AV_OPT_SEARCH_CHILDREN,
> (uint8_t**)_cookies);

 Did you mean & instead of ^?
>>>
>>> No, the original code was structured to set *u to null (and thus did not
>>> copy cookies) iff AVFMT_FLAG_CUSTOM_IO was set in the flags.  So using ^
>>> is logically equivalent -- cookies are copied only if
>>> AVFMT_FLAG_CUSTOM_IO is not set.
>>>
 Did you find out yet what difference AVFMT_FLAG_CUSTOM_IO is supposed
 to make in the existing code?
>>>
>>> Yes, I wrote back about it a day or two ago... here is the reference to
>>> its original inclusion:
>>> https://lists.ffmpeg.org/pipermail/ffmpeg-trac/2013-February/013020.html.
>>> When the code was originally implemented we were using s->pb->opaque,
>>> which in FFMPEG we could assume was a URLContext with options (one of
>>> them possibly being "cookies").
>>>
>>> Based on the email above, that wasn't true for other apps like mplayer,
>>> and could cause crashes trying to treat opaque as a URLContext. So that
>>> is the purpose of checking the AVFMT_FLAG_CUSTOM_IO flag (way back in
>>> 2013).
>>>
>>> Now though, we don't seem to touch opaque at all. The options are stored
>>> in AVIOContext's av_class, which does have options.  Based on this I
>>> think both patches are safe: this version has less change, but the
>>> original gets rid of the check against AVFMT_FLAG_CUSTOM_IO that I don't
>>> think we need anymore.
>>>
>>> I hope that clears things up.
>>
>> Sorry, I missed that. I guess this code is an artifact of some severely
>> unclean hook up of the AVOPtion API to AVIOContext, that breaks with
>> custom I/O. I guess your patch is fine then.
>>
>> I just wonder how an API user is supposed to pass along cookies then.
>> My code passes an AVDictionary via a "cookies" entry when opening the
>> HLS demuxer with custom I/O, so I was wondering whether the patch
>> changes behavior here.
>
>
> I wouldn't have expected anyone to pass the cookies to the HLS demuxer
> directly -- there is an http protocol AVOption that should pass it along to
> the demuxer. But I guess thats the whole point of the custom IO, right? It'd
> replace the http demuxer?
>
> Even so, I don't think this is a good reason to hold up the the patch - it
> corrects the problem for apps that use the http protocol and maintains the
> existing behavior -- cookies are not (and were not) copied when the custom
> flag is set because u was set to null. Am I wrong in that interpretation of
> the existing behavior?

Hi,

What else do I need to do to get this fix checked in?

Thanks
-- 
"The mark of an immature man is that he wants to die nobly for a
cause, while the mark of the mature man is that he wants to live
humbly for one."   --W. Stekel
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 3/4] avformat/mov: add support for reading Mastering Display Metadata Box

2017-05-26 Thread James Almer
On 5/26/2017 8:55 PM, James Almer wrote:
> On 5/26/2017 8:05 PM, Michael Niedermayer wrote:
>> On Wed, May 17, 2017 at 09:49:40PM -0300, James Almer wrote:
>>> As defined in "VP Codec ISO Media File Format Binding v1.0"
>>> https://github.com/webmproject/vp9-dash/blob/master/VPCodecISOMediaFileFormatBinding.md
>>>
>>> Partially based on Matroska decoder code.
>>>
>>> Signed-off-by: James Almer 
>>> ---
>>>  libavformat/isom.h |  2 ++
>>>  libavformat/mov.c  | 65 
>>> ++
>>>  2 files changed, 67 insertions(+)
>>>
>>> diff --git a/libavformat/isom.h b/libavformat/isom.h
>>> index d9956cf63a..426f732247 100644
>>> --- a/libavformat/isom.h
>>> +++ b/libavformat/isom.h
>>> @@ -27,6 +27,7 @@
>>>  #include 
>>>  #include 
>>>  
>>> +#include "libavutil/mastering_display_metadata.h"
>>>  #include "libavutil/spherical.h"
>>>  #include "libavutil/stereo3d.h"
>>>  
>>> @@ -194,6 +195,7 @@ typedef struct MOVStreamContext {
>>>  AVStereo3D *stereo3d;
>>>  AVSphericalMapping *spherical;
>>>  size_t spherical_size;
>>> +AVMasteringDisplayMetadata *mastering;
>>>  
>>>  uint32_t format;
>>>  
>>> diff --git a/libavformat/mov.c b/libavformat/mov.c
>>> index afef53b79a..0b5fd849f3 100644
>>> --- a/libavformat/mov.c
>>> +++ b/libavformat/mov.c
>>> @@ -4612,6 +4612,60 @@ static int mov_read_tmcd(MOVContext *c, AVIOContext 
>>> *pb, MOVAtom atom)
>>>  return 0;
>>>  }
>>>  
>>> +static int mov_read_smdm(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>>> +{
>>> +MOVStreamContext *sc;
>>> +const int chroma_den = 5;
>>> +const int luma_den = 1;
>>> +int version;
>>> +
>>> +if (c->fc->nb_streams < 1)
>>> +return AVERROR_INVALIDDATA;
>>> +
>>> +sc = c->fc->streams[c->fc->nb_streams - 1]->priv_data;
>>> +
>>> +if (atom.size < 5) {
>>> +av_log(c->fc, AV_LOG_ERROR, "Empty Mastering Display Metadata 
>>> box\n");
>>> +return AVERROR_INVALIDDATA;
>>> +}
>>> +
>>> +version = avio_r8(pb);
>>> +if (version) {
>>> +av_log(c->fc, AV_LOG_WARNING, "Unsupported Mastering Display 
>>> Metadata box version %d\n", version);
>>> +return 0;
>>> +}
>>> +avio_skip(pb, 3); /* flags */
>>> +
>>> +sc->mastering = av_mastering_display_metadata_alloc();
>>> +if (!sc->mastering)
>>> +return AVERROR(ENOMEM);
>>> +
>>
>>> +sc->mastering->display_primaries[0][0] =
>>> +av_make_q(lrint(((double)avio_rb16(pb) / (1 << 16)) * chroma_den), 
>>> chroma_den);
>>
>> this is not optimal, precission wise
>> av_d2q() should produce closer rationals
>> alternativly av_reduce() can be used directly
>>
>> but iam not sure why a fixed chroma_den and luma_den is fixed
>> maybe iam missing something
> 
> I took the constants from hevcdec.c and matroskadec.c and basically
> copied how they handled these values.
> At least based on what lavf's dump.c and mkvtoolnix's mkvinfo reported,
> this implementation is the most precise i tried. Using av_d2q() directly
> was worse.

By dump.c i mean i remuxed an mkv file with mastering metadata using the
muxer implementation of this same patchset then compared the reported
values.
By mkvinfo i mean mkv -> mov (using my muxer implementation) -> mkv
roundtrip then compared the double precision values stored in the
original mkv and the new mkv.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 1/4] avformat/movenc: add support for writing Mastering Display Metadata Box

2017-05-26 Thread James Almer
On 5/26/2017 7:48 PM, Michael Niedermayer wrote:
> On Wed, May 17, 2017 at 09:49:38PM -0300, James Almer wrote:
>> As defined in "VP Codec ISO Media File Format Binding v1.0"
>> https://github.com/webmproject/vp9-dash/blob/master/VPCodecISOMediaFileFormatBinding.md
>>
>> Signed-off-by: James Almer 
>> ---
>>  libavformat/movenc.c | 37 +
>>  1 file changed, 37 insertions(+)
>>
>> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
>> index a6c0662cd0..cd436df7a4 100644
>> --- a/libavformat/movenc.c
>> +++ b/libavformat/movenc.c
>> @@ -42,6 +42,7 @@
>>  #include "libavutil/avstring.h"
>>  #include "libavutil/intfloat.h"
>>  #include "libavutil/mathematics.h"
>> +#include "libavutil/mastering_display_metadata.h"
>>  #include "libavutil/libm.h"
>>  #include "libavutil/opt.h"
>>  #include "libavutil/dict.h"
>> @@ -1118,6 +1119,41 @@ static int mov_write_vpcc_tag(AVFormatContext *s, 
>> AVIOContext *pb, MOVTrack *tra
>>  return update_size(pb, pos);
>>  }
>>  
>> +static int mov_write_smdm_tag(AVFormatContext *s, AVIOContext *pb, MOVTrack 
>> *track)
>> +{
>> +int size = 0;
>> +int64_t pos;
>> +const AVMasteringDisplayMetadata *mastering =
>> +(const AVMasteringDisplayMetadata *) 
>> av_stream_get_side_data(track->st,
>> + 
>> AV_PKT_DATA_MASTERING_DISPLAY_METADATA,
>> + );
>> +if (!size)
>> +return 0;
>> +
>> +if (!mastering->has_primaries || !mastering->has_luminance) {
>> +av_log(s, AV_LOG_WARNING, "Incomplete Mastering Display metadata. 
>> Both luminance "
>> +  "and display primaries are needed\n");
>> +return 0;
>> +}
>> +
>> +pos = avio_tell(pb);
>> +
>> +avio_wb32(pb, 0);
>> +ffio_wfourcc(pb, "SmDm");
>> +avio_wb32(pb, 0); /* version & flags */
> 
>> +avio_wb16(pb, lrint(av_q2d(mastering->display_primaries[0][0]) * (1 << 
>> 16)));
>> +avio_wb16(pb, lrint(av_q2d(mastering->display_primaries[0][1]) * (1 << 
>> 16)));
>> +avio_wb16(pb, lrint(av_q2d(mastering->display_primaries[1][0]) * (1 << 
>> 16)));
>> +avio_wb16(pb, lrint(av_q2d(mastering->display_primaries[1][1]) * (1 << 
>> 16)));
>> +avio_wb16(pb, lrint(av_q2d(mastering->display_primaries[2][0]) * (1 << 
>> 16)));
>> +avio_wb16(pb, lrint(av_q2d(mastering->display_primaries[2][1]) * (1 << 
>> 16)));
>> +avio_wb16(pb, lrint(av_q2d(mastering->white_point[0])  * (1 << 
>> 16)));
>> +avio_wb16(pb, lrint(av_q2d(mastering->white_point[1])  * (1 << 
>> 16)));
>> +avio_wb32(pb, lrint(av_q2d(mastering->max_luminance)   * (1 <<  
>> 8)));
>> +avio_wb32(pb, lrint(av_q2d(mastering->min_luminance)   * (1 << 
>> 14)));
> 
> These may need range checks.
> Our API doesnt seem to define limits, so they might fall outside the
> 16/32 bit range used to store them, unless i miss something

I guess. How do you suggest to check this? Range checking the
AVRationals with av_cmp_q()?

This could have been avoided if mastering metadata fields were
implemented as integer values, like content light does.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 2/4] avformat/movenc: add support for writing Content Light Level Box

2017-05-26 Thread James Almer
On 5/26/2017 7:56 PM, Michael Niedermayer wrote:
> On Wed, May 17, 2017 at 09:49:39PM -0300, James Almer wrote:
>> As defined in "VP Codec ISO Media File Format Binding v1.0"
>> https://github.com/webmproject/vp9-dash/blob/master/VPCodecISOMediaFileFormatBinding.md
>>
>> Signed-off-by: James Almer 
>> ---
>>  libavformat/movenc.c | 22 ++
>>  1 file changed, 22 insertions(+)
>>
>> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
>> index cd436df7a4..eab7bbc8a7 100644
>> --- a/libavformat/movenc.c
>> +++ b/libavformat/movenc.c
>> @@ -1154,6 +1154,27 @@ static int mov_write_smdm_tag(AVFormatContext *s, 
>> AVIOContext *pb, MOVTrack *tra
>>  return update_size(pb, pos);
>>  }
>>  
>> +static int mov_write_coll_tag(AVIOContext *pb, MOVTrack *track)
>> +{
>> +int size = 0;
>> +int64_t pos;
>> +const AVContentLightMetadata *coll =
>> +(const AVContentLightMetadata *) av_stream_get_side_data(track->st,
>> + 
>> AV_PKT_DATA_CONTENT_LIGHT_LEVEL,
>> + );
>> +if (!size)
>> +return 0;
> 
> Is there anything that checks the validity of size for a
> AVContentLightMetadata ?
> (that is, is this checked anywhere from side data creation to
>  its use here)

If created by av_content_light_metadata_alloc() and the returned size
passed to av_stream_add_side_data(), then it's guaranteed to be correct.
Any other case, guess it depends on if whatever created the side data
ignored the "size not part of the ABI" warning and sizeof'd the struct
or not.

> 
> If not then this can be too small and crash

That's apparently a risk on every current usage of side data (mastering,
stereo3d, spherical, content light, etc) in demuxing code, except those
doing sizeof() against what the doxy says.

I suggested once adding functions that return the size of the struct,
much like the alloc() ones do. Some thought it was a good solution,
others thought it was added complexity to a simple API, and others
wanted to rewrite the entire side data handling code API from scratch.
In the end, none of them were done.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 3/4] avformat/mov: add support for reading Mastering Display Metadata Box

2017-05-26 Thread James Almer
On 5/26/2017 8:05 PM, Michael Niedermayer wrote:
> On Wed, May 17, 2017 at 09:49:40PM -0300, James Almer wrote:
>> As defined in "VP Codec ISO Media File Format Binding v1.0"
>> https://github.com/webmproject/vp9-dash/blob/master/VPCodecISOMediaFileFormatBinding.md
>>
>> Partially based on Matroska decoder code.
>>
>> Signed-off-by: James Almer 
>> ---
>>  libavformat/isom.h |  2 ++
>>  libavformat/mov.c  | 65 
>> ++
>>  2 files changed, 67 insertions(+)
>>
>> diff --git a/libavformat/isom.h b/libavformat/isom.h
>> index d9956cf63a..426f732247 100644
>> --- a/libavformat/isom.h
>> +++ b/libavformat/isom.h
>> @@ -27,6 +27,7 @@
>>  #include 
>>  #include 
>>  
>> +#include "libavutil/mastering_display_metadata.h"
>>  #include "libavutil/spherical.h"
>>  #include "libavutil/stereo3d.h"
>>  
>> @@ -194,6 +195,7 @@ typedef struct MOVStreamContext {
>>  AVStereo3D *stereo3d;
>>  AVSphericalMapping *spherical;
>>  size_t spherical_size;
>> +AVMasteringDisplayMetadata *mastering;
>>  
>>  uint32_t format;
>>  
>> diff --git a/libavformat/mov.c b/libavformat/mov.c
>> index afef53b79a..0b5fd849f3 100644
>> --- a/libavformat/mov.c
>> +++ b/libavformat/mov.c
>> @@ -4612,6 +4612,60 @@ static int mov_read_tmcd(MOVContext *c, AVIOContext 
>> *pb, MOVAtom atom)
>>  return 0;
>>  }
>>  
>> +static int mov_read_smdm(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>> +{
>> +MOVStreamContext *sc;
>> +const int chroma_den = 5;
>> +const int luma_den = 1;
>> +int version;
>> +
>> +if (c->fc->nb_streams < 1)
>> +return AVERROR_INVALIDDATA;
>> +
>> +sc = c->fc->streams[c->fc->nb_streams - 1]->priv_data;
>> +
>> +if (atom.size < 5) {
>> +av_log(c->fc, AV_LOG_ERROR, "Empty Mastering Display Metadata 
>> box\n");
>> +return AVERROR_INVALIDDATA;
>> +}
>> +
>> +version = avio_r8(pb);
>> +if (version) {
>> +av_log(c->fc, AV_LOG_WARNING, "Unsupported Mastering Display 
>> Metadata box version %d\n", version);
>> +return 0;
>> +}
>> +avio_skip(pb, 3); /* flags */
>> +
>> +sc->mastering = av_mastering_display_metadata_alloc();
>> +if (!sc->mastering)
>> +return AVERROR(ENOMEM);
>> +
> 
>> +sc->mastering->display_primaries[0][0] =
>> +av_make_q(lrint(((double)avio_rb16(pb) / (1 << 16)) * chroma_den), 
>> chroma_den);
> 
> this is not optimal, precission wise
> av_d2q() should produce closer rationals
> alternativly av_reduce() can be used directly
> 
> but iam not sure why a fixed chroma_den and luma_den is fixed
> maybe iam missing something

I took the constants from hevcdec.c and matroskadec.c and basically
copied how they handled these values.
At least based on what lavf's dump.c and mkvtoolnix's mkvinfo reported,
this implementation is the most precise i tried. Using av_d2q() directly
was worse.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 4/4] avformat/mov: add support for reading Content Light Level Box

2017-05-26 Thread Michael Niedermayer
On Wed, May 17, 2017 at 09:49:41PM -0300, James Almer wrote:
> As defined in "VP Codec ISO Media File Format Binding v1.0"
> https://github.com/webmproject/vp9-dash/blob/master/VPCodecISOMediaFileFormatBinding.md
> 
> Signed-off-by: James Almer 
> ---
>  libavformat/isom.h |  2 ++
>  libavformat/mov.c  | 43 +++
>  2 files changed, 45 insertions(+)

should be ok

thx

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

If you fake or manipulate statistics in a paper in physics you will never
get a job again.
If you fake or manipulate statistics in a paper in medicin you will get
a job for life at the pharma industry.


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


Re: [FFmpeg-devel] [PATCH] avcodec/fft_template: Fix multiple runtime error: signed integer overflow: -1943918714 - 1935113003 cannot be represented in type 'int'

2017-05-26 Thread Ronald S. Bultje
Hi,

On Fri, May 26, 2017 at 6:34 PM, Michael Niedermayer  wrote:

> On Fri, May 26, 2017 at 06:07:35PM -0400, Ronald S. Bultje wrote:
> > Hi,
> >
> > On Fri, May 26, 2017 at 5:50 PM, Michael Niedermayer
>  > > wrote:
> >
> > > On Fri, May 26, 2017 at 11:18:12PM +0200, Hendrik Leppkes wrote:
> > > > On Fri, May 26, 2017 at 11:11 PM, Michael Niedermayer
> > > >  wrote:
> > > > > On Fri, May 26, 2017 at 03:20:14PM +0100, Rostislav Pehlivanov
> wrote:
> > > > >> On 26 May 2017 at 12:21, wm4  wrote:
> > > > >>
> > > > >> > On Thu, 25 May 2017 16:10:49 +0200
> > > > >> > Michael Niedermayer  wrote:
> > > > >> >
> > > > >> > > Fixes: 1735/clusterfuzz-testcase-minimized-5350472347025408
> > > > >> > >
> > > > >> > > Found-by: continuous fuzzing process
> > > https://github.com/google/oss-
> > > > >> > fuzz/tree/master/projects/ffmpeg
> > > > >> > > Signed-off-by: Michael Niedermayer 
> > > > >> > > ---
> > > > >> > >  libavcodec/fft_template.c | 50 +++---
> > > > >> > -
> > > > >> > >  1 file changed, 25 insertions(+), 25 deletions(-)
> > > > >> > >
> > > > >> > > diff --git a/libavcodec/fft_template.c
> b/libavcodec/fft_template.c
> > > > >> > > index 480557f49f..e3a37e5d69 100644
> > > > >> > > --- a/libavcodec/fft_template.c
> > > > >> > > +++ b/libavcodec/fft_template.c
> > > > >> > > @@ -249,7 +249,7 @@ static void fft_calc_c(FFTContext *s,
> > > FFTComplex *z)
> > > > >> > {
> > > > >> > >
> > > > >> > >  int nbits, i, n, num_transforms, offset, step;
> > > > >> > >  int n4, n2, n34;
> > > > >> > > -FFTSample tmp1, tmp2, tmp3, tmp4, tmp5, tmp6, tmp7, tmp8;
> > > > >> > > +SUINT tmp1, tmp2, tmp3, tmp4, tmp5, tmp6, tmp7, tmp8;
> > > > >> >
> > > > >> > I want this SUINT thing gone, not have more of it.
> > > > >> > ___
> > > > >> > ffmpeg-devel mailing list
> > > > >> > ffmpeg-devel@ffmpeg.org
> > > > >> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > > > >> >
> > > > >>
> > > > >> I agree, especially here.
> > > > >
> > > > >> Overflows should be left to happen in transforms if the input is
> > > corrupt.
> > > > >
> > > > > signed int overflow is not allowed in C, if that is what you meant.
> > > > >
> > > > >
> > > >
> > > > Its "undefined", which means what the result will be is not defined -
> > > > which in such a DSP function is irrelevant, if all it causes is
> > > > corrupt output ... from corrupt input.
> > >
> > > no, this is not correct.
> > > undefined behavior does not mean the effect will be limited to
> > > the output.
> > > It could cause the process to hard lockup, trigger an exception or
> > > set a flag causing errors in later unrelated code.
> >
> >
>
> > We've discussed this before, if you believe this to be exploitable, why
> > allow it in our repository at all? I know of no other project that even
> > remotely allows such ridiculous things. Please limit exploitable code to
> > your personal tree, ffmpeg is not a rootkit.
>
> please calm down, you make all kinds of statments and implications in
> the text above which are not true.
>
> This specific code in git triggers undefined behavior, the patch fixes
> this undefined behavior.
> If that is exploitable (which i neither claim it is nor that it isnt)
> its a issue that exists before the patch but not afterwards.


*unsigned* removes the exploit. SUINT keeps it, and is therefore part of a
rootkit.

Or there is no exploit, in which case SUINT is not necessary.

??

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


Re: [FFmpeg-devel] [PATCH 3/4] avformat/mov: add support for reading Mastering Display Metadata Box

2017-05-26 Thread Michael Niedermayer
On Wed, May 17, 2017 at 09:49:40PM -0300, James Almer wrote:
> As defined in "VP Codec ISO Media File Format Binding v1.0"
> https://github.com/webmproject/vp9-dash/blob/master/VPCodecISOMediaFileFormatBinding.md
> 
> Partially based on Matroska decoder code.
> 
> Signed-off-by: James Almer 
> ---
>  libavformat/isom.h |  2 ++
>  libavformat/mov.c  | 65 
> ++
>  2 files changed, 67 insertions(+)
> 
> diff --git a/libavformat/isom.h b/libavformat/isom.h
> index d9956cf63a..426f732247 100644
> --- a/libavformat/isom.h
> +++ b/libavformat/isom.h
> @@ -27,6 +27,7 @@
>  #include 
>  #include 
>  
> +#include "libavutil/mastering_display_metadata.h"
>  #include "libavutil/spherical.h"
>  #include "libavutil/stereo3d.h"
>  
> @@ -194,6 +195,7 @@ typedef struct MOVStreamContext {
>  AVStereo3D *stereo3d;
>  AVSphericalMapping *spherical;
>  size_t spherical_size;
> +AVMasteringDisplayMetadata *mastering;
>  
>  uint32_t format;
>  
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index afef53b79a..0b5fd849f3 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -4612,6 +4612,60 @@ static int mov_read_tmcd(MOVContext *c, AVIOContext 
> *pb, MOVAtom atom)
>  return 0;
>  }
>  
> +static int mov_read_smdm(MOVContext *c, AVIOContext *pb, MOVAtom atom)
> +{
> +MOVStreamContext *sc;
> +const int chroma_den = 5;
> +const int luma_den = 1;
> +int version;
> +
> +if (c->fc->nb_streams < 1)
> +return AVERROR_INVALIDDATA;
> +
> +sc = c->fc->streams[c->fc->nb_streams - 1]->priv_data;
> +
> +if (atom.size < 5) {
> +av_log(c->fc, AV_LOG_ERROR, "Empty Mastering Display Metadata 
> box\n");
> +return AVERROR_INVALIDDATA;
> +}
> +
> +version = avio_r8(pb);
> +if (version) {
> +av_log(c->fc, AV_LOG_WARNING, "Unsupported Mastering Display 
> Metadata box version %d\n", version);
> +return 0;
> +}
> +avio_skip(pb, 3); /* flags */
> +
> +sc->mastering = av_mastering_display_metadata_alloc();
> +if (!sc->mastering)
> +return AVERROR(ENOMEM);
> +

> +sc->mastering->display_primaries[0][0] =
> +av_make_q(lrint(((double)avio_rb16(pb) / (1 << 16)) * chroma_den), 
> chroma_den);

this is not optimal, precission wise
av_d2q() should produce closer rationals
alternativly av_reduce() can be used directly

but iam not sure why a fixed chroma_den and luma_den is fixed
maybe iam missing something

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

Opposition brings concord. Out of discord comes the fairest harmony.
-- Heraclitus


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/4] avformat/movenc: add support for writing Content Light Level Box

2017-05-26 Thread Michael Niedermayer
On Wed, May 17, 2017 at 09:49:39PM -0300, James Almer wrote:
> As defined in "VP Codec ISO Media File Format Binding v1.0"
> https://github.com/webmproject/vp9-dash/blob/master/VPCodecISOMediaFileFormatBinding.md
> 
> Signed-off-by: James Almer 
> ---
>  libavformat/movenc.c | 22 ++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
> index cd436df7a4..eab7bbc8a7 100644
> --- a/libavformat/movenc.c
> +++ b/libavformat/movenc.c
> @@ -1154,6 +1154,27 @@ static int mov_write_smdm_tag(AVFormatContext *s, 
> AVIOContext *pb, MOVTrack *tra
>  return update_size(pb, pos);
>  }
>  
> +static int mov_write_coll_tag(AVIOContext *pb, MOVTrack *track)
> +{
> +int size = 0;
> +int64_t pos;
> +const AVContentLightMetadata *coll =
> +(const AVContentLightMetadata *) av_stream_get_side_data(track->st,
> + AV_PKT_DATA_CONTENT_LIGHT_LEVEL,
> + );
> +if (!size)
> +return 0;

Is there anything that checks the validity of size for a
AVContentLightMetadata ?
(that is, is this checked anywhere from side data creation to
 its use here)

If not then this can be too small and crash


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

It is dangerous to be right in matters on which the established authorities
are wrong. -- Voltaire


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/4] avformat/movenc: add support for writing Mastering Display Metadata Box

2017-05-26 Thread Michael Niedermayer
On Wed, May 17, 2017 at 09:49:38PM -0300, James Almer wrote:
> As defined in "VP Codec ISO Media File Format Binding v1.0"
> https://github.com/webmproject/vp9-dash/blob/master/VPCodecISOMediaFileFormatBinding.md
> 
> Signed-off-by: James Almer 
> ---
>  libavformat/movenc.c | 37 +
>  1 file changed, 37 insertions(+)
> 
> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
> index a6c0662cd0..cd436df7a4 100644
> --- a/libavformat/movenc.c
> +++ b/libavformat/movenc.c
> @@ -42,6 +42,7 @@
>  #include "libavutil/avstring.h"
>  #include "libavutil/intfloat.h"
>  #include "libavutil/mathematics.h"
> +#include "libavutil/mastering_display_metadata.h"
>  #include "libavutil/libm.h"
>  #include "libavutil/opt.h"
>  #include "libavutil/dict.h"
> @@ -1118,6 +1119,41 @@ static int mov_write_vpcc_tag(AVFormatContext *s, 
> AVIOContext *pb, MOVTrack *tra
>  return update_size(pb, pos);
>  }
>  
> +static int mov_write_smdm_tag(AVFormatContext *s, AVIOContext *pb, MOVTrack 
> *track)
> +{
> +int size = 0;
> +int64_t pos;
> +const AVMasteringDisplayMetadata *mastering =
> +(const AVMasteringDisplayMetadata *) 
> av_stream_get_side_data(track->st,
> + 
> AV_PKT_DATA_MASTERING_DISPLAY_METADATA,
> + );
> +if (!size)
> +return 0;
> +
> +if (!mastering->has_primaries || !mastering->has_luminance) {
> +av_log(s, AV_LOG_WARNING, "Incomplete Mastering Display metadata. 
> Both luminance "
> +  "and display primaries are needed\n");
> +return 0;
> +}
> +
> +pos = avio_tell(pb);
> +
> +avio_wb32(pb, 0);
> +ffio_wfourcc(pb, "SmDm");
> +avio_wb32(pb, 0); /* version & flags */

> +avio_wb16(pb, lrint(av_q2d(mastering->display_primaries[0][0]) * (1 << 
> 16)));
> +avio_wb16(pb, lrint(av_q2d(mastering->display_primaries[0][1]) * (1 << 
> 16)));
> +avio_wb16(pb, lrint(av_q2d(mastering->display_primaries[1][0]) * (1 << 
> 16)));
> +avio_wb16(pb, lrint(av_q2d(mastering->display_primaries[1][1]) * (1 << 
> 16)));
> +avio_wb16(pb, lrint(av_q2d(mastering->display_primaries[2][0]) * (1 << 
> 16)));
> +avio_wb16(pb, lrint(av_q2d(mastering->display_primaries[2][1]) * (1 << 
> 16)));
> +avio_wb16(pb, lrint(av_q2d(mastering->white_point[0])  * (1 << 
> 16)));
> +avio_wb16(pb, lrint(av_q2d(mastering->white_point[1])  * (1 << 
> 16)));
> +avio_wb32(pb, lrint(av_q2d(mastering->max_luminance)   * (1 <<  
> 8)));
> +avio_wb32(pb, lrint(av_q2d(mastering->min_luminance)   * (1 << 
> 14)));

These may need range checks.
Our API doesnt seem to define limits, so they might fall outside the
16/32 bit range used to store them, unless i miss something

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I have never wished to cater to the crowd; for what I know they do not
approve, and what they approve I do not know. -- Epicurus


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


Re: [FFmpeg-devel] [PATCH] avcodec/fft_template: Fix multiple runtime error: signed integer overflow: -1943918714 - 1935113003 cannot be represented in type 'int'

2017-05-26 Thread Michael Niedermayer
On Fri, May 26, 2017 at 06:07:35PM -0400, Ronald S. Bultje wrote:
> Hi,
> 
> On Fri, May 26, 2017 at 5:50 PM, Michael Niedermayer  > wrote:
> 
> > On Fri, May 26, 2017 at 11:18:12PM +0200, Hendrik Leppkes wrote:
> > > On Fri, May 26, 2017 at 11:11 PM, Michael Niedermayer
> > >  wrote:
> > > > On Fri, May 26, 2017 at 03:20:14PM +0100, Rostislav Pehlivanov wrote:
> > > >> On 26 May 2017 at 12:21, wm4  wrote:
> > > >>
> > > >> > On Thu, 25 May 2017 16:10:49 +0200
> > > >> > Michael Niedermayer  wrote:
> > > >> >
> > > >> > > Fixes: 1735/clusterfuzz-testcase-minimized-5350472347025408
> > > >> > >
> > > >> > > Found-by: continuous fuzzing process
> > https://github.com/google/oss-
> > > >> > fuzz/tree/master/projects/ffmpeg
> > > >> > > Signed-off-by: Michael Niedermayer 
> > > >> > > ---
> > > >> > >  libavcodec/fft_template.c | 50 +++---
> > > >> > -
> > > >> > >  1 file changed, 25 insertions(+), 25 deletions(-)
> > > >> > >
> > > >> > > diff --git a/libavcodec/fft_template.c b/libavcodec/fft_template.c
> > > >> > > index 480557f49f..e3a37e5d69 100644
> > > >> > > --- a/libavcodec/fft_template.c
> > > >> > > +++ b/libavcodec/fft_template.c
> > > >> > > @@ -249,7 +249,7 @@ static void fft_calc_c(FFTContext *s,
> > FFTComplex *z)
> > > >> > {
> > > >> > >
> > > >> > >  int nbits, i, n, num_transforms, offset, step;
> > > >> > >  int n4, n2, n34;
> > > >> > > -FFTSample tmp1, tmp2, tmp3, tmp4, tmp5, tmp6, tmp7, tmp8;
> > > >> > > +SUINT tmp1, tmp2, tmp3, tmp4, tmp5, tmp6, tmp7, tmp8;
> > > >> >
> > > >> > I want this SUINT thing gone, not have more of it.
> > > >> > ___
> > > >> > ffmpeg-devel mailing list
> > > >> > ffmpeg-devel@ffmpeg.org
> > > >> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > > >> >
> > > >>
> > > >> I agree, especially here.
> > > >
> > > >> Overflows should be left to happen in transforms if the input is
> > corrupt.
> > > >
> > > > signed int overflow is not allowed in C, if that is what you meant.
> > > >
> > > >
> > >
> > > Its "undefined", which means what the result will be is not defined -
> > > which in such a DSP function is irrelevant, if all it causes is
> > > corrupt output ... from corrupt input.
> >
> > no, this is not correct.
> > undefined behavior does not mean the effect will be limited to
> > the output.
> > It could cause the process to hard lockup, trigger an exception or
> > set a flag causing errors in later unrelated code.
> 
> 

> We've discussed this before, if you believe this to be exploitable, why
> allow it in our repository at all? I know of no other project that even
> remotely allows such ridiculous things. Please limit exploitable code to
> your personal tree, ffmpeg is not a rootkit.

please calm down, you make all kinds of statments and implications in
the text above which are not true.

This specific code in git triggers undefined behavior, the patch fixes
this undefined behavior.
If that is exploitable (which i neither claim it is nor that it isnt)
its a issue that exists before the patch but not afterwards.

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

Those who are best at talking, realize last or never when they are wrong.


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


Re: [FFmpeg-devel] [PATCH] lavc/mpegvideo_enc: allow to force low_delay by increasing strict_std_compliance

2017-05-26 Thread Michael Niedermayer
On Wed, May 24, 2017 at 10:31:10AM +0200, Stefano Sabatini wrote:
> On date Tuesday 2017-05-23 15:07:31 +0200, Moritz Barsnick encoded:
> > On Tue, May 23, 2017 at 12:28:48 +0200, Stefano Sabatini wrote:
> > > -if (s->codec_id != AV_CODEC_ID_MPEG2VIDEO) {
> > > +if (s->codec_id != AV_CODEC_ID_MPEG2VIDEO &&
> > > +s->strict_std_compliance >= FF_COMPLIANCE_NORMAL) {
> > >  av_log(avctx, AV_LOG_ERROR,
> > > -  "low delay forcing is only available for mpeg2\n");
> > > +  "low delay forcing is only available for mpeg2, 
> > > increase strict_std_compliance to force it\n");
> > 
> > Mathematically speaking, you need to decrease, not increase it (so that
> > it makes s->strict_std_compliance >= FF_COMPLIANCE_NORMAL false).
> > 
> > #define FF_COMPLIANCE_VERY_STRICT   2 ///< Strictly conform to an older 
> > more strict version of the spec or reference software.
> > #define FF_COMPLIANCE_STRICT1 ///< Strictly conform to all the 
> > things in the spec no matter what consequences.
> > #define FF_COMPLIANCE_NORMAL0
> > #define FF_COMPLIANCE_UNOFFICIAL   -1 ///< Allow unofficial extensions
> > #define FF_COMPLIANCE_EXPERIMENTAL -2 ///< Allow nonstandardized 
> > experimental things.
> > 
> > The text is also confusing (while correct) in that you'll be "forcing
> > the forcing (of low delay)".
> > 
> > Perhaps something like:
> 
> > "[...] set strict_std_compliance to 'unofficial' or lower in order to
> > allow it", or "enable it nevertheless". (I'm too lazy to check for
> > other wordings.)
> 
> This sounds fine. Patch amended.
> 
> Thanks.
> -- 
> FFmpeg = Fabulous & Fascinating Maxi Power Elitist Gangster

>  mpegvideo_enc.c |6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 0fa1dff6e9dbb5122cbea81ba56eb1892a0bb398  
> 0003-lavc-mpegvideo_enc-allow-low_delay-for-non-MPEG2-cod.patch
> From 536d88be287613a3a49dd60c1023c2418e9b8810 Mon Sep 17 00:00:00 2001
> From: Stefano Sabatini 
> Date: Tue, 23 May 2017 12:22:41 +0200
> Subject: [PATCH] lavc/mpegvideo_enc: allow low_delay for non MPEG2 codecs
>  depending on strict_std_compliance
> 
> Forcing low_delay can be useful, even if not officially supported.
> ---
>  libavcodec/mpegvideo_enc.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)

probably ok

thanks

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I know you won't believe me, but the highest form of Human Excellence is
to question oneself and others. -- Socrates


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


Re: [FFmpeg-devel] [PATCH] avcodec/fft_template: Fix multiple runtime error: signed integer overflow: -1943918714 - 1935113003 cannot be represented in type 'int'

2017-05-26 Thread Ronald S. Bultje
Hi,

On Fri, May 26, 2017 at 5:50 PM, Michael Niedermayer  wrote:

> On Fri, May 26, 2017 at 11:18:12PM +0200, Hendrik Leppkes wrote:
> > On Fri, May 26, 2017 at 11:11 PM, Michael Niedermayer
> >  wrote:
> > > On Fri, May 26, 2017 at 03:20:14PM +0100, Rostislav Pehlivanov wrote:
> > >> On 26 May 2017 at 12:21, wm4  wrote:
> > >>
> > >> > On Thu, 25 May 2017 16:10:49 +0200
> > >> > Michael Niedermayer  wrote:
> > >> >
> > >> > > Fixes: 1735/clusterfuzz-testcase-minimized-5350472347025408
> > >> > >
> > >> > > Found-by: continuous fuzzing process
> https://github.com/google/oss-
> > >> > fuzz/tree/master/projects/ffmpeg
> > >> > > Signed-off-by: Michael Niedermayer 
> > >> > > ---
> > >> > >  libavcodec/fft_template.c | 50 +++---
> > >> > -
> > >> > >  1 file changed, 25 insertions(+), 25 deletions(-)
> > >> > >
> > >> > > diff --git a/libavcodec/fft_template.c b/libavcodec/fft_template.c
> > >> > > index 480557f49f..e3a37e5d69 100644
> > >> > > --- a/libavcodec/fft_template.c
> > >> > > +++ b/libavcodec/fft_template.c
> > >> > > @@ -249,7 +249,7 @@ static void fft_calc_c(FFTContext *s,
> FFTComplex *z)
> > >> > {
> > >> > >
> > >> > >  int nbits, i, n, num_transforms, offset, step;
> > >> > >  int n4, n2, n34;
> > >> > > -FFTSample tmp1, tmp2, tmp3, tmp4, tmp5, tmp6, tmp7, tmp8;
> > >> > > +SUINT tmp1, tmp2, tmp3, tmp4, tmp5, tmp6, tmp7, tmp8;
> > >> >
> > >> > I want this SUINT thing gone, not have more of it.
> > >> > ___
> > >> > ffmpeg-devel mailing list
> > >> > ffmpeg-devel@ffmpeg.org
> > >> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > >> >
> > >>
> > >> I agree, especially here.
> > >
> > >> Overflows should be left to happen in transforms if the input is
> corrupt.
> > >
> > > signed int overflow is not allowed in C, if that is what you meant.
> > >
> > >
> >
> > Its "undefined", which means what the result will be is not defined -
> > which in such a DSP function is irrelevant, if all it causes is
> > corrupt output ... from corrupt input.
>
> no, this is not correct.
> undefined behavior does not mean the effect will be limited to
> the output.
> It could cause the process to hard lockup, trigger an exception or
> set a flag causing errors in later unrelated code.


We've discussed this before, if you believe this to be exploitable, why
allow it in our repository at all? I know of no other project that even
remotely allows such ridiculous things. Please limit exploitable code to
your personal tree, ffmpeg is not a rootkit.

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


Re: [FFmpeg-devel] [PATCH] avcodec/fft_template: Fix multiple runtime error: signed integer overflow: -1943918714 - 1935113003 cannot be represented in type 'int'

2017-05-26 Thread Michael Niedermayer
On Fri, May 26, 2017 at 11:18:12PM +0200, Hendrik Leppkes wrote:
> On Fri, May 26, 2017 at 11:11 PM, Michael Niedermayer
>  wrote:
> > On Fri, May 26, 2017 at 03:20:14PM +0100, Rostislav Pehlivanov wrote:
> >> On 26 May 2017 at 12:21, wm4  wrote:
> >>
> >> > On Thu, 25 May 2017 16:10:49 +0200
> >> > Michael Niedermayer  wrote:
> >> >
> >> > > Fixes: 1735/clusterfuzz-testcase-minimized-5350472347025408
> >> > >
> >> > > Found-by: continuous fuzzing process https://github.com/google/oss-
> >> > fuzz/tree/master/projects/ffmpeg
> >> > > Signed-off-by: Michael Niedermayer 
> >> > > ---
> >> > >  libavcodec/fft_template.c | 50 +++---
> >> > -
> >> > >  1 file changed, 25 insertions(+), 25 deletions(-)
> >> > >
> >> > > diff --git a/libavcodec/fft_template.c b/libavcodec/fft_template.c
> >> > > index 480557f49f..e3a37e5d69 100644
> >> > > --- a/libavcodec/fft_template.c
> >> > > +++ b/libavcodec/fft_template.c
> >> > > @@ -249,7 +249,7 @@ static void fft_calc_c(FFTContext *s, FFTComplex 
> >> > > *z)
> >> > {
> >> > >
> >> > >  int nbits, i, n, num_transforms, offset, step;
> >> > >  int n4, n2, n34;
> >> > > -FFTSample tmp1, tmp2, tmp3, tmp4, tmp5, tmp6, tmp7, tmp8;
> >> > > +SUINT tmp1, tmp2, tmp3, tmp4, tmp5, tmp6, tmp7, tmp8;
> >> >
> >> > I want this SUINT thing gone, not have more of it.
> >> > ___
> >> > ffmpeg-devel mailing list
> >> > ffmpeg-devel@ffmpeg.org
> >> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >> >
> >>
> >> I agree, especially here.
> >
> >> Overflows should be left to happen in transforms if the input is corrupt.
> >
> > signed int overflow is not allowed in C, if that is what you meant.
> >
> >
> 
> Its "undefined", which means what the result will be is not defined -
> which in such a DSP function is irrelevant, if all it causes is
> corrupt output ... from corrupt input.

no, this is not correct.
undefined behavior does not mean the effect will be limited to
the output.
It could cause the process to hard lockup, trigger an exception or
set a flag causing errors in later unrelated code.
A C compiler can assume that undefined behavior does not occur so
it can completely ignore any possibility that implies
undefined behavior.

As referece the text from iso C:
undefined behavior
behavior, upon use of a nonportable or erroneous program construct or of 
erroneous data,
for which this International Standard imposes no requirements
NOTE Possible undefined behavior ranges from ignoring the situation 
completely with unpredictable
results, to behaving during translation or program execution in a 
documented manner characteristic of the
environment (with or without the issuance of a diagnostic message), to 
terminating a translation or
execution (with the issuance of a diagnostic message).
EXAMPLE An example of undefined behavior is the behavior on integer 
overflow.

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Why not whip the teacher when the pupil misbehaves? -- Diogenes of Sinope


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


Re: [FFmpeg-devel] [PATCH] avcodec/fft_template: Fix multiple runtime error: signed integer overflow: -1943918714 - 1935113003 cannot be represented in type 'int'

2017-05-26 Thread Hendrik Leppkes
On Fri, May 26, 2017 at 11:11 PM, Michael Niedermayer
 wrote:
> On Fri, May 26, 2017 at 03:20:14PM +0100, Rostislav Pehlivanov wrote:
>> On 26 May 2017 at 12:21, wm4  wrote:
>>
>> > On Thu, 25 May 2017 16:10:49 +0200
>> > Michael Niedermayer  wrote:
>> >
>> > > Fixes: 1735/clusterfuzz-testcase-minimized-5350472347025408
>> > >
>> > > Found-by: continuous fuzzing process https://github.com/google/oss-
>> > fuzz/tree/master/projects/ffmpeg
>> > > Signed-off-by: Michael Niedermayer 
>> > > ---
>> > >  libavcodec/fft_template.c | 50 +++---
>> > -
>> > >  1 file changed, 25 insertions(+), 25 deletions(-)
>> > >
>> > > diff --git a/libavcodec/fft_template.c b/libavcodec/fft_template.c
>> > > index 480557f49f..e3a37e5d69 100644
>> > > --- a/libavcodec/fft_template.c
>> > > +++ b/libavcodec/fft_template.c
>> > > @@ -249,7 +249,7 @@ static void fft_calc_c(FFTContext *s, FFTComplex *z)
>> > {
>> > >
>> > >  int nbits, i, n, num_transforms, offset, step;
>> > >  int n4, n2, n34;
>> > > -FFTSample tmp1, tmp2, tmp3, tmp4, tmp5, tmp6, tmp7, tmp8;
>> > > +SUINT tmp1, tmp2, tmp3, tmp4, tmp5, tmp6, tmp7, tmp8;
>> >
>> > I want this SUINT thing gone, not have more of it.
>> > ___
>> > ffmpeg-devel mailing list
>> > ffmpeg-devel@ffmpeg.org
>> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> >
>>
>> I agree, especially here.
>
>> Overflows should be left to happen in transforms if the input is corrupt.
>
> signed int overflow is not allowed in C, if that is what you meant.
>
>

Its "undefined", which means what the result will be is not defined -
which in such a DSP function is irrelevant, if all it causes is
corrupt output ... from corrupt input.
Its not like SUINT actually fixes them, it just silences them in debug builds.

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


Re: [FFmpeg-devel] [PATCH] avcodec/fft_template: Fix multiple runtime error: signed integer overflow: -1943918714 - 1935113003 cannot be represented in type 'int'

2017-05-26 Thread Michael Niedermayer
On Fri, May 26, 2017 at 03:20:14PM +0100, Rostislav Pehlivanov wrote:
> On 26 May 2017 at 12:21, wm4  wrote:
> 
> > On Thu, 25 May 2017 16:10:49 +0200
> > Michael Niedermayer  wrote:
> >
> > > Fixes: 1735/clusterfuzz-testcase-minimized-5350472347025408
> > >
> > > Found-by: continuous fuzzing process https://github.com/google/oss-
> > fuzz/tree/master/projects/ffmpeg
> > > Signed-off-by: Michael Niedermayer 
> > > ---
> > >  libavcodec/fft_template.c | 50 +++---
> > -
> > >  1 file changed, 25 insertions(+), 25 deletions(-)
> > >
> > > diff --git a/libavcodec/fft_template.c b/libavcodec/fft_template.c
> > > index 480557f49f..e3a37e5d69 100644
> > > --- a/libavcodec/fft_template.c
> > > +++ b/libavcodec/fft_template.c
> > > @@ -249,7 +249,7 @@ static void fft_calc_c(FFTContext *s, FFTComplex *z)
> > {
> > >
> > >  int nbits, i, n, num_transforms, offset, step;
> > >  int n4, n2, n34;
> > > -FFTSample tmp1, tmp2, tmp3, tmp4, tmp5, tmp6, tmp7, tmp8;
> > > +SUINT tmp1, tmp2, tmp3, tmp4, tmp5, tmp6, tmp7, tmp8;
> >
> > I want this SUINT thing gone, not have more of it.
> > ___
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> 
> I agree, especially here.

> Overflows should be left to happen in transforms if the input is corrupt.

signed int overflow is not allowed in C, if that is what you meant.


> Codecs are designed such that transforms won't overflow unless corrupt data
> is fed. We allow for that to happen already (in the VP9 DCTs), so FFTs
> shouldn't be excluded.
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Modern terrorism, a quick summary: Need oil, start war with country that
has oil, kill hundread thousand in war. Let country fall into chaos,
be surprised about raise of fundamantalists. Drop more bombs, kill more
people, be surprised about them taking revenge and drop even more bombs
and strip your own citizens of their rights and freedoms. to be continued


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


Re: [FFmpeg-devel] [PATCH] avfilter: add audio surround upmixer

2017-05-26 Thread Michael Niedermayer
On Fri, May 26, 2017 at 10:04:40PM +0200, Paul B Mahol wrote:
> On 5/26/17, Michael Niedermayer  wrote:
> > On Fri, May 26, 2017 at 06:54:33PM +0200, Paul B Mahol wrote:
> >> On 5/26/17, Michael Niedermayer  wrote:
> >> > On Fri, May 26, 2017 at 01:11:38PM +0200, Paul B Mahol wrote:
> >> >> On 5/26/17, Nicolas George  wrote:
> >> >> > Le septidi 7 prairial, an CCXXV, Paul B Mahol a ecrit :
> >> >> >> > This belongs in libswresample
> >> >> >> No it does not.
> >> >> >
> >> >> > I think it does too.
> >> >>
> >> >> You want to link libswresample with libavcodec?
> >> >
> >> > While this question was directed at nicolas ...
> >> >
> >> > I dont think audio upmix code should depend on a lib of encoders and
> >> > decoders (libavcodec)
> >> > No matter if the upmix code would be in libavfilter or libswresample
> >> >
> >> > I belive a temporary dependancy would be ok, if there is intend to
> >> > factor/move things to remove the dependancy in the future.
> >> >
> >> > But IMO libavcodec is the wrong place to export generic FFT
> >> > functionality.
> >> > We need FFTs in codecs, we need them in filters, we need them in
> >> > swresample (the soxr resampler we support also uses a FFT internally)
> >> >
> >> > Also moving FFT to a different lib should be quite easy compared to
> >> > other ugly dependancies we have (as in snow motion estimation, which
> >> > is not as easy to move. none the less none of these ugly dependancies
> >> > should be there except temporary)
> >>
> >
> >> This code does upmixing, and there could by myriad variants of upmixing.
> >
> > This is true for any format convertion or more genericaly for anything.
> > there are always many ways to do something.
> >
> > the way FFmpeg is modularized is that we have a lib for audio format
> > convertion, resampling, rematrixing including upmixing and
> > downmixing, ...
> >
> >
> >>
> >> Having it in libswresample is flawed design.
> >>
> >> So I will not do the transitions.
> >>
> >> If you still object to leave it as it is, in lavfi. You will need to take
> >> care of the necessary changes by yourself.
> >
> > Do you agree that we need a part of code and API that does
> > audio format convertion, amongth it upmixing and downmixing ?
> > Something thats used by default
> >
> > if you agree on the need of such code, why would it be flawed design
> > to add a improved upmixing implementation in there so it gets used ?
> > (be that by default or users choice of what to use by default or
> >  by specific choice for a specific instance)
> >
> > I want the best and most correct code to be used.
> > I dont want to object or demand anything. I belive though that putting
> > upmixing code in 2 different places and 2 different libs will give us
> > headaches in the future
> 
> Why is pan filter not part of libswresample? It does upmixing and downmixing.
> Therefore it should be in libswresample by your logic.

libswresample supports what pan does through the C API
pan in libavfilter provides a convenient way to access the same
functionality through a filtergraph description string

> 
> Why is stereotools not part of libswresample?

> 
> Why is volume not part of libswresample?

same as with pan


> 
> Hey why not put there ambisonics decoder too?
> 
> Or equalizer?
> 
> Why is eq filter not part of libswscale?
> 
> Or any damn A->A or V->V filter?

This is becoming increasingly specific and outside the scope these
libs.


> 
> Besides the surround filter is not "correct". There is no "correct"
> way to do upmixing.
> 
> Consider mono upmixing, which is just spreading different bands to
> different channels,
> do you want that too in libswresample?
> 
> What about synthesizers (we have anoisesrc) but I plan to add
> wavetable, do you want that
> in libswresample too?

You misunderstand me i think.

The filter patch you posted is descibed by you in the patch as:
+@section surround
+Apply audio surround upmix filter.
+
+This filter allows to produce multichannel output from stereo audio stream.
+
+The filter accepts the following options:

That is not a special case or something that falls outside the scope.
(which is what you argue about above with listing other filters)
this is a description of a generic stereo upmix.
In fact this very description could be used for the stereo upmix code
in swresample

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

If a bugfix only changes things apparently unrelated to the bug with no
further explanation, that is a good sign that the bugfix is wrong.


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


[FFmpeg-devel] [PATCH] avformat/mux: stop delaying writing the header

2017-05-26 Thread James Almer
There's no need to wait for the first packet of every stream now that
every bitstream filter behaves as intended.

Signed-off-by: James Almer 
---
What should we do with the AVFMT_FLAG_AUTO_BSF flag? Do we deprecate
it and force the automatic insertion of muxer-required bitstream
filters now that the generic muxing code will always behave the same,
or keep it to give the user a choice to apply said bitstream filters?
I ask because some filters, like vp9_superframe, are necessary to
avoid creating broken files, so it's not wise to allow the user to
disable it.
It would also go in line with AVCodec.bsfs, which is essentially a
non-optional (for reasons made obvious in fa1749dd34) autobsf at the
codec level instead of container level.

The change to fate-rgb24-mkv is a regression that can be prevented by
committing https://ffmpeg.org/pipermail/ffmpeg-devel/2017-May/211311.html
or a similar solution, maybe using avformat_init_output(), to make sure
ffmpeg.c sets AVCodecParameters->field_order for the output stream before
calling avformat_write_header().

 libavformat/internal.h |  6 -
 libavformat/mux.c  | 53 +-
 libavformat/options_table.h|  2 +-
 libavformat/tests/fifo_muxer.c | 52 -
 tests/ref/fate/fifo-muxer-tst  |  1 -
 tests/ref/fate/rgb24-mkv   |  4 ++--
 6 files changed, 14 insertions(+), 104 deletions(-)

diff --git a/libavformat/internal.h b/libavformat/internal.h
index c856945ce9..9c6cd77166 100644
--- a/libavformat/internal.h
+++ b/libavformat/internal.h
@@ -121,12 +121,6 @@ struct AVFormatInternal {
 int avoid_negative_ts_use_pts;
 
 /**
- * Whether or not a header has already been written
- */
-int header_written;
-int write_header_ret;
-
-/**
  * Timestamp of the end of the shortest stream.
  */
 int64_t shortest_end;
diff --git a/libavformat/mux.c b/libavformat/mux.c
index 01dcb362ae..f931d6a928 100644
--- a/libavformat/mux.c
+++ b/libavformat/mux.c
@@ -468,26 +468,6 @@ static int init_pts(AVFormatContext *s)
 return 0;
 }
 
-static int write_header_internal(AVFormatContext *s)
-{
-if (!(s->oformat->flags & AVFMT_NOFILE) && s->pb)
-avio_write_marker(s->pb, AV_NOPTS_VALUE, AVIO_DATA_MARKER_HEADER);
-if (s->oformat->write_header) {
-int ret = s->oformat->write_header(s);
-if (ret >= 0 && s->pb && s->pb->error < 0)
-ret = s->pb->error;
-s->internal->write_header_ret = ret;
-if (ret < 0)
-return ret;
-if (s->flush_packets && s->pb && s->pb->error >= 0 && s->flags & 
AVFMT_FLAG_FLUSH_PACKETS)
-avio_flush(s->pb);
-}
-s->internal->header_written = 1;
-if (!(s->oformat->flags & AVFMT_NOFILE) && s->pb)
-avio_write_marker(s->pb, AV_NOPTS_VALUE, AVIO_DATA_MARKER_UNKNOWN);
-return 0;
-}
-
 int avformat_init_output(AVFormatContext *s, AVDictionary **options)
 {
 int ret = 0;
@@ -526,11 +506,19 @@ int avformat_write_header(AVFormatContext *s, 
AVDictionary **options)
 if ((ret = avformat_init_output(s, options)) < 0)
 return ret;
 
-if (!(s->oformat->check_bitstream && s->flags & AVFMT_FLAG_AUTO_BSF)) {
-ret = write_header_internal(s);
+if (!(s->oformat->flags & AVFMT_NOFILE) && s->pb)
+avio_write_marker(s->pb, AV_NOPTS_VALUE, AVIO_DATA_MARKER_HEADER);
+if (s->oformat->write_header) {
+int ret = s->oformat->write_header(s);
+if (ret >= 0 && s->pb && s->pb->error < 0)
+ret = s->pb->error;
 if (ret < 0)
 goto fail;
+if (s->flush_packets && s->pb && s->pb->error >= 0 && s->flags & 
AVFMT_FLAG_FLUSH_PACKETS)
+avio_flush(s->pb);
 }
+if (!(s->oformat->flags & AVFMT_NOFILE) && s->pb)
+avio_write_marker(s->pb, AV_NOPTS_VALUE, AVIO_DATA_MARKER_UNKNOWN);
 
 if (!s->internal->streams_initialized) {
 if ((ret = init_pts(s)) < 0)
@@ -756,12 +744,6 @@ FF_DISABLE_DEPRECATION_WARNINGS
 FF_ENABLE_DEPRECATION_WARNINGS
 #endif
 
-if (!s->internal->header_written) {
-ret = s->internal->write_header_ret ? s->internal->write_header_ret : 
write_header_internal(s);
-if (ret < 0)
-goto fail;
-}
-
 if ((pkt->flags & AV_PKT_FLAG_UNCODED_FRAME)) {
 AVFrame *frame = (AVFrame *)pkt->data;
 av_assert0(pkt->size == UNCODED_FRAME_PACKET_SIZE);
@@ -778,7 +760,6 @@ FF_ENABLE_DEPRECATION_WARNINGS
 ret = s->pb->error;
 }
 
-fail:
 #if FF_API_LAVF_MERGE_SD
 FF_DISABLE_DEPRECATION_WARNINGS
 if (did_split)
@@ -926,11 +907,6 @@ int av_write_frame(AVFormatContext *s, AVPacket *pkt)
 
 if (!pkt) {
 if (s->oformat->flags & AVFMT_ALLOW_FLUSH) {
-if (!s->internal->header_written) {
-ret = s->internal->write_header_ret ? 
s->internal->write_header_ret : write_header_internal(s);
-if (ret < 0)

Re: [FFmpeg-devel] [PATCH] avfilter: add audio surround upmixer

2017-05-26 Thread Paul B Mahol
On 5/26/17, Michael Niedermayer  wrote:
> On Fri, May 26, 2017 at 06:54:33PM +0200, Paul B Mahol wrote:
>> On 5/26/17, Michael Niedermayer  wrote:
>> > On Fri, May 26, 2017 at 01:11:38PM +0200, Paul B Mahol wrote:
>> >> On 5/26/17, Nicolas George  wrote:
>> >> > Le septidi 7 prairial, an CCXXV, Paul B Mahol a ecrit :
>> >> >> > This belongs in libswresample
>> >> >> No it does not.
>> >> >
>> >> > I think it does too.
>> >>
>> >> You want to link libswresample with libavcodec?
>> >
>> > While this question was directed at nicolas ...
>> >
>> > I dont think audio upmix code should depend on a lib of encoders and
>> > decoders (libavcodec)
>> > No matter if the upmix code would be in libavfilter or libswresample
>> >
>> > I belive a temporary dependancy would be ok, if there is intend to
>> > factor/move things to remove the dependancy in the future.
>> >
>> > But IMO libavcodec is the wrong place to export generic FFT
>> > functionality.
>> > We need FFTs in codecs, we need them in filters, we need them in
>> > swresample (the soxr resampler we support also uses a FFT internally)
>> >
>> > Also moving FFT to a different lib should be quite easy compared to
>> > other ugly dependancies we have (as in snow motion estimation, which
>> > is not as easy to move. none the less none of these ugly dependancies
>> > should be there except temporary)
>>
>
>> This code does upmixing, and there could by myriad variants of upmixing.
>
> This is true for any format convertion or more genericaly for anything.
> there are always many ways to do something.
>
> the way FFmpeg is modularized is that we have a lib for audio format
> convertion, resampling, rematrixing including upmixing and
> downmixing, ...
>
>
>>
>> Having it in libswresample is flawed design.
>>
>> So I will not do the transitions.
>>
>> If you still object to leave it as it is, in lavfi. You will need to take
>> care of the necessary changes by yourself.
>
> Do you agree that we need a part of code and API that does
> audio format convertion, amongth it upmixing and downmixing ?
> Something thats used by default
>
> if you agree on the need of such code, why would it be flawed design
> to add a improved upmixing implementation in there so it gets used ?
> (be that by default or users choice of what to use by default or
>  by specific choice for a specific instance)
>
> I want the best and most correct code to be used.
> I dont want to object or demand anything. I belive though that putting
> upmixing code in 2 different places and 2 different libs will give us
> headaches in the future

Why is pan filter not part of libswresample? It does upmixing and downmixing.
Therefore it should be in libswresample by your logic.

Why is stereotools not part of libswresample?

Why is volume not part of libswresample?

Hey why not put there ambisonics decoder too?

Or equalizer?

Why is eq filter not part of libswscale?

Or any damn A->A or V->V filter?

Besides the surround filter is not "correct". There is no "correct"
way to do upmixing.

Consider mono upmixing, which is just spreading different bands to
different channels,
do you want that too in libswresample?

What about synthesizers (we have anoisesrc) but I plan to add
wavetable, do you want that
in libswresample too?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avfilter: add audio surround upmixer

2017-05-26 Thread Michael Niedermayer
On Fri, May 26, 2017 at 06:54:33PM +0200, Paul B Mahol wrote:
> On 5/26/17, Michael Niedermayer  wrote:
> > On Fri, May 26, 2017 at 01:11:38PM +0200, Paul B Mahol wrote:
> >> On 5/26/17, Nicolas George  wrote:
> >> > Le septidi 7 prairial, an CCXXV, Paul B Mahol a ecrit :
> >> >> > This belongs in libswresample
> >> >> No it does not.
> >> >
> >> > I think it does too.
> >>
> >> You want to link libswresample with libavcodec?
> >
> > While this question was directed at nicolas ...
> >
> > I dont think audio upmix code should depend on a lib of encoders and
> > decoders (libavcodec)
> > No matter if the upmix code would be in libavfilter or libswresample
> >
> > I belive a temporary dependancy would be ok, if there is intend to
> > factor/move things to remove the dependancy in the future.
> >
> > But IMO libavcodec is the wrong place to export generic FFT
> > functionality.
> > We need FFTs in codecs, we need them in filters, we need them in
> > swresample (the soxr resampler we support also uses a FFT internally)
> >
> > Also moving FFT to a different lib should be quite easy compared to
> > other ugly dependancies we have (as in snow motion estimation, which
> > is not as easy to move. none the less none of these ugly dependancies
> > should be there except temporary)
> 

> This code does upmixing, and there could by myriad variants of upmixing.

This is true for any format convertion or more genericaly for anything.
there are always many ways to do something.

the way FFmpeg is modularized is that we have a lib for audio format
convertion, resampling, rematrixing including upmixing and
downmixing, ...


> 
> Having it in libswresample is flawed design.
> 
> So I will not do the transitions.
> 
> If you still object to leave it as it is, in lavfi. You will need to take
> care of the necessary changes by yourself.

Do you agree that we need a part of code and API that does
audio format convertion, amongth it upmixing and downmixing ?
Something thats used by default

if you agree on the need of such code, why would it be flawed design
to add a improved upmixing implementation in there so it gets used ?
(be that by default or users choice of what to use by default or
 by specific choice for a specific instance)

I want the best and most correct code to be used.
I dont want to object or demand anything. I belive though that putting
upmixing code in 2 different places and 2 different libs will give us
headaches in the future

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

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


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


Re: [FFmpeg-devel] [PATCH 2/2] ffmpeg: Made execution of reap_filters() more deterministic with respect to when headers are written

2017-05-26 Thread Michael Niedermayer
On Fri, May 12, 2017 at 01:28:14PM -0700, Aaron Levinson wrote:
> Purpose: Made execution of reap_filters() more deterministic with
> respect to when headers are written in relationship with the
> initialization of output streams and the processing of input audio
> and/or video frames.  This change fixes a bug in ffmpeg encountered
> when decoding interlaced video.  In some cases, ffmpeg would treat it
> as progressive.
> 
> Detailed description of problem: Run the following command: "ffmpeg -i
> test8_1080i.h264 -c:v mpeg2video test8_1080i_mp2.ts".  In this case,
> test8_1080i.h264 is an H.264-encoded file with 1080i59.94
> (interlaced).  Prior to the patch, the following output is generated:
> 
> Input #0, h264, from 'test8_1080i.h264':
>   Duration: N/A, bitrate: N/A
> Stream #0:0: Video: h264 (High), yuv420p(top first), 1920x1080 [SAR 1:1 
> DAR 16:9], 29.97 fps, 29.97 tbr, 1200k tbn, 59.94 tbc
> Stream mapping:
>   Stream #0:0 -> #0:0 (h264 (native) -> mpeg2video (native))
> Press [q] to stop, [?] for help
> Output #0, mpegts, to 'test8_1080i_mp2_2.ts':
>   Metadata:
> encoder : Lavf57.72.100
> Stream #0:0: Video: mpeg2video (Main), yuv420p, 1920x1080 [SAR 1:1 DAR 
> 16:9], q=2-31, 200 kb/s, 29.97 fps, 90k tbn, 29.97 tbc
> Metadata:
>   encoder : Lavc57.92.100 mpeg2video
> 
> which demonstrates the bug.  The output stream should instead look like:
> 
> Stream #0:0: Video: mpeg2video (Main), yuv420p(top coded first 
> (swapped)), 1920x1080 [SAR 1:1 DAR 16:9], q=2-31, 200 kb/s, 29.97 fps, 90k 
> tbn, 29.97 tbc
> 
> The bug is caused by the fact that reap_filters() calls
> init_output_stream(), which is then immediately followed by a call to
> check_init_output_file(), and this is all done prior to the first call
> to do_video_out().  An initial call to do_video_out() is necessary to
> populate the interlaced video information to the output stream's
> codecpar (mux_par->field_order in do_video_out()).  However,
> check_init_output_file() calls avformat_write_header() prior to the
> initial call to do_video_out(), so field_order is populated too late,
> and the header is written with the default field_order value,
> progressive.
> 
> Signed-off-by: Aaron Levinson 
> ---
>  ffmpeg.c | 43 ---
>  1 file changed, 40 insertions(+), 3 deletions(-)

This breaks

./ffmpeg -itsoffset 2 -re -i matrixbench_mpeg2.mpg -vcodec libx264 -an -t 3 -f 
rtp rtp://127.0.0.1:17755 > h264.sdp 2> log1 &
sleep 2
./ffmpeg -protocol_whitelist file,rtp,udp -i h264.sdp -y -t 0.9 out-h264.avi

(h264.sdp: Invalid data found when processing input)

for the input see:
http://samples.ffmpeg.org/benchmark/testsuite1/matrixbench_mpeg2.mpg

also increasing sleep to 2.5 or 3 does not make it work, it fails
differently (h264 decoder errors)

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

If you fake or manipulate statistics in a paper in physics you will never
get a job again.
If you fake or manipulate statistics in a paper in medicin you will get
a job for life at the pharma industry.


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


Re: [FFmpeg-devel] [PATCH] avfilter: add audio surround upmixer

2017-05-26 Thread Paul B Mahol
On 5/26/17, Michael Niedermayer  wrote:
> On Fri, May 26, 2017 at 01:11:38PM +0200, Paul B Mahol wrote:
>> On 5/26/17, Nicolas George  wrote:
>> > Le septidi 7 prairial, an CCXXV, Paul B Mahol a ecrit :
>> >> > This belongs in libswresample
>> >> No it does not.
>> >
>> > I think it does too.
>>
>> You want to link libswresample with libavcodec?
>
> While this question was directed at nicolas ...
>
> I dont think audio upmix code should depend on a lib of encoders and
> decoders (libavcodec)
> No matter if the upmix code would be in libavfilter or libswresample
>
> I belive a temporary dependancy would be ok, if there is intend to
> factor/move things to remove the dependancy in the future.
>
> But IMO libavcodec is the wrong place to export generic FFT
> functionality.
> We need FFTs in codecs, we need them in filters, we need them in
> swresample (the soxr resampler we support also uses a FFT internally)
>
> Also moving FFT to a different lib should be quite easy compared to
> other ugly dependancies we have (as in snow motion estimation, which
> is not as easy to move. none the less none of these ugly dependancies
> should be there except temporary)

This code does upmixing, and there could by myriad variants of upmixing.

Having it in libswresample is flawed design.

So I will not do the transitions.

If you still object to leave it as it is, in lavfi. You will need to take
care of the necessary changes by yourself.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] I'm looking for a programmer that would be able to develope for me a script that work with ffmpeg/osx (PAID JOB)

2017-05-26 Thread nuvolablux
hello everybody
is there anybody here that is be able to develope for me a script that work
with ffmpeg/osx to split 1 hours long movie into few clips? This is a paid
project.

The script will have to work on mac osx
I recommend using ffmpeg as a video engine
Input File: Apple prores422hq MOV files / text file containing time code
and file names
The script must cut the videos with accuracy to the frame, the time code in
the text file is in this format: HH: MM: SS: frames (1 second contains 25
frames - from 00 to 24 -) while ffmpeg uses different syntax:

FFMPEG Time unit syntax

Note that you can use two different time unit formats: sexagesimal (HOURS:
MM: SS.MICROSECONDS, as in 01: 23: 45.678), or in seconds. If a fraction is
used, such as 02: 30.05, this is interpreted as "5 100ths of a second", not
as frame 5. For example, 02: 30.5 would be 2 minutes, 30 seconds and half a
second Would be the same as using 150.5 in seconds.
The ffmpeg command to use is the copy, which provides a crop and not the
file regeneration.

The script would be splitted into two parts

PART A:

Starting from a master video file (MOV apple prores 422HQ full HD) the
script must be able to divide it into many files according to the time
codes and names in the TXT file.
Below you can download the sample video file and the EDL(TXT) file
http://wetransfer.red-line.us/xzms4iU
Looking at the text contained in the edl file (see below) the script will
have to:
Generate video file called "Video Source256485.mov" from video master from
time code 00: 00: 00: 00 to 00: 00: 11: 14
Generate the second video file called "Video Source Copy 54.mov" from the
video master file from 00: 00: 11: 14 to 00: 00: 23: 03
...and so on...
Up to file 11 with the name "Video Source copy 2342342.mov" which in this
test file will be the last file that will process the script.
I have more than 1000 files to split and name, hence the need to automate
this process.


TITLE: Video Source Copy
FCM: NON-DROP FRAME

01 001 V 00: 00: 00: 00 00: 00: 11: 14 00: 00: 00: 00 00: 00: 11: 14
* FROM CLIP NAME: Video Source256485.mov

02 001 V C 00: 00: 00: 00 00: 00: 11: 14 00: 00: 11: 14 00: 00: 23: 03
* FROM CLIP NAME: Video Source Copy 54.mov

03 001 V 00: 00: 00: 00 00: 00: 11: 14 00: 00: 23: 03 00: 00: 34: 17
* FROM CLIP NAME: Video Source copy 33.mov

04 001 V C 00: 00: 00: 00 00: 00: 11: 14 00: 00: 34: 17 00: 00: 46: 06
* FROM CLIP NAME: Video Source Copy 433.mov

05 001 V C 00: 00: 00: 00 00: 00: 11: 14 00: 00: 46: 06 00: 00: 57: 20
* FROM CLIP NAME: Video Source Copy 255.mov

06 001 V C 00: 00: 00: 00 00: 00: 11: 14 00: 00: 57: 20 00: 01: 09: 09
* FROM CLIP NAME: Video Source Copy 6123.mov

07 001 V 00: 00: 00: 00 00: 00: 11: 14 00: 01: 09: 09 00: 01: 20: 23
* FROM CLIP NAME: Video Source Copy 2357.mov

08 001 V 00: 00: 00: 00 00: 00: 11: 14 00: 01: 20: 23 00: 01: 32: 12
* FROM CLIP NAME: Video Source 123434.mov copy

09 001 V 00: 00: 00: 00 00: 00: 11: 14 00: 01: 32: 12 00: 01: 44: 01
* FROM CLIP NAME: Video Source 74345fdf.mov copy

10 001 V C 00: 00: 00: 00 00: 00: 11: 14 00: 01: 44: 01 00: 01: 55: 15
* FROM CLIP NAME: Video Source copy 2342342.mov

11 001 V C 00: 00: 00: 00 00: 00: 11: 14 00: 01: 55: 15 00: 02: 07: 04
* FROM CLIP NAME: Video Source copia.mov


PART B)

Starting with two distinct folders of video files, containing files named
the same way and of the same duration

eg:

Folder A:
Aaa.mov duration 00: 00: 10: 12
Bbb.mov duration 00: 01: 14: 10
Ccc.mov duration 00: 10: 00: 14
and so on...

Folder B:
Aaa.mov duration 00: 00: 10: 12
Bbb.mov duration 00: 01: 14: 10
Ccc.mov duration 00: 10: 00: 14
and so on...

The script will have to:

Extract an image file (png?) From each movie that contains the frame that
is in the exact half of the movie.
Create an image from the audio of movie files (
https://trac.ffmpeg.org/wiki/Waveform)
Picture the exported frames and the audio image in a document according to
a predefined pattern and add text such as the name of the files from which
extra pictures and other standard texts have been extracted. .
Get a pdf document or a offline reference database that can serve as a
processing report.
Consider that we speak about 1000 images, then consider the format that
allows fast consultation (pdf too heavy?) And an alphanumeric string search.

Clearly I'm available for any further explanation.
nuvolab...@gmail.com
I would need a quote with splitted times and costs for the two scripts.
Thank you.
S.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 2/2] ffmpeg: Made execution of reap_filters() more deterministic with respect to when headers are written

2017-05-26 Thread James Almer
On 5/13/2017 3:44 AM, wm4 wrote:
> On Fri, 12 May 2017 13:28:14 -0700
> Aaron Levinson  wrote:
> 
>> Purpose: Made execution of reap_filters() more deterministic with
>> respect to when headers are written in relationship with the
>> initialization of output streams and the processing of input audio
>> and/or video frames.  This change fixes a bug in ffmpeg encountered
>> when decoding interlaced video.  In some cases, ffmpeg would treat it
>> as progressive.
>>
>> Detailed description of problem: Run the following command: "ffmpeg -i
>> test8_1080i.h264 -c:v mpeg2video test8_1080i_mp2.ts".  In this case,
>> test8_1080i.h264 is an H.264-encoded file with 1080i59.94
>> (interlaced).  Prior to the patch, the following output is generated:
>>
>> Input #0, h264, from 'test8_1080i.h264':
>>   Duration: N/A, bitrate: N/A
>> Stream #0:0: Video: h264 (High), yuv420p(top first), 1920x1080 [SAR 1:1 
>> DAR 16:9], 29.97 fps, 29.97 tbr, 1200k tbn, 59.94 tbc
>> Stream mapping:
>>   Stream #0:0 -> #0:0 (h264 (native) -> mpeg2video (native))
>> Press [q] to stop, [?] for help
>> Output #0, mpegts, to 'test8_1080i_mp2_2.ts':
>>   Metadata:
>> encoder : Lavf57.72.100
>> Stream #0:0: Video: mpeg2video (Main), yuv420p, 1920x1080 [SAR 1:1 DAR 
>> 16:9], q=2-31, 200 kb/s, 29.97 fps, 90k tbn, 29.97 tbc
>> Metadata:
>>   encoder : Lavc57.92.100 mpeg2video
>>
>> which demonstrates the bug.  The output stream should instead look like:
>>
>> Stream #0:0: Video: mpeg2video (Main), yuv420p(top coded first 
>> (swapped)), 1920x1080 [SAR 1:1 DAR 16:9], q=2-31, 200 kb/s, 29.97 fps, 90k 
>> tbn, 29.97 tbc
>>
>> The bug is caused by the fact that reap_filters() calls
>> init_output_stream(), which is then immediately followed by a call to
>> check_init_output_file(), and this is all done prior to the first call
>> to do_video_out().  An initial call to do_video_out() is necessary to
>> populate the interlaced video information to the output stream's
>> codecpar (mux_par->field_order in do_video_out()).  However,
>> check_init_output_file() calls avformat_write_header() prior to the
>> initial call to do_video_out(), so field_order is populated too late,
>> and the header is written with the default field_order value,
>> progressive.
>>
>> Signed-off-by: Aaron Levinson 
>> ---
>>  ffmpeg.c | 43 ---
>>  1 file changed, 40 insertions(+), 3 deletions(-)
>>
>> diff --git a/ffmpeg.c b/ffmpeg.c
>> index 3cd45ba665..7b044b068c 100644
>> --- a/ffmpeg.c
>> +++ b/ffmpeg.c
>> @@ -1434,6 +1434,7 @@ static int reap_filters(int flush)
>>  AVFilterContext *filter;
>>  AVCodecContext *enc = ost->enc_ctx;
>>  int ret = 0;
>> +int do_check_init_output_file = 0;
>>  
>>  if (!ost->filter || !ost->filter->graph->graph)
>>  continue;
>> @@ -1448,9 +1449,7 @@ static int reap_filters(int flush)
>>  exit_program(1);
>>  }
>>  
>> -ret = check_init_output_file(of, ost->file_index);
>> -if (ret < 0)
>> -exit_program(1);
>> +do_check_init_output_file = 1;
>>  }
>>  
>>  if (!ost->filtered_frame && !(ost->filtered_frame = 
>> av_frame_alloc())) {
>> @@ -1526,6 +1525,44 @@ static int reap_filters(int flush)
>>  }
>>  
>>  av_frame_unref(filtered_frame);
>> +
>> +/*
>> + * It is important to call check_init_output_file() here, after
>> + * do_video_out() was called, instead of in 
>> init_output_stream(),
>> + * as was done previously.
>> + * If called from init_output_stream(), it is possible that not
>> + * everything will have been fully initialized by the time that
>> + * check_init_output_file() is called, and if
>> + * check_init_output_file() determines that all output streams
>> + * have been initialized, it will write the header.  An example
>> + * of initialization that depends on do_video_out() being called
>> + * at least once is the specification of interlaced video, which
>> + * happens in do_video_out().  This is particularly relevant 
>> when
>> + * decoding--without processing a video frame, the interlaced
>> + * video setting is not propagated before the header is written,
>> + * and that causes problems.
>> + * TODO:  should probably handle interlaced video differently
>> + * and not depend on it being setup in do_video_out().  Another
>> + * solution would be to set it up once by examining the input
>> + * header.
>> + */
>> +if (do_check_init_output_file) {
>> +ret = check_init_output_file(of, ost->file_index);
>> +if (ret < 0)
>> +exit_program(1);
>> +   

Re: [FFmpeg-devel] [PATCH] avcodec/fft_template: Fix multiple runtime error: signed integer overflow: -1943918714 - 1935113003 cannot be represented in type 'int'

2017-05-26 Thread Rostislav Pehlivanov
On 26 May 2017 at 12:21, wm4  wrote:

> On Thu, 25 May 2017 16:10:49 +0200
> Michael Niedermayer  wrote:
>
> > Fixes: 1735/clusterfuzz-testcase-minimized-5350472347025408
> >
> > Found-by: continuous fuzzing process https://github.com/google/oss-
> fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer 
> > ---
> >  libavcodec/fft_template.c | 50 +++---
> -
> >  1 file changed, 25 insertions(+), 25 deletions(-)
> >
> > diff --git a/libavcodec/fft_template.c b/libavcodec/fft_template.c
> > index 480557f49f..e3a37e5d69 100644
> > --- a/libavcodec/fft_template.c
> > +++ b/libavcodec/fft_template.c
> > @@ -249,7 +249,7 @@ static void fft_calc_c(FFTContext *s, FFTComplex *z)
> {
> >
> >  int nbits, i, n, num_transforms, offset, step;
> >  int n4, n2, n34;
> > -FFTSample tmp1, tmp2, tmp3, tmp4, tmp5, tmp6, tmp7, tmp8;
> > +SUINT tmp1, tmp2, tmp3, tmp4, tmp5, tmp6, tmp7, tmp8;
>
> I want this SUINT thing gone, not have more of it.
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

I agree, especially here.
Overflows should be left to happen in transforms if the input is corrupt.
Codecs are designed such that transforms won't overflow unless corrupt data
is fed. We allow for that to happen already (in the VP9 DCTs), so FFTs
shouldn't be excluded.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 2/4] h264dec: be more explicit in handling container cropping

2017-05-26 Thread James Almer
On 5/20/2017 1:55 PM, James Almer wrote:
> On 5/11/2017 9:56 AM, Michael Niedermayer wrote:
>> On Wed, May 10, 2017 at 11:06:52PM -0300, James Almer wrote:
>>> On 5/10/2017 9:55 PM, Michael Niedermayer wrote:
 On Wed, May 10, 2017 at 10:41:30AM -0300, James Almer wrote:
> On 5/9/2017 11:56 PM, Michael Niedermayer wrote:
>> On Mon, May 08, 2017 at 03:46:23PM -0300, James Almer wrote:
>>> From: Anton Khirnov 
>>>
>>> The current condition can trigger in cases where it shouldn't, with
>>> unexpected results.
>>> Make sure that:
>>> - container cropping is really based on the original dimensions from the
>>>   caller
>>> - those dimenions are discarded on size change
>>>
>>> The code is still quite hacky and eventually should be deprecated and
>>> removed, with the decision about which cropping is used delegated to the
>>> caller.
>>> ---
>>> This merges commit 4fded0480f20f4d7ca5e776a85574de34dfead14 from libav
>>>
>>>  libavcodec/h264_slice.c | 20 +---
>>>  libavcodec/h264dec.c|  3 +++
>>>  libavcodec/h264dec.h|  5 +
>>>  3 files changed, 21 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c
>>> index acf6a73f60..a7916e09ce 100644
>>> --- a/libavcodec/h264_slice.c
>>> +++ b/libavcodec/h264_slice.c
>>> @@ -378,6 +378,8 @@ int ff_h264_update_thread_context(AVCodecContext 
>>> *dst,
>>>  h->avctx->coded_width   = h1->avctx->coded_width;
>>>  h->avctx->width = h1->avctx->width;
>>>  h->avctx->height= h1->avctx->height;
>>> +h->width_from_caller= h1->width_from_caller;
>>> +h->height_from_caller   = h1->height_from_caller;
>>>  h->coded_picture_number = h1->coded_picture_number;
>>>  h->first_field  = h1->first_field;
>>>  h->picture_structure= h1->picture_structure;
>>
>>> @@ -874,13 +876,17 @@ static int init_dimensions(H264Context *h)
>>>  av_assert0(sps->crop_top + sps->crop_bottom < (unsigned)h->height);
>>>  
>>>  /* handle container cropping */
>>> -if (FFALIGN(h->avctx->width,  16) == FFALIGN(width,  16) &&
>>> -FFALIGN(h->avctx->height, 16) == FFALIGN(height, 16) &&
>>> -h->avctx->width  <= width &&
>>> -h->avctx->height <= height
>>> -) {
>>> -width  = h->avctx->width;
>>> -height = h->avctx->height;
>>> +if (h->width_from_caller > 0 && h->height_from_caller > 0 &&
>>> +!sps->crop_top && !sps->crop_left &&
>>> +FFALIGN(h->width_from_caller,  16) == FFALIGN(width,  16) &&
>>> +FFALIGN(h->height_from_caller, 16) == FFALIGN(height, 16) &&
>>> +h->width_from_caller  <= width &&
>>> +h->height_from_caller <= height) {
>>> +width  = h->width_from_caller;
>>> +height = h->height_from_caller;
>>> +} else {
>>> +h->width_from_caller  = 0;
>>> +h->height_from_caller = 0;
>>>  }
>>
>> With this, seeking in a file could affect if croping is used
>> would something break if croping was unaffected by what was priorly
>> decoded ?
>
> I don't know. Do you have a test case where this could break that i can
> check?

 no, it was just an thought that came to my mind when looking at the
 code. I dont remember seeing this break anything, it changed some
 files output though unless these were caused by another patch i had
 locally
>>>
>>> Could you try to confirm they weren't changed by this patch? Unless i'm
>>> reading it wrong, this set afaik isn't supposed to change the output of
>>> the decoder (at least not negatively), as reflected by fate, just move
>>> the cropping logic to decode.c
>>
>> I retested, it was
>> [3/4] h264dec: export cropping information instead of handling it internally
>> that caused the changes
>> changes seen are with CVFC1_Sony_C.jsv and tickets/4274/sample.ts
>>
>> 4247 needs "-threads 1  -flags2 showall -ss 7" for showin the
>> difference, the sony file shows a difference with just plain default
>> reencoding to avi
>>
>> Our fate test doesnt change, i guess due to -flags unaligned in it
>>
>> thx
> 
> CVFC1_Sony_C.jsv is fixed now that the cropping logic works correctly.
> tickets/4274/sample.ts still shows the difference, but it changes
> garbage output with slightly different, less ugly but still garbage output.
> 
> Old: http://0x0.st/ghF.png
> New: http://0x0.st/ghC.png
> 
> Unless this can be reproduced with negative effects with a sane file and
> not with a badly cut mpegts stream with missing references that requires
> seeking and a some specific flags, i'm inclined to not consider it worth
> bothering with.
> 
> I'll be pushing the set sometime next week if no other 

Re: [FFmpeg-devel] [PATCH] Disable MSA for big-endian mips cpu

2017-05-26 Thread Shivraj Patil

The current upstreamed code has been written and tested for Little Endian 
systems.
We do have plans to add the Big Endian support in near future, but till that 
time, need to disable all to avoid its usage and failures.

-Original Message-
From: Michael Niedermayer [mailto:mich...@niedermayer.cc] 
Sent: 26 May 2017 19:13
To: FFmpeg development discussions and patches
Cc: Shivraj Patil
Subject: Re: [FFmpeg-devel] [PATCH] Disable MSA for big-endian mips cpu

On Fri, May 26, 2017 at 03:40:20PM +0200, Michael Niedermayer wrote:
> On Fri, May 26, 2017 at 04:08:55PM +0530, shivraj.pa...@imgtec.com wrote:
> > From: Shivraj Patil 
> > 
> > Signed-off-by: Shivraj Patil 
> > ---
> >  libavcodec/mips/Makefile|2 ++
> >  libavcodec/mips/blockdsp_init_mips.c|8 
> >  libavcodec/mips/h263dsp_init_mips.c |8 
> >  libavcodec/mips/h264chroma_init_mips.c  |8 
> >  libavcodec/mips/h264dsp_init_mips.c |8 
> >  libavcodec/mips/h264pred_init_mips.c|8 
> >  libavcodec/mips/h264qpel_init_mips.c|8 
> >  libavcodec/mips/hevcdsp_init_mips.c |8 
> >  libavcodec/mips/hevcpred_init_mips.c|8 
> >  libavcodec/mips/hpeldsp_init_mips.c |8 
> >  libavcodec/mips/idctdsp_init_mips.c |8 
> >  libavcodec/mips/me_cmp_init_mips.c  |8 
> >  libavcodec/mips/mpegvideo_init_mips.c   |8 
> >  libavcodec/mips/mpegvideoencdsp_init_mips.c |8 
> >  libavcodec/mips/pixblockdsp_init_mips.c |8 
> >  libavcodec/mips/qpeldsp_init_mips.c |8 
> >  libavcodec/mips/vp8dsp_init_mips.c  |8 
> >  libavcodec/mips/vp9dsp_init_mips.c  |8 
> >  18 files changed, 70 insertions(+), 68 deletions(-)
> 
> Why does none of this code work on big endian mips ?
> 
> Is it difficult to make it work ?
> 
> Is it certain that the "disabled" code does not work on big endian 
> mips ?
> 
> Is it known that the reason for it not working is the endianness or 
> could it be a unrelated issue that makes it work on neither endianness?

and i forgot the CC, so repling with CC (sorry)

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The misfortune of the wise is better than the prosperity of the fool.
-- Epicurus
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] Disable MSA for big-endian mips cpu

2017-05-26 Thread Michael Niedermayer
On Fri, May 26, 2017 at 03:40:20PM +0200, Michael Niedermayer wrote:
> On Fri, May 26, 2017 at 04:08:55PM +0530, shivraj.pa...@imgtec.com wrote:
> > From: Shivraj Patil 
> > 
> > Signed-off-by: Shivraj Patil 
> > ---
> >  libavcodec/mips/Makefile|2 ++
> >  libavcodec/mips/blockdsp_init_mips.c|8 
> >  libavcodec/mips/h263dsp_init_mips.c |8 
> >  libavcodec/mips/h264chroma_init_mips.c  |8 
> >  libavcodec/mips/h264dsp_init_mips.c |8 
> >  libavcodec/mips/h264pred_init_mips.c|8 
> >  libavcodec/mips/h264qpel_init_mips.c|8 
> >  libavcodec/mips/hevcdsp_init_mips.c |8 
> >  libavcodec/mips/hevcpred_init_mips.c|8 
> >  libavcodec/mips/hpeldsp_init_mips.c |8 
> >  libavcodec/mips/idctdsp_init_mips.c |8 
> >  libavcodec/mips/me_cmp_init_mips.c  |8 
> >  libavcodec/mips/mpegvideo_init_mips.c   |8 
> >  libavcodec/mips/mpegvideoencdsp_init_mips.c |8 
> >  libavcodec/mips/pixblockdsp_init_mips.c |8 
> >  libavcodec/mips/qpeldsp_init_mips.c |8 
> >  libavcodec/mips/vp8dsp_init_mips.c  |8 
> >  libavcodec/mips/vp9dsp_init_mips.c  |8 
> >  18 files changed, 70 insertions(+), 68 deletions(-)
> 
> Why does none of this code work on big endian mips ?
> 
> Is it difficult to make it work ?
> 
> Is it certain that the "disabled" code does not work on
> big endian mips ?
> 
> Is it known that the reason for it not working is the endianness or
> could it be a unrelated issue that makes it work on neither endianness?

and i forgot the CC, so repling with CC (sorry)

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The misfortune of the wise is better than the prosperity of the fool.
-- Epicurus


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


Re: [FFmpeg-devel] [PATCH] Disable MSA for big-endian mips cpu

2017-05-26 Thread Michael Niedermayer
On Fri, May 26, 2017 at 04:08:55PM +0530, shivraj.pa...@imgtec.com wrote:
> From: Shivraj Patil 
> 
> Signed-off-by: Shivraj Patil 
> ---
>  libavcodec/mips/Makefile|2 ++
>  libavcodec/mips/blockdsp_init_mips.c|8 
>  libavcodec/mips/h263dsp_init_mips.c |8 
>  libavcodec/mips/h264chroma_init_mips.c  |8 
>  libavcodec/mips/h264dsp_init_mips.c |8 
>  libavcodec/mips/h264pred_init_mips.c|8 
>  libavcodec/mips/h264qpel_init_mips.c|8 
>  libavcodec/mips/hevcdsp_init_mips.c |8 
>  libavcodec/mips/hevcpred_init_mips.c|8 
>  libavcodec/mips/hpeldsp_init_mips.c |8 
>  libavcodec/mips/idctdsp_init_mips.c |8 
>  libavcodec/mips/me_cmp_init_mips.c  |8 
>  libavcodec/mips/mpegvideo_init_mips.c   |8 
>  libavcodec/mips/mpegvideoencdsp_init_mips.c |8 
>  libavcodec/mips/pixblockdsp_init_mips.c |8 
>  libavcodec/mips/qpeldsp_init_mips.c |8 
>  libavcodec/mips/vp8dsp_init_mips.c  |8 
>  libavcodec/mips/vp9dsp_init_mips.c  |8 
>  18 files changed, 70 insertions(+), 68 deletions(-)

Why does none of this code work on big endian mips ?

Is it difficult to make it work ?

Is it certain that the "disabled" code does not work on
big endian mips ?

Is it known that the reason for it not working is the endianness or
could it be a unrelated issue that makes it work on neither endianness?

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

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


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


Re: [FFmpeg-devel] [PATCH] Disable MSA for big-endian mips cpu

2017-05-26 Thread Shivraj Patil

Is this on top of the configure patch?
Shivraj: No, this is complete new patch.

I'm a little confused. It seems the configure patch would be much simpler, no?
Shivraj: This patch is according to michael’s suggestion.


From: Ronald S. Bultje [mailto:rsbul...@gmail.com]
Sent: 26 May 2017 17:26
To: FFmpeg development discussions and patches
Cc: Shivraj Patil
Subject: Re: [FFmpeg-devel] [PATCH] Disable MSA for big-endian mips cpu

Hi,

On Fri, May 26, 2017 at 6:38 AM, 
> wrote:
From: Shivraj Patil >

Signed-off-by: Shivraj Patil 
>
---
 libavcodec/mips/Makefile|2 ++
 libavcodec/mips/blockdsp_init_mips.c|8 
 libavcodec/mips/h263dsp_init_mips.c |8 
 libavcodec/mips/h264chroma_init_mips.c  |8 
 libavcodec/mips/h264dsp_init_mips.c |8 
 libavcodec/mips/h264pred_init_mips.c|8 
 libavcodec/mips/h264qpel_init_mips.c|8 
 libavcodec/mips/hevcdsp_init_mips.c |8 
 libavcodec/mips/hevcpred_init_mips.c|8 
 libavcodec/mips/hpeldsp_init_mips.c |8 
 libavcodec/mips/idctdsp_init_mips.c |8 
 libavcodec/mips/me_cmp_init_mips.c  |8 
 libavcodec/mips/mpegvideo_init_mips.c   |8 
 libavcodec/mips/mpegvideoencdsp_init_mips.c |8 
 libavcodec/mips/pixblockdsp_init_mips.c |8 
 libavcodec/mips/qpeldsp_init_mips.c |8 
 libavcodec/mips/vp8dsp_init_mips.c  |8 
 libavcodec/mips/vp9dsp_init_mips.c  |8 
 18 files changed, 70 insertions(+), 68 deletions(-)

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


Re: [FFmpeg-devel] [PATCH] Disable MSA for big-endian mips cpu

2017-05-26 Thread Ronald S. Bultje
Hi,

On Fri, May 26, 2017 at 6:38 AM,  wrote:

> From: Shivraj Patil 
>
> Signed-off-by: Shivraj Patil 
> ---
>  libavcodec/mips/Makefile|2 ++
>  libavcodec/mips/blockdsp_init_mips.c|8 
>  libavcodec/mips/h263dsp_init_mips.c |8 
>  libavcodec/mips/h264chroma_init_mips.c  |8 
>  libavcodec/mips/h264dsp_init_mips.c |8 
>  libavcodec/mips/h264pred_init_mips.c|8 
>  libavcodec/mips/h264qpel_init_mips.c|8 
>  libavcodec/mips/hevcdsp_init_mips.c |8 
>  libavcodec/mips/hevcpred_init_mips.c|8 
>  libavcodec/mips/hpeldsp_init_mips.c |8 
>  libavcodec/mips/idctdsp_init_mips.c |8 
>  libavcodec/mips/me_cmp_init_mips.c  |8 
>  libavcodec/mips/mpegvideo_init_mips.c   |8 
>  libavcodec/mips/mpegvideoencdsp_init_mips.c |8 
>  libavcodec/mips/pixblockdsp_init_mips.c |8 
>  libavcodec/mips/qpeldsp_init_mips.c |8 
>  libavcodec/mips/vp8dsp_init_mips.c  |8 
>  libavcodec/mips/vp9dsp_init_mips.c  |8 
>  18 files changed, 70 insertions(+), 68 deletions(-)


Is this on top of the configure patch? I'm a little confused. It seems the
configure patch would be much simpler, no?

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


Re: [FFmpeg-devel] [PATCH] avfilter: add audio surround upmixer

2017-05-26 Thread Michael Niedermayer
On Fri, May 26, 2017 at 01:11:38PM +0200, Paul B Mahol wrote:
> On 5/26/17, Nicolas George  wrote:
> > Le septidi 7 prairial, an CCXXV, Paul B Mahol a ecrit :
> >> > This belongs in libswresample
> >> No it does not.
> >
> > I think it does too.
> 
> You want to link libswresample with libavcodec?

While this question was directed at nicolas ...

I dont think audio upmix code should depend on a lib of encoders and
decoders (libavcodec)
No matter if the upmix code would be in libavfilter or libswresample

I belive a temporary dependancy would be ok, if there is intend to
factor/move things to remove the dependancy in the future.

But IMO libavcodec is the wrong place to export generic FFT
functionality.
We need FFTs in codecs, we need them in filters, we need them in
swresample (the soxr resampler we support also uses a FFT internally)

Also moving FFT to a different lib should be quite easy compared to
other ugly dependancies we have (as in snow motion estimation, which
is not as easy to move. none the less none of these ugly dependancies
should be there except temporary)

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

Its not that you shouldnt use gotos but rather that you should write
readable code and code with gotos often but not always is less readable


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


Re: [FFmpeg-devel] [PATCH] avfilter: add audio surround upmixer

2017-05-26 Thread Hendrik Leppkes
On Fri, May 26, 2017 at 1:13 PM, Nicolas George  wrote:
> Le septidi 7 prairial, an CCXXV, Paul B Mahol a écrit :
>> You want to link libswresample with libavcodec?
>
> I want to merge all libraries, I do not make a secret of it. The
> internal separation is nothing but trouble. But API-wise, this feature
> belongs in resampling.
>

Implementation-wise, swresample has nothing of the required primitives
already, and as Paul pointed out it relies on the FFT from avcodec,
linking swresample to avcodec is a big no-no (a breaking change, and a
circular dependency since avcodec links to swresample already), while
avfilter does already link to avcodec for a variety of things.

Until such a day when everything becomes merged, if it ever happens,
there is several technical aspects that make it very impractical to
put this into swresample.

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


Re: [FFmpeg-devel] [PATCH] avcodec/fft_template: Fix multiple runtime error: signed integer overflow: -1943918714 - 1935113003 cannot be represented in type 'int'

2017-05-26 Thread wm4
On Thu, 25 May 2017 16:10:49 +0200
Michael Niedermayer  wrote:

> Fixes: 1735/clusterfuzz-testcase-minimized-5350472347025408
> 
> Found-by: continuous fuzzing process 
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/fft_template.c | 50 
> +++
>  1 file changed, 25 insertions(+), 25 deletions(-)
> 
> diff --git a/libavcodec/fft_template.c b/libavcodec/fft_template.c
> index 480557f49f..e3a37e5d69 100644
> --- a/libavcodec/fft_template.c
> +++ b/libavcodec/fft_template.c
> @@ -249,7 +249,7 @@ static void fft_calc_c(FFTContext *s, FFTComplex *z) {
>  
>  int nbits, i, n, num_transforms, offset, step;
>  int n4, n2, n34;
> -FFTSample tmp1, tmp2, tmp3, tmp4, tmp5, tmp6, tmp7, tmp8;
> +SUINT tmp1, tmp2, tmp3, tmp4, tmp5, tmp6, tmp7, tmp8;

I want this SUINT thing gone, not have more of it.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avcodec/htmlsubtitles: Be a bit more picky on syntax

2017-05-26 Thread wm4
On Thu, 25 May 2017 19:32:06 +0200
Michael Niedermayer  wrote:

> This reduces the number of strstr() calls per byte
> 
> Fixes timeout
> Fixes: 1817/clusterfuzz-testcase-minimized-5104230530547712
> 
> Found-by: continuous fuzzing process 
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/htmlsubtitles.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/libavcodec/htmlsubtitles.c b/libavcodec/htmlsubtitles.c
> index 16295daa0c..e69681f31c 100644
> --- a/libavcodec/htmlsubtitles.c
> +++ b/libavcodec/htmlsubtitles.c
> @@ -92,7 +92,7 @@ int ff_htmlmarkup_to_ass(void *log_ctx, AVBPrint *dst, 
> const char *in)
>  case '<':
>  tag_close = in[1] == '/';
>  len = 0;
> -if (sscanf(in+tag_close+1, "%127[^>]>%n", buffer, ) >= 1 && 
> len > 0) {
> +if (sscanf(in+tag_close+1, "%127[^<>]>%n", buffer, ) >= 1 && 
> len > 0) {
>  const char *tagname = buffer;
>  while (*tagname == ' ')
>  tagname++;
> @@ -149,7 +149,7 @@ int ff_htmlmarkup_to_ass(void *log_ctx, AVBPrint *dst, 
> const char *in)
>  }
>  } else if (tagname[0] && !tagname[1] && strspn(tagname, 
> "bisu") == 1) {
>  av_bprintf(dst, "{\\%c%d}", tagname[0], !tag_close);
> -} else {
> +} else if (*tagname) {
>  unknown = 1;
>  snprintf(tmp, sizeof(tmp), "", tagname);
>  }

Does it change semantics, and how?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avfilter: add audio surround upmixer

2017-05-26 Thread Nicolas George
Le septidi 7 prairial, an CCXXV, Paul B Mahol a écrit :
> You want to link libswresample with libavcodec?

I want to merge all libraries, I do not make a secret of it. The
internal separation is nothing but trouble. But API-wise, this feature
belongs in resampling.

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] avfilter: add audio surround upmixer

2017-05-26 Thread wm4
On Fri, 26 May 2017 13:08:10 +0200
Nicolas George  wrote:

> Le septidi 7 prairial, an CCXXV, Paul B Mahol a écrit :
> > > This belongs in libswresample  
> > No it does not.  
> 
> I think it does too.

There's plenty of precedent of not moving filters into libswresample or
libswscale, even if in theory it could have been done.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avfilter: add audio surround upmixer

2017-05-26 Thread Paul B Mahol
On 5/26/17, Nicolas George  wrote:
> Le septidi 7 prairial, an CCXXV, Paul B Mahol a ecrit :
>> > This belongs in libswresample
>> No it does not.
>
> I think it does too.

You want to link libswresample with libavcodec?

Besides libswresample is quite simple.
And point of this filter it to not be simple
at all or even to be some sort of standard.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avfilter: add audio surround upmixer

2017-05-26 Thread Nicolas George
Le septidi 7 prairial, an CCXXV, Paul B Mahol a écrit :
> > This belongs in libswresample
> No it does not.

I think it does too.

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] avfilter: add audio surround upmixer

2017-05-26 Thread Paul B Mahol
On 5/26/17, Michael Niedermayer  wrote:
> On Thu, May 25, 2017 at 04:45:46PM +0200, Paul B Mahol wrote:
>> Signed-off-by: Paul B Mahol 
>> ---
>>  doc/filters.texi  |  27 ++
>>  libavfilter/Makefile  |   1 +
>>  libavfilter/af_surround.c | 853
>> ++
>>  libavfilter/allfilters.c  |   1 +
>>  4 files changed, 882 insertions(+)
>>  create mode 100644 libavfilter/af_surround.c
>
> This belongs in libswresample

No it does not.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH] Disable MSA for big-endian mips cpu

2017-05-26 Thread shivraj.patil
From: Shivraj Patil 

Signed-off-by: Shivraj Patil 
---
 libavcodec/mips/Makefile|2 ++
 libavcodec/mips/blockdsp_init_mips.c|8 
 libavcodec/mips/h263dsp_init_mips.c |8 
 libavcodec/mips/h264chroma_init_mips.c  |8 
 libavcodec/mips/h264dsp_init_mips.c |8 
 libavcodec/mips/h264pred_init_mips.c|8 
 libavcodec/mips/h264qpel_init_mips.c|8 
 libavcodec/mips/hevcdsp_init_mips.c |8 
 libavcodec/mips/hevcpred_init_mips.c|8 
 libavcodec/mips/hpeldsp_init_mips.c |8 
 libavcodec/mips/idctdsp_init_mips.c |8 
 libavcodec/mips/me_cmp_init_mips.c  |8 
 libavcodec/mips/mpegvideo_init_mips.c   |8 
 libavcodec/mips/mpegvideoencdsp_init_mips.c |8 
 libavcodec/mips/pixblockdsp_init_mips.c |8 
 libavcodec/mips/qpeldsp_init_mips.c |8 
 libavcodec/mips/vp8dsp_init_mips.c  |8 
 libavcodec/mips/vp9dsp_init_mips.c  |8 
 18 files changed, 70 insertions(+), 68 deletions(-)

diff --git a/libavcodec/mips/Makefile b/libavcodec/mips/Makefile
index 797df09..563700f 100644
--- a/libavcodec/mips/Makefile
+++ b/libavcodec/mips/Makefile
@@ -38,6 +38,7 @@ OBJS-$(CONFIG_ME_CMP) += 
mips/me_cmp_init_mips.o
 OBJS-$(CONFIG_MPEG4_DECODER)  += mips/xvididct_init_mips.o
 OBJS-$(CONFIG_VC1DSP) += mips/vc1dsp_init_mips.o
 OBJS-$(CONFIG_WMV2DSP)+= mips/wmv2dsp_init_mips.o
+ifndef HAVE_BIGENDIAN
 MSA-OBJS-$(CONFIG_HEVC_DECODER)   += mips/hevcdsp_msa.o\
  mips/hevc_mc_uni_msa.o\
  mips/hevc_mc_uniw_msa.o   \
@@ -68,6 +69,7 @@ MSA-OBJS-$(CONFIG_IDCTDSP)+= 
mips/idctdsp_msa.o   \
 MSA-OBJS-$(CONFIG_MPEGVIDEO)  += mips/mpegvideo_msa.o
 MSA-OBJS-$(CONFIG_MPEGVIDEOENC)   += mips/mpegvideoencdsp_msa.o
 MSA-OBJS-$(CONFIG_ME_CMP) += mips/me_cmp_msa.o
+endif
 MMI-OBJS  += mips/constants.o
 MMI-OBJS-$(CONFIG_H264DSP)+= mips/h264dsp_mmi.o
 MMI-OBJS-$(CONFIG_H264CHROMA) += mips/h264chroma_mmi.o
diff --git a/libavcodec/mips/blockdsp_init_mips.c 
b/libavcodec/mips/blockdsp_init_mips.c
index 30ae95f..8243f8b 100644
--- a/libavcodec/mips/blockdsp_init_mips.c
+++ b/libavcodec/mips/blockdsp_init_mips.c
@@ -21,7 +21,7 @@
 
 #include "blockdsp_mips.h"
 
-#if HAVE_MSA
+#if HAVE_MSA && !HAVE_BIGENDIAN
 static av_cold void blockdsp_init_msa(BlockDSPContext *c)
 {
 c->clear_block = ff_clear_block_msa;
@@ -30,7 +30,7 @@ static av_cold void blockdsp_init_msa(BlockDSPContext *c)
 c->fill_block_tab[0] = ff_fill_block16_msa;
 c->fill_block_tab[1] = ff_fill_block8_msa;
 }
-#endif  // #if HAVE_MSA
+#endif  // #if HAVE_MSA && !HAVE_BIGENDIAN
 
 #if HAVE_MMI
 static av_cold void blockdsp_init_mmi(BlockDSPContext *c)
@@ -45,9 +45,9 @@ static av_cold void blockdsp_init_mmi(BlockDSPContext *c)
 
 void ff_blockdsp_init_mips(BlockDSPContext *c)
 {
-#if HAVE_MSA
+#if HAVE_MSA && !HAVE_BIGENDIAN
 blockdsp_init_msa(c);
-#endif  // #if HAVE_MSA
+#endif  // #if HAVE_MSA && !HAVE_BIGENDIAN
 #if HAVE_MMI
 blockdsp_init_mmi(c);
 #endif /* HAVE_MMI */
diff --git a/libavcodec/mips/h263dsp_init_mips.c 
b/libavcodec/mips/h263dsp_init_mips.c
index 09bd937..7c5d228 100644
--- a/libavcodec/mips/h263dsp_init_mips.c
+++ b/libavcodec/mips/h263dsp_init_mips.c
@@ -20,17 +20,17 @@
 
 #include "h263dsp_mips.h"
 
-#if HAVE_MSA
+#if HAVE_MSA && !HAVE_BIGENDIAN
 static av_cold void h263dsp_init_msa(H263DSPContext *c)
 {
 c->h263_h_loop_filter = ff_h263_h_loop_filter_msa;
 c->h263_v_loop_filter = ff_h263_v_loop_filter_msa;
 }
-#endif  // #if HAVE_MSA
+#endif  // #if HAVE_MSA && !HAVE_BIGENDIAN
 
 av_cold void ff_h263dsp_init_mips(H263DSPContext *c)
 {
-#if HAVE_MSA
+#if HAVE_MSA && !HAVE_BIGENDIAN
 h263dsp_init_msa(c);
-#endif  // #if HAVE_MSA
+#endif  // #if HAVE_MSA && !HAVE_BIGENDIAN
 }
diff --git a/libavcodec/mips/h264chroma_init_mips.c 
b/libavcodec/mips/h264chroma_init_mips.c
index 122148d..4cb04d4 100644
--- a/libavcodec/mips/h264chroma_init_mips.c
+++ b/libavcodec/mips/h264chroma_init_mips.c
@@ -21,7 +21,7 @@
 
 #include "h264chroma_mips.h"
 
-#if HAVE_MSA
+#if HAVE_MSA && !HAVE_BIGENDIAN
 static av_cold void h264chroma_init_msa(H264ChromaContext *c, int bit_depth)
 {
 const int high_bit_depth = bit_depth > 8;
@@ -36,7 +36,7 @@ static av_cold void h264chroma_init_msa(H264ChromaContext *c, 
int bit_depth)
 c->avg_h264_chroma_pixels_tab[2] = ff_avg_h264_chroma_mc2_msa;
 }
 }
-#endif  // #if HAVE_MSA
+#endif  // #if HAVE_MSA && !HAVE_BIGENDIAN
 
 #if HAVE_MMI
 static 

Re: [FFmpeg-devel] [PATCH] lavc/golomb: Fix UE golomb overwrite issue.

2017-05-26 Thread Carl Eugen Hoyos
2017-05-26 3:19 GMT+02:00 Jun Zhao :

Please explain how we can reproduce the issue you want to fix.

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


Re: [FFmpeg-devel] [PATCH] zscale: Add pixdesc-API compatible color names to filter options

2017-05-26 Thread Paul B Mahol
On 5/25/17, Vittorio Giovara  wrote:
> On Thu, May 25, 2017 at 2:53 PM, Vittorio Giovara
>  wrote:
>> Reviewed-by: Paul B Mahol 
>> Signed-off-by: Vittorio Giovara 
>> ---
>>  libavfilter/vf_zscale.c | 22 +-
>>  1 file changed, 21 insertions(+), 1 deletion(-)

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


Re: [FFmpeg-devel] [PATCH] zscale: Add range options aliases to match scale ones

2017-05-26 Thread Paul B Mahol
On 5/25/17, Vittorio Giovara  wrote:
> ---
>  libavfilter/vf_zscale.c | 2 ++
>  1 file changed, 2 insertions(+)
>

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


Re: [FFmpeg-devel] [PATCH] minterpolate: added codec_me_mode

2017-05-26 Thread Paul B Mahol
On 5/26/17, Michael Niedermayer  wrote:
> On Mon, May 08, 2017 at 07:40:25PM +, Davinder Singh wrote:
>> hi,
>>
>> On Mon, Apr 24, 2017 at 9:43 PM Paul B Mahol  wrote:
>>
>> > On 4/24/17, Davinder Singh  wrote:
>> > > Patch attached.
>> > >
>> >
>> > So this encodes video frames to generate motion vectors?
>> >
>>
>> yes. it significantly improves the frame quality. can please you test it?
>>
>> the filter will be made independent of libavcodec.
>
> In the absence of further comments, i intend to apply this patch
> (unless i spot some issue)

Shouldn't this be applied after its extracted from snow encoder?

Having filter to encode video just to get motion vectors seems quite bad for me.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avfilter: add audio surround upmixer

2017-05-26 Thread Paul B Mahol
On 5/26/17, Carl Eugen Hoyos  wrote:
> 2017-05-25 22:39 GMT+02:00 Paul B Mahol :
>> On 5/25/17, Carl Eugen Hoyos  wrote:
>>> 2017-05-25 16:45 GMT+02:00 Paul B Mahol :
>>>
 +@section surround
 +Apply audio surround upmix filter.
 +
 +This filter allows to produce multichannel output from
 stereo audio stream.
>>>
>>> Does this work on Pro Logic-encoded files (ticket #4085)
>>> or does this filter use a different algorithm?
>>
>> It works with sample from that ticket.
>
>> It uses own algorithm.
>
> Is it related to one of the algorithms listed here?
> https://en.wikipedia.org/wiki/Matrix_decoder
> It may make sense to mention the used algorithm in the
> documentation (or the source code).

All those are passive algorithms, while this one is active.

>
>>> Does a filter have advantages over an implementation in
>>> libswresample?
>>
>> libswresample doesnt do this.
>
> Yes.
> My question was if your implementation shouldn't be
> part of libswresample, or rather what would be the
> disadvantage.

This filter relies on FFT, and it should probably stay in libavfilter library.

>
>>> Iirc, our Pro Logic downmixer has an issue with "phase shift"
>>> (ticket 3455 / 4175): Does this filter give expected output
>>> for FFmpeg-encoded files or original Pro Logic files (or is
>>> there no difference on upmixing)?
>>
>> FFmpeg downmixing, while certainly not correct, is giving
>> reasonable output when used with this filter.
>
> But it works better for Pro Logic files?

I only have one such file, and its not that useful.

FFmpeg have only downmixing from 5.1 layout to stereo.

I also plan to add 5.1 to 7.1 upmixing.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel