Re: [FFmpeg-devel] [PATCH] ffprobe: do not exit in case the demuxer returns AVERROR(EAGAIN)

[Nicolas George]

Le decadi 30 brumaire, an CCXXIV, Stefano Sabatini a écrit :
> Yes, updated patch with EAGAIN (if this is fine, I'll send patches to
> fix the examples).

The API use seems... not good, because it is not possible, but at least ok.

> In my case the EAGAIN is triggered by the flv demuxer code in
> flvdec.c:935:

> Not sure if this is a defect.

It is; I pointed it out a few days ago but had not yet had time to post a
follow-up.

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] [PATCHv2] avformat/mp3dec, rmdec: check return value of ffio_ensure_seekback

[Ganesh Ajjanagadde]

On Fri, Nov 20, 2015 at 9:30 AM, Ronald S. Bultje  wrote:
> Hi Ganesh,
>
> On Fri, Nov 20, 2015 at 8:42 AM, Ganesh Ajjanagadde 
> wrote:
>>
>> On Wed, Nov 18, 2015 at 8:26 AM, Ganesh Ajjanagadde 
>> wrote:
>> > On Wed, Nov 18, 2015 at 7:31 AM, wm4  wrote:
>> >> On Tue, 17 Nov 2015 17:39:31 -0500
>> >> Ganesh Ajjanagadde  wrote:
>> >>
>> >>> ffio_ensure_seekback can fail due to e.g ENOMEM. This return value is
>> >>> checked here and a diagnostic is logged.
>> >>>
>> >>> All usage of ffio_ensure_seekback in the codebase now has the return
>> >>> value checked.
>> >>>
>> >>> Signed-off-by: Ganesh Ajjanagadde 
>> >>> ---
>> >>>  libavformat/mp3dec.c | 6 --
>> >>>  libavformat/rmdec.c  | 3 ++-
>> >>>  2 files changed, 6 insertions(+), 3 deletions(-)
>> >>>
>> >>> diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
>> >>> index 32ca00c..a14bccd 100644
>> >>> --- a/libavformat/mp3dec.c
>> >>> +++ b/libavformat/mp3dec.c
>> >>> @@ -380,11 +380,13 @@ static int mp3_read_header(AVFormatContext *s)
>> >>>  uint32_t header, header2;
>> >>>  int frame_size;
>> >>>  if (!(i&1023))
>> >>> -ffio_ensure_seekback(s->pb, i + 1024 + 4);
>> >>> +if ((ret = ffio_ensure_seekback(s->pb, i + 1024 + 4)) <
>> >>> 0)
>> >>> +av_log(s, AV_LOG_WARNING, "ffio_ensure_seekback():
>> >>> %s\n", av_err2str(ret));
>> >>>  frame_size = check(s->pb, off + i, );
>> >>>  if (frame_size > 0) {
>> >>>  avio_seek(s->pb, off, SEEK_SET);
>> >>> -ffio_ensure_seekback(s->pb, i + 1024 + frame_size + 4);
>> >>> +if ((ret = ffio_ensure_seekback(s->pb, i + 1024 +
>> >>> frame_size + 4)) < 0)
>> >>> +av_log(s, AV_LOG_WARNING, "ffio_ensure_seekback():
>> >>> %s\n", av_err2str(ret));
>> >>>  if (check(s->pb, off + i + frame_size, ) >= 0 &&
>> >>>  (header & SAME_HEADER_MASK) == (header2 &
>> >>> SAME_HEADER_MASK))
>> >>>  {
>> >>> diff --git a/libavformat/rmdec.c b/libavformat/rmdec.c
>> >>> index 4ec78ef..1cf0548 100644
>> >>> --- a/libavformat/rmdec.c
>> >>> +++ b/libavformat/rmdec.c
>> >>> @@ -576,7 +576,8 @@ static int rm_read_header(AVFormatContext *s)
>> >>>  size = avio_rb32(pb);
>> >>>  codec_pos = avio_tell(pb);
>> >>>
>> >>> -ffio_ensure_seekback(pb, 4);
>> >>> +if ((ret = ffio_ensure_seekback(pb, 4)) < 0)
>> >>> +av_log(s, AV_LOG_WARNING, "ffio_ensure_seekback():
>> >>> %s\n", av_err2str(ret));
>> >>>  v = avio_rb32(pb);
>> >>>  if (v == MKBETAG('M', 'L', 'T', 'I')) {
>> >>>  int number_of_streams = avio_rb16(pb);
>> >>
>> >> So why do you not just add the message to the function itself?
>> >
>> > Because a client may not like the generic message, see e.g the message
>> > printed in avformat/apngdec. Handling of the error may vary from
>> > context to context, again see the apngdec example where the client
>> > does something else in addition to error printing.
>> >
>> >>
>> >> I also question the usefulness of the message. In such cases of OOM
>> >> everything goes to hell anyway. No reason to bloat the code with error
>> >> printing.
>> >
>> > Taking that logic a bit further, why bother returning ENOMEM? If the
>> > ENOMEM being returned by ensure_seekback is not checked, the
>> > information is just silently discarded or garbled with further errors
>> > down the road.
>> >
>> > Anyway, I do not consider it a bloat: the buffer allocated can be big
>> > (see in mp3dec > 1024 bytes). A user has a right to know when things
>> > "go to hell". Envisioning what may happen, ffmpeg crashes due to OOM,
>> > upon which a coredump takes place. That crash happens in a somewhat
>> > random location, since OOM handling is a heuristic and determining
>> > which process should be killed is an intractable problem. Thus at the
>> > time when the core dump is written out, it is not guaranteed that the
>> > context is the one where the allocation failed first. Then there are
>> > issues of overcommit where malloc may return a non-NULL pointer but
>> > OOM happens upon a dereference. Such issues are beyond the scope of
>> > error handling, as nothing can be done in such cases anyway.
>> >
>> > Warning the user when a reasonably large allocation failed in my view
>> > is not bloat. It is giving them as much information as we can. Also
>> > think of it from a debugging perspective: it can help us find places
>> > where the alloc size was unreasonable.
>>
>> Another reason related to the randomness of the process killed for
>> OOM: ffmpeg/some other client may not be the process killed when OOM
>> occurs, and the malloc's here fail. Thus, seek back, something a user
>> does care about, fails, but without the error message, there is no
>> information whatsoever that such 

Re: [FFmpeg-devel] [PATCH] avfilter/all: handle ff_formats_unref correctly

[Ganesh Ajjanagadde]

On Sun, Nov 15, 2015 at 7:21 PM, Ganesh Ajjanagadde
 wrote:
> Recent commits 6aaac24d72a7da631173209841a3944fcb4a3309 and
> 3835554bf8ed78539a3492c239f979c0ab03a15f made progress towards cleaning
> up usage of the formats API, and in particular fixed possible NULL pointer
> dereferences.
>
> This commit addresses the issue of possible resource leaks when one call
> fails and the others don't.
>
> Fixes: CID 1338330, 1338329, 1338327, 1338326.
>
> Signed-off-by: Ganesh Ajjanagadde 
> ---
>  libavfilter/af_channelmap.c |  7 +--
>  libavfilter/vf_alphamerge.c |  7 +--
>  libavfilter/vf_overlay.c| 30 +-
>  3 files changed, 31 insertions(+), 13 deletions(-)
>
> diff --git a/libavfilter/af_channelmap.c b/libavfilter/af_channelmap.c
> index 9e95a98..45000d0 100644
> --- a/libavfilter/af_channelmap.c
> +++ b/libavfilter/af_channelmap.c
> @@ -296,8 +296,11 @@ static int channelmap_query_formats(AVFilterContext *ctx)
>  (ret = ff_set_common_formats (ctx , 
> ff_planar_sample_fmts() )) < 0 ||
>  (ret = ff_set_common_samplerates (ctx , 
> ff_all_samplerates())) < 0 ||
>  (ret = ff_channel_layouts_ref(layouts , 
> >inputs[0]->out_channel_layouts)) < 0 ||
> -(ret = ff_channel_layouts_ref(channel_layouts , 
> >outputs[0]->in_channel_layouts)) < 0)
> -return ret;
> +(ret = ff_channel_layouts_ref(channel_layouts , 
> >outputs[0]->in_channel_layouts)) < 0) {
> +ff_channel_layouts_unref(_layouts);
> +ff_channel_layouts_unref();
> +return ret;
> +}
>
>  return 0;
>  }
> diff --git a/libavfilter/vf_alphamerge.c b/libavfilter/vf_alphamerge.c
> index 8a1ca22..dcb7f6c 100644
> --- a/libavfilter/vf_alphamerge.c
> +++ b/libavfilter/vf_alphamerge.c
> @@ -65,8 +65,11 @@ static int query_formats(AVFilterContext *ctx)
>  return AVERROR(ENOMEM);
>  if ((ret = ff_formats_ref(main_formats , >inputs[0]->out_formats)) 
> < 0 ||
>  (ret = ff_formats_ref(alpha_formats, >inputs[1]->out_formats)) 
> < 0 ||
> -(ret = ff_formats_ref(main_formats , >outputs[0]->in_formats)) 
> < 0)
> -return ret;
> +(ret = ff_formats_ref(main_formats , >outputs[0]->in_formats)) 
> < 0) {
> +ff_formats_unref(_formats);
> +ff_formats_unref(_formats);
> +return ret;
> +}
>  return 0;
>  }
>
> diff --git a/libavfilter/vf_overlay.c b/libavfilter/vf_overlay.c
> index 3c61731..4873775 100644
> --- a/libavfilter/vf_overlay.c
> +++ b/libavfilter/vf_overlay.c
> @@ -252,23 +252,31 @@ static int query_formats(AVFilterContext *ctx)
>  switch (s->format) {
>  case OVERLAY_FORMAT_YUV420:
>  if (!(main_formats= ff_make_format_list(main_pix_fmts_yuv420)) ||
> -!(overlay_formats = 
> ff_make_format_list(overlay_pix_fmts_yuv420)))
> -return AVERROR(ENOMEM);
> +!(overlay_formats = 
> ff_make_format_list(overlay_pix_fmts_yuv420))) {
> +ret = AVERROR(ENOMEM);
> +goto fail;
> +}
>  break;
>  case OVERLAY_FORMAT_YUV422:
>  if (!(main_formats= ff_make_format_list(main_pix_fmts_yuv422)) ||
> -!(overlay_formats = 
> ff_make_format_list(overlay_pix_fmts_yuv422)))
> -return AVERROR(ENOMEM);
> +!(overlay_formats = 
> ff_make_format_list(overlay_pix_fmts_yuv422))) {
> +ret = AVERROR(ENOMEM);
> +goto fail;
> +}
>  break;
>  case OVERLAY_FORMAT_YUV444:
>  if (!(main_formats= ff_make_format_list(main_pix_fmts_yuv444)) ||
> -!(overlay_formats = 
> ff_make_format_list(overlay_pix_fmts_yuv444)))
> -return AVERROR(ENOMEM);
> +!(overlay_formats = 
> ff_make_format_list(overlay_pix_fmts_yuv444))) {
> +ret = AVERROR(ENOMEM);
> +goto fail;
> +}
>  break;
>  case OVERLAY_FORMAT_RGB:
>  if (!(main_formats= ff_make_format_list(main_pix_fmts_rgb)) ||
> -!(overlay_formats = ff_make_format_list(overlay_pix_fmts_rgb)))
> -return AVERROR(ENOMEM);
> +!(overlay_formats = ff_make_format_list(overlay_pix_fmts_rgb))) {
> +ret = AVERROR(ENOMEM);
> +goto fail;
> +}
>  break;
>  default:
>  av_assert0(0);
> @@ -277,9 +285,13 @@ static int query_formats(AVFilterContext *ctx)
>  if ((ret = ff_formats_ref(main_formats   , 
> >inputs[MAIN]->out_formats   )) < 0 ||
>  (ret = ff_formats_ref(overlay_formats, 
> >inputs[OVERLAY]->out_formats)) < 0 ||
>  (ret = ff_formats_ref(main_formats   , 
> >outputs[MAIN]->in_formats   )) < 0)
> -return ret;
> +goto fail;
>
>  return 0;
> +fail:
> +ff_formats_unref(_formats);
> +

Re: [FFmpeg-devel] [PATCH 8/8] avcodec/resample2: replace #define by typedef

[Ganesh Ajjanagadde]

On Thu, Nov 19, 2015 at 9:30 AM, Michael Niedermayer  wrote:
> On Thu, Nov 19, 2015 at 08:14:15AM -0500, Ganesh Ajjanagadde wrote:
>> See e.g 
>> https://stackoverflow.com/questions/1666353/are-typedef-and-define-the-same-in-c
>> for rationale.
>>
>> Signed-off-by: Ganesh Ajjanagadde 
>> ---
>>  libavcodec/resample2.c | 12 ++--
>>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> LGTM
>
> thx

pushed, thanks

>
> [...]
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Observe your enemies, for they first find out your faults. -- Antisthenes
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCHv2] avformat/mp3dec, rmdec: check return value of ffio_ensure_seekback

[Ronald S. Bultje]

Hi Ganesh,

On Fri, Nov 20, 2015 at 8:42 AM, Ganesh Ajjanagadde 
wrote:

> On Wed, Nov 18, 2015 at 8:26 AM, Ganesh Ajjanagadde 
> wrote:
> > On Wed, Nov 18, 2015 at 7:31 AM, wm4  wrote:
> >> On Tue, 17 Nov 2015 17:39:31 -0500
> >> Ganesh Ajjanagadde  wrote:
> >>
> >>> ffio_ensure_seekback can fail due to e.g ENOMEM. This return value is
> >>> checked here and a diagnostic is logged.
> >>>
> >>> All usage of ffio_ensure_seekback in the codebase now has the return
> value checked.
> >>>
> >>> Signed-off-by: Ganesh Ajjanagadde 
> >>> ---
> >>>  libavformat/mp3dec.c | 6 --
> >>>  libavformat/rmdec.c  | 3 ++-
> >>>  2 files changed, 6 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
> >>> index 32ca00c..a14bccd 100644
> >>> --- a/libavformat/mp3dec.c
> >>> +++ b/libavformat/mp3dec.c
> >>> @@ -380,11 +380,13 @@ static int mp3_read_header(AVFormatContext *s)
> >>>  uint32_t header, header2;
> >>>  int frame_size;
> >>>  if (!(i&1023))
> >>> -ffio_ensure_seekback(s->pb, i + 1024 + 4);
> >>> +if ((ret = ffio_ensure_seekback(s->pb, i + 1024 + 4)) < 0)
> >>> +av_log(s, AV_LOG_WARNING, "ffio_ensure_seekback():
> %s\n", av_err2str(ret));
> >>>  frame_size = check(s->pb, off + i, );
> >>>  if (frame_size > 0) {
> >>>  avio_seek(s->pb, off, SEEK_SET);
> >>> -ffio_ensure_seekback(s->pb, i + 1024 + frame_size + 4);
> >>> +if ((ret = ffio_ensure_seekback(s->pb, i + 1024 +
> frame_size + 4)) < 0)
> >>> +av_log(s, AV_LOG_WARNING, "ffio_ensure_seekback():
> %s\n", av_err2str(ret));
> >>>  if (check(s->pb, off + i + frame_size, ) >= 0 &&
> >>>  (header & SAME_HEADER_MASK) == (header2 &
> SAME_HEADER_MASK))
> >>>  {
> >>> diff --git a/libavformat/rmdec.c b/libavformat/rmdec.c
> >>> index 4ec78ef..1cf0548 100644
> >>> --- a/libavformat/rmdec.c
> >>> +++ b/libavformat/rmdec.c
> >>> @@ -576,7 +576,8 @@ static int rm_read_header(AVFormatContext *s)
> >>>  size = avio_rb32(pb);
> >>>  codec_pos = avio_tell(pb);
> >>>
> >>> -ffio_ensure_seekback(pb, 4);
> >>> +if ((ret = ffio_ensure_seekback(pb, 4)) < 0)
> >>> +av_log(s, AV_LOG_WARNING, "ffio_ensure_seekback():
> %s\n", av_err2str(ret));
> >>>  v = avio_rb32(pb);
> >>>  if (v == MKBETAG('M', 'L', 'T', 'I')) {
> >>>  int number_of_streams = avio_rb16(pb);
> >>
> >> So why do you not just add the message to the function itself?
> >
> > Because a client may not like the generic message, see e.g the message
> > printed in avformat/apngdec. Handling of the error may vary from
> > context to context, again see the apngdec example where the client
> > does something else in addition to error printing.
> >
> >>
> >> I also question the usefulness of the message. In such cases of OOM
> >> everything goes to hell anyway. No reason to bloat the code with error
> >> printing.
> >
> > Taking that logic a bit further, why bother returning ENOMEM? If the
> > ENOMEM being returned by ensure_seekback is not checked, the
> > information is just silently discarded or garbled with further errors
> > down the road.
> >
> > Anyway, I do not consider it a bloat: the buffer allocated can be big
> > (see in mp3dec > 1024 bytes). A user has a right to know when things
> > "go to hell". Envisioning what may happen, ffmpeg crashes due to OOM,
> > upon which a coredump takes place. That crash happens in a somewhat
> > random location, since OOM handling is a heuristic and determining
> > which process should be killed is an intractable problem. Thus at the
> > time when the core dump is written out, it is not guaranteed that the
> > context is the one where the allocation failed first. Then there are
> > issues of overcommit where malloc may return a non-NULL pointer but
> > OOM happens upon a dereference. Such issues are beyond the scope of
> > error handling, as nothing can be done in such cases anyway.
> >
> > Warning the user when a reasonably large allocation failed in my view
> > is not bloat. It is giving them as much information as we can. Also
> > think of it from a debugging perspective: it can help us find places
> > where the alloc size was unreasonable.
>
> Another reason related to the randomness of the process killed for
> OOM: ffmpeg/some other client may not be the process killed when OOM
> occurs, and the malloc's here fail. Thus, seek back, something a user
> does care about, fails, but without the error message, there is no
> information whatsoever that such a thing failed and why. We should
> make a "best effort" in such cases, especially since the alloc may be
> non-trivial in size and has sufficient client impact.
>
> Anyway, I by no means claim above patch is 

Re: [FFmpeg-devel] [PATCH 1/8] avcodec/lpc: replace #define by typedef

[Ganesh Ajjanagadde]

On Thu, Nov 19, 2015 at 9:29 AM, Michael Niedermayer  wrote:
> On Thu, Nov 19, 2015 at 08:14:08AM -0500, Ganesh Ajjanagadde wrote:
>> See e.g 
>> https://stackoverflow.com/questions/1666353/are-typedef-and-define-the-same-in-c
>> for rationale.
>>
>> Signed-off-by: Ganesh Ajjanagadde 
>> ---
>>  libavcodec/lpc.h | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> LGTM
>
> thx

pushed, thanks

>
> [...]
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Observe your enemies, for they first find out your faults. -- Antisthenes
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCHv2] avformat/mp3dec, rmdec: check return value of ffio_ensure_seekback

[Ganesh Ajjanagadde]

On Wed, Nov 18, 2015 at 8:26 AM, Ganesh Ajjanagadde  wrote:
> On Wed, Nov 18, 2015 at 7:31 AM, wm4  wrote:
>> On Tue, 17 Nov 2015 17:39:31 -0500
>> Ganesh Ajjanagadde  wrote:
>>
>>> ffio_ensure_seekback can fail due to e.g ENOMEM. This return value is
>>> checked here and a diagnostic is logged.
>>>
>>> All usage of ffio_ensure_seekback in the codebase now has the return value 
>>> checked.
>>>
>>> Signed-off-by: Ganesh Ajjanagadde 
>>> ---
>>>  libavformat/mp3dec.c | 6 --
>>>  libavformat/rmdec.c  | 3 ++-
>>>  2 files changed, 6 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
>>> index 32ca00c..a14bccd 100644
>>> --- a/libavformat/mp3dec.c
>>> +++ b/libavformat/mp3dec.c
>>> @@ -380,11 +380,13 @@ static int mp3_read_header(AVFormatContext *s)
>>>  uint32_t header, header2;
>>>  int frame_size;
>>>  if (!(i&1023))
>>> -ffio_ensure_seekback(s->pb, i + 1024 + 4);
>>> +if ((ret = ffio_ensure_seekback(s->pb, i + 1024 + 4)) < 0)
>>> +av_log(s, AV_LOG_WARNING, "ffio_ensure_seekback(): %s\n", 
>>> av_err2str(ret));
>>>  frame_size = check(s->pb, off + i, );
>>>  if (frame_size > 0) {
>>>  avio_seek(s->pb, off, SEEK_SET);
>>> -ffio_ensure_seekback(s->pb, i + 1024 + frame_size + 4);
>>> +if ((ret = ffio_ensure_seekback(s->pb, i + 1024 + frame_size + 
>>> 4)) < 0)
>>> +av_log(s, AV_LOG_WARNING, "ffio_ensure_seekback(): %s\n", 
>>> av_err2str(ret));
>>>  if (check(s->pb, off + i + frame_size, ) >= 0 &&
>>>  (header & SAME_HEADER_MASK) == (header2 & 
>>> SAME_HEADER_MASK))
>>>  {
>>> diff --git a/libavformat/rmdec.c b/libavformat/rmdec.c
>>> index 4ec78ef..1cf0548 100644
>>> --- a/libavformat/rmdec.c
>>> +++ b/libavformat/rmdec.c
>>> @@ -576,7 +576,8 @@ static int rm_read_header(AVFormatContext *s)
>>>  size = avio_rb32(pb);
>>>  codec_pos = avio_tell(pb);
>>>
>>> -ffio_ensure_seekback(pb, 4);
>>> +if ((ret = ffio_ensure_seekback(pb, 4)) < 0)
>>> +av_log(s, AV_LOG_WARNING, "ffio_ensure_seekback(): %s\n", 
>>> av_err2str(ret));
>>>  v = avio_rb32(pb);
>>>  if (v == MKBETAG('M', 'L', 'T', 'I')) {
>>>  int number_of_streams = avio_rb16(pb);
>>
>> So why do you not just add the message to the function itself?
>
> Because a client may not like the generic message, see e.g the message
> printed in avformat/apngdec. Handling of the error may vary from
> context to context, again see the apngdec example where the client
> does something else in addition to error printing.
>
>>
>> I also question the usefulness of the message. In such cases of OOM
>> everything goes to hell anyway. No reason to bloat the code with error
>> printing.
>
> Taking that logic a bit further, why bother returning ENOMEM? If the
> ENOMEM being returned by ensure_seekback is not checked, the
> information is just silently discarded or garbled with further errors
> down the road.
>
> Anyway, I do not consider it a bloat: the buffer allocated can be big
> (see in mp3dec > 1024 bytes). A user has a right to know when things
> "go to hell". Envisioning what may happen, ffmpeg crashes due to OOM,
> upon which a coredump takes place. That crash happens in a somewhat
> random location, since OOM handling is a heuristic and determining
> which process should be killed is an intractable problem. Thus at the
> time when the core dump is written out, it is not guaranteed that the
> context is the one where the allocation failed first. Then there are
> issues of overcommit where malloc may return a non-NULL pointer but
> OOM happens upon a dereference. Such issues are beyond the scope of
> error handling, as nothing can be done in such cases anyway.
>
> Warning the user when a reasonably large allocation failed in my view
> is not bloat. It is giving them as much information as we can. Also
> think of it from a debugging perspective: it can help us find places
> where the alloc size was unreasonable.

Another reason related to the randomness of the process killed for
OOM: ffmpeg/some other client may not be the process killed when OOM
occurs, and the malloc's here fail. Thus, seek back, something a user
does care about, fails, but without the error message, there is no
information whatsoever that such a thing failed and why. We should
make a "best effort" in such cases, especially since the alloc may be
non-trivial in size and has sufficient client impact.

Anyway, I by no means claim above patch is great, but I strongly
disagree with wm4 here.

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

Re: [FFmpeg-devel] [PATCH 7/8] avcodec/faanidct: replace #define by typedef

[Ganesh Ajjanagadde]

On Thu, Nov 19, 2015 at 9:29 AM, Michael Niedermayer  wrote:
> On Thu, Nov 19, 2015 at 08:14:14AM -0500, Ganesh Ajjanagadde wrote:
>> See e.g 
>> https://stackoverflow.com/questions/1666353/are-typedef-and-define-the-same-in-c
>> for rationale.
>>
>> Signed-off-by: Ganesh Ajjanagadde 
>> ---
>>  libavcodec/faanidct.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> LGTM
>
> thx

pushed, thanks

>
> [...]
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> The real ebay dictionary, page 3
> "Rare item" - "Common item with rare defect or maybe just a lie"
> "Professional" - "'Toy' made in china, not functional except as doorstop"
> "Experts will know" - "The seller hopes you are not an expert"
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 6/8] tests/tiny_ssim: replace #define by typedef

[Ganesh Ajjanagadde]

On Thu, Nov 19, 2015 at 9:22 AM, Ganesh Ajjanagadde  wrote:
> On Thu, Nov 19, 2015 at 9:19 AM, Hendrik Leppkes  wrote:
>> On Thu, Nov 19, 2015 at 2:50 PM, Ganesh Ajjanagadde
>>  wrote:
>>> On Thu, Nov 19, 2015 at 8:36 AM, Ronald S. Bultje  
>>> wrote:
 Hi,

 On Thu, Nov 19, 2015 at 8:14 AM, Ganesh Ajjanagadde 
 
 wrote:
>
> See e.g
> https://stackoverflow.com/questions/1666353/are-typedef-and-define-the-same-in-c
> for rationale.
>
> Signed-off-by: Ganesh Ajjanagadde 
> ---
>  tests/tiny_ssim.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/tests/tiny_ssim.c b/tests/tiny_ssim.c
> index 9f355a3..ebd0216 100644
> --- a/tests/tiny_ssim.c
> +++ b/tests/tiny_ssim.c
> @@ -79,11 +79,11 @@ static float ssim_end1( int s1, int s2, int ss, int
> s12 )
>   * s1*s1, s2*s2, and s1*s2 also obtain this value for edge cases:
> ((2^10-1)*16*4)^2 = 4286582784.
>   * Maximum value for 9-bit is: ss*64 = (2^9-1)^2*16*4*64 = 1069551616,
> which will not overflow. */
>  #if BIT_DEPTH > 9
> -#define type float
> +typedef float type;
>  static const float ssim_c1 = .01*.01*PIXEL_MAX*PIXEL_MAX*64;
>  static const float ssim_c2 = .03*.03*PIXEL_MAX*PIXEL_MAX*64*63;
>  #else
> -#define type int
> +typedef int type;
>  static const int ssim_c1 = (int)(.01*.01*PIXEL_MAX*PIXEL_MAX*64 +
> .5);
>  static const int ssim_c2 = (int)(.03*.03*PIXEL_MAX*PIXEL_MAX*64*63 +
> .5);
>  #endif


 Bad indentation?
>>>
>>> I noticed it, suspect it was due to the macro earlier. I followed
>>> general philosophy of avoiding reindents in code changes, keeping them
>>> for a separate commit.
>>
>> Please just indent here, doing a separate commit for one-line changes
>> is not needed. The only reason is for readability, which is not really
>> affected here.
>
> noted, thanks.

changed and pushed, thanks

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

Re: [FFmpeg-devel] [PATCH] all: use M_SQRT1_2, M_SQRT2, M_PI

[Ganesh Ajjanagadde]

On Thu, Nov 19, 2015 at 2:35 PM, Paul B Mahol  wrote:
> On 11/19/15, Ganesh Ajjanagadde  wrote:
>> On Thu, Nov 19, 2015 at 12:35 PM, Nicolas George  wrote:
>>> Le nonidi 29 brumaire, an CCXXIV, Derek Buitenhuis a ecrit :
 Paul uses FreeBSD, if I recall. This is probably with glibc.
>>>
>>> Still, M_PI is meant to have the full precision of the underlying type,
>>> and
>>> I very much doubt that kind of bug to be present in any common
>>> implementation. And let us be realistic, the number of digits in
>>> af_dynaudnorm.c is nothing but ludicrous.
>>
>> Exactly. Can't pull the source for bsd easily, but it likely is
>> identical to openlibm since openlibm imported from freebsd:
>> https://github.com/JuliaLang/openlibm/blob/master/include/openlibm_math.h
>> which has an identical definition to gnu libm for M_PI.
>>
>> If such a silly, easily fixed bug exists with some libm, it can be
>> fixed in a few minutes on their end, and I personally would do my best
>> to publicize it to highlight the fundamental brokenness of said libm
>> for the benefit of their users.
>>
>
> OK, you can change it.

Pushed just this change for now, thanks. Will push the remainder soon.

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

Re: [FFmpeg-devel] [PATCHv2] avformat/mp3dec, rmdec: check return value of ffio_ensure_seekback

[Ronald S. Bultje]

Hi,

On Fri, Nov 20, 2015 at 9:40 AM, Ganesh Ajjanagadde 
wrote:

> On Fri, Nov 20, 2015 at 9:30 AM, Ronald S. Bultje 
> wrote:
> > Hi Ganesh,
> >
> > On Fri, Nov 20, 2015 at 8:42 AM, Ganesh Ajjanagadde 
> > wrote:
> >>
> >> On Wed, Nov 18, 2015 at 8:26 AM, Ganesh Ajjanagadde 
> >> wrote:
> >> > On Wed, Nov 18, 2015 at 7:31 AM, wm4  wrote:
> >> >> On Tue, 17 Nov 2015 17:39:31 -0500
> >> >> Ganesh Ajjanagadde  wrote:
> >> >>
> >> >>> ffio_ensure_seekback can fail due to e.g ENOMEM. This return value
> is
> >> >>> checked here and a diagnostic is logged.
> >> >>>
> >> >>> All usage of ffio_ensure_seekback in the codebase now has the return
> >> >>> value checked.
> >> >>>
> >> >>> Signed-off-by: Ganesh Ajjanagadde 
> >> >>> ---
> >> >>>  libavformat/mp3dec.c | 6 --
> >> >>>  libavformat/rmdec.c  | 3 ++-
> >> >>>  2 files changed, 6 insertions(+), 3 deletions(-)
> >> >>>
> >> >>> diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
> >> >>> index 32ca00c..a14bccd 100644
> >> >>> --- a/libavformat/mp3dec.c
> >> >>> +++ b/libavformat/mp3dec.c
> >> >>> @@ -380,11 +380,13 @@ static int mp3_read_header(AVFormatContext *s)
> >> >>>  uint32_t header, header2;
> >> >>>  int frame_size;
> >> >>>  if (!(i&1023))
> >> >>> -ffio_ensure_seekback(s->pb, i + 1024 + 4);
> >> >>> +if ((ret = ffio_ensure_seekback(s->pb, i + 1024 + 4)) <
> >> >>> 0)
> >> >>> +av_log(s, AV_LOG_WARNING, "ffio_ensure_seekback():
> >> >>> %s\n", av_err2str(ret));
> >> >>>  frame_size = check(s->pb, off + i, );
> >> >>>  if (frame_size > 0) {
> >> >>>  avio_seek(s->pb, off, SEEK_SET);
> >> >>> -ffio_ensure_seekback(s->pb, i + 1024 + frame_size + 4);
> >> >>> +if ((ret = ffio_ensure_seekback(s->pb, i + 1024 +
> >> >>> frame_size + 4)) < 0)
> >> >>> +av_log(s, AV_LOG_WARNING, "ffio_ensure_seekback():
> >> >>> %s\n", av_err2str(ret));
> >> >>>  if (check(s->pb, off + i + frame_size, ) >= 0
> &&
> >> >>>  (header & SAME_HEADER_MASK) == (header2 &
> >> >>> SAME_HEADER_MASK))
> >> >>>  {
> >> >>> diff --git a/libavformat/rmdec.c b/libavformat/rmdec.c
> >> >>> index 4ec78ef..1cf0548 100644
> >> >>> --- a/libavformat/rmdec.c
> >> >>> +++ b/libavformat/rmdec.c
> >> >>> @@ -576,7 +576,8 @@ static int rm_read_header(AVFormatContext *s)
> >> >>>  size = avio_rb32(pb);
> >> >>>  codec_pos = avio_tell(pb);
> >> >>>
> >> >>> -ffio_ensure_seekback(pb, 4);
> >> >>> +if ((ret = ffio_ensure_seekback(pb, 4)) < 0)
> >> >>> +av_log(s, AV_LOG_WARNING, "ffio_ensure_seekback():
> >> >>> %s\n", av_err2str(ret));
> >> >>>  v = avio_rb32(pb);
> >> >>>  if (v == MKBETAG('M', 'L', 'T', 'I')) {
> >> >>>  int number_of_streams = avio_rb16(pb);
> >> >>
> >> >> So why do you not just add the message to the function itself?
> >> >
> >> > Because a client may not like the generic message, see e.g the message
> >> > printed in avformat/apngdec. Handling of the error may vary from
> >> > context to context, again see the apngdec example where the client
> >> > does something else in addition to error printing.
> >> >
> >> >>
> >> >> I also question the usefulness of the message. In such cases of OOM
> >> >> everything goes to hell anyway. No reason to bloat the code with
> error
> >> >> printing.
> >> >
> >> > Taking that logic a bit further, why bother returning ENOMEM? If the
> >> > ENOMEM being returned by ensure_seekback is not checked, the
> >> > information is just silently discarded or garbled with further errors
> >> > down the road.
> >> >
> >> > Anyway, I do not consider it a bloat: the buffer allocated can be big
> >> > (see in mp3dec > 1024 bytes). A user has a right to know when things
> >> > "go to hell". Envisioning what may happen, ffmpeg crashes due to OOM,
> >> > upon which a coredump takes place. That crash happens in a somewhat
> >> > random location, since OOM handling is a heuristic and determining
> >> > which process should be killed is an intractable problem. Thus at the
> >> > time when the core dump is written out, it is not guaranteed that the
> >> > context is the one where the allocation failed first. Then there are
> >> > issues of overcommit where malloc may return a non-NULL pointer but
> >> > OOM happens upon a dereference. Such issues are beyond the scope of
> >> > error handling, as nothing can be done in such cases anyway.
> >> >
> >> > Warning the user when a reasonably large allocation failed in my view
> >> > is not bloat. It is giving them as much information as we can. Also
> >> > think of it from a debugging perspective: it can help us find places
> >> > where the alloc size was unreasonable.
> >>
> >> Another reason related to 

Re: [FFmpeg-devel] [PATCH] avfilter/WIP: add streamselect filter

[Clément Bœsch]

On Thu, Nov 19, 2015 at 06:47:07PM +0100, Nicolas George wrote:
> Le nonidi 29 brumaire, an CCXXIV, Clement Boesch a écrit :
> > - I always receive frames from the first input link... I suppose I need
> >   to write a "smart" request_frame callback, but things changed recently
> >   so maybe there are new guidelines for this? Nicolas, can I call for
> >   your help again?
> 
> Nothing new except you do not have to loop: just forward the request on the
> output to the connected input. But you have to decide what to do when an
> output is not connected to any input; EAGAIN comes to mind, but I am not
> entirely sure it will work correctly. For now I would suggest to make it
> unsupported: either an input is connected or it is EOF.
> 

Alright, so if I understand well, all I have to do is implement a
forwarding such as this:

static int request_frame(AVFilterLink *outlink)
{
AVFilterContext *ctx = outlink->src;
StreamSelectContext *s = ctx->priv;
const int outlink_idx = FF_OUTLINK_IDX(outlink);
const int inlink_idx = s->map[outlink_idx];
AVFilterLink *inlink = ctx->inputs[inlink_idx];
return ff_request_frame(inlink);
}

Which indeed seems to do the trick when set for to every output pad
request_frame callback.

Unfortunately, it seems the other stream(s) in "background" (not present
in the mapping) will not be consumed at all (filter_frame() is not called)
so frames are never dropped. As a result, when switching to that
background stream, the frames are delayed by a large margin (the amount of
time the main stream has been in use).

While I still need to add a timing check in the context (to avoid small
non monotonically problems), how would it be possible to continue
consuming the background streams?

-- 
Clément B.


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

Re: [FFmpeg-devel] [PATCH 3/8] avcodec/ac3: replace #define by typedef

[Ganesh Ajjanagadde]

On Thu, Nov 19, 2015 at 10:55 AM, Michael Niedermayer  wrote:
> On Thu, Nov 19, 2015 at 08:14:10AM -0500, Ganesh Ajjanagadde wrote:
>> See e.g 
>> https://stackoverflow.com/questions/1666353/are-typedef-and-define-the-same-in-c
>> for rationale.
>>
>> Signed-off-by: Ganesh Ajjanagadde 
>> ---
>>  libavcodec/ac3.h | 8 
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> should be ok

pushed, thanks

>
> [...]
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> No human being will ever know the Truth, for even if they happen to say it
> by chance, they would not even known they had done so. -- Xenophanes
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCHv2] avformat/mp3dec, rmdec: check return value of ffio_ensure_seekback

[Ganesh Ajjanagadde]

On Fri, Nov 20, 2015 at 9:48 AM, Ronald S. Bultje  wrote:
> Hi,
>
> On Fri, Nov 20, 2015 at 9:40 AM, Ganesh Ajjanagadde 
> wrote:
>>
>> On Fri, Nov 20, 2015 at 9:30 AM, Ronald S. Bultje 
>> wrote:
>> > Hi Ganesh,
>> >
>> > On Fri, Nov 20, 2015 at 8:42 AM, Ganesh Ajjanagadde 
>> > wrote:
>> >>
>> >> On Wed, Nov 18, 2015 at 8:26 AM, Ganesh Ajjanagadde 
>> >> wrote:
>> >> > On Wed, Nov 18, 2015 at 7:31 AM, wm4  wrote:
>> >> >> On Tue, 17 Nov 2015 17:39:31 -0500
>> >> >> Ganesh Ajjanagadde  wrote:
>> >> >>
>> >> >>> ffio_ensure_seekback can fail due to e.g ENOMEM. This return value
>> >> >>> is
>> >> >>> checked here and a diagnostic is logged.
>> >> >>>
>> >> >>> All usage of ffio_ensure_seekback in the codebase now has the
>> >> >>> return
>> >> >>> value checked.
>> >> >>>
>> >> >>> Signed-off-by: Ganesh Ajjanagadde 
>> >> >>> ---
>> >> >>>  libavformat/mp3dec.c | 6 --
>> >> >>>  libavformat/rmdec.c  | 3 ++-
>> >> >>>  2 files changed, 6 insertions(+), 3 deletions(-)
>> >> >>>
>> >> >>> diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
>> >> >>> index 32ca00c..a14bccd 100644
>> >> >>> --- a/libavformat/mp3dec.c
>> >> >>> +++ b/libavformat/mp3dec.c
>> >> >>> @@ -380,11 +380,13 @@ static int mp3_read_header(AVFormatContext
>> >> >>> *s)
>> >> >>>  uint32_t header, header2;
>> >> >>>  int frame_size;
>> >> >>>  if (!(i&1023))
>> >> >>> -ffio_ensure_seekback(s->pb, i + 1024 + 4);
>> >> >>> +if ((ret = ffio_ensure_seekback(s->pb, i + 1024 + 4))
>> >> >>> <
>> >> >>> 0)
>> >> >>> +av_log(s, AV_LOG_WARNING, "ffio_ensure_seekback():
>> >> >>> %s\n", av_err2str(ret));
>> >> >>>  frame_size = check(s->pb, off + i, );
>> >> >>>  if (frame_size > 0) {
>> >> >>>  avio_seek(s->pb, off, SEEK_SET);
>> >> >>> -ffio_ensure_seekback(s->pb, i + 1024 + frame_size +
>> >> >>> 4);
>> >> >>> +if ((ret = ffio_ensure_seekback(s->pb, i + 1024 +
>> >> >>> frame_size + 4)) < 0)
>> >> >>> +av_log(s, AV_LOG_WARNING, "ffio_ensure_seekback():
>> >> >>> %s\n", av_err2str(ret));
>> >> >>>  if (check(s->pb, off + i + frame_size, ) >= 0
>> >> >>> &&
>> >> >>>  (header & SAME_HEADER_MASK) == (header2 &
>> >> >>> SAME_HEADER_MASK))
>> >> >>>  {
>> >> >>> diff --git a/libavformat/rmdec.c b/libavformat/rmdec.c
>> >> >>> index 4ec78ef..1cf0548 100644
>> >> >>> --- a/libavformat/rmdec.c
>> >> >>> +++ b/libavformat/rmdec.c
>> >> >>> @@ -576,7 +576,8 @@ static int rm_read_header(AVFormatContext *s)
>> >> >>>  size = avio_rb32(pb);
>> >> >>>  codec_pos = avio_tell(pb);
>> >> >>>
>> >> >>> -ffio_ensure_seekback(pb, 4);
>> >> >>> +if ((ret = ffio_ensure_seekback(pb, 4)) < 0)
>> >> >>> +av_log(s, AV_LOG_WARNING, "ffio_ensure_seekback():
>> >> >>> %s\n", av_err2str(ret));
>> >> >>>  v = avio_rb32(pb);
>> >> >>>  if (v == MKBETAG('M', 'L', 'T', 'I')) {
>> >> >>>  int number_of_streams = avio_rb16(pb);
>> >> >>
>> >> >> So why do you not just add the message to the function itself?
>> >> >
>> >> > Because a client may not like the generic message, see e.g the
>> >> > message
>> >> > printed in avformat/apngdec. Handling of the error may vary from
>> >> > context to context, again see the apngdec example where the client
>> >> > does something else in addition to error printing.
>> >> >
>> >> >>
>> >> >> I also question the usefulness of the message. In such cases of OOM
>> >> >> everything goes to hell anyway. No reason to bloat the code with
>> >> >> error
>> >> >> printing.
>> >> >
>> >> > Taking that logic a bit further, why bother returning ENOMEM? If the
>> >> > ENOMEM being returned by ensure_seekback is not checked, the
>> >> > information is just silently discarded or garbled with further errors
>> >> > down the road.
>> >> >
>> >> > Anyway, I do not consider it a bloat: the buffer allocated can be big
>> >> > (see in mp3dec > 1024 bytes). A user has a right to know when things
>> >> > "go to hell". Envisioning what may happen, ffmpeg crashes due to OOM,
>> >> > upon which a coredump takes place. That crash happens in a somewhat
>> >> > random location, since OOM handling is a heuristic and determining
>> >> > which process should be killed is an intractable problem. Thus at the
>> >> > time when the core dump is written out, it is not guaranteed that the
>> >> > context is the one where the allocation failed first. Then there are
>> >> > issues of overcommit where malloc may return a non-NULL pointer but
>> >> > OOM happens upon a dereference. Such issues are beyond the scope of
>> >> > error handling, as nothing can be done in such cases anyway.
>> >> >
>> >> > Warning the user when a reasonably large 

Re: [FFmpeg-devel] [PATCH] avutil/motion_vector: export subpel motion information

[Clément Bœsch]

On Fri, Nov 20, 2015 at 10:36:36AM +0100, Clément Bœsch wrote:
> On Fri, Nov 20, 2015 at 10:33:04AM +0100, Clément Bœsch wrote:
> [...]
> > > i think this doesnt support libavcodec/tpeldsp.c as used in svq3
> > > that would need a motion_xy / 3
> > > 
> > > i might be missing something but
> > > 
> > >  * Motion vector
> > >  * src_x = dst_x + motion_x / motion_scale
> > >  * src_y = dst_y + motion_y / motion_scale
> > >  */
> > > int32_t motion_x, motion_y;
> > > uint8_t motion_scale;   // This is almost always a power of 2
> > > 
> > > should be enough to support all things
> > > 
> > 
> > ok, so always a div then
> > 
> > > The "*motion_scale" should not be needed, the exportet vectors
> > > can just be multiplied up if thats needed
> > 
> > what is the range of mv_scale in snow? how much precision i'm going to
> > loose by doing avmv->motion_scale = mv_scale/8?
> > 
> 
> ah wait my bad, forget this.
> 
> note: can't we make the motion_scale always a power of two by
> pre-multiplying motion_[xy]?
> 

I can't math. New patch incoming.

-- 
Clément B.


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

Re: [FFmpeg-devel] [PATCH] avcodec/h264, videotoolbox: do not return invalid frames on failure

[wm4]

On Tue, 17 Nov 2015 15:19:29 +0100
wm4  wrote:

> If videotoolbox_common_end_frame failed, then the AVFrame was returned
> to the API user with the dummy buffer (in AVFrame.buf[0]) still set, and
> the decode call indicating success.
> 
> These "half-set" AVFrames with dummy buffer are a videotoolbox specific
> hack, because the decoder requires an allocated AVFrame for its internal
> logic. Videotoolbox on the other hand allocates its frame itself
> internally, and outputs it only on end_frame. At this point, the dummy
> buffer is replaced with the real frame (unless decoding fails).
> ---
> Better solutions welcome. For now, this fixes the API returning garbage
> which can crash or confuse API users,
> ---
>  libavcodec/h264_refs.c| 3 ++-
>  libavcodec/videotoolbox.c | 2 ++
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/libavcodec/h264_refs.c b/libavcodec/h264_refs.c
> index 619f2ed..9d0641a 100644
> --- a/libavcodec/h264_refs.c
> +++ b/libavcodec/h264_refs.c
> @@ -515,7 +515,8 @@ void ff_h264_remove_all_refs(H264Context *h)
>  
>  if (h->short_ref_count && !h->last_pic_for_ec.f->data[0]) {
>  ff_h264_unref_picture(h, >last_pic_for_ec);
> -ff_h264_ref_picture(h, >last_pic_for_ec, h->short_ref[0]);
> +if (h->short_ref[0]->f->buf[0])
> +ff_h264_ref_picture(h, >last_pic_for_ec, h->short_ref[0]);
>  }
>  
>  for (i = 0; i < h->short_ref_count; i++) {
> diff --git a/libavcodec/videotoolbox.c b/libavcodec/videotoolbox.c
> index cc1e592..2f4d531 100644
> --- a/libavcodec/videotoolbox.c
> +++ b/libavcodec/videotoolbox.c
> @@ -353,6 +353,8 @@ static int videotoolbox_common_end_frame(AVCodecContext 
> *avctx, AVFrame *frame)
>  AVVideotoolboxContext *videotoolbox = avctx->hwaccel_context;
>  VTContext *vtctx = avctx->internal->hwaccel_priv_data;
>  
> +av_buffer_unref(>buf[0]);
> +
>  if (!videotoolbox->session || !vtctx->bitstream)
>  return AVERROR_INVALIDDATA;
>  

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

Re: [FFmpeg-devel] [PATCH] ffprobe: do not exit in case the demuxer returns AVERROR(EAGAIN)

[wm4]

On Fri, 20 Nov 2015 13:30:50 +0100
Stefano Sabatini  wrote:

> On date Wednesday 2015-11-18 15:21:15 +0100, Nicolas George encoded:
> > Le septidi 27 brumaire, an CCXXIV, Stefano Sabatini a écrit :  
> > > Fix demuxing of files for which the demuxer returns AVERROR(EAGAIN) at
> > > some point. Also returns error code to the caller in case of different
> > > non EOF error.
> > > ---
> > >  ffprobe.c | 11 ++-
> > >  1 file changed, 10 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/ffprobe.c b/ffprobe.c
> > > index c304a6d..481ff0b 100644
> > > --- a/ffprobe.c
> > > +++ b/ffprobe.c
> > > @@ -2027,7 +2027,16 @@ static int read_interval_packets(WriterContext *w, 
> > > AVFormatContext *fmt_ctx,
> > >  ret = AVERROR(ENOMEM);
> > >  goto end;
> > >  }
> > > -while (!av_read_frame(fmt_ctx, )) {  
> >   
> > > +while (1) {
> > > +ret = av_read_frame(fmt_ctx, );
> > > +if (ret == AVERROR(EAGAIN))
> > > +continue;  
> > 
> > This is a busy wait, this is one of the evilest constructions possible.
> >   
> 
> > For real EAGAIN cases, a sleep would be needed, until we have a real event
> > loop. Other cases are bugs in the demuxers.  
> 
> Yes, updated patch with EAGAIN (if this is fine, I'll send patches to
> fix the examples).
> 
> In my case the EAGAIN is triggered by the flv demuxer code in
> flvdec.c:935:
> 
> /* skip empty data packets */
> if (!size) {
> ret = AVERROR(EAGAIN);
> goto leave;
> }
> 
> introduced in 527c2e64295671bfcd9c86ca2cfd75e0f4d79f73.
> 
> Not sure if this is a defect.

Isn't it actually fcb4228c863ec80c9498a6c5e815d79ee9f89a7f and
a308737bd66a1c669b9be71a1b834d8419fe70ca?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avutil/motion_vector: export subpel motion information

[Clément Bœsch]

On Fri, Nov 20, 2015 at 10:33:04AM +0100, Clément Bœsch wrote:
[...]
> > i think this doesnt support libavcodec/tpeldsp.c as used in svq3
> > that would need a motion_xy / 3
> > 
> > i might be missing something but
> > 
> >  * Motion vector
> >  * src_x = dst_x + motion_x / motion_scale
> >  * src_y = dst_y + motion_y / motion_scale
> >  */
> > int32_t motion_x, motion_y;
> > uint8_t motion_scale;   // This is almost always a power of 2
> > 
> > should be enough to support all things
> > 
> 
> ok, so always a div then
> 
> > The "*motion_scale" should not be needed, the exportet vectors
> > can just be multiplied up if thats needed
> 
> what is the range of mv_scale in snow? how much precision i'm going to
> loose by doing avmv->motion_scale = mv_scale/8?
> 

ah wait my bad, forget this.

note: can't we make the motion_scale always a power of two by
pre-multiplying motion_[xy]?

-- 
Clément B.


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

Re: [FFmpeg-devel] [PATCH 1/3] aacsbr_fixed: avoid division by zero in sbr_gain_calc

[Michael Niedermayer]

On Fri, Nov 20, 2015 at 12:16:09AM +0100, Andreas Cadhalpun wrote:
> On 19.11.2015 01:31, Michael Niedermayer wrote:
> > On Thu, Nov 19, 2015 at 12:31:17AM +0100, Andreas Cadhalpun wrote:
> >> So far so good. However, the next time sbr_dequant is called this breaks:
> >> noise_facs[1][0].exp = 6 - 536870912 + 1 = -536870905;
> >>
> > 
> >> This is obviously completely bogus.
> > 
> > yes
> > 
> > 
> >> Instead this code needs a function like av_exp2_sf.
> > 
> > no, thats not the problem
> > this code is missing error checks and only adding error checks will
> > fix that
> > 
> > there is read_sbr_noise() which reads data
> > there is sbr_dequant() which converts the data from "read data" to
> > lets call it "dequantized data"
> 
> That makes sense, thanks for explaining.
> 
> > what you describe sounds like that sbr_dequant() is called on top of
> > the output from sbr_dequant(), that sounds like a error condition
> > for both fixed and float
> 
> Indeed. A patch checking for that is attached.
> 
> Best regards,
> Andreas
> 

>  aacsbr_template.c |8 
>  1 file changed, 8 insertions(+)
> 43ace0364265ef16eecd6ca2d9564cc9585847f7  
> 0001-aacsbr-don-t-call-sbr_dequant-twice-without-intermed.patch
> From 0237ebfca9571d2d3e51f5c5dc15f8f5a516510a Mon Sep 17 00:00:00 2001
> From: Andreas Cadhalpun 
> Date: Fri, 20 Nov 2015 00:04:50 +0100
> Subject: [PATCH] aacsbr: don't call sbr_dequant twice without intermediate
>  read_sbr_data
> 
> Doing that doesn't make sense, because the only purpose of sbr_dequant
> is to process the data from read_sbr_data.
> 
> Signed-off-by: Andreas Cadhalpun 
> ---
>  libavcodec/aacsbr_template.c | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/libavcodec/aacsbr_template.c b/libavcodec/aacsbr_template.c
> index cf18862..564930a 100644
> --- a/libavcodec/aacsbr_template.c
> +++ b/libavcodec/aacsbr_template.c
> @@ -1041,6 +1041,7 @@ static unsigned int read_sbr_data(AACContext *ac, 
> SpectralBandReplication *sbr,
>  unsigned int cnt = get_bits_count(gb);
>  
>  sbr->id_aac = id_aac;
> +sbr->start = 2;

the new value should be documented
or maybe it would be possible to use a new field with self explanatory
name

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Asymptotically faster algorithms should always be preferred if you have
asymptotical amounts of data


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

Re: [FFmpeg-devel] [PATCH 5/5] lavfi/select: add support for concatdec_select option

[Marton Balint]

On Mon, 16 Nov 2015, Stefano Sabatini wrote:


+@item concatdec_select
+The concat demuxer can set the @var{lavf.concat.start_time} and the
+@var{lavf.concat.duration} packet metadata values which are also present in the
+decoded frames.
+



+The @var{concatdec_select} variable is -1 if the frame pts is at least
+start_time and either the duration metadata is missing or the frame pts is less
+than start_time + duration, 0 otherwise, and NaN if the start_time metadata is
+missing.


Add also a single explanation, like this:

That means that an input frame is selected if its pts is within the
interval set by the concat demuxer.


Ok, will do.



I wonder if it would make sense to export the input index
(lavf.concat.index), and use it here in place of -1.



Maybe, but we should probably provide this as separate variable, because 
you cannot use 0 for the first segent, only -1 etc... And maybe there will 
be a time when the expression evaluation code will be able to access all 
fields and metadata of an AVFrame...


Anyway, can be added later.


+@item
+Select useful frames from an ffconcat file which is using inpoints and
+outpoints but where the source files are not intra frame only.
+@example
+ffmpeg -copyts -vsync 0 -segment_time_metadata 1 -i input.ffconcat -vf 
select=concatdec_select -af aselect=concatdec_select output.avi
+@end example


I guess that the lavf.concat metadata is set so that start_time and
duration corresponds to the inpoint and outpoint interval, right?


Yes, only inpoint and outpoint in the concat file are timestamps of the
source files referenced by the concat file,  start_time and 
duration metadata are timestamps of the "output" from the concat demuxer.



 @end itemize

 @section selectivecolor
diff --git a/libavfilter/f_select.c b/libavfilter/f_select.c
index 2b926e1..14a23d2 100644
--- a/libavfilter/f_select.c
+++ b/libavfilter/f_select.c
@@ -82,6 +82,8 @@ static const char *const var_names[] = {

 "scene",

+"concatdec_select",  ///< frame usefulness based on pts and frame metadata 
originating from the concat demuxer


probably you can give a more explicative description ("useful" is
pretty generic)


Ok.

Will send a new version soon.

Regards,
Marton
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCHv2 5/5] lavfi/select: add support for concatdec_select option

[Marton Balint]

This option can be used to select useful frames from an ffconcat file which is
using inpoints and outpoints but where the source files are not intra frame
only.

Signed-off-by: Marton Balint 
---
 doc/filters.texi   | 20 
 libavfilter/f_select.c | 27 +++
 2 files changed, 47 insertions(+)

diff --git a/doc/filters.texi b/doc/filters.texi
index 471ec3f..c886976 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -13196,6 +13196,19 @@ value between 0 and 1 to indicate a new scene; a low 
value reflects a low
 probability for the current frame to introduce a new scene, while a higher
 value means the current frame is more likely to be one (see the example below)
 
+@item concatdec_select
+The concat demuxer can set the @var{lavf.concat.start_time} and the
+@var{lavf.concat.duration} packet metadata values which are also present in the
+decoded frames.
+
+The @var{concatdec_select} variable is -1 if the frame pts is at least
+start_time and either the duration metadata is missing or the frame pts is less
+than start_time + duration, 0 otherwise, and NaN if the start_time metadata is
+missing.
+
+That basically means that an input frame is selected if its pts is within the
+interval set by the concat demuxer.
+
 @end table
 
 The default value of the select expression is "1".
@@ -13270,6 +13283,13 @@ Send even and odd frames to separate outputs, and 
compose them:
 @example
 select=n=2:e='mod(n, 2)+1' [odd][even]; [odd] pad=h=2*ih [tmp]; [tmp][even] 
overlay=y=h
 @end example
+
+@item
+Select useful frames from an ffconcat file which is using inpoints and
+outpoints but where the source files are not intra frame only.
+@example
+ffmpeg -copyts -vsync 0 -segment_time_metadata 1 -i input.ffconcat -vf 
select=concatdec_select -af aselect=concatdec_select output.avi
+@end example
 @end itemize
 
 @section selectivecolor
diff --git a/libavfilter/f_select.c b/libavfilter/f_select.c
index 2b926e1..52f474e 100644
--- a/libavfilter/f_select.c
+++ b/libavfilter/f_select.c
@@ -82,6 +82,8 @@ static const char *const var_names[] = {
 
 "scene",
 
+"concatdec_select",  ///< frame is within the interval set by the concat 
demuxer
+
 NULL
 };
 
@@ -132,6 +134,8 @@ enum var_name {
 
 VAR_SCENE,
 
+VAR_CONCATDEC_SELECT,
+
 VAR_VARS_NB
 };
 
@@ -278,6 +282,28 @@ static double get_scene_score(AVFilterContext *ctx, 
AVFrame *frame)
 return ret;
 }
 
+static double get_concatdec_select(AVFrame *frame, int64_t pts)
+{
+AVDictionary *metadata = av_frame_get_metadata(frame);
+AVDictionaryEntry *start_time_entry = av_dict_get(metadata, 
"lavf.concatdec.start_time", NULL, 0);
+AVDictionaryEntry *duration_entry = av_dict_get(metadata, 
"lavf.concatdec.duration", NULL, 0);
+if (start_time_entry) {
+int64_t start_time = strtoll(start_time_entry->value, NULL, 10);
+if (pts >= start_time) {
+if (duration_entry) {
+  int64_t duration = strtoll(duration_entry->value, NULL, 10);
+  if (pts < start_time + duration)
+  return -1;
+  else
+  return 0;
+}
+return -1;
+}
+return 0;
+}
+return NAN;
+}
+
 #define D2TS(d)  (isnan(d) ? AV_NOPTS_VALUE : (int64_t)(d))
 #define TS2D(ts) ((ts) == AV_NOPTS_VALUE ? NAN : (double)(ts))
 
@@ -297,6 +323,7 @@ static void select_frame(AVFilterContext *ctx, AVFrame 
*frame)
 select->var_values[VAR_T  ] = TS2D(frame->pts) * av_q2d(inlink->time_base);
 select->var_values[VAR_POS] = av_frame_get_pkt_pos(frame) == -1 ? NAN : 
av_frame_get_pkt_pos(frame);
 select->var_values[VAR_KEY] = frame->key_frame;
+select->var_values[VAR_CONCATDEC_SELECT] = get_concatdec_select(frame, 
av_rescale_q(frame->pts, inlink->time_base, AV_TIME_BASE_Q));
 
 switch (inlink->type) {
 case AVMEDIA_TYPE_AUDIO:
-- 
2.6.2

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

Re: [FFmpeg-devel] [PATCH] avutil/motion_vector: export subpel motion information

[Clément Bœsch]

On Mon, Nov 16, 2015 at 02:49:05AM +0100, Michael Niedermayer wrote:
> On Thu, Nov 12, 2015 at 03:03:33PM +0100, Clément Bœsch wrote:
> > ---
> >  libavcodec/mpegvideo.c | 36 +---
> >  libavcodec/snowdec.c   |  4 
> >  libavfilter/vf_codecview.c |  7 +--
> >  libavutil/motion_vector.h  |  8 
> >  libavutil/version.h|  2 +-
> >  5 files changed, 39 insertions(+), 18 deletions(-)
> > 
> > diff --git a/libavcodec/mpegvideo.c b/libavcodec/mpegvideo.c
> > index 96634ec..9ba1e09 100644
> > --- a/libavcodec/mpegvideo.c
> > +++ b/libavcodec/mpegvideo.c
> > @@ -1556,15 +1556,21 @@ static void draw_arrow(uint8_t *buf, int sx, int 
> > sy, int ex,
> >  
> >  static int add_mb(AVMotionVector *mb, uint32_t mb_type,
> >int dst_x, int dst_y,
> > -  int src_x, int src_y,
> > +  int motion_x, int motion_y, int motion_shift,
> >int direction)
> >  {
> >  mb->w = IS_8X8(mb_type) || IS_8X16(mb_type) ? 8 : 16;
> >  mb->h = IS_8X8(mb_type) || IS_16X8(mb_type) ? 8 : 16;
> > -mb->src_x = src_x;
> > -mb->src_y = src_y;
> > +
> > +mb->motion_x = motion_x;
> > +mb->motion_y = motion_y;
> > +mb->motion_scale = 1;
> > +mb->motion_shift = motion_shift;
> > +
> >  mb->dst_x = dst_x;
> >  mb->dst_y = dst_y;
> > +mb->src_x = dst_x + (motion_x >> motion_shift);
> > +mb->src_y = dst_y + (motion_y >> motion_shift);
> >  mb->source = direction ? 1 : -1;
> >  mb->flags = 0; // XXX: does mb_type contain extra information that 
> > could be exported here?
> >  return 1;
> > @@ -1603,43 +1609,43 @@ void ff_print_debug_info2(AVCodecContext *avctx, 
> > AVFrame *pict, uint8_t *mbskip_
> >  int sy = mb_y * 16 + 4 + 8 * (i >> 1);
> >  int xy = (mb_x * 2 + (i & 1) +
> >(mb_y * 2 + (i >> 1)) * mv_stride) 
> > << (mv_sample_log2 - 1);
> > -int mx = (motion_val[direction][xy][0] >> 
> > shift) + sx;
> > -int my = (motion_val[direction][xy][1] >> 
> > shift) + sy;
> > -mbcount += add_mb(mvs + mbcount, mb_type, sx, 
> > sy, mx, my, direction);
> > +int mx = motion_val[direction][xy][0];
> > +int my = motion_val[direction][xy][1];
> > +mbcount += add_mb(mvs + mbcount, mb_type, sx, 
> > sy, mx, my, shift, direction);
> >  }
> >  } else if (IS_16X8(mb_type)) {
> >  for (i = 0; i < 2; i++) {
> >  int sx = mb_x * 16 + 8;
> >  int sy = mb_y * 16 + 4 + 8 * i;
> >  int xy = (mb_x * 2 + (mb_y * 2 + i) * 
> > mv_stride) << (mv_sample_log2 - 1);
> > -int mx = (motion_val[direction][xy][0] >> 
> > shift);
> > -int my = (motion_val[direction][xy][1] >> 
> > shift);
> > +int mx = motion_val[direction][xy][0];
> > +int my = motion_val[direction][xy][1];
> >  
> >  if (IS_INTERLACED(mb_type))
> >  my *= 2;
> >  
> > -mbcount += add_mb(mvs + mbcount, mb_type, sx, 
> > sy, mx + sx, my + sy, direction);
> > +mbcount += add_mb(mvs + mbcount, mb_type, sx, 
> > sy, mx, my, shift, direction);
> >  }
> >  } else if (IS_8X16(mb_type)) {
> >  for (i = 0; i < 2; i++) {
> >  int sx = mb_x * 16 + 4 + 8 * i;
> >  int sy = mb_y * 16 + 8;
> >  int xy = (mb_x * 2 + i + mb_y * 2 * mv_stride) 
> > << (mv_sample_log2 - 1);
> > -int mx = motion_val[direction][xy][0] >> shift;
> > -int my = motion_val[direction][xy][1] >> shift;
> > +int mx = motion_val[direction][xy][0];
> > +int my = motion_val[direction][xy][1];
> >  
> >  if (IS_INTERLACED(mb_type))
> >  my *= 2;
> >  
> > -mbcount += add_mb(mvs + mbcount, mb_type, sx, 
> > sy, mx + sx, my + sy, direction);
> > +mbcount += add_mb(mvs + mbcount, mb_type, sx, 
> > sy, mx, my, shift, direction);
> >  }
> >  } else {
> >int sx = mb_x * 16 + 8;
> >int sy = mb_y * 16 + 8;
> >int xy = (mb_x + mb_y * mv_stride) << 
> > mv_sample_log2;
> > -  int mx = (motion_val[direction][xy][0]>>shift) + 
> > sx;
> 

Re: [FFmpeg-devel] [PATCH] ffprobe: do not exit in case the demuxer returns AVERROR(EAGAIN)

[Stefano Sabatini]

On date Wednesday 2015-11-18 15:21:15 +0100, Nicolas George encoded:
> Le septidi 27 brumaire, an CCXXIV, Stefano Sabatini a écrit :
> > Fix demuxing of files for which the demuxer returns AVERROR(EAGAIN) at
> > some point. Also returns error code to the caller in case of different
> > non EOF error.
> > ---
> >  ffprobe.c | 11 ++-
> >  1 file changed, 10 insertions(+), 1 deletion(-)
> > 
> > diff --git a/ffprobe.c b/ffprobe.c
> > index c304a6d..481ff0b 100644
> > --- a/ffprobe.c
> > +++ b/ffprobe.c
> > @@ -2027,7 +2027,16 @@ static int read_interval_packets(WriterContext *w, 
> > AVFormatContext *fmt_ctx,
> >  ret = AVERROR(ENOMEM);
> >  goto end;
> >  }
> > -while (!av_read_frame(fmt_ctx, )) {
> 
> > +while (1) {
> > +ret = av_read_frame(fmt_ctx, );
> > +if (ret == AVERROR(EAGAIN))
> > +continue;
> 
> This is a busy wait, this is one of the evilest constructions possible.
> 

> For real EAGAIN cases, a sleep would be needed, until we have a real event
> loop. Other cases are bugs in the demuxers.

Yes, updated patch with EAGAIN (if this is fine, I'll send patches to
fix the examples).

In my case the EAGAIN is triggered by the flv demuxer code in
flvdec.c:935:

/* skip empty data packets */
if (!size) {
ret = AVERROR(EAGAIN);
goto leave;
}

introduced in 527c2e64295671bfcd9c86ca2cfd75e0f4d79f73.

Not sure if this is a defect.
-- 
FFmpeg = Fast and Furious Magic Proud Eager Guide
>From 50ead71633e99197ead0bae8d99640dbac28e1d0 Mon Sep 17 00:00:00 2001
From: Stefano Sabatini 
Date: Tue, 17 Nov 2015 15:11:09 +0100
Subject: [PATCH] ffprobe: Sleep and retry when the demuxer returns
 AVERROR(EAGAIN)

Fix demuxing of files for which the demuxer returns AVERROR(EAGAIN) at
some point. Also return error code to the caller in case of non-EOF
error.
---
 ffprobe.c | 14 +-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/ffprobe.c b/ffprobe.c
index c304a6d..b4ab286 100644
--- a/ffprobe.c
+++ b/ffprobe.c
@@ -41,6 +41,7 @@
 #include "libavutil/intreadwrite.h"
 #include "libavutil/libm.h"
 #include "libavutil/parseutils.h"
+#include "libavutil/time.h"
 #include "libavutil/timecode.h"
 #include "libavutil/timestamp.h"
 #include "libavdevice/avdevice.h"
@@ -2027,7 +2028,18 @@ static int read_interval_packets(WriterContext *w, AVFormatContext *fmt_ctx,
 ret = AVERROR(ENOMEM);
 goto end;
 }
-while (!av_read_frame(fmt_ctx, )) {
+while (1) {
+ret = av_read_frame(fmt_ctx, );
+if (ret == AVERROR(EAGAIN)) {
+av_usleep(1);
+continue;
+}
+if (ret < 0) {
+if (ret == AVERROR_EOF)
+ret = 0;
+break;
+}
+
 if (fmt_ctx->nb_streams > nb_streams) {
 REALLOCZ_ARRAY_STREAM(nb_streams_frames,  nb_streams, fmt_ctx->nb_streams);
 REALLOCZ_ARRAY_STREAM(nb_streams_packets, nb_streams, fmt_ctx->nb_streams);
-- 
1.9.1

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

[FFmpeg-devel] [PATCH] avutil/motion_vector: export subpel motion information

[Clément Bœsch]

---
FATE test changes because of the replacement of >>n with /(1<w = IS_8X8(mb_type) || IS_8X16(mb_type) ? 8 : 16;
 mb->h = IS_8X8(mb_type) || IS_16X8(mb_type) ? 8 : 16;
-mb->src_x = src_x;
-mb->src_y = src_y;
+
+mb->motion_x = motion_x;
+mb->motion_y = motion_y;
+mb->motion_scale = motion_scale;
+
 mb->dst_x = dst_x;
 mb->dst_y = dst_y;
+mb->src_x = dst_x + motion_x / motion_scale;
+mb->src_y = dst_y + motion_y / motion_scale;
 mb->source = direction ? 1 : -1;
 mb->flags = 0; // XXX: does mb_type contain extra information that could 
be exported here?
 return 1;
@@ -1581,6 +1586,7 @@ void ff_print_debug_info2(AVCodecContext *avctx, AVFrame 
*pict, uint8_t *mbskip_
 {
 if ((avctx->flags2 & AV_CODEC_FLAG2_EXPORT_MVS) && mbtype_table && 
motion_val[0]) {
 const int shift = 1 + quarter_sample;
+const int scale = 1 << shift;
 const int mv_sample_log2 = avctx->codec_id == AV_CODEC_ID_H264 || 
avctx->codec_id == AV_CODEC_ID_SVQ3 ? 2 : 1;
 const int mv_stride  = (mb_width << mv_sample_log2) +
(avctx->codec->id == AV_CODEC_ID_H264 ? 0 : 
1);
@@ -1604,43 +1610,43 @@ void ff_print_debug_info2(AVCodecContext *avctx, 
AVFrame *pict, uint8_t *mbskip_
 int sy = mb_y * 16 + 4 + 8 * (i >> 1);
 int xy = (mb_x * 2 + (i & 1) +
   (mb_y * 2 + (i >> 1)) * mv_stride) << 
(mv_sample_log2 - 1);
-int mx = (motion_val[direction][xy][0] >> shift) + 
sx;
-int my = (motion_val[direction][xy][1] >> shift) + 
sy;
-mbcount += add_mb(mvs + mbcount, mb_type, sx, sy, 
mx, my, direction);
+int mx = motion_val[direction][xy][0];
+int my = motion_val[direction][xy][1];
+mbcount += add_mb(mvs + mbcount, mb_type, sx, sy, 
mx, my, scale, direction);
 }
 } else if (IS_16X8(mb_type)) {
 for (i = 0; i < 2; i++) {
 int sx = mb_x * 16 + 8;
 int sy = mb_y * 16 + 4 + 8 * i;
 int xy = (mb_x * 2 + (mb_y * 2 + i) * mv_stride) 
<< (mv_sample_log2 - 1);
-int mx = (motion_val[direction][xy][0] >> shift);
-int my = (motion_val[direction][xy][1] >> shift);
+int mx = motion_val[direction][xy][0];
+int my = motion_val[direction][xy][1];
 
 if (IS_INTERLACED(mb_type))
 my *= 2;
 
-mbcount += add_mb(mvs + mbcount, mb_type, sx, sy, 
mx + sx, my + sy, direction);
+mbcount += add_mb(mvs + mbcount, mb_type, sx, sy, 
mx, my, scale, direction);
 }
 } else if (IS_8X16(mb_type)) {
 for (i = 0; i < 2; i++) {
 int sx = mb_x * 16 + 4 + 8 * i;
 int sy = mb_y * 16 + 8;
 int xy = (mb_x * 2 + i + mb_y * 2 * mv_stride) << 
(mv_sample_log2 - 1);
-int mx = motion_val[direction][xy][0] >> shift;
-int my = motion_val[direction][xy][1] >> shift;
+int mx = motion_val[direction][xy][0];
+int my = motion_val[direction][xy][1];
 
 if (IS_INTERLACED(mb_type))
 my *= 2;
 
-mbcount += add_mb(mvs + mbcount, mb_type, sx, sy, 
mx + sx, my + sy, direction);
+mbcount += add_mb(mvs + mbcount, mb_type, sx, sy, 
mx, my, scale, direction);
 }
 } else {
   

Re: [FFmpeg-devel] [PATCH] ffprobe: do not exit in case the demuxer returns AVERROR(EAGAIN)

[Nicolas George]

Le decadi 30 brumaire, an CCXXIV, Stefano Sabatini a écrit :
> Please elaborate, what does it means that "it is not possible"?

It is no longer busy-waiting, but it is still polling: ffprobe will wake up
a hundred times per second waiting for network packets that may not come for
several minutes or hours, preventing the processor from going into deep
sleep, reducing the battery life, consuming energy and speeding up the
climate change.

And the other way around, in one hundredth of second the kernel buffer for
the stream may fill up, causing the peer to discard data or other bad
behaviours.

So, no, it is definitely not good.

But FFmpeg does not have a better API for that yet, so the best we can do
for now is to use polling, make a wet-finger estimate of a good sleep time,
and hope for the best. I would have gone for 10 milliseconds too.

(The bit about climate change was exaggeration, but the rest is really
relevant: a friend of mine used to work on lightweight embedded systems, and
useless polling wakeups did reduce the battery life by a huge amount. And we
had to add a thread on the UDP protocol reader to avoid packets from being
discarded, amongst other things while the process was sleeping.)

> Or in other words, are you fine with the patch? If not, what fix do
> you suggest instead?

I can not comment on the loop and the setting of the return value, because I
do not know the code of ffprobe well enough and this part seems a bit
convoluted (probably out of necessity), so I can not comment the patch as a
whole. The part that I know seems ok, as well as can be with the current
API.

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] ffprobe: do not exit in case the demuxer returns AVERROR(EAGAIN)

[wm4]

On Fri, 20 Nov 2015 16:44:56 +0100
Nicolas George  wrote:

> Le decadi 30 brumaire, an CCXXIV, Stefano Sabatini a écrit :
> > Please elaborate, what does it means that "it is not possible"?  
> 
> It is no longer busy-waiting, but it is still polling: ffprobe will wake up
> a hundred times per second waiting for network packets that may not come for
> several minutes or hours, preventing the processor from going into deep
> sleep, reducing the battery life, consuming energy and speeding up the
> climate change.
> 
> And the other way around, in one hundredth of second the kernel buffer for
> the stream may fill up, causing the peer to discard data or other bad
> behaviours.
> 
> So, no, it is definitely not good.
> 
> But FFmpeg does not have a better API for that yet, so the best we can do
> for now is to use polling, make a wet-finger estimate of a good sleep time,
> and hope for the best. I would have gone for 10 milliseconds too.
> 
> (The bit about climate change was exaggeration, but the rest is really
> relevant: a friend of mine used to work on lightweight embedded systems, and
> useless polling wakeups did reduce the battery life by a huge amount. And we
> had to add a thread on the UDP protocol reader to avoid packets from being
> discarded, amongst other things while the process was sleeping.)
> 
> > Or in other words, are you fine with the patch? If not, what fix do
> > you suggest instead?  
> 
> I can not comment on the loop and the setting of the return value, because I
> do not know the code of ffprobe well enough and this part seems a bit
> convoluted (probably out of necessity), so I can not comment the patch as a
> whole. The part that I know seems ok, as well as can be with the current
> API.
> 
> Regards,
> 

Not sure why you argue for a better API. The semantics are to block
until there's a new packet, or EOF. That's it. There is no room for
EAGAIN with "waiting". (EAGAIN that doesn't imply that the caller
should "wait", but can happen immediately, is ok, but makes not much
sense.)
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] ffprobe: do not exit in case the demuxer returns AVERROR(EAGAIN)

[Nicolas George]

Le decadi 30 brumaire, an CCXXIV, wm4 a écrit :
> Not sure why you argue for a better API. The semantics are to block
> until there's a new packet, or EOF. That's it.

Did you not realize yet that blocking APIs are unsuited for any complex
application? Even ffmpeg.c suffers from this, and it is not very complex
with regard to I/O.

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] [PATCHv2 5/5] lavfi/select: add support for concatdec_select option

[Stefano Sabatini]

On date Friday 2015-11-20 13:54:26 +0100, Marton Balint encoded:
> This option can be used to select useful frames from an ffconcat file which is
> using inpoints and outpoints but where the source files are not intra frame
> only.
> 
> Signed-off-by: Marton Balint 
> ---
>  doc/filters.texi   | 20 
>  libavfilter/f_select.c | 27 +++
>  2 files changed, 47 insertions(+)
> 
> diff --git a/doc/filters.texi b/doc/filters.texi
> index 471ec3f..c886976 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -13196,6 +13196,19 @@ value between 0 and 1 to indicate a new scene; a low 
> value reflects a low
>  probability for the current frame to introduce a new scene, while a higher
>  value means the current frame is more likely to be one (see the example 
> below)
>  
> +@item concatdec_select
> +The concat demuxer can set the @var{lavf.concat.start_time} and the
> +@var{lavf.concat.duration} packet metadata values which are also present in 
> the
> +decoded frames.

Maybe here you could elaborate. Something like:

The concat demuxer can select only part of a concat input file by
setting an inpoint and an outpoint, but the output packets may not be
entirely contained in the selected interval. By using this variable,
it is possible to skip frames generated by the concat demuxer which
are not exactly contained in the selected interval.

> +
> +The @var{concatdec_select} variable is -1 if the frame pts is at least
> +start_time and either the duration metadata is missing or the frame pts is 
> less
> +than start_time + duration, 0 otherwise, and NaN if the start_time metadata 
> is
> +missing.
> +
> +That basically means that an input frame is selected if its pts is within the
> +interval set by the concat demuxer.
[...]

Looks good otherwise.
-- 
FFmpeg = Fanciful Free Merciless Proud Erroneous Ghost
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avfilter/WIP: add streamselect filter

[Nicolas George]

Le decadi 30 brumaire, an CCXXIV, Clement Boesch a écrit :
> Alright, so if I understand well, all I have to do is implement a
> forwarding such as this:
> 
> static int request_frame(AVFilterLink *outlink)
> {
> AVFilterContext *ctx = outlink->src;
> StreamSelectContext *s = ctx->priv;
> const int outlink_idx = FF_OUTLINK_IDX(outlink);
> const int inlink_idx = s->map[outlink_idx];
> AVFilterLink *inlink = ctx->inputs[inlink_idx];
> return ff_request_frame(inlink);
> }

Indeed. Note: you may want to keep both "in->out" and "out->in" maps in
arrays.

> Unfortunately, it seems the other stream(s) in "background" (not present
> in the mapping) will not be consumed at all (filter_frame() is not called)
> so frames are never dropped. As a result, when switching to that
> background stream, the frames are delayed by a large margin (the amount of
> time the main stream has been in use).

If the corresponding input is connected to a buffersrc fed from a file, that
is perfectly normal.

> While I still need to add a timing check in the context (to avoid small
> non monotonically problems), how would it be possible to continue
> consuming the background streams?

I had not thought about this issue before now. I think it is not possible in
principle. Consider this graph:

  +---+
[v1] >|.  |   ++
  | `>|>[ setpts ×2 ]>||
[v2] >|   |   | vstack |>
  | .>|-->||
[v3] >|´  |   ++
  +---+

With the ×2 setpts, v1 must be consumed twice as fast as v3 (or the other
way around, does not matter). But then, what happens when v2 gets connected
to one of the outputs?

You need to decide the constraints or the user interface to solve this
before you can implement anything.

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] Map description metadata to ©inf tag in .mov wrapper

[Michael Niedermayer]

On Thu, Nov 19, 2015 at 11:20:04AM -0500, Brian Wheeler wrote:
> The description metadata isn't mapped to anything when the mov
> wrapper is used.This patch maps description to ©inf for both
> reading and writing.
> The change to mov.c makes the mapping round-trip.
> 
> Signed-off-by: Brian Wheeler 
> ---
>  libavformat/mov.c| 2 +-
>  libavformat/movenc.c | 1 +
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index 0eb7272..960667f 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -342,7 +342,7 @@ static int mov_read_udta_string(MOVContext *c,
> AVIOContext *pb, MOVAtom atom)
>  case MKTAG(0xa9,'g','e','n'): key = "genre"; break;

this is corrupted by extra newlines

Applying: Map description metadata to ©inf tag in .mov wrapper
fatal: corrupt patch at line 11
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 Map description metadata to ©inf tag in .mov wrapper
When you have resolved this problem run "git am --resolved".
If you would prefer to skip this patch, instead run "git am --skip".
To restore the original branch and stop patching run "git am --abort".

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

What does censorship reveal? It reveals fear. -- Julian Assange


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

Re: [FFmpeg-devel] [PATCH] ffprobe: do not exit in case the demuxer returns AVERROR(EAGAIN)

[Stefano Sabatini]

On date Friday 2015-11-20 14:35:20 +0100, Nicolas George encoded:
> Le decadi 30 brumaire, an CCXXIV, Stefano Sabatini a écrit :
> > Yes, updated patch with EAGAIN (if this is fine, I'll send patches to
> > fix the examples).
> 

> The API use seems... not good, because it is not possible, but at least ok.

Please elaborate, what does it means that "it is not possible"?

Or in other words, are you fine with the patch? If not, what fix do
you suggest instead?
-- 
FFmpeg = Fantastic and Frightening MultiPurpose Encoding/decoding Geisha
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] ffprobe: do not exit in case the demuxer returns AVERROR(EAGAIN)

[wm4]

On Fri, 20 Nov 2015 17:09:50 +0100
Nicolas George  wrote:

> Le decadi 30 brumaire, an CCXXIV, wm4 a écrit :
> > Not sure why you argue for a better API. The semantics are to block
> > until there's a new packet, or EOF. That's it.  
> 
> Did you not realize yet that blocking APIs are unsuited for any complex
> application? Even ffmpeg.c suffers from this, and it is not very complex
> with regard to I/O.
> 

Uh what? Blocking I/O is perfectly fine, especially for something like
a demuxer, or something that does file I/O. Just use threads.

But this has absolutely nothing to do with the problem at hand.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCHv2] avformat/mp3dec, rmdec: check return value of ffio_ensure_seekback

[Ganesh Ajjanagadde]

[...]
On Fri, Nov 20, 2015 at 9:30 AM, Ronald S. Bultje  wrote:
> He pointed out that you added multiple instances of the exact same line in
> the code:
>
> av_log(s, AV_LOG_WARNING, "ffio_ensure_seekback(): %s\n", av_err2str(ret));
>
> which is both duplicate as well as meaningless. There is nothing to disagree
> with here, his criticism is just and needs to be addressed. Stop being
> defensive about trivial things like that. Patch review is all about finding
> issues, fixing them, and moving on.
>
> Ronald

To get things straight, are you unhappy about repetition of the same
message, or unhappy about the choice of message but fine with the
repetition per se?
If it is the former, we will need an mp3 person to suggest what
message to use for the two failures, and you may want to look at your
own avcodec/vp9:4194.

If it is the latter, I have an idea.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCHv2] avformat/mp3dec, rmdec: check return value of ffio_ensure_seekback

[Michael Niedermayer]

On Tue, Nov 17, 2015 at 05:39:31PM -0500, Ganesh Ajjanagadde wrote:
> ffio_ensure_seekback can fail due to e.g ENOMEM. This return value is
> checked here and a diagnostic is logged.
> 
> All usage of ffio_ensure_seekback in the codebase now has the return value 
> checked.
> 
> Signed-off-by: Ganesh Ajjanagadde 
> ---
>  libavformat/mp3dec.c | 6 --
>  libavformat/rmdec.c  | 3 ++-
>  2 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
> index 32ca00c..a14bccd 100644
> --- a/libavformat/mp3dec.c
> +++ b/libavformat/mp3dec.c
> @@ -380,11 +380,13 @@ static int mp3_read_header(AVFormatContext *s)
>  uint32_t header, header2;
>  int frame_size;
>  if (!(i&1023))
> -ffio_ensure_seekback(s->pb, i + 1024 + 4);
> +if ((ret = ffio_ensure_seekback(s->pb, i + 1024 + 4)) < 0)
> +av_log(s, AV_LOG_WARNING, "ffio_ensure_seekback(): %s\n", 
> av_err2str(ret));
>  frame_size = check(s->pb, off + i, );
>  if (frame_size > 0) {
>  avio_seek(s->pb, off, SEEK_SET);
> -ffio_ensure_seekback(s->pb, i + 1024 + frame_size + 4);
> +if ((ret = ffio_ensure_seekback(s->pb, i + 1024 + frame_size + 
> 4)) < 0)
> +av_log(s, AV_LOG_WARNING, "ffio_ensure_seekback(): %s\n", 
> av_err2str(ret));
>  if (check(s->pb, off + i + frame_size, ) >= 0 &&
>  (header & SAME_HEADER_MASK) == (header2 & SAME_HEADER_MASK))
>  {

it is probably better to also break out of the loop if these fail

and as a message yu could use something like
initial junk detection and skiping not possible due to 

[]
-- 
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] ffprobe: do not exit in case the demuxer returns AVERROR(EAGAIN)

[Nicolas George]

Le decadi 30 brumaire, an CCXXIV, wm4 a écrit :
> Interrupt callback.

Which requires polling. You are responsible for climate change and poor
battery life. Seriously, go read a book on proper I/O design or ten.

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] [PATCHv2] avformat/mp3dec, rmdec: check return value of ffio_ensure_seekback

[Ronald S. Bultje]

Hi,

On Fri, Nov 20, 2015 at 11:39 AM, Ganesh Ajjanagadde 
wrote:

> [...]
> On Fri, Nov 20, 2015 at 9:30 AM, Ronald S. Bultje 
> wrote:
> > He pointed out that you added multiple instances of the exact same line
> in
> > the code:
> >
> > av_log(s, AV_LOG_WARNING, "ffio_ensure_seekback(): %s\n",
> av_err2str(ret));
> >
> > which is both duplicate as well as meaningless. There is nothing to
> disagree
> > with here, his criticism is just and needs to be addressed. Stop being
> > defensive about trivial things like that. Patch review is all about
> finding
> > issues, fixing them, and moving on.
> >
> > Ronald
>
> To get things straight, are you unhappy about repetition of the same
> message, or unhappy about the choice of message but fine with the
> repetition per se?
> If it is the former, we will need an mp3 person to suggest what
> message to use for the two failures, and you may want to look at your
> own avcodec/vp9:4194.
>
> If it is the latter, I have an idea.
>

Both, to some extend, but I guess if you improve the message to explain
what went wrong, the other isn't an issue anymore. So I would suggest to
improve the message and that's sufficient for me.

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

Re: [FFmpeg-devel] [PATCH 2/5] concatdec: calculate duration early if outpoint is known

[Marton Balint]

On Sat, 14 Nov 2015, Marton Balint wrote:



On Fri, 13 Nov 2015, Nicolas George wrote:


Le decadi 20 brumaire, an CCXXIV, Marton Balint a écrit :

Signed-off-by: Marton Balint 
---
 libavformat/concatdec.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/libavformat/concatdec.c b/libavformat/concatdec.c
index 0180a7e..560aa64 100644
--- a/libavformat/concatdec.c
+++ b/libavformat/concatdec.c
@@ -316,6 +316,8 @@ static int open_file(AVFormatContext *avf, unsigned 

fileno)

cat->files[fileno - 1].duration;
 file->file_start_time = (cat->avf->start_time == AV_NOPTS_VALUE) ? 0 

: cat->avf->start_time;
 file->file_inpoint = (file->inpoint == AV_NOPTS_VALUE) ? 

file->file_start_time : file->inpoint;


+if (file->duration == AV_NOPTS_VALUE && file->outpoint != 

AV_NOPTS_VALUE)

+file->duration = file->outpoint - file->file_inpoint;


Is it not already done? In read_header():

   if (cat->files[i].duration == AV_NOPTS_VALUE) {
   if (cat->files[i].inpoint == AV_NOPTS_VALUE || 

cat->files[i].outpoint == AV_NOPTS_VALUE)

   break;
   cat->files[i].duration = cat->files[i].outpoint - 

cat->files[i].inpoint;

   }

is done for every file, it seems it does exactly the same thing.


In read_header, duration is only set if both inpoint and outpoint is set 
by the user.


In open_file duration is set if outpoint is set by the user which is 
the superset of the first case.


Do you have any further questions? Can I apply the series?

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

Re: [FFmpeg-devel] [PATCH 2/3] ffserver: Use AVOption API to access ffm demuxer instead of direct access depending on ABI

[Andreas Cadhalpun]

On 20.11.2015 03:26, Michael Niedermayer wrote:
> From: Michael Niedermayer 
> 
> Signed-off-by: Michael Niedermayer 
> ---
>  ffserver.c |6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/ffserver.c b/ffserver.c
> index fb8ab7d..7e4f620 100644
> --- a/ffserver.c
> +++ b/ffserver.c
> @@ -300,9 +300,9 @@ bail_eio:
>  static void ffm_set_write_index(AVFormatContext *s, int64_t pos,
>  int64_t file_size)
>  {
> -FFMContext *ffm = s->priv_data;
> -ffm->write_index = pos;
> -ffm->file_size = file_size;
> +av_opt_set_int(s, "server_attached", 1, AV_OPT_SEARCH_CHILDREN);
> +av_opt_set_int(s, "write_index", pos, AV_OPT_SEARCH_CHILDREN);
> +av_opt_set_int(s, "file_size", file_size, AV_OPT_SEARCH_CHILDREN);
>  }
>  
>  static char *ctime1(char *buf2, size_t buf_size)

The code change looks very good, but it would be nice, if the commit
message would explain that server_attached is newly added (and why).
Otherwise one might wonder how it was set previously.

Best regards,
Andreas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] ffprobe: do not exit in case the demuxer returns AVERROR(EAGAIN)

[wm4]

On Fri, 20 Nov 2015 18:35:33 +0100
Nicolas George  wrote:

> Le decadi 30 brumaire, an CCXXIV, wm4 a écrit :
> > Uh what? Blocking I/O is perfectly fine, especially for something like
> > a demuxer, or something that does file I/O. Just use threads.  
> 
> Just use threads... I was waiting for it. Trick question: an application is
> reading from a live stream, the server goes down, the user switches to
> another stream: how do you close the first stream?

Interrupt callback.

> > But this has absolutely nothing to do with the problem at hand.  
> 
> It has everything to do with it.

No.

> Regards,
> 

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

Re: [FFmpeg-devel] [PATCH 1/3] avformat/ffmdec: Add cleaner API for ffserver to interface without depending on internal ABI

[Andreas Cadhalpun]

On 20.11.2015 03:26, Michael Niedermayer wrote:
> From: Michael Niedermayer 
> 
> Signed-off-by: Michael Niedermayer 
> ---
>  libavformat/ffm.h |2 ++
>  libavformat/ffmdec.c  |   14 ++
>  libavformat/version.h |2 +-
>  3 files changed, 17 insertions(+), 1 deletion(-)
> 

This looks good.

> diff --git a/libavformat/version.h b/libavformat/version.h
> index 55198e7..d66eeff 100644
> --- a/libavformat/version.h
> +++ b/libavformat/version.h
> @@ -30,7 +30,7 @@
>  #include "libavutil/version.h"
>  
>  #define LIBAVFORMAT_VERSION_MAJOR  57
> -#define LIBAVFORMAT_VERSION_MINOR  15
> +#define LIBAVFORMAT_VERSION_MINOR  16
>  #define LIBAVFORMAT_VERSION_MICRO 100
>  
>  #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \
> 

Only MINOR is already at 16 now.

Best regards,
Andreas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] AAC sequence header data setup if stream copy

[Michael Niedermayer]

On Wed, Nov 18, 2015 at 09:56:05PM +0200, Maksym Veremeyenko wrote:
> On 18.11.2015 19:37, Michael Niedermayer wrote:
> [...]
> >the first field of the context must be a AVClass
> 
> updated patch attached
> 
> -- 
> Maksym Veremeyenko
> 

>  ffmpeg.c |1 
>  libavformat/flvenc.c |   54 
> +++
>  2 files changed, 55 insertions(+)
> 198cb19b08cf98f9eb6e0c6eeaa003a1715d5eff  
> 0001-AAC-sequence-header-data-setup-if-stream-copy.patch
> From 665751883fd515c753ada7bbf4d24c89a24f3d30 Mon Sep 17 00:00:00 2001
> From: Maksym Veremeyenko 
> Date: Wed, 18 Nov 2015 23:44:08 +0200
> Subject: [PATCH] AAC sequence header data setup if stream copy
> 
> ---
>  ffmpeg.c |1 +

split and applied


>  libavformat/flvenc.c |   54 
> ++
>  2 files changed, 55 insertions(+), 0 deletions(-)

applied the aac_seq_header_detect part

the rest could be made more generic, not restricted to flv or aac

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Democracy is the form of government in which you can choose your dictator


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

Re: [FFmpeg-devel] [PATCH] ffprobe: do not exit in case the demuxer returns AVERROR(EAGAIN)

[wm4]

On Fri, 20 Nov 2015 19:00:31 +0100
Nicolas George  wrote:

> Le decadi 30 brumaire, an CCXXIV, wm4 a écrit :
> > Interrupt callback.  
> 
> Which requires polling.

I never claimed otherwise. But the topic of the discussion is whether
Stefano's patch adds or fixes a bug. Even if the implementation has to
do polling currently, there's no need for EAGAIN poll semantics.

> You are responsible for climate change and poor
> battery life. Seriously, go read a book on proper I/O design or ten.

Do you have anything other than lame trolling and thread derailing to
offer?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] swscale/arm: add ff_nv{12, 21}_to_{argb, rgba, abgr, bgra}_neon

[Michael Niedermayer]

On Thu, Nov 19, 2015 at 06:29:23PM +0100, Clément Bœsch wrote:
> On Thu, Nov 19, 2015 at 04:50:54PM +0100, Michael Niedermayer wrote:
> > On Thu, Nov 19, 2015 at 11:48:53AM +0100, Clément Bœsch wrote:
> > > From: Matthieu Bouron 
> > > 
> > > Signed-off-by: Matthieu Bouron 
> > > Signed-off-by: Clément Bœsch 
> > > 
> > > ---
> > > The function takes about 29ms with a 1080p source (testsrc2) on a
> > > cortex-a8. Though, 16ms (more than half the time!) is spend in the vst2
> > > call. Any suggestion on how to speed up this?
> > > 
> > > Also, the reference code seems to cause some kind of ringing, while our
> > > ASM doesn't:
> > >   http://b.pkh.me/nv12-rgba-ref.png
> > >   http://b.pkh.me/nv12-rgba-neon.png
> > 
> > what did you test exactly here ?
> 
> ./ffmpeg -f lavfi -i testsrc2 -vf format=nv12,format=rgba -ss 1 -frames:v 1 
> -y nv12-rgba-ref.png
> 
> (on ARM though, and with -cpuflags 0)
> 
> > but there are several codepathes for rgb output, one uses LUTs and
> > not all use full resolution chroma
> > 
> 
> Yeah, we noticed...
> 
> Note: on x86 there are some yuv2rgb mmx code but it's not called above
> because it doesn't handle nv12 (only yuv420 & friends), so the chroma
> issue is reproducible (it's calling the LUT path).
> 
> > 
> > > 
> > > Last, we noticed that the y_offset is scaled to 1<<9 for some reason we
> > > couldn't figure out. Hopefully we're doing it correctly here.
> > 
> > [...]
> > > +.macro compute_half_line dst half_y ofmt
> > > +vmovl.u8q7, \half_y@ 
> > > 8px of Y
> > > +vdup.16 q5, r9
> > > +vsub.s16q7, q5
> > > +vmull.s16   q1, d14, d0@ 
> > > q1 = (srcY - y_offset) * y_coeff (left)
> > > +vmull.s16   q2, d15, d0@ 
> > > q2 = (srcY - y_offset) * y_coeff (right)
> > 
> > if you do something like (srcY) * y_coeff - y_offset2
> > then you could keep a bit more precission in the requested brightness
> > correction
> 
> The code in swscale/output.c seems to always use the form we use here. Is
> it on purpose?

if srcY has some extra bits precission then it shuld be fine


> 

> > OTOH maybe you want to be bitexact to some existing codepath
> > 
> 
> Right... I suppose we don't have much tests with custom
> brightness/contrast/saturation. Should I add expose them in vf_scale and
> see how much breaks? :)

contrast/brightness/saturation fate tests are welcome

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

He who knows, does not speak. He who speaks, does not know. -- Lao Tsu


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

Re: [FFmpeg-devel] [PATCH 3/3] avformat/ffmdec: Only return EAGAIN if a server is attached

[Andreas Cadhalpun]

On 20.11.2015 03:26, Michael Niedermayer wrote:
> From: Michael Niedermayer 
> 
> This should fix a infinite loop
> 
> Signed-off-by: Michael Niedermayer 
> ---
>  libavformat/ffmdec.c |9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/libavformat/ffmdec.c b/libavformat/ffmdec.c
> index c3850db..e7c1449 100644
> --- a/libavformat/ffmdec.c
> +++ b/libavformat/ffmdec.c
> @@ -49,7 +49,10 @@ static int ffm_is_avail_data(AVFormatContext *s, int size)
>  } else {
>  if (pos == ffm->write_index) {
>  /* exactly at the end of stream */
> -return AVERROR(EAGAIN);
> +if (ffm->server_attached)
> +return AVERROR(EAGAIN);
> +else
> +return AVERROR_INVALIDDATA;
>  } else if (pos < ffm->write_index) {
>  avail_size = ffm->write_index - pos;
>  } else {
> @@ -59,8 +62,10 @@ static int ffm_is_avail_data(AVFormatContext *s, int size)
>  avail_size = (avail_size / ffm->packet_size) * (ffm->packet_size - 
> FFM_HEADER_SIZE) + len;
>  if (size <= avail_size)
>  return 1;
> -else
> +else if (ffm->server_attached)
>  return AVERROR(EAGAIN);
> +else
> +return AVERROR_INVALIDDATA;
>  }
>  
>  static int ffm_resync(AVFormatContext *s, int state)
> 

This reliably fixes the infinite EAGAIN loops. Thanks a lot!

I haven't tested ffserver, but assuming you did and it still works,
this patch is fine.

Best regards,
Andreas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avfilter/all: handle ff_formats_unref correctly

[Michael Niedermayer]

On Sun, Nov 15, 2015 at 07:21:11PM -0500, Ganesh Ajjanagadde wrote:
> Recent commits 6aaac24d72a7da631173209841a3944fcb4a3309 and
> 3835554bf8ed78539a3492c239f979c0ab03a15f made progress towards cleaning
> up usage of the formats API, and in particular fixed possible NULL pointer
> dereferences.
> 
> This commit addresses the issue of possible resource leaks when one call
> fails and the others don't.
> 
> Fixes: CID 1338330, 1338329, 1338327, 1338326.
> 
> Signed-off-by: Ganesh Ajjanagadde 
> ---
>  libavfilter/af_channelmap.c |  7 +--
>  libavfilter/vf_alphamerge.c |  7 +--
>  libavfilter/vf_overlay.c| 30 +-
>  3 files changed, 31 insertions(+), 13 deletions(-)

if the error pathes have been tested with valgrind or assan (and
simulating/forcing them to trigger) then this should be ok

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

Many that live deserve death. And some that die deserve life. Can you give
it to them? Then do not be too eager to deal out death in judgement. For
even the very wise cannot see all ends. -- Gandalf


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

Re: [FFmpeg-devel] [PATCH] ffmpeg: fix -copy_prior_start 0 with -copyts and input -ss

[Michael Niedermayer]

On Sun, Nov 01, 2015 at 08:04:25PM -0600, Rodger Combs wrote:
> Also rearranged the relevant check to reduce code duplication
> ---
>  ffmpeg.c | 15 +++
>  1 file changed, 7 insertions(+), 8 deletions(-)

applied

thanks

PS: can this be tested in fate ?

[...]
-- 
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 3/3] avformat/ffmdec: Only return EAGAIN if a server is attached

[Michael Niedermayer]

On Fri, Nov 20, 2015 at 07:34:50PM +0100, Andreas Cadhalpun wrote:
> On 20.11.2015 03:26, Michael Niedermayer wrote:
> > From: Michael Niedermayer 
> > 
> > This should fix a infinite loop
> > 
> > Signed-off-by: Michael Niedermayer 
> > ---
> >  libavformat/ffmdec.c |9 +++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> > 
> > diff --git a/libavformat/ffmdec.c b/libavformat/ffmdec.c
> > index c3850db..e7c1449 100644
> > --- a/libavformat/ffmdec.c
> > +++ b/libavformat/ffmdec.c
> > @@ -49,7 +49,10 @@ static int ffm_is_avail_data(AVFormatContext *s, int 
> > size)
> >  } else {
> >  if (pos == ffm->write_index) {
> >  /* exactly at the end of stream */
> > -return AVERROR(EAGAIN);
> > +if (ffm->server_attached)
> > +return AVERROR(EAGAIN);
> > +else
> > +return AVERROR_INVALIDDATA;
> >  } else if (pos < ffm->write_index) {
> >  avail_size = ffm->write_index - pos;
> >  } else {
> > @@ -59,8 +62,10 @@ static int ffm_is_avail_data(AVFormatContext *s, int 
> > size)
> >  avail_size = (avail_size / ffm->packet_size) * (ffm->packet_size - 
> > FFM_HEADER_SIZE) + len;
> >  if (size <= avail_size)
> >  return 1;
> > -else
> > +else if (ffm->server_attached)
> >  return AVERROR(EAGAIN);
> > +else
> > +return AVERROR_INVALIDDATA;
> >  }
> >  
> >  static int ffm_resync(AVFormatContext *s, int state)
> > 
> 
> This reliably fixes the infinite EAGAIN loops. Thanks a lot!
> 
> I haven't tested ffserver, but assuming you did and it still works,
> this patch is fine.

i lightly tested ffserver, yes, more testing surely cannot hurt
though

applied

thx

[...]
-- 
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 5/8] avcodec/amr: replace #define by typedef

[Michael Niedermayer]

On Thu, Nov 19, 2015 at 08:14:12AM -0500, Ganesh Ajjanagadde wrote:
> See e.g 
> https://stackoverflow.com/questions/1666353/are-typedef-and-define-the-same-in-c
> for rationale.
> 
> Signed-off-by: Ganesh Ajjanagadde 
> ---

should be ok

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

Breaking DRM is a little like attempting to break through a door even
though the window is wide open and the only thing in the house is a bunch
of things you dont want and which you would get tomorrow for free anyway


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/all: handle ff_formats_unref correctly

[Ganesh Ajjanagadde]

On Fri, Nov 20, 2015 at 3:00 PM, Michael Niedermayer  wrote:
> On Sun, Nov 15, 2015 at 07:21:11PM -0500, Ganesh Ajjanagadde wrote:
>> Recent commits 6aaac24d72a7da631173209841a3944fcb4a3309 and
>> 3835554bf8ed78539a3492c239f979c0ab03a15f made progress towards cleaning
>> up usage of the formats API, and in particular fixed possible NULL pointer
>> dereferences.
>>
>> This commit addresses the issue of possible resource leaks when one call
>> fails and the others don't.
>>
>> Fixes: CID 1338330, 1338329, 1338327, 1338326.
>>
>> Signed-off-by: Ganesh Ajjanagadde 
>> ---
>>  libavfilter/af_channelmap.c |  7 +--
>>  libavfilter/vf_alphamerge.c |  7 +--
>>  libavfilter/vf_overlay.c| 30 +-
>>  3 files changed, 31 insertions(+), 13 deletions(-)
>
> if the error pathes have been tested with valgrind or assan (and
> simulating/forcing them to trigger) then this should be ok

Unfortunately, I have no idea how to test such code pathways easily. -
how does one force a particular malloc to fail? Isn't the cheapest
solution simply to submit a Coverity build for this and check?

>
> [...]
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Many that live deserve death. And some that die deserve life. Can you give
> it to them? Then do not be too eager to deal out death in judgement. For
> even the very wise cannot see all ends. -- Gandalf
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/3] avformat/ffmdec: Add cleaner API for ffserver to interface without depending on internal ABI

[Michael Niedermayer]

On Fri, Nov 20, 2015 at 07:33:20PM +0100, Andreas Cadhalpun wrote:
> On 20.11.2015 03:26, Michael Niedermayer wrote:
> > From: Michael Niedermayer 
> > 
> > Signed-off-by: Michael Niedermayer 
> > ---
> >  libavformat/ffm.h |2 ++
> >  libavformat/ffmdec.c  |   14 ++
> >  libavformat/version.h |2 +-
> >  3 files changed, 17 insertions(+), 1 deletion(-)
> > 
> 
> This looks good.
> 
> > diff --git a/libavformat/version.h b/libavformat/version.h
> > index 55198e7..d66eeff 100644
> > --- a/libavformat/version.h
> > +++ b/libavformat/version.h
> > @@ -30,7 +30,7 @@
> >  #include "libavutil/version.h"
> >  
> >  #define LIBAVFORMAT_VERSION_MAJOR  57
> > -#define LIBAVFORMAT_VERSION_MINOR  15
> > +#define LIBAVFORMAT_VERSION_MINOR  16
> >  #define LIBAVFORMAT_VERSION_MICRO 100
> >  
> >  #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \
> > 
> 
> Only MINOR is already at 16 now.

updated

applied

thanks

[...]
-- 
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

Re: [FFmpeg-devel] [PATCH 2/3] ffserver: Use AVOption API to access ffm demuxer instead of direct access depending on ABI

[Michael Niedermayer]

On Fri, Nov 20, 2015 at 07:34:02PM +0100, Andreas Cadhalpun wrote:
> On 20.11.2015 03:26, Michael Niedermayer wrote:
> > From: Michael Niedermayer 
> > 
> > Signed-off-by: Michael Niedermayer 
> > ---
> >  ffserver.c |6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/ffserver.c b/ffserver.c
> > index fb8ab7d..7e4f620 100644
> > --- a/ffserver.c
> > +++ b/ffserver.c
> > @@ -300,9 +300,9 @@ bail_eio:
> >  static void ffm_set_write_index(AVFormatContext *s, int64_t pos,
> >  int64_t file_size)
> >  {
> > -FFMContext *ffm = s->priv_data;
> > -ffm->write_index = pos;
> > -ffm->file_size = file_size;
> > +av_opt_set_int(s, "server_attached", 1, AV_OPT_SEARCH_CHILDREN);
> > +av_opt_set_int(s, "write_index", pos, AV_OPT_SEARCH_CHILDREN);
> > +av_opt_set_int(s, "file_size", file_size, AV_OPT_SEARCH_CHILDREN);
> >  }
> >  
> >  static char *ctime1(char *buf2, size_t buf_size)
> 
> The code change looks very good, but it would be nice, if the commit
> message would explain that server_attached is newly added (and why).
> Otherwise one might wonder how it was set previously.

explained and applied

thx

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

You can kill me, but you cannot change the truth.


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

[FFmpeg-devel] [PATCH] avfilter: add deflash filter

[Paul B Mahol]

Signed-off-by: Paul B Mahol 
---
 libavfilter/Makefile |   1 +
 libavfilter/allfilters.c |   1 +
 libavfilter/vf_deflash.c | 199 +++
 3 files changed, 201 insertions(+)
 create mode 100644 libavfilter/vf_deflash.c

diff --git a/libavfilter/Makefile b/libavfilter/Makefile
index 1f4abeb..f801566 100644
--- a/libavfilter/Makefile
+++ b/libavfilter/Makefile
@@ -122,6 +122,7 @@ OBJS-$(CONFIG_CURVES_FILTER) += vf_curves.o
 OBJS-$(CONFIG_DCTDNOIZ_FILTER)   += vf_dctdnoiz.o
 OBJS-$(CONFIG_DEBAND_FILTER) += vf_deband.o
 OBJS-$(CONFIG_DECIMATE_FILTER)   += vf_decimate.o
+OBJS-$(CONFIG_DEFLASH_FILTER)+= vf_deflash.o
 OBJS-$(CONFIG_DEFLATE_FILTER)+= vf_neighbor.o
 OBJS-$(CONFIG_DEJUDDER_FILTER)   += vf_dejudder.o
 OBJS-$(CONFIG_DELOGO_FILTER) += vf_delogo.o
diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
index 63b8fdb..daacbc3 100644
--- a/libavfilter/allfilters.c
+++ b/libavfilter/allfilters.c
@@ -144,6 +144,7 @@ void avfilter_register_all(void)
 REGISTER_FILTER(DCTDNOIZ,   dctdnoiz,   vf);
 REGISTER_FILTER(DEBAND, deband, vf);
 REGISTER_FILTER(DECIMATE,   decimate,   vf);
+REGISTER_FILTER(DEFLASH,deflash,vf);
 REGISTER_FILTER(DEFLATE,deflate,vf);
 REGISTER_FILTER(DEJUDDER,   dejudder,   vf);
 REGISTER_FILTER(DELOGO, delogo, vf);
diff --git a/libavfilter/vf_deflash.c b/libavfilter/vf_deflash.c
new file mode 100644
index 000..3167989
--- /dev/null
+++ b/libavfilter/vf_deflash.c
@@ -0,0 +1,199 @@
+/*
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include "libavutil/opt.h"
+#include "libavutil/pixdesc.h"
+#include "libavutil/imgutils.h"
+#include "libavutil/internal.h"
+#include "avfilter.h"
+
+#define FF_BUFQUEUE_SIZE 13
+#include "bufferqueue.h"
+
+#include "internal.h"
+#include "video.h"
+
+typedef struct DeFlashContext {
+const AVClass *class;
+
+AVFrame *frame;
+float fthrd[4];
+int thrd[4];
+float fthrr[4];
+int thrr[4];
+
+struct FFBufQueue q;
+
+int planeheight[4];
+int planewidth[4];
+} DeFlashContext;
+
+#define OFFSET(x) offsetof(DeFlashContext, x)
+#define FLAGS AV_OPT_FLAG_VIDEO_PARAM|AV_OPT_FLAG_FILTERING_PARAM
+
+static const AVOption deflash_options[] = {
+{ "0fd", "set flash threshold detection for 1st color component",  
OFFSET(fthrd[0]), AV_OPT_TYPE_FLOAT, {.dbl=0.2}, 0, 5, FLAGS },
+{ "0fr", "set ammount of flash reduction for 1st color component", 
OFFSET(fthrr[0]), AV_OPT_TYPE_FLOAT, {.dbl=0.05}, 0, 1, FLAGS },
+{ "1fd", "set flash threshold detection for 2nd color component",  
OFFSET(fthrd[1]), AV_OPT_TYPE_FLOAT, {.dbl=0.2}, 0, 5, FLAGS },
+{ "1fr", "set ammount of flash reduction for 2nd color component", 
OFFSET(fthrr[1]), AV_OPT_TYPE_FLOAT, {.dbl=0.05}, 0, 1, FLAGS },
+{ "2fd", "set flash threshold detection for 3rd color component",  
OFFSET(fthrd[2]), AV_OPT_TYPE_FLOAT, {.dbl=0.2}, 0, 5, FLAGS },
+{ "2fr", "set ammount of flash reduction for 3rd color component", 
OFFSET(fthrr[2]), AV_OPT_TYPE_FLOAT, {.dbl=0.05}, 0, 1, FLAGS },
+{ NULL }
+};
+
+AVFILTER_DEFINE_CLASS(deflash);
+
+static int query_formats(AVFilterContext *ctx)
+{
+static const enum AVPixelFormat pixel_fmts[] = {
+AV_PIX_FMT_YUV410P,  AV_PIX_FMT_YUV411P,
+AV_PIX_FMT_YUV420P,  AV_PIX_FMT_YUV422P,
+AV_PIX_FMT_YUV440P,  AV_PIX_FMT_YUV444P,
+AV_PIX_FMT_YUVJ420P, AV_PIX_FMT_YUVJ422P,
+AV_PIX_FMT_YUVJ440P, AV_PIX_FMT_YUVJ444P,
+AV_PIX_FMT_YUVJ411P,
+AV_PIX_FMT_NONE
+};
+AVFilterFormats *formats = ff_make_format_list(pixel_fmts);
+if (!formats)
+return AVERROR(ENOMEM);
+return ff_set_common_formats(ctx, formats);
+}
+
+static int config_input(AVFilterLink *inlink)
+{
+const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(inlink->format);
+AVFilterContext *ctx = inlink->dst;
+DeFlashContext *s = ctx->priv;
+
+s->planeheight[1] = s->planeheight[2] = FF_CEIL_RSHIFT(inlink->h, 
desc->log2_chroma_h);
+s->planeheight[0] = s->planeheight[3] = 

Re: [FFmpeg-devel] [PATCH 1/3] aacsbr_fixed: avoid division by zero in sbr_gain_calc

[Andreas Cadhalpun]

On 20.11.2015 10:51, Michael Niedermayer wrote:
> On Fri, Nov 20, 2015 at 12:16:09AM +0100, Andreas Cadhalpun wrote:
>>  aacsbr_template.c |8 
>>  1 file changed, 8 insertions(+)
>> 43ace0364265ef16eecd6ca2d9564cc9585847f7  
>> 0001-aacsbr-don-t-call-sbr_dequant-twice-without-intermed.patch
>> From 0237ebfca9571d2d3e51f5c5dc15f8f5a516510a Mon Sep 17 00:00:00 2001
>> From: Andreas Cadhalpun 
>> Date: Fri, 20 Nov 2015 00:04:50 +0100
>> Subject: [PATCH] aacsbr: don't call sbr_dequant twice without intermediate
>>  read_sbr_data
>>
>> Doing that doesn't make sense, because the only purpose of sbr_dequant
>> is to process the data from read_sbr_data.
>>
>> Signed-off-by: Andreas Cadhalpun 
>> ---
>>  libavcodec/aacsbr_template.c | 8 
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/libavcodec/aacsbr_template.c b/libavcodec/aacsbr_template.c
>> index cf18862..564930a 100644
>> --- a/libavcodec/aacsbr_template.c
>> +++ b/libavcodec/aacsbr_template.c
>> @@ -1041,6 +1041,7 @@ static unsigned int read_sbr_data(AACContext *ac, 
>> SpectralBandReplication *sbr,
>>  unsigned int cnt = get_bits_count(gb);
>>  
>>  sbr->id_aac = id_aac;
>> +sbr->start = 2;
> 
> the new value should be documented
> or maybe it would be possible to use a new field with self explanatory
> name

I think using a new field is a good idea for code readability.
Updated patch attached.

Best regards,
Andreas

>From 10acbb3939e576594b43965847ea7b2231722ee4 Mon Sep 17 00:00:00 2001
From: Andreas Cadhalpun 
Date: Fri, 20 Nov 2015 20:15:21 +0100
Subject: [PATCH] aacsbr: don't call sbr_dequant twice without intermediate
 read_sbr_data

Doing that doesn't make sense, because the only purpose of sbr_dequant
is to process the data from read_sbr_data.

Signed-off-by: Andreas Cadhalpun 
---
 libavcodec/aacsbr_template.c | 10 ++
 libavcodec/sbr.h |  1 +
 2 files changed, 11 insertions(+)

diff --git a/libavcodec/aacsbr_template.c b/libavcodec/aacsbr_template.c
index cf18862..1d7a7be 100644
--- a/libavcodec/aacsbr_template.c
+++ b/libavcodec/aacsbr_template.c
@@ -72,6 +72,7 @@ av_cold void AAC_RENAME(ff_aac_sbr_init)(void)
 /** Places SBR in pure upsampling mode. */
 static void sbr_turnoff(SpectralBandReplication *sbr) {
 sbr->start = 0;
+sbr->ready_for_dequant = 0;
 // Init defults used in pure upsampling mode
 sbr->kx[1] = 32; //Typo in spec, kx' inits to 32
 sbr->m[1] = 0;
@@ -179,6 +180,7 @@ static unsigned int read_sbr_header(SpectralBandReplication *sbr, GetBitContext
 SpectrumParameters old_spectrum_params;
 
 sbr->start = 1;
+sbr->ready_for_dequant = 0;
 
 // Save last spectrum parameters variables to compare to new ones
 memcpy(_spectrum_params, >spectrum_params, sizeof(SpectrumParameters));
@@ -1041,6 +1043,7 @@ static unsigned int read_sbr_data(AACContext *ac, SpectralBandReplication *sbr,
 unsigned int cnt = get_bits_count(gb);
 
 sbr->id_aac = id_aac;
+sbr->ready_for_dequant = 1;
 
 if (id_aac == TYPE_SCE || id_aac == TYPE_CCE) {
 if (read_sbr_single_channel_element(ac, sbr, gb)) {
@@ -1476,6 +1479,12 @@ void AAC_RENAME(ff_sbr_apply)(AACContext *ac, SpectralBandReplication *sbr, int
 sbr_turnoff(sbr);
 }
 
+if (sbr->start && !sbr->ready_for_dequant) {
+av_log(ac->avctx, AV_LOG_ERROR,
+   "No quantized data read for sbr_dequant.\n");
+sbr_turnoff(sbr);
+}
+
 if (!sbr->kx_and_m_pushed) {
 sbr->kx[0] = sbr->kx[1];
 sbr->m[0] = sbr->m[1];
@@ -1485,6 +1494,7 @@ void AAC_RENAME(ff_sbr_apply)(AACContext *ac, SpectralBandReplication *sbr, int
 
 if (sbr->start) {
 sbr_dequant(sbr, id_aac);
+sbr->ready_for_dequant = 0;
 }
 for (ch = 0; ch < nch; ch++) {
 /* decode channel */
diff --git a/libavcodec/sbr.h b/libavcodec/sbr.h
index 4a94c4a..eb7d1ae 100644
--- a/libavcodec/sbr.h
+++ b/libavcodec/sbr.h
@@ -139,6 +139,7 @@ typedef struct AACSBRContext {
 struct SpectralBandReplication {
 intsample_rate;
 intstart;
+intready_for_dequant;
 intid_aac;
 intreset;
 SpectrumParameters spectrum_params;
-- 
2.6.2

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

Re: [FFmpeg-devel] [PATCH v2 11/13] Remove the MIPS "generic" core in favor of "*"

[Andreas Cadhalpun]

On 18.11.2015 12:13, Vicente Olivert Riera wrote:
> There is no point to have a "generic" core when we can catch all
> unsupported cores with the "*" option, so remove it.
> 
> Signed-off-by: Vicente Olivert Riera 
> ---
> Changes v1 -> v2:
>  - Nothing.
> 
>  configure |4 
>  1 files changed, 0 insertions(+), 4 deletions(-)
> 
> diff --git a/configure b/configure
> index fc3d559..7d87e5c 100755
> --- a/configure
> +++ b/configure
> @@ -4154,10 +4154,6 @@ elif enabled mips; then
>  ;;
>  esac
>  ;;
> -generic)
> -disable mips64r6
> -disable msa
> -;;
>  *)
>  disable mipsfpu
>  disable mips32r2
> 

This is a bad idea.
If the cpu is not explicitly set via configure options, it is set to generic.
Removing this generic case here, will force-disable all mips optimizations,
even if they were explicitly enabled with configure options.
That would e.g. break how the Debian ffmpeg package is built for mips64el.

Best regards,
Andreas

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

Re: [FFmpeg-devel] [PATCH 1/3] aacsbr_fixed: avoid division by zero in sbr_gain_calc

[Michael Niedermayer]

On Fri, Nov 20, 2015 at 08:42:09PM +0100, Andreas Cadhalpun wrote:
> On 20.11.2015 10:51, Michael Niedermayer wrote:
> > On Fri, Nov 20, 2015 at 12:16:09AM +0100, Andreas Cadhalpun wrote:
> >>  aacsbr_template.c |8 
> >>  1 file changed, 8 insertions(+)
> >> 43ace0364265ef16eecd6ca2d9564cc9585847f7  
> >> 0001-aacsbr-don-t-call-sbr_dequant-twice-without-intermed.patch
> >> From 0237ebfca9571d2d3e51f5c5dc15f8f5a516510a Mon Sep 17 00:00:00 2001
> >> From: Andreas Cadhalpun 
> >> Date: Fri, 20 Nov 2015 00:04:50 +0100
> >> Subject: [PATCH] aacsbr: don't call sbr_dequant twice without intermediate
> >>  read_sbr_data
> >>
> >> Doing that doesn't make sense, because the only purpose of sbr_dequant
> >> is to process the data from read_sbr_data.
> >>
> >> Signed-off-by: Andreas Cadhalpun 
> >> ---
> >>  libavcodec/aacsbr_template.c | 8 
> >>  1 file changed, 8 insertions(+)
> >>
> >> diff --git a/libavcodec/aacsbr_template.c b/libavcodec/aacsbr_template.c
> >> index cf18862..564930a 100644
> >> --- a/libavcodec/aacsbr_template.c
> >> +++ b/libavcodec/aacsbr_template.c
> >> @@ -1041,6 +1041,7 @@ static unsigned int read_sbr_data(AACContext *ac, 
> >> SpectralBandReplication *sbr,
> >>  unsigned int cnt = get_bits_count(gb);
> >>  
> >>  sbr->id_aac = id_aac;
> >> +sbr->start = 2;
> > 
> > the new value should be documented
> > or maybe it would be possible to use a new field with self explanatory
> > name
> 
> I think using a new field is a good idea for code readability.
> Updated patch attached.
> 
> Best regards,
> Andreas
> 

>  aacsbr_template.c |   10 ++
>  sbr.h |1 +
>  2 files changed, 11 insertions(+)
> fd6c7feeeb29eae46242ade296c317a5e4930c12  
> 0001-aacsbr-don-t-call-sbr_dequant-twice-without-intermed.patch
> From 10acbb3939e576594b43965847ea7b2231722ee4 Mon Sep 17 00:00:00 2001
> From: Andreas Cadhalpun 
> Date: Fri, 20 Nov 2015 20:15:21 +0100
> Subject: [PATCH] aacsbr: don't call sbr_dequant twice without intermediate
>  read_sbr_data
> 
> Doing that doesn't make sense, because the only purpose of sbr_dequant
> is to process the data from read_sbr_data.
> 
> Signed-off-by: Andreas Cadhalpun 
> ---
>  libavcodec/aacsbr_template.c | 10 ++
>  libavcodec/sbr.h |  1 +
>  2 files changed, 11 insertions(+)

LGTM

thx

[...]

-- 
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] avfilter/all: handle ff_formats_unref correctly

[Michael Niedermayer]

On Fri, Nov 20, 2015 at 03:07:36PM -0500, Ganesh Ajjanagadde wrote:
> On Fri, Nov 20, 2015 at 3:00 PM, Michael Niedermayer  wrote:
> > On Sun, Nov 15, 2015 at 07:21:11PM -0500, Ganesh Ajjanagadde wrote:
> >> Recent commits 6aaac24d72a7da631173209841a3944fcb4a3309 and
> >> 3835554bf8ed78539a3492c239f979c0ab03a15f made progress towards cleaning
> >> up usage of the formats API, and in particular fixed possible NULL pointer
> >> dereferences.
> >>
> >> This commit addresses the issue of possible resource leaks when one call
> >> fails and the others don't.
> >>
> >> Fixes: CID 1338330, 1338329, 1338327, 1338326.
> >>
> >> Signed-off-by: Ganesh Ajjanagadde 
> >> ---
> >>  libavfilter/af_channelmap.c |  7 +--
> >>  libavfilter/vf_alphamerge.c |  7 +--
> >>  libavfilter/vf_overlay.c| 30 +-
> >>  3 files changed, 31 insertions(+), 13 deletions(-)
> >
> > if the error pathes have been tested with valgrind or assan (and
> > simulating/forcing them to trigger) then this should be ok
> 
> Unfortunately, I have no idea how to test such code pathways easily. -
> how does one force a particular malloc to fail? Isn't the cheapest

-this = particular_malloc()
+if (somerandom() % someconstant  == 0)
+this = NULL;
+ else
+this = particular_malloc()

this also should/would make some (not neccesarily the first) call
to fail


> solution simply to submit a Coverity build for this and check?

coverity seems not to detect everything


> 
> >
> > [...]
> > --
> > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> >
> > Many that live deserve death. And some that die deserve life. Can you give
> > it to them? Then do not be too eager to deal out death in judgement. For
> > even the very wise cannot see all ends. -- Gandalf
> >
> > ___
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

No great genius has ever existed without some touch of madness. -- Aristotle


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

Re: [FFmpeg-devel] Ffmpeg Consulting

[Mayank Agarwal]

Hi,

I have good expereince in ffmpeg and multimedia domain and i am interested
in that

Regards
Mayank

On Thu, Nov 19, 2015 at 8:08 AM, d...@faceflow.com 
wrote:

> Hi,
>
> I am currently working on a project in which we want to use Ffmpeg.
>
> I have a lot of data to handle with different options on the Ffmpeg
> command — mostly combining segments of several small A/V files in one big
> file A/V file at specific times.
>
> Anybody available for a more detailed talk?
>
> If so please let me know your rate as well.
>
> Dany Pelletier
> Developer, Demio.com 
> d...@faceflow.com 
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/3] aacsbr_fixed: avoid division by zero in sbr_gain_calc

[Andreas Cadhalpun]

On 20.11.2015 21:51, Michael Niedermayer wrote:
> On Fri, Nov 20, 2015 at 08:42:09PM +0100, Andreas Cadhalpun wrote:
>> On 20.11.2015 10:51, Michael Niedermayer wrote:
>>> On Fri, Nov 20, 2015 at 12:16:09AM +0100, Andreas Cadhalpun wrote:
  aacsbr_template.c |8 
  1 file changed, 8 insertions(+)
 43ace0364265ef16eecd6ca2d9564cc9585847f7  
 0001-aacsbr-don-t-call-sbr_dequant-twice-without-intermed.patch
 From 0237ebfca9571d2d3e51f5c5dc15f8f5a516510a Mon Sep 17 00:00:00 2001
 From: Andreas Cadhalpun 
 Date: Fri, 20 Nov 2015 00:04:50 +0100
 Subject: [PATCH] aacsbr: don't call sbr_dequant twice without intermediate
  read_sbr_data

 Doing that doesn't make sense, because the only purpose of sbr_dequant
 is to process the data from read_sbr_data.

 Signed-off-by: Andreas Cadhalpun 
 ---
  libavcodec/aacsbr_template.c | 8 
  1 file changed, 8 insertions(+)

 diff --git a/libavcodec/aacsbr_template.c b/libavcodec/aacsbr_template.c
 index cf18862..564930a 100644
 --- a/libavcodec/aacsbr_template.c
 +++ b/libavcodec/aacsbr_template.c
 @@ -1041,6 +1041,7 @@ static unsigned int read_sbr_data(AACContext *ac, 
 SpectralBandReplication *sbr,
  unsigned int cnt = get_bits_count(gb);
  
  sbr->id_aac = id_aac;
 +sbr->start = 2;
>>>
>>> the new value should be documented
>>> or maybe it would be possible to use a new field with self explanatory
>>> name
>>
>> I think using a new field is a good idea for code readability.
>> Updated patch attached.
>>
>> Best regards,
>> Andreas
>>
> 
>>  aacsbr_template.c |   10 ++
>>  sbr.h |1 +
>>  2 files changed, 11 insertions(+)
>> fd6c7feeeb29eae46242ade296c317a5e4930c12  
>> 0001-aacsbr-don-t-call-sbr_dequant-twice-without-intermed.patch
>> From 10acbb3939e576594b43965847ea7b2231722ee4 Mon Sep 17 00:00:00 2001
>> From: Andreas Cadhalpun 
>> Date: Fri, 20 Nov 2015 20:15:21 +0100
>> Subject: [PATCH] aacsbr: don't call sbr_dequant twice without intermediate
>>  read_sbr_data
>>
>> Doing that doesn't make sense, because the only purpose of sbr_dequant
>> is to process the data from read_sbr_data.
>>
>> Signed-off-by: Andreas Cadhalpun 
>> ---
>>  libavcodec/aacsbr_template.c | 10 ++
>>  libavcodec/sbr.h |  1 +
>>  2 files changed, 11 insertions(+)
> 
> LGTM

Pushed.

Best regards,
Andreas

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

Re: [FFmpeg-devel] Error in ATRAC1 decoder?

[Даниил Чередник]

I rechecked it.
The issue with test is the test uses decoded from aea sample pcm file. And
it looks like this file is not correctly decoded (it is easy to see at
0.7221 sec, https://yadi.sk/i/8WBO701XkcHUv). Same place decoded by fixed
version is https://yadi.sk/i/efG3lkEGkcHYE.

Also I made test with newly encoded ATRAC1 file. I included source pcm and
encoded aea in to the fate test. These files are here
https://yadi.sk/d/DA-xxzM_kcJ5t



On Wed, Nov 18, 2015 at 10:19 PM, Michael Niedermayer 
wrote:

> On Tue, Nov 17, 2015 at 04:08:29AM +0300, Даниил Чередник wrote:
> > Thank you for answer.
> >
> > If I understood available ATRAC1 docs, the purpose of this delay line is
> > just to compensate delay of 1st QMF because for 1st and 2nd band we have
> > two QMF but for 3rd band just one.
> >
> > About test, yes for some reason this patch brakes it. I will try to find
> > out. Sorry, I should have done it before.
>
> np
> if the changed / new version is more correct then please also provide
> a new reference file for the fate samples with a new filename and
> update the fate test so it uses the new file
>
> thanks
>
> >
> > I tried to decode "The Four Seasons, Concerto Alla Rustica in G - Winter-
> > Largo.aea" sample, the results are:
> > without patch: https://yadi.sk/i/PCFplrafkWEFC
> > with patch: https://yadi.sk/i/iLUR188gkWEFn
> > On zoomed spectrogram it is possible to find aliasing if decode without
> > patch. I can share decoded wav file if needed.
> >
> >
> >
> > On Mon, Nov 16, 2015 at 3:45 AM, Michael Niedermayer
>  > > wrote:
> >
> > > On Sat, Nov 14, 2015 at 02:59:30AM +0300, Даниил Чередник wrote:
> > > > Hello!
> > > >
> > > > I have noticed if decode
> > > >
> > >
> https://samples.ffmpeg.org/A-codecs/ATRAC1/Test%20tones%20disc%20-%20Chirp.aea
> > > > file by ffmpeg we got aliasing near 11025Hz. Screenshots:
> > > > https://yadi.sk/i/r-95jZkKkSnbu https://yadi.sk/i/PV92LNESkSnby
> > > >
> > > > I was tried to solve it:
> > > >
> > > > ATRAC1 - hybrid codec, has two stacked QMF and splits the signal
> into 3
> > > > band before MDCT. Thereby we need to compensate delay of one QMF to
> > > > achieve reconstruction.
> > > > There is a delay line but it looks like delay for 23 sample is not
> > > correct.
> > > > I have done some experiments and got 39 should be right delay.
> Results of
> > > > decoding with patch: https://yadi.sk/i/yooaIQrmkSncB
> > > > https://yadi.sk/i/CUXsH7-CkSncK
> > > >
> > >
> > > > To be honest I am not a prof in math and it would be great if someone
> > > > recheck it from math perspective.
> > >
> > > i dont think math can help here
> > > whatever a format requires that has to be done, this could even be
> > > wrong from a math point of view of a ideal transform
> > >
> > >
> > > >
> > > > There is a patch in attach.
> > >
> > > the patch also affects the atrac1 fate test
> > > have you checked if the new output for that test is better than the
> > > old ?
> > >
> > > make V=2 fate-atrac1
> > >
> > > also have you looked at other files, is this improving all files ?
> > > any that get worse ?
> > >
> > >
> > > [...]
> > >
> > > --
> > > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> > >
> > > Many that live deserve death. And some that die deserve life. Can you
> give
> > > it to them? Then do not be too eager to deal out death in judgement.
> For
> > > even the very wise cannot see all ends. -- Gandalf
> > >
> > > ___
> > > ffmpeg-devel mailing list
> > > ffmpeg-devel@ffmpeg.org
> > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > >
> > >
> >
> >
> > --
> > Daniil Cherednik
> > ___
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> The greatest way to live with honor in this world is to be what we pretend
> to be. -- Socrates
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>


-- 
Daniil Cherednik


0001-fix-atrac1-decoder-QMF-delay-compensation-should-be-.patch
Description: Binary data
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] Error in ATRAC1 decoder?

[Michael Niedermayer]

On Sat, Nov 21, 2015 at 01:24:35AM +0300, Даниил Чередник wrote:
> I rechecked it.
> The issue with test is the test uses decoded from aea sample pcm file. And
> it looks like this file is not correctly decoded (it is easy to see at
> 0.7221 sec, https://yadi.sk/i/8WBO701XkcHUv). Same place decoded by fixed
> version is https://yadi.sk/i/efG3lkEGkcHYE.
> 
> Also I made test with newly encoded ATRAC1 file. I included source pcm and
> encoded aea in to the fate test. These files are here
> https://yadi.sk/d/DA-xxzM_kcJ5t
> 
> 
> 
> On Wed, Nov 18, 2015 at 10:19 PM, Michael Niedermayer 
> wrote:
> 
> > On Tue, Nov 17, 2015 at 04:08:29AM +0300, Даниил Чередник wrote:
> > > Thank you for answer.
> > >
> > > If I understood available ATRAC1 docs, the purpose of this delay line is
> > > just to compensate delay of 1st QMF because for 1st and 2nd band we have
> > > two QMF but for 3rd band just one.
> > >
> > > About test, yes for some reason this patch brakes it. I will try to find
> > > out. Sorry, I should have done it before.
> >
> > np
> > if the changed / new version is more correct then please also provide
> > a new reference file for the fate samples with a new filename and
> > update the fate test so it uses the new file
> >
> > thanks
> >
> > >
> > > I tried to decode "The Four Seasons, Concerto Alla Rustica in G - Winter-
> > > Largo.aea" sample, the results are:
> > > without patch: https://yadi.sk/i/PCFplrafkWEFC
> > > with patch: https://yadi.sk/i/iLUR188gkWEFn
> > > On zoomed spectrogram it is possible to find aliasing if decode without
> > > patch. I can share decoded wav file if needed.
> > >
> > >
> > >
> > > On Mon, Nov 16, 2015 at 3:45 AM, Michael Niedermayer
> >  > > > wrote:
> > >
> > > > On Sat, Nov 14, 2015 at 02:59:30AM +0300, Даниил Чередник wrote:
> > > > > Hello!
> > > > >
> > > > > I have noticed if decode
> > > > >
> > > >
> > https://samples.ffmpeg.org/A-codecs/ATRAC1/Test%20tones%20disc%20-%20Chirp.aea
> > > > > file by ffmpeg we got aliasing near 11025Hz. Screenshots:
> > > > > https://yadi.sk/i/r-95jZkKkSnbu https://yadi.sk/i/PV92LNESkSnby
> > > > >
> > > > > I was tried to solve it:
> > > > >
> > > > > ATRAC1 - hybrid codec, has two stacked QMF and splits the signal
> > into 3
> > > > > band before MDCT. Thereby we need to compensate delay of one QMF to
> > > > > achieve reconstruction.
> > > > > There is a delay line but it looks like delay for 23 sample is not
> > > > correct.
> > > > > I have done some experiments and got 39 should be right delay.
> > Results of
> > > > > decoding with patch: https://yadi.sk/i/yooaIQrmkSncB
> > > > > https://yadi.sk/i/CUXsH7-CkSncK
> > > > >
> > > >
> > > > > To be honest I am not a prof in math and it would be great if someone
> > > > > recheck it from math perspective.
> > > >
> > > > i dont think math can help here
> > > > whatever a format requires that has to be done, this could even be
> > > > wrong from a math point of view of a ideal transform
> > > >
> > > >
> > > > >
> > > > > There is a patch in attach.
> > > >
> > > > the patch also affects the atrac1 fate test
> > > > have you checked if the new output for that test is better than the
> > > > old ?
> > > >
> > > > make V=2 fate-atrac1
> > > >
> > > > also have you looked at other files, is this improving all files ?
> > > > any that get worse ?
> > > >
> > > >
> > > > [...]
> > > >
> > > > --
> > > > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> > > >
> > > > Many that live deserve death. And some that die deserve life. Can you
> > give
> > > > it to them? Then do not be too eager to deal out death in judgement.
> > For
> > > > even the very wise cannot see all ends. -- Gandalf
> > > >
> > > > ___
> > > > ffmpeg-devel mailing list
> > > > ffmpeg-devel@ffmpeg.org
> > > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > > >
> > > >
> > >
> > >
> > > --
> > > Daniil Cherednik
> > > ___
> > > ffmpeg-devel mailing list
> > > ffmpeg-devel@ffmpeg.org
> > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> > --
> > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> >
> > The greatest way to live with honor in this world is to be what we pretend
> > to be. -- Socrates
> >
> > ___
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> >
> 
> 
> -- 
> Daniil Cherednik

>  libavcodec/atrac1.c  |8 
>  tests/fate/atrac.mak |   13 ++---
>  2 files changed, 14 insertions(+), 7 deletions(-)
> dd5fc1eb5d68026fa06b90f6c20cad643c0280a0  
> 0001-fix-atrac1-decoder-QMF-delay-compensation-should-be-.patch
> From 00d07b0c655cd6171e77b6a2b9140cb7a80c0ade Mon Sep 17 00:00:00 2001
> From: Daniil Cherednik 
> Date: Sat, 14 Nov 2015 01:57:25 +0300
> Subject: [PATCH] fix 

[FFmpeg-devel] [PATCH 1/3] imgutils: Use designated initializers for AVClass

[Timothy Gu]

More readable and less breakable.
---
 libavutil/imgutils.c | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/libavutil/imgutils.c b/libavutil/imgutils.c
index 8956ae3..a189a50 100644
--- a/libavutil/imgutils.c
+++ b/libavutil/imgutils.c
@@ -236,11 +236,21 @@ typedef struct ImgUtils {
 void *log_ctx;
 } ImgUtils;
 
-static const AVClass imgutils_class = { "IMGUTILS", av_default_item_name, 
NULL, LIBAVUTIL_VERSION_INT, offsetof(ImgUtils, log_offset), offsetof(ImgUtils, 
log_ctx) };
+static const AVClass imgutils_class = {
+.class_name = "IMGUTILS",
+.item_name  = av_default_item_name,
+.version= LIBAVUTIL_VERSION_INT,
+.log_level_offset_offset   = offsetof(ImgUtils, log_offset),
+.parent_log_context_offset = offsetof(ImgUtils, log_ctx)
+};
 
 int av_image_check_size(unsigned int w, unsigned int h, int log_offset, void 
*log_ctx)
 {
-ImgUtils imgutils = { _class, log_offset, log_ctx };
+ImgUtils imgutils = {
+.class  = _class,
+.log_offset = log_offset,
+.log_ctx= log_ctx
+};
 
 if ((int)w>0 && (int)h>0 && (w+128)*(uint64_t)(h+128) < INT_MAX/8)
 return 0;
-- 
2.1.4

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

Re: [FFmpeg-devel] [PATCH] configure: Add option to use gmp

[Matt Oliver]

On 19 November 2015 at 00:42, Ricardo Constantino  wrote:

> I previously had an option for gcrypt too but it seemed to not work
> (FFmpeg crashed, iirc), so I just gave up on it.
>

If ffmpeg crashes when using gcrypt then thats a separate bug not related
to configure. So you should post a bug report to trac. Its possible that
the gcrypt code isnt well tested due to gmp being picked up in preference,
so gcrypt will only be used currently on systems without gmp. This will be
easier to demonstrate once I submit a patch to manually choose to enable
either option.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH 2/3] avformat/rawvideodec: Rework packet size calculation

[Timothy Gu]

Calculate packet size only once, and propagate errors earlier in the chain.

Also remove use of the deprecated av_image_get_buffer_size().
---
 libavformat/rawvideodec.c | 17 +
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/libavformat/rawvideodec.c b/libavformat/rawvideodec.c
index 7f355ef..5fd58d3 100644
--- a/libavformat/rawvideodec.c
+++ b/libavformat/rawvideodec.c
@@ -39,6 +39,7 @@ static int rawvideo_read_header(AVFormatContext *ctx)
 RawVideoDemuxerContext *s = ctx->priv_data;
 enum AVPixelFormat pix_fmt;
 AVStream *st;
+int packet_size;
 
 st = avformat_new_stream(ctx, NULL);
 if (!st)
@@ -59,7 +60,11 @@ static int rawvideo_read_header(AVFormatContext *ctx)
 st->codec->width  = s->width;
 st->codec->height = s->height;
 st->codec->pix_fmt = pix_fmt;
-st->codec->bit_rate = av_rescale_q(avpicture_get_size(st->codec->pix_fmt, 
s->width, s->height),
+packet_size = av_image_get_buffer_size(st->codec->pix_fmt, s->width, 
s->height, 1);
+if (packet_size < 0)
+return packet_size;
+ctx->packet_size = packet_size;
+st->codec->bit_rate = av_rescale_q(ctx->packet_size,
(AVRational){8,1}, st->time_base);
 
 return 0;
@@ -68,18 +73,14 @@ static int rawvideo_read_header(AVFormatContext *ctx)
 
 static int rawvideo_read_packet(AVFormatContext *s, AVPacket *pkt)
 {
-int packet_size, ret, width, height;
+int ret, width, height;
 AVStream *st = s->streams[0];
 
 width = st->codec->width;
 height = st->codec->height;
 
-packet_size = av_image_get_buffer_size(st->codec->pix_fmt, width, height, 
1);
-if (packet_size < 0)
-return -1;
-
-ret = av_get_packet(s->pb, pkt, packet_size);
-pkt->pts = pkt->dts = pkt->pos / packet_size;
+ret = av_get_packet(s->pb, pkt, s->packet_size);
+pkt->pts = pkt->dts = pkt->pos / s->packet_size;
 
 pkt->stream_index = 0;
 if (ret < 0)
-- 
2.1.4

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

[FFmpeg-devel] [PATCH 3/3] avformat/v210: Check width and height

[Timothy Gu]

Fixes a floating point exception when width and height are not supplied
(and therefore are zero).
---
 libavformat/v210.c | 17 +
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/libavformat/v210.c b/libavformat/v210.c
index ab29171..7503d5d 100644
--- a/libavformat/v210.c
+++ b/libavformat/v210.c
@@ -39,6 +39,7 @@ static int v210_read_header(AVFormatContext *ctx)
 {
 V210DemuxerContext *s = ctx->priv_data;
 AVStream *st;
+int ret;
 
 st = avformat_new_stream(ctx, NULL);
 if (!st)
@@ -50,11 +51,15 @@ static int v210_read_header(AVFormatContext *ctx)
 
 avpriv_set_pts_info(st, 64, s->framerate.den, s->framerate.num);
 
+ret = av_image_check_size(s->width, s->height, 0, ctx);
+if (ret < 0)
+return ret;
 st->codec->width= s->width;
 st->codec->height   = s->height;
 st->codec->pix_fmt  = ctx->iformat->raw_codec_id == AV_CODEC_ID_V210 ?
   AV_PIX_FMT_YUV422P10 : AV_PIX_FMT_YUV422P16;
-st->codec->bit_rate = av_rescale_q(GET_PACKET_SIZE(s->width, s->height),
+ctx->packet_size= GET_PACKET_SIZE(s->width, s->height);
+st->codec->bit_rate = av_rescale_q(ctx->packet_size,
(AVRational){8,1}, st->time_base);
 
 return 0;
@@ -63,18 +68,14 @@ static int v210_read_header(AVFormatContext *ctx)
 
 static int v210_read_packet(AVFormatContext *s, AVPacket *pkt)
 {
-int packet_size, ret, width, height;
+int ret, width, height;
 AVStream *st = s->streams[0];
 
 width = st->codec->width;
 height = st->codec->height;
 
-packet_size = GET_PACKET_SIZE(width, height);
-if (packet_size < 0)
-return -1;
-
-ret = av_get_packet(s->pb, pkt, packet_size);
-pkt->pts = pkt->dts = pkt->pos / packet_size;
+ret = av_get_packet(s->pb, pkt, s->packet_size);
+pkt->pts = pkt->dts = pkt->pos / s->packet_size;
 
 pkt->stream_index = 0;
 if (ret < 0)
-- 
2.1.4

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