Re: [FFmpeg-devel] 2.9/3.0, 2.8.5, ...

[Carl Eugen Hoyos]

Michael Niedermayer  niedermayer.cc> writes:

> Also ill likely make another round of point releases 
> from the 2.8/2.7/2.6/2.5 branches soon

Will you backport the fix for ticket #5096?

Carl Eugen

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

Re: [FFmpeg-devel] [PATCH] avcodec: Remove libstagefright

[Carl Eugen Hoyos]

Matthieu Bouron  gmail.com> writes:

> some patch can be expected at the end of the month

So perhaps libstagefright should be removed once your 
patch hits the git repo?

Carl Eugen

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

Re: [FFmpeg-devel] [PATCH]lavf/decimate: Fix total difference for the first frame

[Carl Eugen Hoyos]

Carl Eugen Hoyos  ag.or.at> writes:

> Clément Bœsch  pkh.me> writes:
> 
> > +if (cyclestart == 0) {
> > +vdm->vmi[0].maxbdiff = vdm->vmi[1].maxbdiff;
> > +vdm->vmi[0].totdiff = vdm->scthresh + 1;
> > +}
> > +
> 
> > I suggest to "cherry-pick" this if it works.
> 
> Nicolas has already found a very similar solution 
> that only differs in rare cases (although the change 
> should be ported) and not for the file in question.

This should not be ported because Nicolas' approach 
is better:
The difference of above approach is that if the first 
five frames contain a scene change, the first frame 
after the scene change would be dropped if no 
duplicate was found. This is a rare case and it is 
not obvious why it's better to drop the first frame 
after the scene change instead of the very first 
frame (as currently with Nicolas' patch).
But the important and imo more typical case is when 
the first frame of the original telecined video was 
an interlaced frame that could not be matched by 
fieldmatch (because the missing field is not 
available): In this case, the first frame should be 
dropped if no duplicate was found (there will be no 
duplicate for the first five frames if the input 
stream was a clean telecined stream starting with an 
interlaced frame) and this is what currently happens 
in FFmpeg.

This does not explain the issue in ticket #4990:
I will not test with Avisynth / whatever, if nobody 
can test, the ticket will stay open.
The OP there claims that the original filter drops 
the correct frame, this does not happen with FFmpeg 
and the default options which are - afaict - 
identical for the original filter.

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

Re: [FFmpeg-devel] [PATCH] Add frame side data when SEI green metadata are detected

[Nicolas Derouineau]

>this should be in a seperate patch and libavutil/version.h should have
>its minor version bumped

Ok, so I'm going to do two separate patch (commit ?) for libavformat and for 
libavutil.
Where should I bump libavutil/version.h ? (I'm not sure I really understand 
this action).

>GreenMetaData is defined in h264.h, which is an internal header
>while AVFrameSideDataType is external. public API should not refer to
>internal/private headers

Ok

>also the struct, as currently defined is platform specific. Theres no
>gurantee that a compiler doesnt add padding betwen the elements. 

I'm not sure how to deal with this issue in ffmpeg. I guess #pragma packed is 
not an option, right ?

>Also the code initializing it only matches the platform dependant struct
>on little endian

Can you explain to me which part of the code is platform dependant on little 
endian ? (because memset seems allright to me, but I guess I'm wrong).

Thanks,

Nicolas DEROUINEAU
Research Engineer
VITEC

T.  +33 1 46 73 06 06
E.  nicolas.derouin...@vitec.com
W. www.vitec.com


De : ffmpeg-devel  de la part de Michael 
Niedermayer 
Envoyé : lundi 4 janvier 2016 19:37
À : FFmpeg development discussions and patches
Objet : Re: [FFmpeg-devel] [PATCH] Add frame side data when SEI green   
metadata are detected

On Mon, Jan 04, 2016 at 02:50:44PM +0100, Nicolas DEROUINEAU wrote:
> ---
>  libavcodec/h264.c | 21 +
>  libavcodec/h264.h |  1 +
>  libavcodec/h264_sei.c |  3 +++

>  libavutil/frame.h |  8 

this should be in a seperate patch and libavutil/version.h should have
its minor version bumped


>  4 files changed, 33 insertions(+)
>
> diff --git a/libavcodec/h264.c b/libavcodec/h264.c
> index 089a86f..e90bcc0 100644
> --- a/libavcodec/h264.c
> +++ b/libavcodec/h264.c
> @@ -879,6 +879,27 @@ static void decode_postinit(H264Context *h, int 
> setup_finished)
>  }
>  }
>
> +if (h->sei_green_metadata_present) {
> +AVFrameSideData *Greenmd = av_frame_new_side_data(cur->f, 
> AV_FRAME_DATA_GREENMD,
> +sizeof(GreenMetaData));
> +if (Greenmd) {
> +memset((uint8_t*)Greenmd->data, 0, sizeof(GreenMetaData));
> +Greenmd->data[0] = h->sei_green_metadata.green_metadata_type;
> +Greenmd->data[1] = h->sei_green_metadata.period_type;
> +Greenmd->data[2] = 
> (uint8_t)(h->sei_green_metadata.num_seconds>>8);
> +Greenmd->data[3] = 
> (uint8_t)(h->sei_green_metadata.num_seconds&0xFF);
> +Greenmd->data[4] = 
> (uint8_t)(h->sei_green_metadata.num_pictures>>8);
> +Greenmd->data[5] = 
> (uint8_t)(h->sei_green_metadata.num_pictures&0xFF);
> +Greenmd->data[6] = 
> h->sei_green_metadata.percent_non_zero_macroblocks;
> +Greenmd->data[7] = 
> h->sei_green_metadata.percent_intra_coded_macroblocks;
> +Greenmd->data[8] = 
> h->sei_green_metadata.percent_six_tap_filtering;
> +Greenmd->data[9] = 
> h->sei_green_metadata.percent_alpha_point_deblocking_instance;
> +Greenmd->data[10] = h->sei_green_metadata.xsd_metric_type;
> +Greenmd->data[11] = 
> (uint8_t)(h->sei_green_metadata.xsd_metric_value>>8);
> +Greenmd->data[12] = 
> (uint8_t)(h->sei_green_metadata.xsd_metric_value&0xFF);
> +}
> +}
> +
>  if (h->sei_reguserdata_afd_present) {
>  AVFrameSideData *sd = av_frame_new_side_data(cur->f, 
> AV_FRAME_DATA_AFD,
>   sizeof(uint8_t));
> diff --git a/libavcodec/h264.h b/libavcodec/h264.h
> index 5d9aecd..51490d6 100644
> --- a/libavcodec/h264.h
> +++ b/libavcodec/h264.h
> @@ -839,6 +839,7 @@ typedef struct H264Context {
>  qpel_mc_func (*qpel_avg)[16];
>
>  /*Green Metadata */
> +int sei_green_metadata_present;
>  GreenMetaData sei_green_metadata;
>
>  } H264Context;
> diff --git a/libavcodec/h264_sei.c b/libavcodec/h264_sei.c
> index 0411b87..4a021b8 100644
> --- a/libavcodec/h264_sei.c
> +++ b/libavcodec/h264_sei.c
> @@ -43,6 +43,7 @@ void ff_h264_reset_sei(H264Context *h)
>  h->sei_frame_packing_present=  0;
>  h->sei_display_orientation_present = 0;
>  h->sei_reguserdata_afd_present  =  0;
> +h->sei_green_metadata_present   =  0;
>
>  h->a53_caption_size = 0;
>  av_freep(>a53_caption);
> @@ -363,6 +364,8 @@ static int decode_GreenMetadata(H264Context *h)
>  if (h->avctx->debug & FF_DEBUG_GREEN_MD)
>  av_log(h->avctx, AV_LOG_DEBUG,  "Green Metadata Info SEI 
> message\n");
>
> +h->sei_green_metadata_present = 1;
> +
>  h->sei_green_metadata.green_metadata_type=get_bits(>gb, 8);
>
>  if (h->avctx->debug & FF_DEBUG_GREEN_MD)


> diff --git a/libavutil/frame.h b/libavutil/frame.h
> index 9c6061a..89a57ad 100644
> --- a/libavutil/frame.h
> +++ 

Re: [FFmpeg-devel] [PATCH] decklink: support all valid numbers of audio channels

[Matthias Hunstock]

Am 20.12.2015 um 12:57 schrieb Matthias Hunstock:
> As it is already written in the documentation, BMD DeckLink cards
> are capable of capturing 2, 8 or 16 audio channels (for SDI Inputs).
> Currently the value is hardcoded to 2. Introduces new option.

Any more opinions on this?

I'd like to add several options to make the decklink support more
feature complete, since it is very widespread hardware. But I can save
some effort if it is not accepted anyway.

Things on my personal roadmap:

- change order of streams (currently :0 is audio and :1 is video)
- introduce options to select video and audio input, e.g. the new
low-cost cards have SDI and HDMI inputs
- port those changes to decklink output code
- more stuff I don't remember right now

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

Re: [FFmpeg-devel] [PATCH] decklink: support all valid numbers of audio channels

[Dave Rice]

> On Jan 5, 2016, at 10:38 AM, Matthias Hunstock  wrote:
> 
> Am 20.12.2015 um 12:57 schrieb Matthias Hunstock:
>> As it is already written in the documentation, BMD DeckLink cards
>> are capable of capturing 2, 8 or 16 audio channels (for SDI Inputs).
>> Currently the value is hardcoded to 2. Introduces new option.
> 
> Any more opinions on this?

I suggest adding options for one 2, 8, or 16 channel stream. Documentation 
could add examples of how to do mapping within a filterchain with that input.

> I'd like to add several options to make the decklink support more
> feature complete, since it is very widespread hardware. But I can save
> some effort if it is not accepted anyway.
> 
> Things on my personal roadmap:
> 
> - change order of streams (currently :0 is audio and :1 is video)

+1. Also it may be possible to add caption (c608) and timecode streams.

> - introduce options to select video and audio input, e.g. the new
> low-cost cards have SDI and HDMI inputs

I have a BlackMagic UltraStudio which adds Composite and Component inputs. 
Happy to test patches.

> - port those changes to decklink output code
> - more stuff I don't remember right now

[...]

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

Re: [FFmpeg-devel] [PATCH 1/3] libavcodec/ccaption_dec: remove unnecessary include

[Michael Niedermayer]

On Mon, Jan 04, 2016 at 07:28:01PM -0800, Aman Gupta wrote:
> From: Aman Gupta 
> 
> ---
>  libavcodec/ccaption_dec.c | 1 -
>  1 file changed, 1 deletion(-)

applied

thanks

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

In a rich man's house there is no place to spit but his face.
-- 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] [PATCHv2] lavc/cbrt_tablegen: speed up tablegen

[Ganesh Ajjanagadde]

On Tue, Jan 5, 2016 at 8:08 AM, Ganesh Ajjanagadde  wrote:
> On Tue, Jan 5, 2016 at 7:44 AM, Daniel Serpell  wrote:
>> Hi!,
>>
>> El Mon, Jan 04, 2016 at 06:33:59PM -0800, Ganesh Ajjanagadde escribio:
>>> This exploits an approach based on the sieve of Eratosthenes, a popular
>>> method for generating prime numbers.
>>>
>>> Tables are identical to previous ones.
>>>
>>> Tested with FATE with/without --enable-hardcoded-tables.
>>>
>>> Sample benchmark (Haswell, GNU/Linux+gcc):
>>> prev:
>>> 7860100 decicycles in cbrt_tableinit,   1 runs,  0 skips
>>> 490 decicycles in cbrt_tableinit,   2 runs,  0 skips
>>> [...]
>>> 7582339 decicycles in cbrt_tableinit, 256 runs,  0 skips
>>> 7563556 decicycles in cbrt_tableinit, 512 runs,  0 skips
>>>
>>> new:
>>> 2099480 decicycles in cbrt_tableinit,   1 runs,  0 skips
>>> 2044470 decicycles in cbrt_tableinit,   2 runs,  0 skips
>>> [...]
>>> 1796544 decicycles in cbrt_tableinit, 256 runs,  0 skips
>>> 1791631 decicycles in cbrt_tableinit, 512 runs,  0 skips
>>>
>>
>> See attached code, function "test1", based on an approximation of:
>>
>>   (i+1)^(1/3) ~= i^(1/3) * ( 1 + 1/(3i) - 1/(9i) + 5/(81i) -  )
>
> I assume 1/(3i), 1/(9i^2), etc obtained via a Taylor series at x = 0.
>
>>
>> Generated values are the same as original floats (max error in double
>> is < 4*10^-10), it is faster (and I think, simpler) than your version.
>
> Had thought of these ideas, but did not examine as I was a little
> concerned about accuracy. Thanks, will give it a spin. Or
> alternatively, you can submit a patch since you put it into action.
>
> Alternatively, one could directly expand the series for (i+1)^(4/3).
> And it may be possible to tighten the number of terms needed by
> expanding not about x = 0, but x = i to get i+1.

Scratch the x = 0 remark, I misread the code. Remainder still applies.

> Or fancier polynomial
> approximations can be used. Have you tried these?
>
>>
>> Perhaps altering the constants it could be made faster still, but it is
>> currently dominated by de division in the main loop.
>
> Thanks for letting me know.
>
>>
>> Daniel.
>>
>> ___
>> 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] lavc/cbrt_tablegen: speed up tablegen

[Ganesh Ajjanagadde]

On Tue, Jan 5, 2016 at 7:44 AM, Daniel Serpell  wrote:
> Hi!,
>
> El Mon, Jan 04, 2016 at 06:33:59PM -0800, Ganesh Ajjanagadde escribio:
>> This exploits an approach based on the sieve of Eratosthenes, a popular
>> method for generating prime numbers.
>>
>> Tables are identical to previous ones.
>>
>> Tested with FATE with/without --enable-hardcoded-tables.
>>
>> Sample benchmark (Haswell, GNU/Linux+gcc):
>> prev:
>> 7860100 decicycles in cbrt_tableinit,   1 runs,  0 skips
>> 490 decicycles in cbrt_tableinit,   2 runs,  0 skips
>> [...]
>> 7582339 decicycles in cbrt_tableinit, 256 runs,  0 skips
>> 7563556 decicycles in cbrt_tableinit, 512 runs,  0 skips
>>
>> new:
>> 2099480 decicycles in cbrt_tableinit,   1 runs,  0 skips
>> 2044470 decicycles in cbrt_tableinit,   2 runs,  0 skips
>> [...]
>> 1796544 decicycles in cbrt_tableinit, 256 runs,  0 skips
>> 1791631 decicycles in cbrt_tableinit, 512 runs,  0 skips
>>
>
> See attached code, function "test1", based on an approximation of:
>
>   (i+1)^(1/3) ~= i^(1/3) * ( 1 + 1/(3i) - 1/(9i) + 5/(81i) -  )

I assume 1/(3i), 1/(9i^2), etc obtained via a Taylor series at x = 0.

>
> Generated values are the same as original floats (max error in double
> is < 4*10^-10), it is faster (and I think, simpler) than your version.

Had thought of these ideas, but did not examine as I was a little
concerned about accuracy. Thanks, will give it a spin. Or
alternatively, you can submit a patch since you put it into action.

Alternatively, one could directly expand the series for (i+1)^(4/3).
And it may be possible to tighten the number of terms needed by
expanding not about x = 0, but x = i to get i+1. Or fancier polynomial
approximations can be used. Have you tried these?

>
> Perhaps altering the constants it could be made faster still, but it is
> currently dominated by de division in the main loop.

Thanks for letting me know.

>
> Daniel.
>
> ___
> 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] ffmpeg: check return value of avio_closep for progress report

[Michael Niedermayer]

On Mon, Jan 04, 2016 at 07:39:42PM -0800, Ganesh Ajjanagadde wrote:
> avio_closep is not guaranteed to succeed, and its return value can
> contain information regarding failure of preceding writes and silent
> loss of data (man 2 close, man fclose). Users should know when the
> progress was not successfully logged, and so a diagnostic is printed
> here.
> 
> The reason only one of these is changed in the patch is to get feedback:
> a quick glance shows a large number of unchecked avio_close operations.
> It may be tedious to check all of them, and relative importance varies
> across instances. This was one which I feel is important.
> 
> Signed-off-by: Ganesh Ajjanagadde 
> ---
>  ffmpeg.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)

patch LGTM

thx

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

In fact, the RIAA has been known to suggest that students drop out
of college or go to community college in order to be able to afford
settlements. -- The RIAA


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 Tue, Jan 5, 2016 at 5:29 AM, wm4  wrote:
> On Mon,  4 Jan 2016 17:50:01 -0800
> 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.
>>
>> Reviewed-by: wm4 
>> Reviewed-by: Ronald S. Bultje 
>> Reviewed-by: Michael Niedermayer 
>> Signed-off-by: Ganesh Ajjanagadde 
>> ---
>>  libavformat/mp3dec.c | 13 +++--
>>  libavformat/rmdec.c  |  3 ++-
>>  2 files changed, 13 insertions(+), 3 deletions(-)
>>
>> diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
>> index c76b21e..57ebcc8 100644
>> --- a/libavformat/mp3dec.c
>> +++ b/libavformat/mp3dec.c
>> @@ -372,11 +372,19 @@ 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,
>> +   "initial junk detection and skipping impossible due 
>> to: %s\n", av_err2str(ret));
>> +goto skip_fail;
>> +}
>>  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,
>> +   "initial junk detection and skipping impossible due 
>> to: %s\n", av_err2str(ret));
>> +goto skip_fail;
>> +}
>>  if (check(s->pb, off + i + frame_size, ) >= 0 &&
>>  (header & SAME_HEADER_MASK) == (header2 & SAME_HEADER_MASK))
>>  {
>> @@ -387,6 +395,7 @@ static int mp3_read_header(AVFormatContext *s)
>>  }
>>  avio_seek(s->pb, off, SEEK_SET);
>>  }
>> +skip_fail:
>>
>>  // the seek index is relative to the end of the xing vbr headers
>>  for (i = 0; i < st->nb_index_entries; i++)
>> diff --git a/libavformat/rmdec.c b/libavformat/rmdec.c
>> index 4e46a3d..470e227 100644
>> --- a/libavformat/rmdec.c
>> +++ b/libavformat/rmdec.c
>> @@ -618,7 +618,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, "seeking back impossible due to: 
>> %s\n", av_err2str(ret));
>>  v = avio_rb32(pb);
>>  if (v == MKBETAG('M', 'L', 'T', 'I')) {
>>  ret = rm_read_multi(s, s->pb, st, mime);
>
> I maintain that this should not be done, because it makes the code
> verbose for no reason.

Michael has pointed out that when seekback fails, one should really
break out of the junk detection loop in mp3dec. Your idea fails to
achieve it.

> Or if you insist, do it in the ffio_ function
> itself.

> ___
> 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] ffmpeg: check return value of avio_closep for progress report

[James Almer]

On 1/5/2016 12:39 AM, Ganesh Ajjanagadde wrote:
> avio_closep is not guaranteed to succeed, and its return value can
> contain information regarding failure of preceding writes and silent
> loss of data (man 2 close, man fclose). Users should know when the
> progress was not successfully logged, and so a diagnostic is printed
> here.
> 
> The reason only one of these is changed in the patch is to get feedback:
> a quick glance shows a large number of unchecked avio_close operations.
> It may be tedious to check all of them, and relative importance varies
> across instances. This was one which I feel is important.
> 
> Signed-off-by: Ganesh Ajjanagadde 
> ---
>  ffmpeg.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/ffmpeg.c b/ffmpeg.c
> index ee72f91..81f3697 100644
> --- a/ffmpeg.c
> +++ b/ffmpeg.c
> @@ -1512,6 +1512,7 @@ static void print_report(int is_last_report, int64_t 
> timer_start, int64_t cur_ti
>  static int64_t last_time = -1;
>  static int qp_histogram[52];
>  int hours, mins, secs, us;
> +int ret;
>  float t;
>  
>  if (!print_stats && !is_last_report && !progress_avio)
> @@ -1678,7 +1679,9 @@ static void print_report(int is_last_report, int64_t 
> timer_start, int64_t cur_ti
>  avio_flush(progress_avio);
>  av_bprint_finalize(_script, NULL);
>  if (is_last_report) {
> -avio_closep(_avio);
> +if ((ret = avio_closep(_avio)) < 0)
> +av_log(NULL, AV_LOG_ERROR,
> +   "not possible to close progress log, loss of 
> information possible: %s\n", av_err2str(ret));

"Unable to close progress log" or "Error closing progress log" sounds better 
IMO.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH]lavc/opus: Use original sample rate for decoding

[Hendrik Leppkes]

Am 05.01.2016 16:07 schrieb "Carl Eugen Hoyos" :
>
> Hi!
>
> Attached patch intends to match what opusdec does, sorry if I miss
something.
>
> Please review, Carl Eugen

I dont think its as simple as this. Only the SILK parts of the audio are
automatically resampled, the CELT parts are always decoded to 48kHz. Just
changing the samplerate would then cause corrupted audio.

Personally I'm wouldn't like forcing a resampling of all audio anyway, Opus
as a format is specifically designed to not exactly preserve the input
sample rate, unlike other formats.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH]lavc/opus: Use original sample rate for decoding

[Carl Eugen Hoyos]

Hi!

Attached patch intends to match what opusdec does, sorry if I miss something.

Please review, Carl Eugen
diff --git a/libavcodec/opus.c b/libavcodec/opus.c
index f2b8ecc..df04f05 100644
--- a/libavcodec/opus.c
+++ b/libavcodec/opus.c
@@ -332,6 +332,18 @@ av_cold int ff_opus_parse_extradata(AVCodecContext *avctx,
 return AVERROR_INVALIDDATA;
 }
 
+avctx->sample_rate = AV_RL32(extradata + 12);
+if (   avctx->sample_rate != 48000
+&& avctx->sample_rate != 24000
+&& avctx->sample_rate != 16000
+&& avctx->sample_rate != 12000
+&& avctx->sample_rate !=  8000) {
+av_log(avctx, AV_LOG_WARNING,
+   "Invalid sample rate %d specified in the extradata, using 48000 
instead\n",
+   avctx->sample_rate);
+avctx->sample_rate = 48000;
+}
+
 s->gain_i = AV_RL16(extradata + 16);
 if (s->gain_i)
 s->gain = ff_exp10(s->gain_i / (20.0 * 256));
diff --git a/libavcodec/opusdec.c b/libavcodec/opusdec.c
index 95a2435..66f2a19 100644
--- a/libavcodec/opusdec.c
+++ b/libavcodec/opusdec.c
@@ -663,7 +663,6 @@ static av_cold int opus_decode_init(AVCodecContext *avctx)
 int ret, i, j;
 
 avctx->sample_fmt  = AV_SAMPLE_FMT_FLTP;
-avctx->sample_rate = 48000;
 
 c->fdsp = avpriv_float_dsp_alloc(0);
 if (!c->fdsp)
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH v4] lavf/matroskadec: A_QUICKTIME/AV_CODEC_ID_NONE + SMI->SVQ3

[Mats Peterson]

Incorrectly used codec_movaudio_tags instead of codec_movvideo_tags.
Description follows:

In many older QuickTime files, the audio format, or "fourcc", is
0x (AV_CODEC_ID_NONE). The QuickTime File Format Specification
states the following regarding this situation:

"This format descriptor should not be used, but may be found in some
files. Samples are assumed to be stored in either 'raw ' or 'twos'
format, depending on the sample size field in the sound description."

MPlayer handles this logic by itself, but FFmpeg/FFplay currently does
not.

Also, Michael Niedermayer, at least in this case, MPlayer seems to look
at the codec tag rather than the codec ID in order to determine the
codec. Therefore, your patch from 2014 for SMI -> SVQ3 needs to set the
'fourcc' variable to 'SVQ3' as well, which is later copied to
st->codec->codec_tag in matroskadec.c.

Mats

--
Mats Peterson
http://matsp888.no-ip.org/~mats/
>From 6ce829b5be54d86ebee49202c09adbd026c19845 Mon Sep 17 00:00:00 2001
From: Mats Peterson 
Date: Tue, 5 Jan 2016 14:49:17 +0100
Subject: [PATCH v4] lavf/matroskadec: A_QUICKTIME/AV_CODEC_ID_NONE + SMI->SVQ3

Incorrectly used codec_movaudio_tags instead of codec_movvideo_tags.
Description follows:

In many older QuickTime files, the audio format, or "fourcc", is
0x (AV_CODEC_ID_NONE). The QuickTime File Format Specification
states the following regarding this situation:

"This format descriptor should not be used, but may be found in some
files. Samples are assumed to be stored in either 'raw ' or 'twos'
format, depending on the sample size field in the sound description."

MPlayer handles this logic by itself, but FFmpeg/FFplay currently does
not.

Also, Michael Niedermayer, at least in this case, MPlayer seems to look
at the codec tag rather than the codec ID in order to determine the
codec. Therefore, your patch from 2014 for SMI -> SVQ3 needs to set the
'fourcc' variable to 'SVQ3' as well, which is later copied to
st->codec->codec_tag in matroskadec.c.

Mats

---
 libavformat/matroskadec.c |   15 +--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index 9de7cfb..d9d87cc 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -1869,6 +1869,15 @@ static int matroska_parse_tracks(AVFormatContext *s)
 fourcc = AV_RL32(track->codec_priv.data);
 codec_id = ff_codec_get_id(ff_codec_movaudio_tags, fourcc);
 }
+if (codec_id == AV_CODEC_ID_NONE) {
+if (track->audio.bitdepth == 8) {
+fourcc = MKTAG('r','a','w',' ');
+codec_id = ff_codec_get_id(ff_codec_movaudio_tags, fourcc);
+} else if (track->audio.bitdepth == 16) {
+fourcc = MKTAG('t','w','o','s');
+codec_id = ff_codec_get_id(ff_codec_movaudio_tags, fourcc);
+}
+}
 } else if (!strcmp(track->codec_id, "V_QUICKTIME") &&
(track->codec_priv.size >= 21)  &&
(track->codec_priv.data)) {
@@ -1878,8 +1887,10 @@ static int matroska_parse_tracks(AVFormatContext *s)
 fourcc   = AV_RL32(track->codec_priv.data);
 codec_id = ff_codec_get_id(ff_codec_movvideo_tags, fourcc);
 }
-if (codec_id == AV_CODEC_ID_NONE && AV_RL32(track->codec_priv.data+4) == AV_RL32("SMI "))
-codec_id = AV_CODEC_ID_SVQ3;
+if (codec_id == AV_CODEC_ID_NONE && AV_RL32(track->codec_priv.data+4) == AV_RL32("SMI ")) {
+fourcc = MKTAG('S','V','Q','3');
+codec_id = ff_codec_get_id(ff_codec_movvideo_tags, fourcc);
+}
 if (codec_id == AV_CODEC_ID_NONE) {
 char buf[32];
 av_get_codec_tag_string(buf, sizeof(buf), fourcc);
-- 
1.7.10.4

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

Re: [FFmpeg-devel] [PATCH v4] lavf/matroskadec: A_QUICKTIME/AV_CODEC_ID_NONE + SMI->SVQ3

[Carl Eugen Hoyos]

Mats Peterson  ffmpeg.org> writes:

> Therefore, your patch from 2014 for SMI -> SVQ3 needs to set the
> 'fourcc' variable to 'SVQ3' as well, which is later copied to
> st->codec->codec_tag in matroskadec.c.

This cannot be part of the same patch.

Carl Eugen

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

Re: [FFmpeg-devel] [PATCHv2] lavc/cbrt_tablegen: speed up tablegen

[Daniel Serpell]

Hi!,

El Mon, Jan 04, 2016 at 06:33:59PM -0800, Ganesh Ajjanagadde escribio:
> This exploits an approach based on the sieve of Eratosthenes, a popular
> method for generating prime numbers.
> 
> Tables are identical to previous ones.
> 
> Tested with FATE with/without --enable-hardcoded-tables.
> 
> Sample benchmark (Haswell, GNU/Linux+gcc):
> prev:
> 7860100 decicycles in cbrt_tableinit,   1 runs,  0 skips
> 490 decicycles in cbrt_tableinit,   2 runs,  0 skips
> [...]
> 7582339 decicycles in cbrt_tableinit, 256 runs,  0 skips
> 7563556 decicycles in cbrt_tableinit, 512 runs,  0 skips
> 
> new:
> 2099480 decicycles in cbrt_tableinit,   1 runs,  0 skips
> 2044470 decicycles in cbrt_tableinit,   2 runs,  0 skips
> [...]
> 1796544 decicycles in cbrt_tableinit, 256 runs,  0 skips
> 1791631 decicycles in cbrt_tableinit, 512 runs,  0 skips
>

See attached code, function "test1", based on an approximation of:

  (i+1)^(1/3) ~= i^(1/3) * ( 1 + 1/(3i) - 1/(9i) + 5/(81i) -  )

Generated values are the same as original floats (max error in double
is < 4*10^-10), it is faster (and I think, simpler) than your version.

Perhaps altering the constants it could be made faster still, but it is
currently dominated by de division in the main loop.

Daniel.

#include 
#include 
#include 
#include 

void test1(float *out)
{
int i;
double cb;

// Really small values are filled with original function:
for(i=0; i<64; i++)
out[i] = cbrt(i) * i;

// "cb" is the cube-root approximation to "i"
cb = 4;
for(i=64; i<343; i++)
{
double s, r, t;
out[i] = cb * i;
// For small values, we use 5 terms:
r = 1.0/(3*i);// 0 A , 1 M , 1 D
t = r;//
s = 1.0 + r;  // 1 A , 1 M , 1 D
r = r * r;// 1 A , 2 M , 1 D
s = s - r;// 2 A , 2 M , 1 D
r = r * t;// 2 A , 3 M , 1 D
s = s + 1.155 *r; // 3 A , 4 M , 1 D
r = r * t;// 3 A , 5 M , 1 D
s = s - 3.289935 *r;  // 4 A , 6 M , 1 D
cb = cb * s;  // 4 A , 7 M , 1 D
}

cb = 7;
for(i=343; i<8192; i++)
{
double s, r, t;
out[i] = cb * i;
// For big values, we use 4 terms:
r = 1.0/(3*i);// 0 A , 1 M , 1 D
t = r;//
s = 1.0 + r;  // 1 A , 1 M , 1 D
r = r * r;// 1 A , 2 M , 1 D
s = s - r;// 2 A , 2 M , 1 D
r = r * t;// 2 A , 3 M , 1 D
s = s + 1.6644 *r;// 3 A , 4 M , 1 D
cb = cb * s;  // 3 A , 5 M , 1 D
}
}

void test2(float *out)
{
int i;

for(i=0; i<8192; i++)
out[i] = cbrt(i) * i;
}

static double get_time(void)
{
struct timeval t1;
gettimeofday(,0);
return t1.tv_sec * 100.0 + t1.tv_usec;
}

int main()
{
int i;
double t1, t2, t3;
char *orig, *fast;
orig = malloc(sizeof(float) * 8192);
fast = malloc(sizeof(float) * 8192);

// Repeat 200 times
for(i=0; i<200; i++)
{
t1 = get_time();
test1((float *)fast);
t2 = get_time();
test2((float *)orig);
t3 = get_time();

printf("Orig: %f\tFast: %f\n", (t3-t2), (t2-t1));
}

// Compare
for(i=0; i

Re: [FFmpeg-devel] [PATCH v4] lavf/matroskadec: A_QUICKTIME/AV_CODEC_ID_NONE + SMI->SVQ3

[Mats Peterson]

On 01/05/2016 03:13 PM, Carl Eugen Hoyos wrote:

Mats Peterson  ffmpeg.org> writes:


Therefore, your patch from 2014 for SMI -> SVQ3 needs to set the
'fourcc' variable to 'SVQ3' as well, which is later copied to
st->codec->codec_tag in matroskadec.c.


This cannot be part of the same patch.

Carl Eugen

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



I hope he can split it for me. I'm fully aware of the fact that it 
shouldn't be part of the same patch. Sorry for that.


Mats

--
Mats Peterson
http://matsp888.no-ip.org/~mats/
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [RFC][PATCH]lavf/matroskaenc: Assume 48kHz sample rate for Opus initial padding.

[Michael Niedermayer]

On Tue, Jan 05, 2016 at 01:23:54PM +0100, Carl Eugen Hoyos wrote:
> Hi!
> 
> Attached patch may fix this issue reported for Firefox:
> https://bugzilla.mozilla.org/show_bug.cgi?id=1227153
> Completely untested but I believe the patch matches a 
> comment in libopusenc.c line 90.
> 
> Please comment, Carl Eugen

please bump the libavformat version when fixing this so it can be
detected on the demuxer side if a file was generated before or after
the fix.


[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Let us carefully observe those good qualities wherein our enemies excel us
and endeavor to excel them, by avoiding what is faulty, and imitating what
is excellent in them. -- Plutarch


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/formats: fix leak of channel_layouts on error

[Paul B Mahol]

On 1/2/16, Ganesh Ajjanagadde  wrote:
[...]
>
> Thanks a lot for addressing this. But a client may not necessarily
> attempt initialization of the channel layouts first, so instead of
> do_nothing calls, shouldn't there be an equivalent for
> AVFilterFormats*?
>
> Of course, the commit message will need rewording.

After second look I see no need for this code, the present code already
frees all allocated memory.

Whichever instructions you used to expose leaks were flawed.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avfilter/formats: fix leak of channel_layouts on error

[Paul B Mahol]

On 1/5/16, Ganesh Ajjanagadde  wrote:
> On Tue, Jan 5, 2016 at 11:06 AM, Paul B Mahol  wrote:
>> On 1/2/16, Ganesh Ajjanagadde  wrote:
>> [...]
>>>
>>> Thanks a lot for addressing this. But a client may not necessarily
>>> attempt initialization of the channel layouts first, so instead of
>>> do_nothing calls, shouldn't there be an equivalent for
>>> AVFilterFormats*?
>>>
>>> Of course, the commit message will need rewording.
>>
>> After second look I see no need for this code, the present code already
>> frees all allocated memory.
>>
>> Whichever instructions you used to expose leaks were flawed.
>
> care to explain: I did not know valgrind (as well as coverity) had
> such simple bugs if you are indeed correct?

I didn't claim valgrind is buggy in this case, instead the way to
trigger leak is wrong.

The ->list translates to ->channel_layouts and its freed on error IIRC.
___
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

[wm4]

On Tue, 5 Jan 2016 08:32:02 -0800
Ganesh Ajjanagadde  wrote:

> On Tue, Jan 5, 2016 at 5:29 AM, wm4  wrote:
> > On Mon,  4 Jan 2016 17:50:01 -0800
> > 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.
> >>
> >> Reviewed-by: wm4 
> >> Reviewed-by: Ronald S. Bultje 
> >> Reviewed-by: Michael Niedermayer 
> >> Signed-off-by: Ganesh Ajjanagadde 
> >> ---
> >>  libavformat/mp3dec.c | 13 +++--
> >>  libavformat/rmdec.c  |  3 ++-
> >>  2 files changed, 13 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
> >> index c76b21e..57ebcc8 100644
> >> --- a/libavformat/mp3dec.c
> >> +++ b/libavformat/mp3dec.c
> >> @@ -372,11 +372,19 @@ 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,
> >> +   "initial junk detection and skipping impossible 
> >> due to: %s\n", av_err2str(ret));
> >> +goto skip_fail;
> >> +}
> >>  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,
> >> +   "initial junk detection and skipping impossible 
> >> due to: %s\n", av_err2str(ret));
> >> +goto skip_fail;
> >> +}
> >>  if (check(s->pb, off + i + frame_size, ) >= 0 &&
> >>  (header & SAME_HEADER_MASK) == (header2 & 
> >> SAME_HEADER_MASK))
> >>  {
> >> @@ -387,6 +395,7 @@ static int mp3_read_header(AVFormatContext *s)
> >>  }
> >>  avio_seek(s->pb, off, SEEK_SET);
> >>  }
> >> +skip_fail:
> >>
> >>  // the seek index is relative to the end of the xing vbr headers
> >>  for (i = 0; i < st->nb_index_entries; i++)
> >> diff --git a/libavformat/rmdec.c b/libavformat/rmdec.c
> >> index 4e46a3d..470e227 100644
> >> --- a/libavformat/rmdec.c
> >> +++ b/libavformat/rmdec.c
> >> @@ -618,7 +618,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, "seeking back impossible due 
> >> to: %s\n", av_err2str(ret));
> >>  v = avio_rb32(pb);
> >>  if (v == MKBETAG('M', 'L', 'T', 'I')) {
> >>  ret = rm_read_multi(s, s->pb, st, mime);  
> >
> > I maintain that this should not be done, because it makes the code
> > verbose for no reason.  
> 
> Michael has pointed out that when seekback fails, one should really
> break out of the junk detection loop in mp3dec. Your idea fails to
> achieve it.

Why? It's not really harmful. You could even argue that this will make
the common case (skipping jumk and finding a valid frame in a seekable
file) work in low memory situations, while your patch breaks it.

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

Re: [FFmpeg-devel] [PATCH] avfilter/formats: fix leak of channel_layouts on error

[Ganesh Ajjanagadde]

On Tue, Jan 5, 2016 at 11:06 AM, Paul B Mahol  wrote:
> On 1/2/16, Ganesh Ajjanagadde  wrote:
> [...]
>>
>> Thanks a lot for addressing this. But a client may not necessarily
>> attempt initialization of the channel layouts first, so instead of
>> do_nothing calls, shouldn't there be an equivalent for
>> AVFilterFormats*?
>>
>> Of course, the commit message will need rewording.
>
> After second look I see no need for this code, the present code already
> frees all allocated memory.
>
> Whichever instructions you used to expose leaks were flawed.

care to explain: I did not know valgrind (as well as coverity) had
such simple bugs if you are indeed correct?

> ___
> 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] lavc/cbrt_tablegen: speed up tablegen

[Ganesh Ajjanagadde]

On Tue, Jan 5, 2016 at 10:10 AM, Daniel Serpell  wrote:
> Hi!,
>
> El Tue, Jan 05, 2016 at 08:08:35AM -0800, Ganesh Ajjanagadde escribio:
>> On Tue, Jan 5, 2016 at 7:44 AM, Daniel Serpell  wrote:
>> > Hi!,
>> >
>> > El Mon, Jan 04, 2016 at 06:33:59PM -0800, Ganesh Ajjanagadde escribio:
>> >> This exploits an approach based on the sieve of Eratosthenes, a popular
>> >> method for generating prime numbers.
>> >>
>> >> Tables are identical to previous ones.
>> >>
>> >> Tested with FATE with/without --enable-hardcoded-tables.
>> >>
>> >> Sample benchmark (Haswell, GNU/Linux+gcc):
>> >> prev:
>> >> 7860100 decicycles in cbrt_tableinit,   1 runs,  0 skips
>> >> 490 decicycles in cbrt_tableinit,   2 runs,  0 skips
>> >> [...]
>> >> 7582339 decicycles in cbrt_tableinit, 256 runs,  0 skips
>> >> 7563556 decicycles in cbrt_tableinit, 512 runs,  0 skips
>> >>
>> >> new:
>> >> 2099480 decicycles in cbrt_tableinit,   1 runs,  0 skips
>> >> 2044470 decicycles in cbrt_tableinit,   2 runs,  0 skips
>> >> [...]
>> >> 1796544 decicycles in cbrt_tableinit, 256 runs,  0 skips
>> >> 1791631 decicycles in cbrt_tableinit, 512 runs,  0 skips
>> >>
>> >
>> > See attached code, function "test1", based on an approximation of:
>> >
>> >   (i+1)^(1/3) ~= i^(1/3) * ( 1 + 1/(3i) - 1/(9i) + 5/(81i) -  )
>>
>> I assume 1/(3i), 1/(9i^2), etc obtained via a Taylor series at x = 0.
>>
>
> Yes, more specifically (in wxmaxima):
>
> (%i1) at( taylor((i+x)^(1/3)/i^(1/3), x, 0, 6) , x=1 );
>
>1 1   5   10   22   154
> (%o1) 1 + --- -  + - - -- + -- - ---
>   3 i  2   345 6
> 9 i81 i243 i729 i6561 i
>
>
> I noticed that if you do a change of variable, the series simplifies:
>
> (%i2) at( taylor((i+x)^(1/3)/i^(1/3), x, 0, 6) , [ x=1, i = (1/3)/j ] );
>
> 65   4  3
>154 j 22 j10 j5 j 2
> (%o2)   (- --) + - - - +  - j  + j + 1
>  9 3   3  3
>
>> >
>> > Generated values are the same as original floats (max error in double
>> > is < 4*10^-10), it is faster (and I think, simpler) than your version.
>>
>> Had thought of these ideas, but did not examine as I was a little
>> concerned about accuracy. Thanks, will give it a spin. Or
>> alternatively, you can submit a patch since you put it into action.
>>
>
> Best if you make the patch, as you can test the speed in your same setup.

Ok, I will still make you the author though.

>
>> Alternatively, one could directly expand the series for (i+1)^(4/3).
>
> Yes, but the first two coefficients are not 1 anymore, so it needs one
> more multiplication, canceling the advantage:
>
> (%i3) at( taylor((i+x)^(4/3)/i^(4/3), x, 0, 6) , [ x=1, i = (4/3)/j ] );
>
>  65   432
>  11 jj 5 jjj
> (%o3)- - --- +  - -- + -- + j + 1
>  9216384   76848   8

Ok, thanks.

>
>
>> And it may be possible to tighten the number of terms needed by
>> expanding not about x = 0, but x = i to get i+1. Or fancier polynomial
>> approximations can be used. Have you tried these?
>
> I think that this is what I'm already doing. As I said, the slowest part
> is the first division ( r = (1.0/3.0) / i ), and I can't think of any way
> to avoid it.

I meant Chebyshev approximation obtained e.g via Remez; it may allow
shaving the degree by one, but I can't think of a simple way to get
rid of the divide. Also, coefficients will likely not be 1 anymore for
leading terms.

>
> I altered the last constants to reduce the error, but stopped trying after
> getting better than 10^-10 absolute error, that is higher than the
> precision of a float.

This is the main thing I slightly did not like: the perturbations are
not documented and they were done in an ad-hoc way. Overall though,
good work! Will do some more testing and post a patch tonight.

>
> Daniel.
>
> ___
> 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] lavc/cbrt_tablegen: speed up tablegen

[Daniel Serpell]

Hi!,

El Tue, Jan 05, 2016 at 08:08:35AM -0800, Ganesh Ajjanagadde escribio:
> On Tue, Jan 5, 2016 at 7:44 AM, Daniel Serpell  wrote:
> > Hi!,
> >
> > El Mon, Jan 04, 2016 at 06:33:59PM -0800, Ganesh Ajjanagadde escribio:
> >> This exploits an approach based on the sieve of Eratosthenes, a popular
> >> method for generating prime numbers.
> >>
> >> Tables are identical to previous ones.
> >>
> >> Tested with FATE with/without --enable-hardcoded-tables.
> >>
> >> Sample benchmark (Haswell, GNU/Linux+gcc):
> >> prev:
> >> 7860100 decicycles in cbrt_tableinit,   1 runs,  0 skips
> >> 490 decicycles in cbrt_tableinit,   2 runs,  0 skips
> >> [...]
> >> 7582339 decicycles in cbrt_tableinit, 256 runs,  0 skips
> >> 7563556 decicycles in cbrt_tableinit, 512 runs,  0 skips
> >>
> >> new:
> >> 2099480 decicycles in cbrt_tableinit,   1 runs,  0 skips
> >> 2044470 decicycles in cbrt_tableinit,   2 runs,  0 skips
> >> [...]
> >> 1796544 decicycles in cbrt_tableinit, 256 runs,  0 skips
> >> 1791631 decicycles in cbrt_tableinit, 512 runs,  0 skips
> >>
> >
> > See attached code, function "test1", based on an approximation of:
> >
> >   (i+1)^(1/3) ~= i^(1/3) * ( 1 + 1/(3i) - 1/(9i) + 5/(81i) -  )
> 
> I assume 1/(3i), 1/(9i^2), etc obtained via a Taylor series at x = 0.
> 

Yes, more specifically (in wxmaxima):

(%i1) at( taylor((i+x)^(1/3)/i^(1/3), x, 0, 6) , x=1 );

   1 1   5   10   22   154
(%o1) 1 + --- -  + - - -- + -- - ---
  3 i  2   345 6
9 i81 i243 i729 i6561 i


I noticed that if you do a change of variable, the series simplifies:

(%i2) at( taylor((i+x)^(1/3)/i^(1/3), x, 0, 6) , [ x=1, i = (1/3)/j ] );

65   4  3
   154 j 22 j10 j5 j 2
(%o2)   (- --) + - - - +  - j  + j + 1
 9 3   3  3

> >
> > Generated values are the same as original floats (max error in double
> > is < 4*10^-10), it is faster (and I think, simpler) than your version.
> 
> Had thought of these ideas, but did not examine as I was a little
> concerned about accuracy. Thanks, will give it a spin. Or
> alternatively, you can submit a patch since you put it into action.
>

Best if you make the patch, as you can test the speed in your same setup.

> Alternatively, one could directly expand the series for (i+1)^(4/3).

Yes, but the first two coefficients are not 1 anymore, so it needs one
more multiplication, canceling the advantage:

(%i3) at( taylor((i+x)^(4/3)/i^(4/3), x, 0, 6) , [ x=1, i = (4/3)/j ] );

 65   432
 11 jj 5 jjj
(%o3)- - --- +  - -- + -- + j + 1
 9216384   76848   8


> And it may be possible to tighten the number of terms needed by
> expanding not about x = 0, but x = i to get i+1. Or fancier polynomial
> approximations can be used. Have you tried these?

I think that this is what I'm already doing. As I said, the slowest part
is the first division ( r = (1.0/3.0) / i ), and I can't think of any way
to avoid it.

I altered the last constants to reduce the error, but stopped trying after
getting better than 10^-10 absolute error, that is higher than the
precision of a float.

Daniel.

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

Re: [FFmpeg-devel] [PATCH 2/3] libavcodec/ccaption_dec: clean up and standardize white space

[Clément Bœsch]

On Mon, Jan 04, 2016 at 07:28:02PM -0800, Aman Gupta wrote:
> From: Aman Gupta 
> 
> ---
>  libavcodec/ccaption_dec.c | 98 
> ++-
>  1 file changed, 45 insertions(+), 53 deletions(-)
> 

There are much more garbage formatting in that file (typically at the
beginning of process_cc608 around braces) but still fine with me.

[...]

-- 
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/3] libavcodec/ccaption_dec: rewrite packet handler as case statement; remove COR3 macro

[Clément Bœsch]

On Mon, Jan 04, 2016 at 07:28:03PM -0800, Aman Gupta wrote:
> From: Aman Gupta 
> 
> ---
>  libavcodec/ccaption_dec.c | 92 
> +++
>  1 file changed, 53 insertions(+), 39 deletions(-)
> 
> diff --git a/libavcodec/ccaption_dec.c b/libavcodec/ccaption_dec.c
> index 96f0ccf..788e96a 100644
> --- a/libavcodec/ccaption_dec.c
> +++ b/libavcodec/ccaption_dec.c
> @@ -453,54 +453,69 @@ static void handle_char(CCaptionSubContext *ctx, char 
> hi, char lo, int64_t pts)
>  static int process_cc608(CCaptionSubContext *ctx, int64_t pts, uint8_t hi, 
> uint8_t lo)
>  {
>  int ret = 0;
> -#define COR3(var, with1, with2, with3)  ( (var) == (with1) ||  (var) == 
> (with2) || (var) == (with3) )
>  if (hi == ctx->prev_cmd[0] && lo == ctx->prev_cmd[1]) {
> -/* ignore redundant command */
> +/* ignore redundant command */
>  } else if ( (hi == 0x10 && (lo >= 0x40 || lo <= 0x5f)) ||
>( (hi >= 0x11 && hi <= 0x17) && (lo >= 0x40 && lo <= 0x7f) ) ) 
> {
>  handle_pac(ctx, hi, lo);
>  } else if ( ( hi == 0x11 && lo >= 0x20 && lo <= 0x2f ) ||
>  ( hi == 0x17 && lo >= 0x2e && lo <= 0x2f) ) {
>  handle_textattr(ctx, hi, lo);
> -} else if ( COR3(hi, 0x14, 0x15, 0x1C) && lo == 0x20 ) {
> -/* resume caption loading */
> -ctx->mode = CCMODE_POPON;
> -} else if ( COR3(hi, 0x14, 0x15, 0x1C) && lo == 0x24 ) {
> -handle_delete_end_of_row(ctx, hi, lo);
> -} else if ( COR3(hi, 0x14, 0x15, 0x1C) && lo == 0x25 ) {
> -ctx->rollup = 2;
> -ctx->mode = CCMODE_ROLLUP_2;
> -} else if ( COR3(hi, 0x14, 0x15, 0x1C) && lo == 0x26 ) {
> -ctx->rollup = 3;
> -ctx->mode = CCMODE_ROLLUP_3;
> -} else if ( COR3(hi, 0x14, 0x15, 0x1C) && lo == 0x27 ) {
> -ctx->rollup = 4;
> -ctx->mode = CCMODE_ROLLUP_4;
> -} else if ( COR3(hi, 0x14, 0x15, 0x1C) && lo == 0x29 ) {
> -/* resume direct captioning */
> -ctx->mode = CCMODE_PAINTON;
> -} else if ( COR3(hi, 0x14, 0x15, 0x1C) && lo == 0x2B ) {
> -/* resume text display */
> -ctx->mode = CCMODE_TEXT;
> -} else if ( COR3(hi, 0x14, 0x15, 0x1C) && lo == 0x2C ) {
> -/* erase display memory */
> -ret = handle_edm(ctx, pts);
> -} else if ( COR3(hi, 0x14, 0x15, 0x1C) && lo == 0x2D ) {
> -/* carriage return */
> -ff_dlog(ctx, "carriage return\n");
> -reap_screen(ctx, pts);
> -roll_up(ctx);
> -ctx->screen_changed = 1;
> -ctx->cursor_column = 0;
> -} else if ( COR3(hi, 0x14, 0x15, 0x1C) && lo == 0x2F ) {
> -/* end of caption */
> -ff_dlog(ctx, "handle_eoc\n");
> -ret = handle_eoc(ctx, pts);
> +} else if (hi == 0x14 || hi == 0x15 || hi == 0x1c) {
> +switch (lo) {
> +case 0x20:
> +/* resume caption loading */
> +ctx->mode = CCMODE_POPON;
> +break;
> +case 0x24:
> +handle_delete_end_of_row(ctx, hi, lo);
> +break;
> +case 0x25:
> +ctx->rollup = 2;
> +ctx->mode = CCMODE_ROLLUP_2;
> +break;
> +case 0x26:
> +ctx->rollup = 3;
> +ctx->mode = CCMODE_ROLLUP_3;
> +break;
> +case 0x27:
> +ctx->rollup = 4;
> +ctx->mode = CCMODE_ROLLUP_4;
> +break;
> +case 0x29:
> +/* resume direct captioning */
> +ctx->mode = CCMODE_PAINTON;
> +break;
> +case 0x2b:
> +/* resume text display */
> +ctx->mode = CCMODE_TEXT;
> +break;
> +case 0x2c:
> +/* erase display memory */
> +ret = handle_edm(ctx, pts);
> +break;
> +case 0x2d:
> +/* carriage return */
> +ff_dlog(ctx, "carriage return\n");
> +reap_screen(ctx, pts);
> +roll_up(ctx);
> +ctx->screen_changed = 1;
> +ctx->cursor_column = 0;
> +break;
> +case 0x2f:
> +/* end of caption */
> +ff_dlog(ctx, "handle_eoc\n");
> +ret = handle_eoc(ctx, pts);
> +break;
> +default:
> +ff_dlog(ctx, "Unknown command 0x%hhx 0x%hhx\n", hi, lo);
> +break;
> +}
>  } else if (hi>=0x20) {
> -/* Standard characters (always in pairs) */
> +/* Standard characters (always in pairs) */
>  handle_char(ctx, hi, lo, pts);
>  } else {
> -/* Ignoring all other non data code */
> +/* Ignoring all other non data code */
>  ff_dlog(ctx, "Unknown command 0x%hhx 0x%hhx\n", hi, lo);
>  }
>  
> @@ -508,7 +523,6 @@ static int process_cc608(CCaptionSubContext *ctx, int64_t 
> pts, uint8_t hi, uint8
>  ctx->prev_cmd[0] = hi;
>  ctx->prev_cmd[1] = lo;
>  
> -#undef COR3
>  return ret;
>  }
>  

Looks OK, but the command handling 

Re: [FFmpeg-devel] [WIP] SDL2 in ffplay

[Andreas Cadhalpun]

On 05.01.2016 21:03, Marton Balint wrote:
> On Tue, 5 Jan 2016, Roger Pack wrote:
>> Perhaps you could update the "sdl out" filter as well? :)
> 
> I guess that shouldn't be too hard, but is there anybody who thinks that we 
> should
> NOT drop SDL1 support? Because if there is, then I am less motivated doing 
> it...

libsdl2 is not present in Debian 6 Squeeze (supported until February 2016).
For Debian 7 Wheezy it's available via Backports and it's regularly included in
Debian 8 Jessie.
Also it's present in all supported Ubuntu releases except 12.04 (supported 
until 2017).

Also it's not available for RHEL 5/6, only for 7 (via EPEL).

So people wanting to use bleeding edge ffplay on these oldest supported LTS 
releases
would have to compile libsdl2 manually or just upgrade.
I don't think that would be a problem.

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

Re: [FFmpeg-devel] [PATCH] parser: error out if the codec doesn't match

[Michael Niedermayer]

On Tue, Jan 05, 2016 at 12:48:10PM +0100, Andreas Cadhalpun wrote:
> On 05.01.2016 12:18, Michael Niedermayer wrote:
> > On Tue, Jan 05, 2016 at 12:02:29PM +0100, Andreas Cadhalpun wrote:
> >> That wouldn't work, as the codec id wasn't changed in force_codec_ids,
> >> but in the API using program.
> >> To reiterate, the problematic steps were:
> >>  * call avformat_find_stream_info, which detects a codec and initializes
> >>a parser for it
> >>  * afterwards change the codec id in the API using program, so it
> >>doesn't match with the parser
> >>
> >> Thus I think the only reliable way to detect this is a check in 
> >> av_parser_parse2.
> > 
> > shouldnt this be a av_assert1/2() then ?
> > i mean this is a programming error in the user app not a error
> > condition that should happen that way in correct usage
> > IMHO if we need 5 extra checks thats fine but if this is just to
> > detect a user app bug then iam not sure these checks should be run
> > for every packet
> 
> I would prefer av_assert0, but if you think that could cause relevant 
> slowdowns,
> I'm also fine with av_assert1. Patch for that attached.
> 
> Best regards,
> Andreas
> 

>  parser.c |7 +++
>  1 file changed, 7 insertions(+)
> 1a29aebd63e39c82039cddb1239de415b1da43df  
> 0001-parser-add-av_assert1-to-make-sure-the-codec-matches.patch
> From 80f6baa6d091807538461bab28656e3cf7303a2d Mon Sep 17 00:00:00 2001
> From: Andreas Cadhalpun 
> Date: Mon, 4 Jan 2016 23:52:20 +0100
> Subject: [PATCH] parser: add av_assert1 to make sure the codec matches
> 
> Otherwise this can have some surprising effects (crashes), so let's
> better not allow it.
> 
> Signed-off-by: Andreas Cadhalpun 

LGTM

thx

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Freedom in capitalist society always remains about the same as it was in
ancient Greek republics: Freedom for slave owners. -- Vladimir Lenin


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

Re: [FFmpeg-devel] [WIP] SDL2 in ffplay

[wm4]

On Tue, 5 Jan 2016 21:03:14 +0100 (CET)
Marton Balint  wrote:

> On Tue, 5 Jan 2016, Roger Pack wrote:
> 
> > On 1/2/16, Marton Balint  wrote:  
> >> Hi,
> >>
> >> To anybody who is interested, I have pushed my experimental SDL2 branch of
> >> ffplay to github. (https://github.com/cus/ffplay.git)
> >>
> >> SDL2 completely replaced SDL_Overlay with 3D textures, so with SDL2 it is
> >> possible to:
> >> - Use textures with odd width/height
> >> - Use RGB textures, not only YUV
> >> - Use the renderer to present subtitles with alpha channel
> >> on top of the video instead of software blending  
> >
> > Perhaps you could update the "sdl out" filter as well? :)  
> 
> I guess that shouldn't be too hard, but is there anybody who thinks that 
> we should NOT drop SDL1 support? Because if there is, then I am less 
> motivated doing it...

AFAIK SDL 1 is unmaintained now. I'm not sure why anyone would
explicitly want SDL 1 over 2. And trying to support both would be an
unholy mess.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [WIP] SDL2 in ffplay

[Roger Pack]

On 1/2/16, Marton Balint  wrote:
> Hi,
>
> To anybody who is interested, I have pushed my experimental SDL2 branch of
> ffplay to github. (https://github.com/cus/ffplay.git)
>
> SDL2 completely replaced SDL_Overlay with 3D textures, so with SDL2 it is
> possible to:
> - Use textures with odd width/height
> - Use RGB textures, not only YUV
> - Use the renderer to present subtitles with alpha channel
> on top of the video instead of software blending

Perhaps you could update the "sdl out" filter as well? :)
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avfilter/formats: fix leak of channel_layouts on error

[Ganesh Ajjanagadde]

On Tue, Jan 5, 2016 at 11:16 AM, Paul B Mahol  wrote:
> On 1/5/16, Ganesh Ajjanagadde  wrote:
>> On Tue, Jan 5, 2016 at 11:06 AM, Paul B Mahol  wrote:
>>> On 1/2/16, Ganesh Ajjanagadde  wrote:
>>> [...]

 Thanks a lot for addressing this. But a client may not necessarily
 attempt initialization of the channel layouts first, so instead of
 do_nothing calls, shouldn't there be an equivalent for
 AVFilterFormats*?

 Of course, the commit message will need rewording.
>>>
>>> After second look I see no need for this code, the present code already
>>> frees all allocated memory.
>>>
>>> Whichever instructions you used to expose leaks were flawed.
>>
>> care to explain: I did not know valgrind (as well as coverity) had
>> such simple bugs if you are indeed correct?
>
> I didn't claim valgrind is buggy in this case, instead the way to
> trigger leak is wrong.
>
> The ->list translates to ->channel_layouts and its freed on error IIRC.

Ok. Let us examine this carefully, and patiently step by step:
1. Patch to trigger leaks:
diff --git a/libavfilter/formats.c b/libavfilter/formats.c
index a2b19e7..dde30ec 100644
--- a/libavfilter/formats.c
+++ b/libavfilter/formats.c
@@ -422,7 +422,8 @@ AVFilterChannelLayouts *ff_all_channel_counts(void)
 if (!f || !ref)
  \
 return AVERROR(ENOMEM);
  \

  \
-tmp = av_realloc_array(f->refs, sizeof(*f->refs), f->refcount +
1); \
+/*tmp = av_realloc_array(f->refs, sizeof(*f->refs), f->refcount +
1);*/ \
+tmp = NULL;
  \
 if (!tmp) {
  \
 unref_fn();
  \
 return AVERROR(ENOMEM);
  \

I think you agree that this is a correct way to simulate.

2. Run valgrind before and after this patch, with the command:
valgrind -v --leak-check=full --show-leak-kinds=all
--log-file=/tmp/avfilter_leak ./ffmpeg_g -i
~/samples/h264/crop-to-container-dims-canon.mov -af compensationdelay
-f null -

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

[FFmpeg-devel] [PATCH] avfilter: add ahistogram multimedia filter

[Paul B Mahol]

Signed-off-by: Paul B Mahol 
---
 doc/filters.texi |  86 ++
 libavfilter/Makefile |   1 +
 libavfilter/allfilters.c |   1 +
 libavfilter/avf_ahistogram.c | 391 +++
 4 files changed, 479 insertions(+)
 create mode 100644 libavfilter/avf_ahistogram.c

diff --git a/doc/filters.texi b/doc/filters.texi
index a48ec3a..108b628 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -13282,6 +13282,92 @@ tools.
 
 Below is a description of the currently available multimedia filters.
 
+@section ahistogram
+
+Convert input audio to a video output, displaying the volume histogram.
+
+The filter accepts the following options:
+
+@table @option
+@item dmode
+Specify how histogram is calculated.
+
+It accepts the following values:
+@table @samp
+@item single
+Use single histogram for all channels.
+@item separate
+Use separate histogram for each channel.
+@end table
+Default is @code{single}.
+
+@item hmode
+Specify histogram mode.
+
+It accepts the following values:
+@table @samp
+@item accumulate
+Accumulate all frames.
+@item current
+Use current frame only.
+@end table
+Default is @code{current}.
+
+@item rate, r
+Set frame rate, expressed as number of frames per second. Default
+value is "25".
+
+@item size, s
+Specify the video size for the output. For the syntax of this option, check the
+@ref{video size syntax,,"Video size" section in the ffmpeg-utils 
manual,ffmpeg-utils}.
+Default value is @code{hd720}.
+
+@item scale
+Set display scale.
+
+It accepts the following values:
+@table @samp
+@item log
+logarithmic
+@item sqrt
+square root
+@item cbrt
+cubic root
+@item lin
+linear
+@item rlog
+reverse logarithmic
+@end table
+Default is @code{log}.
+
+@item ascale
+Set amplitude scale.
+
+It accepts the following values:
+@table @samp
+@item log
+logarithmic
+@item lin
+linear
+@end table
+Default is @code{log}.
+
+@item pheight
+Set histogram percentage of window height.
+
+@item slide
+Set sonogram sliding.
+
+It accepts the following values:
+@table @samp
+@item replace
+logarithmic
+@item scroll
+scroll from to bottom.
+@end table
+Default is @code{replace}.
+@end table
+
 @section aphasemeter
 
 Convert input audio to a video output, displaying the audio phase.
diff --git a/libavfilter/Makefile b/libavfilter/Makefile
index 689da73..4890832 100644
--- a/libavfilter/Makefile
+++ b/libavfilter/Makefile
@@ -280,6 +280,7 @@ OBJS-$(CONFIG_NULLSINK_FILTER)   += 
vsink_nullsink.o
 
 # multimedia filters
 OBJS-$(CONFIG_ADRAWGRAPH_FILTER) += f_drawgraph.o
+OBJS-$(CONFIG_AHISTOGRAM_FILTER) += avf_ahistogram.o
 OBJS-$(CONFIG_APHASEMETER_FILTER)+= avf_aphasemeter.o
 OBJS-$(CONFIG_AVECTORSCOPE_FILTER)   += avf_avectorscope.o
 OBJS-$(CONFIG_CONCAT_FILTER) += avf_concat.o
diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
index 2267e88..7b33404 100644
--- a/libavfilter/allfilters.c
+++ b/libavfilter/allfilters.c
@@ -300,6 +300,7 @@ void avfilter_register_all(void)
 
 /* multimedia filters */
 REGISTER_FILTER(ADRAWGRAPH, adrawgraph, avf);
+REGISTER_FILTER(AHISTOGRAM, ahistogram, avf);
 REGISTER_FILTER(APHASEMETER,aphasemeter,avf);
 REGISTER_FILTER(AVECTORSCOPE,   avectorscope,   avf);
 REGISTER_FILTER(CONCAT, concat, avf);
diff --git a/libavfilter/avf_ahistogram.c b/libavfilter/avf_ahistogram.c
new file mode 100644
index 000..b2ecbae
--- /dev/null
+++ b/libavfilter/avf_ahistogram.c
@@ -0,0 +1,391 @@
+/*
+ * Copyright (c) 2015 Paul B Mahol
+ *
+ * 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/avassert.h"
+#include "libavutil/opt.h"
+#include "libavutil/parseutils.h"
+#include "avfilter.h"
+#include "formats.h"
+#include "audio.h"
+#include "video.h"
+#include "internal.h"
+
+enum DisplayScale   { LINEAR, SQRT, CBRT, LOG, RLOG, NB_SCALES };
+enum AmplitudeScale { ALINEAR, ALOG, NB_ASCALES };
+enum SlideMode  { REPLACE, SCROLL, NB_SLIDES };
+enum DisplayMode{ SINGLE, SEPARATE, NB_DMODES };
+enum HistogramMode  { ACCUMULATE, CURRENT, NB_HMODES };
+
+typedef struct AudioHistogramContext {
+const AVClass *class;
+AVFrame *out;
+int w, 

Re: [FFmpeg-devel] [PATCH] libavcodec/ccaption_dec: remove unnecessary buffering of closed caption packets

[Clément Bœsch]

On Sun, Jan 03, 2016 at 01:07:15PM +0100, Clément Bœsch wrote:
[...]
> This indeed LGTM, but I'm not the maintainer.
> 

OK I finally understood why it's done that way: validate_cc_data_pair()
alters the pkt data, but the decoder isn't supposed to do that.

So this patch is actually incorrect in this state.

-- 
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] [RFC] avcodec: Add native DCA decoder based on libdcadec.

[foo86]

On Tue, Jan 05, 2016 at 08:45:22PM +0100, Andreas Cadhalpun wrote:
> On 03.01.2016 18:49, foo86 wrote:
> > +// 5.3.1 - Bit stream header
> > +static int parse_frame_header(DCA2CoreDecoder *s)
> > +{
> [...]
> > +// Source PCM resolution
> > +s->source_pcm_res = ff_dca_bits_per_sample[pcmr_index = 
> > get_bits(>gb, 3)];
> 
> This can cause an out-of-bounds read if get_bits returns 7, because 
> ff_dca_bits_per_sample
> only has 7 elements.

Fixed locally, thanks.

P.S. To avoid resending this huge patch, I've put the fixes accumulated
so far in a private dcadec2 branch on github [1] (will be rebased
frequently against FFmpeg master).

[1]: https://github.com/foo86/FFmpeg/tree/dcadec2
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] lavc/ccaption_dec: fix always true condition

[Clément Bœsch]

No idea why this wasn't ever detected by a static analyzer.
---
 libavcodec/ccaption_dec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/ccaption_dec.c b/libavcodec/ccaption_dec.c
index 4e478e0..94771d5 100644
--- a/libavcodec/ccaption_dec.c
+++ b/libavcodec/ccaption_dec.c
@@ -462,7 +462,7 @@ static int process_cc608(CCaptionSubContext *ctx, int64_t 
pts, uint8_t hi, uint8
 #define COR3(var, with1, with2, with3)  ( (var) == (with1) ||  (var) == 
(with2) || (var) == (with3) )
 if ( hi == ctx->prev_cmd[0] && lo == ctx->prev_cmd[1]) {
 /* ignore redundant command */
-} else if ( (hi == 0x10 && (lo >= 0x40 || lo <= 0x5f)) ||
+} else if ( (hi == 0x10 && (lo >= 0x40 && lo <= 0x5f)) ||
   ( (hi >= 0x11 && hi <= 0x17) && (lo >= 0x40 && lo <= 0x7f) ) ) {
 handle_pac(ctx, hi, lo);
 } else if ( ( hi == 0x11 && lo >= 0x20 && lo <= 0x2f ) ||
-- 
2.6.4

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

Re: [FFmpeg-devel] [WIP] SDL2 in ffplay

[Marton Balint]

On Tue, 5 Jan 2016, Roger Pack wrote:


On 1/2/16, Marton Balint  wrote:

Hi,

To anybody who is interested, I have pushed my experimental SDL2 branch of
ffplay to github. (https://github.com/cus/ffplay.git)

SDL2 completely replaced SDL_Overlay with 3D textures, so with SDL2 it is
possible to:
- Use textures with odd width/height
- Use RGB textures, not only YUV
- Use the renderer to present subtitles with alpha channel
on top of the video instead of software blending


Perhaps you could update the "sdl out" filter as well? :)


I guess that shouldn't be too hard, but is there anybody who thinks that 
we should NOT drop SDL1 support? Because if there is, then I am less 
motivated doing it...


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

[FFmpeg-devel] [PATCH] avfilter/formats: fix leak of formats on error

[Paul B Mahol]

Signed-off-by: Paul B Mahol 
---
 libavfilter/formats.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/libavfilter/formats.c b/libavfilter/formats.c
index a2b19e7..f12dcf4 100644
--- a/libavfilter/formats.c
+++ b/libavfilter/formats.c
@@ -518,6 +518,8 @@ void ff_formats_changeref(AVFilterFormats **oldref, 
AVFilterFormats **newref)
 int ret = ref_fn(fmts, >inputs[i]->out_fmts);  \
 if (ret < 0) {  \
 unref_fn();\
+av_freep(>list);  \
+av_freep();\
 return ret; \
 }   \
 count++;\
@@ -528,6 +530,8 @@ void ff_formats_changeref(AVFilterFormats **oldref, 
AVFilterFormats **newref)
 int ret = ref_fn(fmts, >outputs[i]->in_fmts);  \
 if (ret < 0) {  \
 unref_fn();\
+av_freep(>list);  \
+av_freep();\
 return ret; \
 }   \
 count++;\
-- 
1.9.1

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

Re: [FFmpeg-devel] [RFC][PATCH]lavf/matroskaenc: Assume 48kHz sample rate for Opus initial padding.

[Carl Eugen Hoyos]

Michael Niedermayer  niedermayer.cc> writes:

> > Attached patch may fix this issue reported for Firefox:
> > https://bugzilla.mozilla.org/show_bug.cgi?id=1227153
> > Completely untested but I believe the patch matches a 
> > comment in libopusenc.c line 90.

> please bump the libavformat version when fixing this so it can be
> detected on the demuxer side if a file was generated before or after
> the fix.

Done and pushed.

Thank you, Carl Eugen

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

Re: [FFmpeg-devel] [PATCH] Add frame side data when SEI green metadata are detected

[Michael Niedermayer]

On Tue, Jan 05, 2016 at 10:19:28AM +, Nicolas Derouineau wrote:
> >this should be in a seperate patch and libavutil/version.h should have
> >its minor version bumped
> 
> Ok, so I'm going to do two separate patch (commit ?) for libavformat and for 
> libavutil.

yes, please do


> Where should I bump libavutil/version.h ? (I'm not sure I really understand 
> this action).

when a new feature is added to a lib, its version should be increased
so that any application can use the version to check if the feature
is available
the middle number is theoreticaly the correct one to increase for
feature additions



> 
> >GreenMetaData is defined in h264.h, which is an internal header
> >while AVFrameSideDataType is external. public API should not refer to
> >internal/private headers
> 
> Ok
> 

> >also the struct, as currently defined is platform specific. Theres no
> >gurantee that a compiler doesnt add padding betwen the elements. 
> 
> I'm not sure how to deal with this issue in ffmpeg. I guess #pragma packed is 
> not an option, right ?

no, #pragma packed, cant be used


> 
> >Also the code initializing it only matches the platform dependant struct
> >on little endian
> 
> Can you explain to me which part of the code is platform dependant on little 
> endian ? (because memset seems allright to me, but I guess I'm wrong).

IIRC C allows fields to have arbitrary padding
this does not happen on actual systems so its hypothetical but we
should not knowingly write code that depends on assumtations that
are outside the standards unless thats somehow hard/ugly to avoid

it should be quite easy to just treat the stored side data as
a array of bytes with defined meaning for each (preferrably with
some tought about future extensibilty)

that can then be read7written into the struct 1 element at a time

as in mystruct.this = AV_RB16(myarray+123)

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

Awnsering whenever a program halts or runs forever is
On a turing machine, in general impossible (turings halting problem).
On any real computer, always possible as a real computer has a finite number
of states N, and will either halt in less than N cycles or never halt.


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 Tue, Jan 5, 2016 at 4:57 PM, Michael Niedermayer
 wrote:
> On Tue, Jan 05, 2016 at 01:57:09PM -0800, Ganesh Ajjanagadde wrote:
>> On Tue, Jan 5, 2016 at 11:01 AM, wm4  wrote:
>> > On Tue, 5 Jan 2016 08:32:02 -0800
>> > Ganesh Ajjanagadde  wrote:
>> >
>> >> On Tue, Jan 5, 2016 at 5:29 AM, wm4  wrote:
>> >> > On Mon,  4 Jan 2016 17:50:01 -0800
>> >> > 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.
>> >> >>
>> >> >> Reviewed-by: wm4 
>> >> >> Reviewed-by: Ronald S. Bultje 
>> >> >> Reviewed-by: Michael Niedermayer 
>> >> >> Signed-off-by: Ganesh Ajjanagadde 
>> >> >> ---
>> >> >>  libavformat/mp3dec.c | 13 +++--
>> >> >>  libavformat/rmdec.c  |  3 ++-
>> >> >>  2 files changed, 13 insertions(+), 3 deletions(-)
>> >> >>
>> >> >> diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
>> >> >> index c76b21e..57ebcc8 100644
>> >> >> --- a/libavformat/mp3dec.c
>> >> >> +++ b/libavformat/mp3dec.c
>> >> >> @@ -372,11 +372,19 @@ 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,
>> >> >> +   "initial junk detection and skipping 
>> >> >> impossible due to: %s\n", av_err2str(ret));
>> >> >> +goto skip_fail;
>> >> >> +}
>> >> >>  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,
>> >> >> +   "initial junk detection and skipping 
>> >> >> impossible due to: %s\n", av_err2str(ret));
>> >> >> +goto skip_fail;
>> >> >> +}
>> >> >>  if (check(s->pb, off + i + frame_size, ) >= 0 &&
>> >> >>  (header & SAME_HEADER_MASK) == (header2 & 
>> >> >> SAME_HEADER_MASK))
>> >> >>  {
>> >> >> @@ -387,6 +395,7 @@ static int mp3_read_header(AVFormatContext *s)
>> >> >>  }
>> >> >>  avio_seek(s->pb, off, SEEK_SET);
>> >> >>  }
>> >> >> +skip_fail:
>> >> >>
>> >> >>  // the seek index is relative to the end of the xing vbr headers
>> >> >>  for (i = 0; i < st->nb_index_entries; i++)
>> >> >> diff --git a/libavformat/rmdec.c b/libavformat/rmdec.c
>> >> >> index 4e46a3d..470e227 100644
>> >> >> --- a/libavformat/rmdec.c
>> >> >> +++ b/libavformat/rmdec.c
>> >> >> @@ -618,7 +618,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, "seeking back impossible 
>> >> >> due to: %s\n", av_err2str(ret));
>> >> >>  v = avio_rb32(pb);
>> >> >>  if (v == MKBETAG('M', 'L', 'T', 'I')) {
>> >> >>  ret = rm_read_multi(s, s->pb, st, mime);
>> >> >
>> >> > I maintain that this should not be done, because it makes the code
>> >> > verbose for no reason.
>> >>
>> >> Michael has pointed out that when seekback fails, one should really
>> >> break out of the junk detection loop in mp3dec. Your idea fails to
>> >> achieve it.
>> >
>> > Why? It's not really harmful. You could even argue that this will make
>> > the common case (skipping jumk and finding a valid frame in a seekable
>> > file) work in low memory situations, while your patch breaks it.
>
> ffio_ensure_seekback() never fails for seekable files
> it checks s->seekable and return 0 if true before attempting to
> allocate anything
>
>
>>
>> I assumed Michael had a good reason for it, but from what you say here
>> and my examination just now, I would at a first glance agree with you.
>> Ronald had concerns about repetition of messages, so if he (and
>> Michael) are fine with logging within ffio_ensure_seekback, then I
>> will log it there.
>
> as the code is written currently, if ffio_ensure_seekback() fails
> and that being ignored, the url_seek() to return to the begin will
> fail too quite likely while forward seeks will succeed.
> this would result in random amounts of 

Re: [FFmpeg-devel] [PATCH 2/3] libavcodec/ccaption_dec: clean up and standardize white space

[Aman Gupta]

On Tue, Jan 5, 2016 at 6:24 PM, Michael Niedermayer 
wrote:

> On Tue, Jan 05, 2016 at 09:34:35PM +0100, Clément Bœsch wrote:
> > On Mon, Jan 04, 2016 at 07:28:02PM -0800, Aman Gupta wrote:
> > > From: Aman Gupta 
> > >
> > > ---
> > >  libavcodec/ccaption_dec.c | 98
> ++-
> > >  1 file changed, 45 insertions(+), 53 deletions(-)
> > >
> >
> > There are much more garbage formatting in that file (typically at the
> > beginning of process_cc608 around braces) but still fine with me.
>
> should i apply this patch and the other?
>

Please do, as my follow-up patchsets are based on this patch.


>
> [...]
>
> --
> 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] libavcodec/ccaption_dec: remove unnecessary buffering of closed caption packets

[Aman Gupta]

On Tue, Jan 5, 2016 at 12:25 PM, Clément Bœsch  wrote:

> On Sun, Jan 03, 2016 at 01:07:15PM +0100, Clément Bœsch wrote:
> [...]
> > This indeed LGTM, but I'm not the maintainer.
> >
>
> OK I finally understood why it's done that way: validate_cc_data_pair()
> alters the pkt data, but the decoder isn't supposed to do that.
>
> So this patch is actually incorrect in this state.
>

Wow, good catch. The memcpy() makes much more sense now. Thanks.


>
> --
> Clément B.
>
> ___
> 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

[FFmpeg-devel] [PATCH 01/10] libavcodec/ccaption_dec: fix whitespace

[Aman Gupta]

From: Aman Gupta 

---
 libavcodec/ccaption_dec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/ccaption_dec.c b/libavcodec/ccaption_dec.c
index 34a7208..12e8f1d 100644
--- a/libavcodec/ccaption_dec.c
+++ b/libavcodec/ccaption_dec.c
@@ -159,7 +159,7 @@ typedef struct CCaptionSubContext {
 AVBPrint buffer;
 int screen_changed;
 int rollup;
-enum  cc_mode mode;
+enum cc_mode mode;
 int64_t start_time;
 /* visible screen time */
 int64_t startv_time;
-- 
2.5.3

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

Re: [FFmpeg-devel] [RFC] avcodec: Add native DCA decoder based on libdcadec.

[Michael Niedermayer]

On Tue, Jan 05, 2016 at 11:27:25PM -0300, James Almer wrote:
> On 1/5/2016 11:21 PM, Michael Niedermayer wrote:
> > On Tue, Jan 05, 2016 at 11:38:00PM +0300, foo86 wrote:
> >> On Tue, Jan 05, 2016 at 08:45:22PM +0100, Andreas Cadhalpun wrote:
> >>> On 03.01.2016 18:49, foo86 wrote:
>  +// 5.3.1 - Bit stream header
>  +static int parse_frame_header(DCA2CoreDecoder *s)
>  +{
> >>> [...]
>  +// Source PCM resolution
>  +s->source_pcm_res = ff_dca_bits_per_sample[pcmr_index = 
>  get_bits(>gb, 3)];
> >>>
> >>> This can cause an out-of-bounds read if get_bits returns 7, because 
> >>> ff_dca_bits_per_sample
> >>> only has 7 elements.
> >>
> >> Fixed locally, thanks.
> >>
> >> P.S. To avoid resending this huge patch, I've put the fixes accumulated
> >> so far in a private dcadec2 branch on github [1] (will be rebased
> >> frequently against FFmpeg master).
> >>
> > 
> >> [1]: https://github.com/foo86/FFmpeg/tree/dcadec2
> > 
> > breaks "make fate", something needs to be updated
> > or a new reference sample uploaded if teh one we have is wrong
> > 
> > stddev:  297.72 PSNR: 46.85 MAXDIFF: 3474 bytes:  8994816/  9601024
> > MAXDIFF: |3474 - 0| >= 1
> > size: |8994816 - 9601024| >= 0
> > Test dca-xll failed. Look at tests/data/fate/dca-xll.err for details.
> > make: *** [fate-dca-xll] Error 1
> > make: *** Waiting for unfinished jobs
> 
> Was this run using foo86's decoder, or the current one? If the former then 
> it's
> not unexpected since the xll test was made for the current decoder, which is 
> not
> bitexact.

i just locally merged the branch and did a make -j12 fate


> 
> Ideally, once this decoder is committed replacing the current one we'd replace
> the samples for the set available here: 
> https://github.com/foo86/dcadec-samples

we can add new fate samples, we cannot replace fate samples
replacing would break previous checkouts and releases
not sure you actually meant to replace any ...

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

Dictatorship: All citizens are under surveillance, all their steps and
actions recorded, for the politicians to enforce control.
Democracy: All politicians are under surveillance, all their steps and
actions recorded, for the citizens to enforce control.


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

Re: [FFmpeg-devel] [RFC] avcodec: Add native DCA decoder based on libdcadec.

[James Almer]

On 1/5/2016 11:35 PM, Michael Niedermayer wrote:
> On Tue, Jan 05, 2016 at 11:27:25PM -0300, James Almer wrote:
>> On 1/5/2016 11:21 PM, Michael Niedermayer wrote:
>>> On Tue, Jan 05, 2016 at 11:38:00PM +0300, foo86 wrote:
 On Tue, Jan 05, 2016 at 08:45:22PM +0100, Andreas Cadhalpun wrote:
> On 03.01.2016 18:49, foo86 wrote:
>> +// 5.3.1 - Bit stream header
>> +static int parse_frame_header(DCA2CoreDecoder *s)
>> +{
> [...]
>> +// Source PCM resolution
>> +s->source_pcm_res = ff_dca_bits_per_sample[pcmr_index = 
>> get_bits(>gb, 3)];
>
> This can cause an out-of-bounds read if get_bits returns 7, because 
> ff_dca_bits_per_sample
> only has 7 elements.

 Fixed locally, thanks.

 P.S. To avoid resending this huge patch, I've put the fixes accumulated
 so far in a private dcadec2 branch on github [1] (will be rebased
 frequently against FFmpeg master).

>>>
 [1]: https://github.com/foo86/FFmpeg/tree/dcadec2
>>>
>>> breaks "make fate", something needs to be updated
>>> or a new reference sample uploaded if teh one we have is wrong
>>>
>>> stddev:  297.72 PSNR: 46.85 MAXDIFF: 3474 bytes:  8994816/  9601024
>>> MAXDIFF: |3474 - 0| >= 1
>>> size: |8994816 - 9601024| >= 0
>>> Test dca-xll failed. Look at tests/data/fate/dca-xll.err for details.
>>> make: *** [fate-dca-xll] Error 1
>>> make: *** Waiting for unfinished jobs
>>
>> Was this run using foo86's decoder, or the current one? If the former then 
>> it's
>> not unexpected since the xll test was made for the current decoder, which is 
>> not
>> bitexact.
> 
> i just locally merged the branch and did a make -j12 fate

I see dca2 is above dca in allcodecs.c on this patch so i guess it's using the
former, which would explain why it fails.

> 
> 
>>
>> Ideally, once this decoder is committed replacing the current one we'd 
>> replace
>> the samples for the set available here: 
>> https://github.com/foo86/dcadec-samples
> 
> we can add new fate samples, we cannot replace fate samples
> replacing would break previous checkouts and releases
> not sure you actually meant to replace any ...

Yeah, forgot about that. Kinda makes for a bloated fate suit in the long run...

Since the lossy tests (fate-dca-core and fate-dts_es) apparently still work with
this decoder then there's no need to remove them, but the new extensions still
need the above sample suit.

> 
> [...]
> 
> 
> 
> ___
> 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] [RFC] avcodec: Add native DCA decoder based on libdcadec.

[Michael Niedermayer]

On Tue, Jan 05, 2016 at 11:44:04PM -0300, James Almer wrote:
> On 1/5/2016 11:35 PM, Michael Niedermayer wrote:
> > On Tue, Jan 05, 2016 at 11:27:25PM -0300, James Almer wrote:
> >> On 1/5/2016 11:21 PM, Michael Niedermayer wrote:
[...]
> > 
> > 
> >>
> >> Ideally, once this decoder is committed replacing the current one we'd 
> >> replace
> >> the samples for the set available here: 
> >> https://github.com/foo86/dcadec-samples
> > 
> > we can add new fate samples, we cannot replace fate samples
> > replacing would break previous checkouts and releases
> > not sure you actually meant to replace any ...
> 
> Yeah, forgot about that. Kinda makes for a bloated fate suit in the long 
> run...

this wouldnt happen if reference files where properly generated
that is with some reference decoder or taken from the input to a
reference encoder or from some referece test suite.

for subtitles or other where files are tiny that is a non issue though
...

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

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


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

Re: [FFmpeg-devel] [RFC] avcodec: Add native DCA decoder based on libdcadec.

[James Almer]

On 1/5/2016 11:51 PM, Michael Niedermayer wrote:
> On Tue, Jan 05, 2016 at 11:44:04PM -0300, James Almer wrote:
>> On 1/5/2016 11:35 PM, Michael Niedermayer wrote:
>>> On Tue, Jan 05, 2016 at 11:27:25PM -0300, James Almer wrote:
 On 1/5/2016 11:21 PM, Michael Niedermayer wrote:
> [...]
>>>
>>>

 Ideally, once this decoder is committed replacing the current one we'd 
 replace
 the samples for the set available here: 
 https://github.com/foo86/dcadec-samples
>>>
>>> we can add new fate samples, we cannot replace fate samples
>>> replacing would break previous checkouts and releases
>>> not sure you actually meant to replace any ...
>>
>> Yeah, forgot about that. Kinda makes for a bloated fate suit in the long 
>> run...
> 
> this wouldnt happen if reference files where properly generated
> that is with some reference decoder or taken from the input to a
> reference encoder or from some referece test suite.
> 
> for subtitles or other where files are tiny that is a non issue though
> ...

Well, whoever made the xll test knew it would probably become obsolete at some
point, since the decoder was not bitexact when the pcm reference was created.
At least i assume that's why the test fails with this new bitexact decoder.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] ffmpeg: replace log2 by faster variant

[Ganesh Ajjanagadde]

On Tue, Jan 5, 2016 at 6:44 PM, Michael Niedermayer
 wrote:
> On Mon, Jan 04, 2016 at 05:38:46PM -0800, Ganesh Ajjanagadde wrote:
>> On Sat, Jan 2, 2016 at 7:59 AM, Ganesh Ajjanagadde  wrote:
>> > On Sat, Jan 2, 2016 at 6:24 AM, Michael Niedermayer
>> >  wrote:
>> >> On Fri, Jan 01, 2016 at 05:55:31PM -0800, Ganesh Ajjanagadde wrote:
>> >>> On Wed, Dec 30, 2015 at 1:01 AM, Hendrik Leppkes  
>> >>> wrote:
>> >>> > On Wed, Dec 30, 2015 at 4:39 AM, Ganesh Ajjanagadde
>> >>> >  wrote:
>> >>> >> The log is anyway rounded to an integer, so one may use an frexp
>> >>> >> based approach. Note that this may be made frexpf; if arguments are 
>> >>> >> less than
>> >>> >> 2^24 there is no loss. Kept as double precision for simplicity; 2^32 
>> >>> >> is
>> >>> >> exactly representable as a double.
>> >>> >>
>> >>> >> Signed-off-by: Ganesh Ajjanagadde 
>> >>> >> ---
>> >>> >>  ffmpeg.c | 13 -
>> >>> >>  1 file changed, 12 insertions(+), 1 deletion(-)
>> >>> >>
>> >>> >> diff --git a/ffmpeg.c b/ffmpeg.c
>> >>> >> index 6d01987..ee72f91 100644
>> >>> >> --- a/ffmpeg.c
>> >>> >> +++ b/ffmpeg.c
>> >>> >> @@ -1486,6 +1486,17 @@ static void print_final_stats(int64_t 
>> >>> >> total_size)
>> >>> >>  }
>> >>> >>  }
>> >>> >>
>> >>> >> +static inline int log2i(double d)
>> >>> >> +{
>> >>> >> +int exp;
>> >>> >> +double mant;
>> >>> >> +
>> >>> >> +mant = frexp(d, );
>> >>> >> +if (mant >= M_SQRT1_2)
>> >>> >> +return exp;
>> >>> >> +return exp-1;
>> >>> >> +}
>> >>> >> +
>> >>> >>  static void print_report(int is_last_report, int64_t timer_start, 
>> >>> >> int64_t cur_time)
>> >>> >>  {
>> >>> >>  char buf[1024];
>> >>> >> @@ -1559,7 +1570,7 @@ static void print_report(int is_last_report, 
>> >>> >> int64_t timer_start, int64_t cur_ti
>> >>> >>  if (qp >= 0 && qp < FF_ARRAY_ELEMS(qp_histogram))
>> >>> >>  qp_histogram[qp]++;
>> >>> >>  for (j = 0; j < 32; j++)
>> >>> >> -snprintf(buf + strlen(buf), sizeof(buf) - 
>> >>> >> strlen(buf), "%X", (int)lrintf(log2(qp_histogram[j] + 1)));
>> >>> >> +snprintf(buf + strlen(buf), sizeof(buf) - 
>> >>> >> strlen(buf), "%X", log2i(qp_histogram[j] + 1));
>> >>> >>  }
>> >>> >>
>> >>> >>  if ((enc->flags & AV_CODEC_FLAG_PSNR) && (ost->pict_type 
>> >>> >> != AV_PICTURE_TYPE_NONE || is_last_report)) {
>> >>> >> --
>> >>> >> 2.6.4
>> >>> >
>> >>> > This isn't exactly a performance critical area, and defining a custom
>> >>> > function just for this seems somewhat like over-optimization.
>> >>> > Just my opinion, of course, I'll leave the decision up to the
>> >>> > maintainers of ffmpeg.c
>> >>>
>> >>> Ping: based on the above, final decision left to you, Michael.
>> >>
>> >> maybe av_log2() could be used, so as to avoid any increase in
>> >> complexity
>> >
>> > Not really, or at least not without additional code. The lrint rounds
>> > the log; av_log2 either returns the log of the next higher power of 2,
>> > or the one before it. Thus they are quite different things. I don't
>> > know if this is wrong or not; I created this to preserve identical
>> > behavior.
>>
>> @Michael: final thoughts?
>
> is there any loss of usefullness of the output with av_log2() ?
> i dont think we need to maintain exactly identical output for
> such fancy / debug output

AFAIK none, but I was not around when it was made, and I am not a
heavy user :). Replaced by av_log2 locally, will push tomorrow in
absence of further comments.

>
>
> [...]
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Why not whip the teacher when the pupil misbehaves? -- Diogenes of Sinope
>
> ___
> 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] lavc/cbrt_tablegen: speed up tablegen

[Ganesh Ajjanagadde]

On Tue, Jan 5, 2016 at 10:46 AM, Ganesh Ajjanagadde  wrote:
> On Tue, Jan 5, 2016 at 10:10 AM, Daniel Serpell  wrote:
>> Hi!,
>>
>> El Tue, Jan 05, 2016 at 08:08:35AM -0800, Ganesh Ajjanagadde escribio:
>>> On Tue, Jan 5, 2016 at 7:44 AM, Daniel Serpell  wrote:
>>> > Hi!,
>>> >
>>> > El Mon, Jan 04, 2016 at 06:33:59PM -0800, Ganesh Ajjanagadde escribio:
>>> >> This exploits an approach based on the sieve of Eratosthenes, a popular
>>> >> method for generating prime numbers.
>>> >>
>>> >> Tables are identical to previous ones.
>>> >>
>>> >> Tested with FATE with/without --enable-hardcoded-tables.
>>> >>
>>> >> Sample benchmark (Haswell, GNU/Linux+gcc):
>>> >> prev:
>>> >> 7860100 decicycles in cbrt_tableinit,   1 runs,  0 skips
>>> >> 490 decicycles in cbrt_tableinit,   2 runs,  0 skips
>>> >> [...]
>>> >> 7582339 decicycles in cbrt_tableinit, 256 runs,  0 skips
>>> >> 7563556 decicycles in cbrt_tableinit, 512 runs,  0 skips
>>> >>
>>> >> new:
>>> >> 2099480 decicycles in cbrt_tableinit,   1 runs,  0 skips
>>> >> 2044470 decicycles in cbrt_tableinit,   2 runs,  0 skips
>>> >> [...]
>>> >> 1796544 decicycles in cbrt_tableinit, 256 runs,  0 skips
>>> >> 1791631 decicycles in cbrt_tableinit, 512 runs,  0 skips
>>> >>
>>> >
>>> > See attached code, function "test1", based on an approximation of:
>>> >
>>> >   (i+1)^(1/3) ~= i^(1/3) * ( 1 + 1/(3i) - 1/(9i) + 5/(81i) -  )
[...]

So I looked up Mr. Fog's manuals, and it seems like divides are
considerably slower than multiplies and adds. This made me somewhat
skeptical of your idea, and so I benched it.
Seems like your patch is actually significantly slower on my setup
(gcc 5.3.0, GNU libm):
3349080 decicycles in cbrt_tableinit,   1 runs,  0 skips
3466605 decicycles in cbrt_tableinit,   2 runs,  0 skips
[...]
3425939 decicycles in cbrt_tableinit, 256 runs,  0 skips
3414528 decicycles in cbrt_tableinit, 512 runs,  0 skips

(clang 3.7.0):
3337590 decicycles in cbrt_tableinit,   1 runs,  0 skips
3276225 decicycles in cbrt_tableinit,   2 runs,  0 skips
[...]
2871065 decicycles in cbrt_tableinit, 256 runs,  0 skips
2832137 decicycles in cbrt_tableinit, 512 runs,  0 skips

The divides seem to be what is killing your approach.
Times (just for the divisions, clang):
1085430 decicycles in cbrt_tableinit,   1 runs,  0 skips
1005165 decicycles in cbrt_tableinit,   2 runs,  0 skips
[...]
 863732 decicycles in cbrt_tableinit, 256 runs,  0 skips
 864907 decicycles in cbrt_tableinit, 512 runs,  0 skips

Another illustration, even if I change the 1.0/(3*i) to simply i to
get rid of the multiplication as well, obviously not possible but
really wishful thinking, you still only get:
(clang)
2013390 decicycles in cbrt_tableinit,   1 runs,  0 skips
1950135 decicycles in cbrt_tableinit,   2 runs,  0 skips
[...]
1668963 decicycles in cbrt_tableinit, 256 runs,  0 skips
1653179 decicycles in cbrt_tableinit, 512 runs,  0 skips

To complete my curiousity,
(gcc)
1485240 decicycles in cbrt_tableinit,   1 runs,  0 skips
1522115 decicycles in cbrt_tableinit,   2 runs,  0 skips
[...]
1324325 decicycles in cbrt_tableinit, 256 runs,  0 skips
1322110 decicycles in cbrt_tableinit, 512 runs,  0 skips

i.e not a whole lot faster than the benchmark I posted.

If you feel this can't be right, I can do give a higher level
justification, which shows that for a reasonable libm cbrt, and
standard architectural assumptions, the approach I have is faster.

>
>>
>> Daniel.
>>
>> ___
>> 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

[FFmpeg-devel] [PATCH 10/10] libavcodec/ccaption_dec: extract ass time base into constant

[Aman Gupta]

From: Aman Gupta 

---
 libavcodec/ccaption_dec.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/libavcodec/ccaption_dec.c b/libavcodec/ccaption_dec.c
index 56b2b52..6dff761 100644
--- a/libavcodec/ccaption_dec.c
+++ b/libavcodec/ccaption_dec.c
@@ -30,6 +30,8 @@
 #define UNSET_FLAG(var, val) ( (var) &=  ~( 1 << (val)) )
 #define CHECK_FLAG(var, val) ( (var) &( 1 << (val)) )
 
+static const AVRational ass_tb = {1, 100};
+
 /*
  * TODO list
  * 1) handle font and color completely
@@ -557,8 +559,8 @@ static int decode(AVCodecContext *avctx, void *data, int 
*got_sub, AVPacket *avp
 
 if (ctx->calculate_duration) {
 if (ctx->prev_string) {
-int start_time = av_rescale_q(ctx->prev_time, 
avctx->time_base, (AVRational){ 1, 100 });
-int end_time = av_rescale_q(avpkt->pts, avctx->time_base, 
(AVRational){ 1, 100 });
+int start_time = av_rescale_q(ctx->prev_time, 
avctx->time_base, ass_tb);
+int end_time = av_rescale_q(avpkt->pts, avctx->time_base, 
ass_tb);
 ret = ff_ass_add_rect(sub, ctx->prev_string, start_time, 
end_time - start_time, 0);
 if (ret < 0)
 return ret;
@@ -570,7 +572,7 @@ static int decode(AVCodecContext *avctx, void *data, int 
*got_sub, AVPacket *avp
 ctx->prev_string = av_strdup(ctx->buffer.str);
 ctx->prev_time = avpkt->pts;
 } else {
-int start_time = av_rescale_q(avpkt->pts, avctx->time_base, 
(AVRational){ 1, 100 });
+int start_time = av_rescale_q(avpkt->pts, avctx->time_base, 
ass_tb);
 ret = ff_ass_add_rect_bprint(sub, >buffer, start_time, -1);
 if (ret < 0)
 return ret;
-- 
2.5.3

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

[FFmpeg-devel] [PATCH 07/10] libavcodec/ass.c: increase hardcoded timestamp for infinite duration

[Aman Gupta]

From: Aman Gupta 

before this change, ffmpeg would sometimes generate invalid ASS events
when dealing with mpegts streams that had a large start_time:

Dialogue: 0,20:57:07.37,9:59:59.99,Default,,0,0,0,,(upbeat music playing)
---
 libavcodec/ass.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/ass.c b/libavcodec/ass.c
index 336c308..0484db4 100644
--- a/libavcodec/ass.c
+++ b/libavcodec/ass.c
@@ -92,7 +92,7 @@ int ff_ass_subtitle_header_default(AVCodecContext *avctx)
 static void insert_ts(AVBPrint *buf, int ts)
 {
 if (ts == -1) {
-av_bprintf(buf, "9:59:59.99,");
+av_bprintf(buf, "999:59:59.99,");
 } else {
 int h, m, s;
 
-- 
2.5.3

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

[FFmpeg-devel] [PATCH 03/10] libavcodec/ccaption_dec: add calculate_duration option

[Aman Gupta]

From: Aman Gupta 

new option defaults to true, to preserve existing behavior. by flipping
the option to false, subtitle events are emitted as soon as they are
received and have an infinite duration.

this new mode is useful for realtime decoding of closed captions so they
can be display along with decoded mpeg2 frames.
---
 libavcodec/ccaption_dec.c | 19 ---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/libavcodec/ccaption_dec.c b/libavcodec/ccaption_dec.c
index f651c88..9f17e77 100644
--- a/libavcodec/ccaption_dec.c
+++ b/libavcodec/ccaption_dec.c
@@ -150,6 +150,7 @@ struct Screen {
 
 typedef struct CCaptionSubContext {
 AVClass *class;
+int calculate_duration;
 struct Screen screen[2];
 int active_screen;
 uint8_t cursor_row;
@@ -545,8 +546,12 @@ static int decode(AVCodecContext *avctx, void *data, int 
*got_sub, AVPacket *avp
 continue;
 else
 process_cc608(ctx, *(bptr + i + 1) & 0x7f, *(bptr + i + 2) & 0x7f);
-if (ctx->screen_changed)
-{
+
+if (!ctx->screen_changed)
+continue;
+ctx->screen_changed = 0;
+
+if (ctx->calculate_duration) {
 if (ctx->prev_string) {
 int start_time = av_rescale_q(ctx->prev_time, 
avctx->time_base, (AVRational){ 1, 100 });
 int end_time = av_rescale_q(avpkt->pts, avctx->time_base, 
(AVRational){ 1, 100 });
@@ -560,7 +565,12 @@ static int decode(AVCodecContext *avctx, void *data, int 
*got_sub, AVPacket *avp
 av_bprintf(>buffer, "\r\n");
 ctx->prev_string = av_strdup(ctx->buffer.str);
 ctx->prev_time = avpkt->pts;
-ctx->screen_changed = 0;
+} else {
+int start_time = av_rescale_q(avpkt->pts, avctx->time_base, 
(AVRational){ 1, 100 });
+ret = ff_ass_add_rect_bprint(sub, >buffer, start_time, -1);
+if (ret < 0)
+return ret;
+sub->pts = av_rescale_q(avpkt->pts, avctx->time_base, 
AV_TIME_BASE_Q);
 }
 }
 
@@ -568,7 +578,10 @@ static int decode(AVCodecContext *avctx, void *data, int 
*got_sub, AVPacket *avp
 return ret;
 }
 
+#define OFFSET(x) offsetof(CCaptionSubContext, x)
+#define SD AV_OPT_FLAG_SUBTITLE_PARAM | AV_OPT_FLAG_DECODING_PARAM
 static const AVOption options[] = {
+{ "calculate_duration", "Buffer closed captions to calculate durations.", 
OFFSET(calculate_duration), AV_OPT_TYPE_BOOL, { .i64 = 1 }, 0, 1, SD },
 {NULL}
 };
 
-- 
2.5.3

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

[FFmpeg-devel] [PATCH 08/10] libavcodec/ccaption_dec.c: fix more whitespace

[Aman Gupta]

From: Aman Gupta 

---
 libavcodec/ccaption_dec.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/libavcodec/ccaption_dec.c b/libavcodec/ccaption_dec.c
index 423c576..3de16bf 100644
--- a/libavcodec/ccaption_dec.c
+++ b/libavcodec/ccaption_dec.c
@@ -220,7 +220,7 @@ static int write_char(CCaptionSubContext *ctx, char *row, 
uint8_t col, char ch)
 return 0;
 }
 else {
-av_log(ctx, AV_LOG_WARNING,"Data Ignored since exceeding screen 
width\n");
+av_log(ctx, AV_LOG_WARNING, "Data Ignored since exceeding screen 
width\n");
 return AVERROR_INVALIDDATA;
 }
 }
@@ -323,7 +323,7 @@ static int reap_screen(CCaptionSubContext *ctx)
 
 for (i = 0; screen->row_used && i < SCREEN_ROWS; i++)
 {
-if (CHECK_FLAG(screen->row_used,i)) {
+if (CHECK_FLAG(screen->row_used, i)) {
 char *str = screen->characters[i];
 /* skip space */
 while (*str == ' ')
@@ -360,7 +360,7 @@ static void handle_textattr(CCaptionSubContext *ctx, 
uint8_t hi, uint8_t lo)
 ctx->cursor_color = pac2_attribs[i][0];
 ctx->cursor_font = pac2_attribs[i][1];
 
-SET_FLAG(screen->row_used,ctx->cursor_row);
+SET_FLAG(screen->row_used, ctx->cursor_row);
 ret = write_char(ctx, row, ctx->cursor_column, ' ');
 if (ret == 0)
 ctx->cursor_column++;
@@ -377,7 +377,7 @@ static void handle_pac(CCaptionSubContext *ctx, uint8_t hi, 
uint8_t lo)
 int indent, i, ret;
 
 if (row_map[index] <= 0) {
-av_log(ctx, AV_LOG_DEBUG,"Invalid pac index encountered\n");
+av_log(ctx, AV_LOG_DEBUG, "Invalid pac index encountered\n");
 return;
 }
 
@@ -534,7 +534,7 @@ static int decode(AVCodecContext *avctx, void *data, int 
*got_sub, AVPacket *avp
 if (ctx->pktbuf->size < len) {
 ret = av_buffer_realloc(>pktbuf, len);
  if (ret < 0) {
-av_log(ctx, AV_LOG_WARNING, "Insufficient Memory of %d truncated 
to %d\n",len, ctx->pktbuf->size);
+av_log(ctx, AV_LOG_WARNING, "Insufficient Memory of %d truncated 
to %d\n", len, ctx->pktbuf->size);
 len = ctx->pktbuf->size;
 ret = 0;
 }
-- 
2.5.3

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

[FFmpeg-devel] [PATCH 04/10] libavcodec/ccaption_dec: reap_screen after flipping on EOC

[Aman Gupta]

From: Aman Gupta 

---
 libavcodec/ccaption_dec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/ccaption_dec.c b/libavcodec/ccaption_dec.c
index 9f17e77..5d4c568 100644
--- a/libavcodec/ccaption_dec.c
+++ b/libavcodec/ccaption_dec.c
@@ -399,9 +399,9 @@ static void handle_erase(CCaptionSubContext *ctx, int 
n_screen)
 
 static void handle_eoc(CCaptionSubContext *ctx)
 {
-reap_screen(ctx);
 ctx->active_screen = !ctx->active_screen;
 ctx->cursor_column = 0;
+reap_screen(ctx);
 }
 
 static void handle_delete_end_of_row(CCaptionSubContext *ctx, char hi, char lo)
-- 
2.5.3

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

[FFmpeg-devel] [PATCH 05/10] libavcodec/ccaption_dec: combine ROLLUP modes as they are identical

[Aman Gupta]

From: Aman Gupta 

---
 libavcodec/ccaption_dec.c | 16 ++--
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/libavcodec/ccaption_dec.c b/libavcodec/ccaption_dec.c
index 5d4c568..424b434 100644
--- a/libavcodec/ccaption_dec.c
+++ b/libavcodec/ccaption_dec.c
@@ -37,9 +37,7 @@
 enum cc_mode {
 CCMODE_POPON,
 CCMODE_PAINTON,
-CCMODE_ROLLUP_2,
-CCMODE_ROLLUP_3,
-CCMODE_ROLLUP_4,
+CCMODE_ROLLUP,
 CCMODE_TEXT,
 };
 
@@ -176,7 +174,7 @@ static av_cold int init_decoder(AVCodecContext *avctx)
 
 av_bprint_init(>buffer, 0, AV_BPRINT_SIZE_UNLIMITED);
 /* taking by default roll up to 2 */
-ctx->mode = CCMODE_ROLLUP_2;
+ctx->mode = CCMODE_ROLLUP;
 ctx->rollup = 2;
 ret = ff_ass_subtitle_header_default(avctx);
 if (ret < 0) {
@@ -266,9 +264,7 @@ static struct Screen *get_writing_screen(CCaptionSubContext 
*ctx)
 // use Inactive screen
 return ctx->screen + !ctx->active_screen;
 case CCMODE_PAINTON:
-case CCMODE_ROLLUP_2:
-case CCMODE_ROLLUP_3:
-case CCMODE_ROLLUP_4:
+case CCMODE_ROLLUP:
 case CCMODE_TEXT:
 // use active screen
 return ctx->screen + ctx->active_screen;
@@ -460,15 +456,15 @@ static void process_cc608(CCaptionSubContext *ctx, 
uint8_t hi, uint8_t lo)
 break;
 case 0x25:
 ctx->rollup = 2;
-ctx->mode = CCMODE_ROLLUP_2;
+ctx->mode = CCMODE_ROLLUP;
 break;
 case 0x26:
 ctx->rollup = 3;
-ctx->mode = CCMODE_ROLLUP_3;
+ctx->mode = CCMODE_ROLLUP;
 break;
 case 0x27:
 ctx->rollup = 4;
-ctx->mode = CCMODE_ROLLUP_4;
+ctx->mode = CCMODE_ROLLUP;
 break;
 case 0x29:
 /* resume direct captioning */
-- 
2.5.3

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

[FFmpeg-devel] [PATCH 02/10] libavcodec/ccaption_dec: fix timestamps on generated subtitles by buffering after EOC/newline commands

[Aman Gupta]

From: Aman Gupta 

before:

  Dialogue: 0,0:00:00.03,0:00:00.67,Default,,0,0,0,,EVERYBODY, TONIGHT WE'LL 
COVER
  Dialogue: 0,0:00:00.67,0:00:02.30,Default,,0,0,0,,EVERYBODY, TONIGHT WE'LL 
COVER\NTHE NEWS WE MISSED OVER THE
  Dialogue: 0,0:00:02.30,0:00:03.57,Default,,0,0,0,,THE NEWS WE MISSED OVER 
THE\NHOLIDAYS AND SEE HOW THE

after:

  Dialogue: 0,0:00:00.67,0:00:02.30,Default,,0,0,0,,EVERYBODY, TONIGHT WE'LL 
COVER
  Dialogue: 0,0:00:02.30,0:00:03.57,Default,,0,0,0,,EVERYBODY, TONIGHT WE'LL 
COVER\NTHE NEWS WE MISSED OVER THE
  Dialogue: 0,0:00:03.57,0:00:04.30,Default,,0,0,0,,THE NEWS WE MISSED OVER 
THE\NHOLIDAYS AND SEE HOW THE
---
 libavcodec/ccaption_dec.c | 47 ---
 1 file changed, 24 insertions(+), 23 deletions(-)

diff --git a/libavcodec/ccaption_dec.c b/libavcodec/ccaption_dec.c
index 12e8f1d..f651c88 100644
--- a/libavcodec/ccaption_dec.c
+++ b/libavcodec/ccaption_dec.c
@@ -160,10 +160,8 @@ typedef struct CCaptionSubContext {
 int screen_changed;
 int rollup;
 enum cc_mode mode;
-int64_t start_time;
-/* visible screen time */
-int64_t startv_time;
-int64_t end_time;
+char *prev_string;
+int64_t prev_time;
 char prev_cmd[2];
 /* buffer to store pkt data */
 AVBufferRef *pktbuf;
@@ -310,12 +308,11 @@ static void roll_up(CCaptionSubContext *ctx)
 UNSET_FLAG(screen->row_used, ctx->cursor_row);
 }
 
-static int reap_screen(CCaptionSubContext *ctx, int64_t pts)
+static int reap_screen(CCaptionSubContext *ctx)
 {
 int i;
 int ret = 0;
 struct Screen *screen = ctx->screen + ctx->active_screen;
-ctx->start_time = ctx->startv_time;
 av_bprint_clear(>buffer);
 
 for (i = 0; screen->row_used && i < SCREEN_ROWS; i++)
@@ -341,8 +338,6 @@ static int reap_screen(CCaptionSubContext *ctx, int64_t pts)
 ctx->screen_changed = 1;
 }
 
-ctx->startv_time = pts;
-ctx->end_time = pts;
 return ret;
 }
 
@@ -401,9 +396,9 @@ static void handle_erase(CCaptionSubContext *ctx, int 
n_screen)
 screen->row_used = 0;
 }
 
-static void handle_eoc(CCaptionSubContext *ctx, int64_t pts)
+static void handle_eoc(CCaptionSubContext *ctx)
 {
-reap_screen(ctx, pts);
+reap_screen(ctx);
 ctx->active_screen = !ctx->active_screen;
 ctx->cursor_column = 0;
 }
@@ -415,7 +410,7 @@ static void handle_delete_end_of_row(CCaptionSubContext 
*ctx, char hi, char lo)
 write_char(ctx, row, ctx->cursor_column, 0);
 }
 
-static void handle_char(CCaptionSubContext *ctx, char hi, char lo, int64_t pts)
+static void handle_char(CCaptionSubContext *ctx, char hi, char lo)
 {
 struct Screen *screen = get_writing_screen(ctx);
 char *row = screen->characters[ctx->cursor_row];
@@ -443,7 +438,7 @@ static void handle_char(CCaptionSubContext *ctx, char hi, 
char lo, int64_t pts)
ff_dlog(ctx, "(%c)\n", hi);
 }
 
-static void process_cc608(CCaptionSubContext *ctx, int64_t pts, uint8_t hi, 
uint8_t lo)
+static void process_cc608(CCaptionSubContext *ctx, uint8_t hi, uint8_t lo)
 {
 if (hi == ctx->prev_cmd[0] && lo == ctx->prev_cmd[1]) {
 /* ignore redundant command */
@@ -489,7 +484,7 @@ static void process_cc608(CCaptionSubContext *ctx, int64_t 
pts, uint8_t hi, uint
 case 0x2d:
 /* carriage return */
 ff_dlog(ctx, "carriage return\n");
-reap_screen(ctx, pts);
+reap_screen(ctx);
 roll_up(ctx);
 ctx->cursor_column = 0;
 break;
@@ -500,7 +495,7 @@ static void process_cc608(CCaptionSubContext *ctx, int64_t 
pts, uint8_t hi, uint
 case 0x2f:
 /* end of caption */
 ff_dlog(ctx, "handle_eoc\n");
-handle_eoc(ctx, pts);
+handle_eoc(ctx);
 break;
 default:
 ff_dlog(ctx, "Unknown command 0x%hhx 0x%hhx\n", hi, lo);
@@ -510,7 +505,7 @@ static void process_cc608(CCaptionSubContext *ctx, int64_t 
pts, uint8_t hi, uint
 /* ignore tab offset */
 } else if (hi >= 0x20) {
 /* Standard characters (always in pairs) */
-handle_char(ctx, hi, lo, pts);
+handle_char(ctx, hi, lo);
 } else {
 /* Ignoring all other non data code */
 ff_dlog(ctx, "Unknown command 0x%hhx 0x%hhx\n", hi, lo);
@@ -549,16 +544,22 @@ static int decode(AVCodecContext *avctx, void *data, int 
*got_sub, AVPacket *avp
 if(cc_type == 1)
 continue;
 else
-process_cc608(ctx, avpkt->pts, *(bptr + i + 1) & 0x7f, *(bptr + i 
+ 2) & 0x7f);
+process_cc608(ctx, *(bptr + i + 1) & 0x7f, *(bptr + i + 2) & 0x7f);
 if (ctx->screen_changed)
 {
-int start_time = av_rescale_q(ctx->start_time, avctx->time_base, 
(AVRational){ 1, 100 });
-int end_time = av_rescale_q(ctx->end_time, avctx->time_base, 
(AVRational){ 1, 100 });
-ff_dlog(ctx, "cdp writing data (%s)\n",ctx->buffer.str);
-ret 

[FFmpeg-devel] [PATCH 06/10] libavcodec/ccaption_dec: flush screen buffer on seek

[Aman Gupta]

From: Aman Gupta 

---
 libavcodec/ccaption_dec.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/libavcodec/ccaption_dec.c b/libavcodec/ccaption_dec.c
index 424b434..423c576 100644
--- a/libavcodec/ccaption_dec.c
+++ b/libavcodec/ccaption_dec.c
@@ -196,6 +196,15 @@ static av_cold int close_decoder(AVCodecContext *avctx)
 return 0;
 }
 
+static void flush_decoder(AVCodecContext *avctx)
+{
+CCaptionSubContext *ctx = avctx->priv_data;
+ctx->screen[0].row_used = 0;
+ctx->screen[1].row_used = 0;
+av_bprint_clear(>buffer);
+ctx->screen_changed = 1;
+}
+
 /**
  * @param ctx closed caption context just to print log
  */
@@ -596,6 +605,7 @@ AVCodec ff_ccaption_decoder = {
 .priv_data_size = sizeof(CCaptionSubContext),
 .init   = init_decoder,
 .close  = close_decoder,
+.flush  = flush_decoder,
 .decode = decode,
 .priv_class = _dec_class,
 };
-- 
2.5.3

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

[FFmpeg-devel] [PATCH 09/10] libavcodec/ccaption_dec: default to simpler POPON mode

[Aman Gupta]

From: Aman Gupta 

the default value does not really have any effect in normal operation,
as the mode command is repeated before every new caption. after seeking,
it makes more sense to default to POPON since that is the more common
mode.
---
 libavcodec/ccaption_dec.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/libavcodec/ccaption_dec.c b/libavcodec/ccaption_dec.c
index 3de16bf..56b2b52 100644
--- a/libavcodec/ccaption_dec.c
+++ b/libavcodec/ccaption_dec.c
@@ -173,9 +173,7 @@ static av_cold int init_decoder(AVCodecContext *avctx)
 CCaptionSubContext *ctx = avctx->priv_data;
 
 av_bprint_init(>buffer, 0, AV_BPRINT_SIZE_UNLIMITED);
-/* taking by default roll up to 2 */
-ctx->mode = CCMODE_ROLLUP;
-ctx->rollup = 2;
+ctx->mode = CCMODE_POPON;
 ret = ff_ass_subtitle_header_default(avctx);
 if (ret < 0) {
 return ret;
@@ -203,6 +201,7 @@ static void flush_decoder(AVCodecContext *avctx)
 ctx->screen[1].row_used = 0;
 av_bprint_clear(>buffer);
 ctx->screen_changed = 1;
+ctx->mode = CCMODE_POPON;
 }
 
 /**
-- 
2.5.3

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

[FFmpeg-devel] [PATCH v5] lavf/matroskadec: A_QUICKTIME and AV_CODEC_ID_NONE

[Mats Peterson]

In many older QuickTime files, the audio format, or "fourcc", is
0x (AV_CODEC_ID_NONE). The QuickTime File Format Specification
states the following regarding this situation:

"This format descriptor should not be used, but may be found in some
files. Samples are assumed to be stored in either 'raw ' or 'twos'
format, depending on the sample size field in the sound description."

MPlayer handles this logic by itself, but FFmpeg/FFplay currently does
not.

Mats

--
Mats Peterson
http://matsp888.no-ip.org/~mats/
>From 9adba279d3ca9dada9774ff204e7fb4a255bfb9e Mon Sep 17 00:00:00 2001
From: Mats Peterson 
Date: Wed, 6 Jan 2016 04:13:47 +0100
Subject: [PATCH v5] lavf/matroskadec: A_QUICKTIME and AV_CODEC_ID_NONE

In many older QuickTime files, the audio format, or "fourcc", is
0x (AV_CODEC_ID_NONE). The QuickTime File Format Specification
states the following regarding this situation:

"This format descriptor should not be used, but may be found in some
files. Samples are assumed to be stored in either 'raw ' or 'twos'
format, depending on the sample size field in the sound description."

MPlayer handles this logic by itself, but FFmpeg/FFplay currently does
not.

Mats

---
 libavformat/matroskadec.c |9 +
 1 file changed, 9 insertions(+)

diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index 9de7cfb..440c9f9 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -1869,6 +1869,15 @@ static int matroska_parse_tracks(AVFormatContext *s)
 fourcc = AV_RL32(track->codec_priv.data);
 codec_id = ff_codec_get_id(ff_codec_movaudio_tags, fourcc);
 }
+if (codec_id == AV_CODEC_ID_NONE) {
+if (track->audio.bitdepth == 8) {
+fourcc = MKTAG('r','a','w',' ');
+codec_id = ff_codec_get_id(ff_codec_movaudio_tags, fourcc);
+} else if (track->audio.bitdepth == 16) {
+fourcc = MKTAG('t','w','o','s');
+codec_id = ff_codec_get_id(ff_codec_movaudio_tags, fourcc);
+}
+}
 } else if (!strcmp(track->codec_id, "V_QUICKTIME") &&
(track->codec_priv.size >= 21)  &&
(track->codec_priv.data)) {
-- 
1.7.10.4

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

[FFmpeg-devel] [PATCH] lavf/matroskadec: SMI to SVQ3

[Mats Peterson]

At least in this case, MPlayer seems to look at the codec tag rather
than the codec ID in order to determine the codec. Therefore, the
'fourcc' variable needs to be set to 'SVQ3' as well, which is later
copied to st->codec->codec_tag in matroskadec.c.

Mats

--
Mats Peterson
http://matsp888.no-ip.org/~mats/
>From 5a515ebc38a821e9312c4b7a62db210cf66488a2 Mon Sep 17 00:00:00 2001
From: Mats Peterson 
Date: Wed, 6 Jan 2016 04:16:32 +0100
Subject: [PATCH] lavf/matroskadec: SMI to SVQ3

At least in this case, MPlayer seems to look at the codec tag rather
than the codec ID in order to determine the codec. Therefore, the
'fourcc' variable needs to be set to 'SVQ3' as well, which is later
copied to st->codec->codec_tag in matroskadec.c.

Mats

---
 libavformat/matroskadec.c |6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index 440c9f9..d9d87cc 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -1887,8 +1887,10 @@ static int matroska_parse_tracks(AVFormatContext *s)
 fourcc   = AV_RL32(track->codec_priv.data);
 codec_id = ff_codec_get_id(ff_codec_movvideo_tags, fourcc);
 }
-if (codec_id == AV_CODEC_ID_NONE && AV_RL32(track->codec_priv.data+4) == AV_RL32("SMI "))
-codec_id = AV_CODEC_ID_SVQ3;
+if (codec_id == AV_CODEC_ID_NONE && AV_RL32(track->codec_priv.data+4) == AV_RL32("SMI ")) {
+fourcc = MKTAG('S','V','Q','3');
+codec_id = ff_codec_get_id(ff_codec_movvideo_tags, fourcc);
+}
 if (codec_id == AV_CODEC_ID_NONE) {
 char buf[32];
 av_get_codec_tag_string(buf, sizeof(buf), fourcc);
-- 
1.7.10.4

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

Re: [FFmpeg-devel] [PATCH] ffmpeg: replace log2 by faster variant

[Michael Niedermayer]

On Mon, Jan 04, 2016 at 05:38:46PM -0800, Ganesh Ajjanagadde wrote:
> On Sat, Jan 2, 2016 at 7:59 AM, Ganesh Ajjanagadde  wrote:
> > On Sat, Jan 2, 2016 at 6:24 AM, Michael Niedermayer
> >  wrote:
> >> On Fri, Jan 01, 2016 at 05:55:31PM -0800, Ganesh Ajjanagadde wrote:
> >>> On Wed, Dec 30, 2015 at 1:01 AM, Hendrik Leppkes  
> >>> wrote:
> >>> > On Wed, Dec 30, 2015 at 4:39 AM, Ganesh Ajjanagadde
> >>> >  wrote:
> >>> >> The log is anyway rounded to an integer, so one may use an frexp
> >>> >> based approach. Note that this may be made frexpf; if arguments are 
> >>> >> less than
> >>> >> 2^24 there is no loss. Kept as double precision for simplicity; 2^32 is
> >>> >> exactly representable as a double.
> >>> >>
> >>> >> Signed-off-by: Ganesh Ajjanagadde 
> >>> >> ---
> >>> >>  ffmpeg.c | 13 -
> >>> >>  1 file changed, 12 insertions(+), 1 deletion(-)
> >>> >>
> >>> >> diff --git a/ffmpeg.c b/ffmpeg.c
> >>> >> index 6d01987..ee72f91 100644
> >>> >> --- a/ffmpeg.c
> >>> >> +++ b/ffmpeg.c
> >>> >> @@ -1486,6 +1486,17 @@ static void print_final_stats(int64_t 
> >>> >> total_size)
> >>> >>  }
> >>> >>  }
> >>> >>
> >>> >> +static inline int log2i(double d)
> >>> >> +{
> >>> >> +int exp;
> >>> >> +double mant;
> >>> >> +
> >>> >> +mant = frexp(d, );
> >>> >> +if (mant >= M_SQRT1_2)
> >>> >> +return exp;
> >>> >> +return exp-1;
> >>> >> +}
> >>> >> +
> >>> >>  static void print_report(int is_last_report, int64_t timer_start, 
> >>> >> int64_t cur_time)
> >>> >>  {
> >>> >>  char buf[1024];
> >>> >> @@ -1559,7 +1570,7 @@ static void print_report(int is_last_report, 
> >>> >> int64_t timer_start, int64_t cur_ti
> >>> >>  if (qp >= 0 && qp < FF_ARRAY_ELEMS(qp_histogram))
> >>> >>  qp_histogram[qp]++;
> >>> >>  for (j = 0; j < 32; j++)
> >>> >> -snprintf(buf + strlen(buf), sizeof(buf) - 
> >>> >> strlen(buf), "%X", (int)lrintf(log2(qp_histogram[j] + 1)));
> >>> >> +snprintf(buf + strlen(buf), sizeof(buf) - 
> >>> >> strlen(buf), "%X", log2i(qp_histogram[j] + 1));
> >>> >>  }
> >>> >>
> >>> >>  if ((enc->flags & AV_CODEC_FLAG_PSNR) && (ost->pict_type 
> >>> >> != AV_PICTURE_TYPE_NONE || is_last_report)) {
> >>> >> --
> >>> >> 2.6.4
> >>> >
> >>> > This isn't exactly a performance critical area, and defining a custom
> >>> > function just for this seems somewhat like over-optimization.
> >>> > Just my opinion, of course, I'll leave the decision up to the
> >>> > maintainers of ffmpeg.c
> >>>
> >>> Ping: based on the above, final decision left to you, Michael.
> >>
> >> maybe av_log2() could be used, so as to avoid any increase in
> >> complexity
> >
> > Not really, or at least not without additional code. The lrint rounds
> > the log; av_log2 either returns the log of the next higher power of 2,
> > or the one before it. Thus they are quite different things. I don't
> > know if this is wrong or not; I created this to preserve identical
> > behavior.
> 
> @Michael: final thoughts?

is there any loss of usefullness of the output with av_log2() ?
i dont think we need to maintain exactly identical output for
such fancy / debug output


[...]
-- 
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] lavfi/af_compensationdelay: replace pow(x, 0.5) by sqrt(x)

[Ganesh Ajjanagadde]

On Tue, Jan 5, 2016 at 2:40 AM, Paul B Mahol  wrote:
> On 1/5/16, Ganesh Ajjanagadde  wrote:
>> sqrt is faster, and is sometimes more accurate depending on the libm.
>>
>> Signed-off-by: Ganesh Ajjanagadde 
>> ---
>>  libavfilter/af_compensationdelay.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/libavfilter/af_compensationdelay.c
>> b/libavfilter/af_compensationdelay.c
>> index 33ee7e4..d5a3484 100644
>> --- a/libavfilter/af_compensationdelay.c
>> +++ b/libavfilter/af_compensationdelay.c
>> @@ -57,7 +57,7 @@ AVFILTER_DEFINE_CLASS(compensationdelay);
>>  // The maximum distance for options
>>  #define COMP_DELAY_MAX_DISTANCE(100.0 * 100.0 + 100.0 * 1.0 +
>> 1.0)
>>  // The actual speed of sound in normal conditions
>> -#define COMP_DELAY_SOUND_SPEED_KM_H(temp)  1.85325 * (643.95 * pow(((temp +
>> 273.15) / 273.15), 0.5))
>> +#define COMP_DELAY_SOUND_SPEED_KM_H(temp)  1.85325 * (643.95 * sqrt(((temp
>> + 273.15) / 273.15)))
>>  #define COMP_DELAY_SOUND_SPEED_CM_S(temp)
>> (COMP_DELAY_SOUND_SPEED_KM_H(temp) * (1000.0 * 100.0) /* cm/km */ / (60.0 *
>> 60.0) /* s/h */)
>>  #define COMP_DELAY_SOUND_FRONT_DELAY(temp) (1.0 /
>> COMP_DELAY_SOUND_SPEED_CM_S(temp))
>>  // The maximum delay may be reached by this filter
>> --
>> 2.6.4
>>
>> ___
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>
> ok

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

Re: [FFmpeg-devel] [PATCH] ffmpeg: check return value of avio_closep for progress report

[Ganesh Ajjanagadde]

On Tue, Jan 5, 2016 at 9:24 AM, James Almer  wrote:
> On 1/5/2016 12:39 AM, Ganesh Ajjanagadde wrote:
>> avio_closep is not guaranteed to succeed, and its return value can
>> contain information regarding failure of preceding writes and silent
>> loss of data (man 2 close, man fclose). Users should know when the
>> progress was not successfully logged, and so a diagnostic is printed
>> here.
>>
>> The reason only one of these is changed in the patch is to get feedback:
>> a quick glance shows a large number of unchecked avio_close operations.
>> It may be tedious to check all of them, and relative importance varies
>> across instances. This was one which I feel is important.
>>
>> Signed-off-by: Ganesh Ajjanagadde 
>> ---
>>  ffmpeg.c | 5 -
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/ffmpeg.c b/ffmpeg.c
>> index ee72f91..81f3697 100644
>> --- a/ffmpeg.c
>> +++ b/ffmpeg.c
>> @@ -1512,6 +1512,7 @@ static void print_report(int is_last_report, int64_t 
>> timer_start, int64_t cur_ti
>>  static int64_t last_time = -1;
>>  static int qp_histogram[52];
>>  int hours, mins, secs, us;
>> +int ret;
>>  float t;
>>
>>  if (!print_stats && !is_last_report && !progress_avio)
>> @@ -1678,7 +1679,9 @@ static void print_report(int is_last_report, int64_t 
>> timer_start, int64_t cur_ti
>>  avio_flush(progress_avio);
>>  av_bprint_finalize(_script, NULL);
>>  if (is_last_report) {
>> -avio_closep(_avio);
>> +if ((ret = avio_closep(_avio)) < 0)
>> +av_log(NULL, AV_LOG_ERROR,
>> +   "not possible to close progress log, loss of 
>> information possible: %s\n", av_err2str(ret));
>
> "Unable to close progress log" or "Error closing progress log" sounds better 
> IMO.

Reworded and pushed, thanks. Now comes the tedious aspect: auditing
other such instances.

> ___
> 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] decklink: support all valid numbers of audio channels

[Michael Niedermayer]

On Tue, Jan 05, 2016 at 04:38:06PM +0100, Matthias Hunstock wrote:
> Am 20.12.2015 um 12:57 schrieb Matthias Hunstock:
> > As it is already written in the documentation, BMD DeckLink cards
> > are capable of capturing 2, 8 or 16 audio channels (for SDI Inputs).
> > Currently the value is hardcoded to 2. Introduces new option.
> 
> Any more opinions on this?
> 
> I'd like to add several options to make the decklink support more
> feature complete, since it is very widespread hardware.

> But I can save
> some effort if it is not accepted anyway.

any patch that was forgotten and should be applied ?
just asking as your comment sounds a bit in that direction


> 
> Things on my personal roadmap:
> 
> - change order of streams (currently :0 is audio and :1 is video)
> - introduce options to select video and audio input, e.g. the new
> low-cost cards have SDI and HDMI inputs
> - port those changes to decklink output code
> - more stuff I don't remember right now
> 
> Cheers
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

When you are offended at any man's fault, turn to yourself and study your
own failings. Then you will forget your anger. -- Epictetus


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

Re: [FFmpeg-devel] [RESEND][PATCH] Fix buffer allocation in pad filter

[Andrey Turkin]

Sure; no-copy padding was broken for some scenarios (namely, left-padding
when target height divides evenly by 32). Lets say I want to use following
filter string: "scale=720x576, pad=880:576:80:0" (scale to 720x576 and add
some black bands on the sides). Pad should be able to allocate buffer for
scale and then just fill remaining area; yet it fails to do so and "Direct
padding impossible allocating new frame" message can be seen in logs (when
log level set to verbose). In this particular case the allocated buffer is
big enough to hold 896x576 image and a few extra bytes; scale filter then
operates on its view of the buffer (720x576 but with pitch of 896).
Interestingly, last row of that view doesn't fit in the buffer (there's
only 720 bytes + a bit of padding and not 880 bytes). So scale does its
thing and then pad checks if there is enough extra space in the buffer for
the padding. There's not; in fact there's no room in the buffer to hold the
view itself - so pad gives up and does another allocation and a copy. So,
basically this patch fixes that.

2016-01-06 0:59 GMT+03:00 Paul B Mahol :

> On 1/5/16, Andrey Turkin  wrote:
> >
> > One extra line must be allocated when adding left padding to make sure
> > that last line in every plane fits in the allocated buffer fully
> > ---
> >  libavfilter/vf_pad.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >
>
> Do you have example this patch fixes?
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [RFC] avcodec: Add native DCA decoder based on libdcadec.

[Andreas Cadhalpun]

On 03.01.2016 18:49, foo86 wrote:
> +// 5.3.1 - Bit stream header
> +static int parse_frame_header(DCA2CoreDecoder *s)
> +{
[...]
> +// Source PCM resolution
> +s->source_pcm_res = ff_dca_bits_per_sample[pcmr_index = get_bits(>gb, 
> 3)];

This can cause an out-of-bounds read if get_bits returns 7, because 
ff_dca_bits_per_sample
only has 7 elements.

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

Re: [FFmpeg-devel] [PATCH] avfilter: add ahistogram multimedia filter

[Moritz Barsnick]

On Tue, Jan 05, 2016 at 21:19:12 +0100, Paul B Mahol wrote:
> +It accepts the following values:
> +@table @samp
> +@item replace
> +logarithmic

   ^^^ This description is a copy/paste error.

> +@item pheight
> +Set histogram percentage of window height.
[...]
> +{ "pheight", "set histogram percentage of window height", 
> OFFSET(phisto), AV_OPT_TYPE_FLOAT, {.dbl=0.10}, 0, 1, FLAGS },

This is not the first of your filters where you describe a parameter as
a "percentage", but expect a float value [0, 1]. That is extremely
confusing, if not even wrong. To me, a percentage covers [0, 100]. You
need to find a different term for the "factor" ranging from zero to
one.

I left the rest of the code functionally unreviewed so far. I will test
later.

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

Re: [FFmpeg-devel] [PATCH]lavc/opus: Use original sample rate for decoding

[Carl Eugen Hoyos]

Hendrik Leppkes  gmail.com> writes:

> > Attached patch intends to match what opusdec does, 
> > sorry if I miss something.

> I dont think its as simple as this. Only the SILK 
> parts of the audio are automatically resampled, the 
> CELT parts are always decoded to 48kHz. Just changing 
> the samplerate would then cause corrupted audio.

Thank you for the explanation!

Carl Eugen

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

Re: [FFmpeg-devel] [PATCH] avpacket: fix size check in packet_alloc

[Michael Niedermayer]

On Tue, Jan 05, 2016 at 01:05:50PM +0100, Andreas Cadhalpun wrote:
> The previous check only caught sizes from -AV_INPUT_BUFFER_PADDING_SIZE
> to -1.
> 
> This fixes ubsan runtime error: signed integer overflow: 2147483647 + 32
> cannot be represented in type 'int'
> 
> Signed-off-by: Andreas Cadhalpun 
> ---
>  libavcodec/avpacket.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

LGTM

thx

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

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


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

[FFmpeg-devel] [RESEND][PATCH] Fix buffer allocation in pad filter

[Andrey Turkin]

One extra line must be allocated when adding left padding to make sure
that last line in every plane fits in the allocated buffer fully
---
 libavfilter/vf_pad.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavfilter/vf_pad.c b/libavfilter/vf_pad.c
index d94ced1..555b318 100644
--- a/libavfilter/vf_pad.c
+++ b/libavfilter/vf_pad.c
@@ -203,7 +203,7 @@ static AVFrame *get_video_buffer(AVFilterLink *inlink, int w, int h)
 
 AVFrame *frame = ff_get_video_buffer(inlink->dst->outputs[0],
  w + (s->w - s->in_w),
- h + (s->h - s->in_h));
+ h + (s->h - s->in_h) + (s->x ? 1 : 0));
 int plane;
 
 if (!frame)
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavfi/af_compensationdelay: replace pow(x, 0.5) by sqrt(x)

[Paul B Mahol]

On 1/5/16, Ganesh Ajjanagadde  wrote:
> sqrt is faster, and is sometimes more accurate depending on the libm.
>
> Signed-off-by: Ganesh Ajjanagadde 
> ---
>  libavfilter/af_compensationdelay.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavfilter/af_compensationdelay.c
> b/libavfilter/af_compensationdelay.c
> index 33ee7e4..d5a3484 100644
> --- a/libavfilter/af_compensationdelay.c
> +++ b/libavfilter/af_compensationdelay.c
> @@ -57,7 +57,7 @@ AVFILTER_DEFINE_CLASS(compensationdelay);
>  // The maximum distance for options
>  #define COMP_DELAY_MAX_DISTANCE(100.0 * 100.0 + 100.0 * 1.0 +
> 1.0)
>  // The actual speed of sound in normal conditions
> -#define COMP_DELAY_SOUND_SPEED_KM_H(temp)  1.85325 * (643.95 * pow(((temp +
> 273.15) / 273.15), 0.5))
> +#define COMP_DELAY_SOUND_SPEED_KM_H(temp)  1.85325 * (643.95 * sqrt(((temp
> + 273.15) / 273.15)))
>  #define COMP_DELAY_SOUND_SPEED_CM_S(temp)
> (COMP_DELAY_SOUND_SPEED_KM_H(temp) * (1000.0 * 100.0) /* cm/km */ / (60.0 *
> 60.0) /* s/h */)
>  #define COMP_DELAY_SOUND_FRONT_DELAY(temp) (1.0 /
> COMP_DELAY_SOUND_SPEED_CM_S(temp))
>  // The maximum delay may be reached by this filter
> --
> 2.6.4
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

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

Re: [FFmpeg-devel] [PATCH] parser: error out if the codec doesn't match

[Andreas Cadhalpun]

On 05.01.2016 11:46, Michael Niedermayer wrote:
> On Tue, Jan 05, 2016 at 12:44:40AM +0100, Andreas Cadhalpun wrote:
>> Otherwise this can have some surprising effects (crashes), so let's
>> better not allow it.
>>
>> Signed-off-by: Andreas Cadhalpun 
>> ---
>>  libavcodec/parser.c | 11 +++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/libavcodec/parser.c b/libavcodec/parser.c
>> index 2809158..1f38edb 100644
>> --- a/libavcodec/parser.c
>> +++ b/libavcodec/parser.c
>> @@ -141,6 +141,17 @@ int av_parser_parse2(AVCodecParserContext *s, 
>> AVCodecContext *avctx,
>>  int index, i;
>>  uint8_t dummy_buf[AV_INPUT_BUFFER_PADDING_SIZE];
>>  
>> +if (avctx->codec_id != s->parser->codec_ids[0] &&
>> +avctx->codec_id != s->parser->codec_ids[1] &&
>> +avctx->codec_id != s->parser->codec_ids[2] &&
>> +avctx->codec_id != s->parser->codec_ids[3] &&
>> +avctx->codec_id != s->parser->codec_ids[4]) {
>> +av_log(avctx, AV_LOG_ERROR,
>> +   "The parser doesn't match the codec %s.\n",
>> +   avcodec_get_name(avctx->codec_id));
>> +return buf_size;
>> +}
> 
> does it also work to check if a parser is set when the codec id
> is changed ? as in below

That wouldn't work, as the codec id wasn't changed in force_codec_ids,
but in the API using program.
To reiterate, the problematic steps were:
 * call avformat_find_stream_info, which detects a codec and initializes
   a parser for it
 * afterwards change the codec id in the API using program, so it
   doesn't match with the parser

Thus I think the only reliable way to detect this is a check in 
av_parser_parse2.

> (that would avoid doing 5 extra checks per packet)

Parsers aren't that speed critical code, I think.

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

Re: [FFmpeg-devel] arm neon float instruction crash the ffmpeg

[Carl Eugen Hoyos]

zhuhb  qiyi.com> writes:

> I managed to compile/build the ffmpeg for the iOS 
> arm32 version. But it crashed

Please consider reading https://ffmpeg.org/contact.html 
and https://ffmpeg.org/bugreports.html (again).

Carl Eugen

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

Re: [FFmpeg-devel] [PATCH] parser: error out if the codec doesn't match

[Michael Niedermayer]

On Tue, Jan 05, 2016 at 12:44:40AM +0100, Andreas Cadhalpun wrote:
> Otherwise this can have some surprising effects (crashes), so let's
> better not allow it.
> 
> Signed-off-by: Andreas Cadhalpun 
> ---
>  libavcodec/parser.c | 11 +++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/libavcodec/parser.c b/libavcodec/parser.c
> index 2809158..1f38edb 100644
> --- a/libavcodec/parser.c
> +++ b/libavcodec/parser.c
> @@ -141,6 +141,17 @@ int av_parser_parse2(AVCodecParserContext *s, 
> AVCodecContext *avctx,
>  int index, i;
>  uint8_t dummy_buf[AV_INPUT_BUFFER_PADDING_SIZE];
>  
> +if (avctx->codec_id != s->parser->codec_ids[0] &&
> +avctx->codec_id != s->parser->codec_ids[1] &&
> +avctx->codec_id != s->parser->codec_ids[2] &&
> +avctx->codec_id != s->parser->codec_ids[3] &&
> +avctx->codec_id != s->parser->codec_ids[4]) {
> +av_log(avctx, AV_LOG_ERROR,
> +   "The parser doesn't match the codec %s.\n",
> +   avcodec_get_name(avctx->codec_id));
> +return buf_size;
> +}

does it also work to check if a parser is set when the codec id
is changed ? as in below

(that would avoid doing 5 extra checks per packet)

--- a/libavformat/utils.c
+++ b/libavformat/utils.c
@@ -521,6 +521,9 @@ fail:

 static void force_codec_ids(AVFormatContext *s, AVStream *st)
 {
+if (st->parser)
+return;
+
 switch (st->codec->codec_type) {
 case AVMEDIA_TYPE_VIDEO:
 if (s->video_codec_id)


[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Rewriting code that is poorly written but fully understood is good.
Rewriting code that one doesnt understand is a sign that one is less smart
then the original author, trying to rewrite it will not make it better.


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

Re: [FFmpeg-devel] [PATCH] parser: error out if the codec doesn't match

[Hendrik Leppkes]

On Tue, Jan 5, 2016 at 11:46 AM, Michael Niedermayer
 wrote:
> On Tue, Jan 05, 2016 at 12:44:40AM +0100, Andreas Cadhalpun wrote:
>> Otherwise this can have some surprising effects (crashes), so let's
>> better not allow it.
>>
>> Signed-off-by: Andreas Cadhalpun 
>> ---
>>  libavcodec/parser.c | 11 +++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/libavcodec/parser.c b/libavcodec/parser.c
>> index 2809158..1f38edb 100644
>> --- a/libavcodec/parser.c
>> +++ b/libavcodec/parser.c
>> @@ -141,6 +141,17 @@ int av_parser_parse2(AVCodecParserContext *s, 
>> AVCodecContext *avctx,
>>  int index, i;
>>  uint8_t dummy_buf[AV_INPUT_BUFFER_PADDING_SIZE];
>>
>> +if (avctx->codec_id != s->parser->codec_ids[0] &&
>> +avctx->codec_id != s->parser->codec_ids[1] &&
>> +avctx->codec_id != s->parser->codec_ids[2] &&
>> +avctx->codec_id != s->parser->codec_ids[3] &&
>> +avctx->codec_id != s->parser->codec_ids[4]) {
>> +av_log(avctx, AV_LOG_ERROR,
>> +   "The parser doesn't match the codec %s.\n",
>> +   avcodec_get_name(avctx->codec_id));
>> +return buf_size;
>> +}
>
> does it also work to check if a parser is set when the codec id
> is changed ? as in below
>
> (that would avoid doing 5 extra checks per packet)
>
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -521,6 +521,9 @@ fail:
>
>  static void force_codec_ids(AVFormatContext *s, AVStream *st)
>  {
> +if (st->parser)
> +return;
> +
>  switch (st->codec->codec_type) {
>  case AVMEDIA_TYPE_VIDEO:
>  if (s->video_codec_id)
>
>

Maybe it should just delete the existing parser if the codec id is
changed here instead of refusing the change?

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

Re: [FFmpeg-devel] [PATCH v2] lavf/matroskadec: A_QUICKTIME and AV_CODEC_ID_NONE

[Mats Peterson]

On 01/04/2016 10:49 PM, Mats Peterson wrote:

In many older QuickTime files, the audio format, or "fourcc", is
0x (AV_CODEC_ID_NONE). The QuickTime File Format Specification
states the following regarding this situation:

"This format descriptor should not be used, but may be found in some
files. Samples are assumed to be stored in either 'raw ' or 'twos'
format, depending on the sample size field in the sound description."

MPlayer handles this logic by itself, but FFmpeg/FFplay currently does
not.

Mats



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



I'm still a bit afraid about the following construct for both 
A_QUICKTIME and V_QUICKTIME. It could lead to false positives. The fact 
is that the format/fourcc ALWAYS starts at offset 4 of the private data 
in a Matroska file. Does anyone sit on a file where it does not?


if (ff_codec_get_id(ff_codec_movaudio_tags, 
AV_RL32(track->codec_priv.data))) {

   fourcc = AV_RL32(track->codec_priv.data);
   codec_id = ff_codec_get_id(ff_codec_movaudio_tags, fourcc);
}

--
Mats Peterson
http://matsp888.no-ip.org/~mats/
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] arm neon float instruction crash the ffmpeg

[zhuhb]

I managed to compile/build the ffmpeg for the iOS arm32 version. But it crashed 
when I play bitstreams containing the aac and H.264 raw bitstreams.
I found that the neon float instruction is the reason, so I disable all neon 
float assembly code and the ffmpeg succeed to play the aac and H.264 bitstream.
What is the reason? How can I succeed to use the neon float instruction 
assembly code?

My configuration is as follows:
FFMPEG_FLAGS="$FFMPEG_FLAGS_IOS"
PLATFORM="iPhoneOS"
EXTRA_CONFIG="--cpu=cortex-a8 --enable-cross-compile --disable-debug 
--disable-programs  --disable-doc --arch=arm --target-os=darwin 
--disable-armv5te"
EXTRA_CFLAGS="-w -arch ${ARCH} -mfpu=neon -marm"
EXTRA_LDFLAGS="-mfpu=neon"

./configure $FFMPEG_FLAGS --prefix="${INTERDIR}/${ARCH}" 
--disable-armv6 --disable-armv6t2 
--sysroot="${DEVELOPER}/Platforms/${PLATFORM}.platform/Developer/SDKs/${PLATFORM}${SDKVERSION}.sdk"
 --cc="${DEVELOPER}/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang" 
--as='gas-preprocessor.pl 
/Applications/XCode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang'
 --extra-cflags="${EXTRA_CFLAGS} -miphoneos-version-min=${MIN_IPHONE_VERSION} 
-I${OUTPUTDIR}/include -I $SOURCE/include/ios" --extra-ldflags="-arch ${ARCH} 
${EXTRA_LDFLAGS} -isysroot 
/Applications/Xcode.app/Contents/Developer/Platforms/${PLATFORM}.platform/Developer/SDKs/${PLATFORM}${SDKVERSION}.sdk
 -miphoneos-version-min=${MIN_IPHONE_VERSION} -L${OUTPUTDIR}/lib -L 
$SOURCE/lib/ios" ${EXTRA_CONFIG} --enable-pic --extra-cxxflags="$CPPFLAGS 
-I${OUTPUTDIR}/include -isysroot 
${DEVELOPER}/Platforms/${PLATFORM}.platform/Developer/SDKs/${PLATFORM}${SDKVERSION}.sdk"
 --extra-cflags="-I/Users/iqiyi/local/opencore-amr/ios/${ARCH}/include" 
--extra-ldflags="-L/Users/iqiyi/local/opencore-amr/ios/${ARCH}/lib" 
--extra-cflags="-fembed-bitcode"

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

Re: [FFmpeg-devel] [PATCH] parser: error out if the codec doesn't match

[Michael Niedermayer]

On Tue, Jan 05, 2016 at 12:02:29PM +0100, Andreas Cadhalpun wrote:
> On 05.01.2016 11:46, Michael Niedermayer wrote:
> > On Tue, Jan 05, 2016 at 12:44:40AM +0100, Andreas Cadhalpun wrote:
> >> Otherwise this can have some surprising effects (crashes), so let's
> >> better not allow it.
> >>
> >> Signed-off-by: Andreas Cadhalpun 
> >> ---
> >>  libavcodec/parser.c | 11 +++
> >>  1 file changed, 11 insertions(+)
> >>
> >> diff --git a/libavcodec/parser.c b/libavcodec/parser.c
> >> index 2809158..1f38edb 100644
> >> --- a/libavcodec/parser.c
> >> +++ b/libavcodec/parser.c
> >> @@ -141,6 +141,17 @@ int av_parser_parse2(AVCodecParserContext *s, 
> >> AVCodecContext *avctx,
> >>  int index, i;
> >>  uint8_t dummy_buf[AV_INPUT_BUFFER_PADDING_SIZE];
> >>  
> >> +if (avctx->codec_id != s->parser->codec_ids[0] &&
> >> +avctx->codec_id != s->parser->codec_ids[1] &&
> >> +avctx->codec_id != s->parser->codec_ids[2] &&
> >> +avctx->codec_id != s->parser->codec_ids[3] &&
> >> +avctx->codec_id != s->parser->codec_ids[4]) {
> >> +av_log(avctx, AV_LOG_ERROR,
> >> +   "The parser doesn't match the codec %s.\n",
> >> +   avcodec_get_name(avctx->codec_id));
> >> +return buf_size;
> >> +}
> > 
> > does it also work to check if a parser is set when the codec id
> > is changed ? as in below
> 
> That wouldn't work, as the codec id wasn't changed in force_codec_ids,
> but in the API using program.
> To reiterate, the problematic steps were:
>  * call avformat_find_stream_info, which detects a codec and initializes
>a parser for it
>  * afterwards change the codec id in the API using program, so it
>doesn't match with the parser
> 
> Thus I think the only reliable way to detect this is a check in 
> av_parser_parse2.

shouldnt this be a av_assert1/2() then ?
i mean this is a programming error in the user app not a error
condition that should happen that way in correct usage
IMHO if we need 5 extra checks thats fine but if this is just to
detect a user app bug then iam not sure these checks should be run
for every packet

[...]


> 
> > (that would avoid doing 5 extra checks per packet)
> 
> Parsers aren't that speed critical code, I think.

if you demux/remux audio with small packets the parser might be a
sizable part speedwise, probably doesnt matter hugely but probably
not entirely irrelevant either


[...]
-- 
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] parser: error out if the codec doesn't match

[Andreas Cadhalpun]

On 05.01.2016 12:18, Michael Niedermayer wrote:
> On Tue, Jan 05, 2016 at 12:02:29PM +0100, Andreas Cadhalpun wrote:
>> That wouldn't work, as the codec id wasn't changed in force_codec_ids,
>> but in the API using program.
>> To reiterate, the problematic steps were:
>>  * call avformat_find_stream_info, which detects a codec and initializes
>>a parser for it
>>  * afterwards change the codec id in the API using program, so it
>>doesn't match with the parser
>>
>> Thus I think the only reliable way to detect this is a check in 
>> av_parser_parse2.
> 
> shouldnt this be a av_assert1/2() then ?
> i mean this is a programming error in the user app not a error
> condition that should happen that way in correct usage
> IMHO if we need 5 extra checks thats fine but if this is just to
> detect a user app bug then iam not sure these checks should be run
> for every packet

I would prefer av_assert0, but if you think that could cause relevant slowdowns,
I'm also fine with av_assert1. Patch for that attached.

Best regards,
Andreas

>From 80f6baa6d091807538461bab28656e3cf7303a2d Mon Sep 17 00:00:00 2001
From: Andreas Cadhalpun 
Date: Mon, 4 Jan 2016 23:52:20 +0100
Subject: [PATCH] parser: add av_assert1 to make sure the codec matches

Otherwise this can have some surprising effects (crashes), so let's
better not allow it.

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

diff --git a/libavcodec/parser.c b/libavcodec/parser.c
index 2809158..d25d261 100644
--- a/libavcodec/parser.c
+++ b/libavcodec/parser.c
@@ -141,6 +141,13 @@ int av_parser_parse2(AVCodecParserContext *s, AVCodecContext *avctx,
 int index, i;
 uint8_t dummy_buf[AV_INPUT_BUFFER_PADDING_SIZE];
 
+/* Parsers only work for the specified codec ids. */
+av_assert1(avctx->codec_id == s->parser->codec_ids[0] ||
+   avctx->codec_id == s->parser->codec_ids[1] ||
+   avctx->codec_id == s->parser->codec_ids[2] ||
+   avctx->codec_id == s->parser->codec_ids[3] ||
+   avctx->codec_id == s->parser->codec_ids[4]);
+
 if (!(s->flags & PARSER_FLAG_FETCHED_OFFSET)) {
 s->next_frame_offset =
 s->cur_offset= pos;
-- 
2.6.4

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

Re: [FFmpeg-devel] [PATCH] lavf/matroskadec: A_QUICKTIME and AV_CODEC_ID_NONE

[Mats Peterson]

On 01/04/2016 10:34 PM, Hendrik Leppkes wrote:



I have also removed some superfluous checks for the format/fourcc at the
very start of the private data for A_QUICKTIME and V_QUICKTIME. This
shouldn't happen, since the private data always starts with the sample
description size, *then* the format/fourcc, in a Matroska file.



Don't remove any existing checks, they usually get added based on actual files.
"Should not" does not mean "does not".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel



I'm still a bit afraid about the following construct for both 
A_QUICKTIME and V_QUICKTIME. It could lead to false positives. The fact 
is that the format/fourcc ALWAYS starts at offset 4 of the private data 
in a Matroska file. Does anyone sit on a file where it does not?


if (ff_codec_get_id(ff_codec_movaudio_tags, 
AV_RL32(track->codec_priv.data))) {

   fourcc = AV_RL32(track->codec_priv.data);
   codec_id = ff_codec_get_id(ff_codec_movaudio_tags, fourcc);
}

Mats

--
Mats Peterson
http://matsp888.no-ip.org/~mats/
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [RFC] avcodec: Add native DCA decoder based on libdcadec.

[Andreas Cadhalpun]

On 05.01.2016 21:38, foo86 wrote:
> On Tue, Jan 05, 2016 at 08:45:22PM +0100, Andreas Cadhalpun wrote:
>> On 03.01.2016 18:49, foo86 wrote:
>>> +// 5.3.1 - Bit stream header
>>> +static int parse_frame_header(DCA2CoreDecoder *s)
>>> +{
>> [...]
>>> +// Source PCM resolution
>>> +s->source_pcm_res = ff_dca_bits_per_sample[pcmr_index = 
>>> get_bits(>gb, 3)];
>>
>> This can cause an out-of-bounds read if get_bits returns 7, because 
>> ff_dca_bits_per_sample
>> only has 7 elements.
> 
> Fixed locally, thanks.

Thanks.

> P.S. To avoid resending this huge patch, I've put the fixes accumulated
> so far in a private dcadec2 branch on github [1] (will be rebased
> frequently against FFmpeg master).
> 
> [1]: https://github.com/foo86/FFmpeg/tree/dcadec2

OK. This decoder seems to be quite robust in handling fuzzed samples,
so from a security point of view it should be fine to replace the
old dca decoder with this one.

Out of curiosity: Can you post a few benchmarks comparing the performance
of the old and the new decoder?

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

Re: [FFmpeg-devel] [WIP] SDL2 in ffplay

[Moritz Barsnick]

On Tue, Jan 05, 2016 at 21:49:58 +0100, wm4 wrote:
> AFAIK SDL 1 is unmaintained now. I'm not sure why anyone would
> explicitly want SDL 1 over 2. And trying to support both would be an
> unholy mess.

For the output device, sdl2 could be created as a copy of sdl (so that
git can follow), then modified. The original sdl device would then
later be deprecated or dropped. Double maintenance in the meantime.

OTOH, that doesn't solve the ffplay scenario at all.

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

Re: [FFmpeg-devel] [PATCH 3/3] libavcodec/ccaption_dec: rewrite packet handler as case statement; remove COR3 macro

[Clément Bœsch]

On Tue, Jan 05, 2016 at 09:35:41PM +0100, Clément Bœsch wrote:
> On Mon, Jan 04, 2016 at 07:28:03PM -0800, Aman Gupta wrote:
> > From: Aman Gupta 
[...]
> >  } else if ( (hi == 0x10 && (lo >= 0x40 || lo <= 0x5f)) ||
> >( (hi >= 0x11 && hi <= 0x17) && (lo >= 0x40 && lo <= 0x7f) ) 
> > ) {
> >  handle_pac(ctx, hi, lo);
> >  } else if ( ( hi == 0x11 && lo >= 0x20 && lo <= 0x2f ) ||
> >  ( hi == 0x17 && lo >= 0x2e && lo <= 0x2f) ) {
> >  handle_textattr(ctx, hi, lo);
[...]
> Looks OK, but the command handling really is clumsy in that decoder. I'm
> pretty sure we can do something better by handling the command as a
> uint16.
> 

To expand on this, the first condition would look something like this:

if ((cmd & 0x7f60) == 0x1040 || (cmd & 0x7940) == 0x1140)

with cmd = bptr[i+1]<<8 | bptr[i+2]

(Note: cmd is assumed to be already masked with a 0x7f7f)

[...]

-- 
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] parser: error out if the codec doesn't match

[Andreas Cadhalpun]

On 05.01.2016 21:40, Michael Niedermayer wrote:
> On Tue, Jan 05, 2016 at 12:48:10PM +0100, Andreas Cadhalpun wrote:
>>  parser.c |7 +++
>>  1 file changed, 7 insertions(+)
>> 1a29aebd63e39c82039cddb1239de415b1da43df  
>> 0001-parser-add-av_assert1-to-make-sure-the-codec-matches.patch
>> From 80f6baa6d091807538461bab28656e3cf7303a2d Mon Sep 17 00:00:00 2001
>> From: Andreas Cadhalpun 
>> Date: Mon, 4 Jan 2016 23:52:20 +0100
>> Subject: [PATCH] parser: add av_assert1 to make sure the codec matches
>>
>> Otherwise this can have some surprising effects (crashes), so let's
>> better not allow it.
>>
>> Signed-off-by: Andreas Cadhalpun 
> 
> LGTM

Pushed.

Best regards,
Andreas

___
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 Tue, Jan 5, 2016 at 11:01 AM, wm4  wrote:
> On Tue, 5 Jan 2016 08:32:02 -0800
> Ganesh Ajjanagadde  wrote:
>
>> On Tue, Jan 5, 2016 at 5:29 AM, wm4  wrote:
>> > On Mon,  4 Jan 2016 17:50:01 -0800
>> > 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.
>> >>
>> >> Reviewed-by: wm4 
>> >> Reviewed-by: Ronald S. Bultje 
>> >> Reviewed-by: Michael Niedermayer 
>> >> Signed-off-by: Ganesh Ajjanagadde 
>> >> ---
>> >>  libavformat/mp3dec.c | 13 +++--
>> >>  libavformat/rmdec.c  |  3 ++-
>> >>  2 files changed, 13 insertions(+), 3 deletions(-)
>> >>
>> >> diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
>> >> index c76b21e..57ebcc8 100644
>> >> --- a/libavformat/mp3dec.c
>> >> +++ b/libavformat/mp3dec.c
>> >> @@ -372,11 +372,19 @@ 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,
>> >> +   "initial junk detection and skipping impossible 
>> >> due to: %s\n", av_err2str(ret));
>> >> +goto skip_fail;
>> >> +}
>> >>  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,
>> >> +   "initial junk detection and skipping impossible 
>> >> due to: %s\n", av_err2str(ret));
>> >> +goto skip_fail;
>> >> +}
>> >>  if (check(s->pb, off + i + frame_size, ) >= 0 &&
>> >>  (header & SAME_HEADER_MASK) == (header2 & 
>> >> SAME_HEADER_MASK))
>> >>  {
>> >> @@ -387,6 +395,7 @@ static int mp3_read_header(AVFormatContext *s)
>> >>  }
>> >>  avio_seek(s->pb, off, SEEK_SET);
>> >>  }
>> >> +skip_fail:
>> >>
>> >>  // the seek index is relative to the end of the xing vbr headers
>> >>  for (i = 0; i < st->nb_index_entries; i++)
>> >> diff --git a/libavformat/rmdec.c b/libavformat/rmdec.c
>> >> index 4e46a3d..470e227 100644
>> >> --- a/libavformat/rmdec.c
>> >> +++ b/libavformat/rmdec.c
>> >> @@ -618,7 +618,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, "seeking back impossible due 
>> >> to: %s\n", av_err2str(ret));
>> >>  v = avio_rb32(pb);
>> >>  if (v == MKBETAG('M', 'L', 'T', 'I')) {
>> >>  ret = rm_read_multi(s, s->pb, st, mime);
>> >
>> > I maintain that this should not be done, because it makes the code
>> > verbose for no reason.
>>
>> Michael has pointed out that when seekback fails, one should really
>> break out of the junk detection loop in mp3dec. Your idea fails to
>> achieve it.
>
> Why? It's not really harmful. You could even argue that this will make
> the common case (skipping jumk and finding a valid frame in a seekable
> file) work in low memory situations, while your patch breaks it.

I assumed Michael had a good reason for it, but from what you say here
and my examination just now, I would at a first glance agree with you.
Ronald had concerns about repetition of messages, so if he (and
Michael) are fine with logging within ffio_ensure_seekback, then I
will log it there.
If that is not fine, then the patch is dropped.
Thanks.

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

Re: [FFmpeg-devel] [PATCH] avpacket: fix size check in packet_alloc

[Andreas Cadhalpun]

On 05.01.2016 21:39, Michael Niedermayer wrote:
> On Tue, Jan 05, 2016 at 01:05:50PM +0100, Andreas Cadhalpun wrote:
>> The previous check only caught sizes from -AV_INPUT_BUFFER_PADDING_SIZE
>> to -1.
>>
>> This fixes ubsan runtime error: signed integer overflow: 2147483647 + 32
>> cannot be represented in type 'int'
>>
>> Signed-off-by: Andreas Cadhalpun 
>> ---
>>  libavcodec/avpacket.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> LGTM

Pushed.

Best regards,
Andreas

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

Re: [FFmpeg-devel] [PATCH] avfilter/formats: fix leak of formats on error

[Ganesh Ajjanagadde]

On Tue, Jan 5, 2016 at 12:11 PM, Paul B Mahol  wrote:
> Signed-off-by: Paul B Mahol 
> ---
>  libavfilter/formats.c | 4 
>  1 file changed, 4 insertions(+)
>
> diff --git a/libavfilter/formats.c b/libavfilter/formats.c
> index a2b19e7..f12dcf4 100644
> --- a/libavfilter/formats.c
> +++ b/libavfilter/formats.c
> @@ -518,6 +518,8 @@ void ff_formats_changeref(AVFilterFormats **oldref, 
> AVFilterFormats **newref)
>  int ret = ref_fn(fmts, >inputs[i]->out_fmts);  \
>  if (ret < 0) {  \
>  unref_fn();\
> +av_freep(>list);  \
> +av_freep();\
>  return ret; \
>  }   \
>  count++;\
> @@ -528,6 +530,8 @@ void ff_formats_changeref(AVFilterFormats **oldref, 
> AVFilterFormats **newref)
>  int ret = ref_fn(fmts, >outputs[i]->in_fmts);  \
>  if (ret < 0) {  \
>  unref_fn();\
> +av_freep(>list);  \
> +av_freep();\
>  return ret; \
>  }   \
>  count++;\

This is a good effort, and I favor it. However, note that no matter
what is done here (ie at the API level in avfilter/formats), it will
not fix all possible error paths in the filters as far as I can tell.
The reason roughly boils down to the calls being able to free their
own stuff on failure, but they are not able to free other things, e.g
samplerates functions can't free channel layouts.

Or put in other words, if one wants to be absolutely correct, a ton of
filters will need updating unfortunately.

> --
> 1.9.1
>
> ___
> 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] avfilter/formats: fix leak of formats on error

[Paul B Mahol]

On 1/5/16, Ganesh Ajjanagadde  wrote:
> On Tue, Jan 5, 2016 at 12:11 PM, Paul B Mahol  wrote:
>> Signed-off-by: Paul B Mahol 
>> ---
>>  libavfilter/formats.c | 4 
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/libavfilter/formats.c b/libavfilter/formats.c
>> index a2b19e7..f12dcf4 100644
>> --- a/libavfilter/formats.c
>> +++ b/libavfilter/formats.c
>> @@ -518,6 +518,8 @@ void ff_formats_changeref(AVFilterFormats **oldref,
>> AVFilterFormats **newref)
>>  int ret = ref_fn(fmts, >inputs[i]->out_fmts);  \
>>  if (ret < 0) {  \
>>  unref_fn();\
>> +av_freep(>list);  \
>> +av_freep();\
>>  return ret; \
>>  }   \
>>  count++;\
>> @@ -528,6 +530,8 @@ void ff_formats_changeref(AVFilterFormats **oldref,
>> AVFilterFormats **newref)
>>  int ret = ref_fn(fmts, >outputs[i]->in_fmts);  \
>>  if (ret < 0) {  \
>>  unref_fn();\
>> +av_freep(>list);  \
>> +av_freep();\
>>  return ret; \
>>  }   \
>>  count++;\
>
> This is a good effort, and I favor it. However, note that no matter
> what is done here (ie at the API level in avfilter/formats), it will
> not fix all possible error paths in the filters as far as I can tell.
> The reason roughly boils down to the calls being able to free their
> own stuff on failure, but they are not able to free other things, e.g
> samplerates functions can't free channel layouts.
>
> Or put in other words, if one wants to be absolutely correct, a ton of
> filters will need updating unfortunately.

Have example of leak this one doesn't solve?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [RESEND][PATCH] Fix buffer allocation in pad filter

[Paul B Mahol]

On 1/5/16, Andrey Turkin  wrote:
>
> One extra line must be allocated when adding left padding to make sure
> that last line in every plane fits in the allocated buffer fully
> ---
>  libavfilter/vf_pad.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
>

Do you have example this patch fixes?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avfilter/formats: fix leak of formats on error

[Ganesh Ajjanagadde]

On Tue, Jan 5, 2016 at 1:54 PM, Paul B Mahol  wrote:
> On 1/5/16, Ganesh Ajjanagadde  wrote:
>> On Tue, Jan 5, 2016 at 12:11 PM, Paul B Mahol  wrote:
>>> Signed-off-by: Paul B Mahol 
>>> ---
>>>  libavfilter/formats.c | 4 
>>>  1 file changed, 4 insertions(+)
>>>
>>> diff --git a/libavfilter/formats.c b/libavfilter/formats.c
>>> index a2b19e7..f12dcf4 100644
>>> --- a/libavfilter/formats.c
>>> +++ b/libavfilter/formats.c
>>> @@ -518,6 +518,8 @@ void ff_formats_changeref(AVFilterFormats **oldref,
>>> AVFilterFormats **newref)
>>>  int ret = ref_fn(fmts, >inputs[i]->out_fmts);  \
>>>  if (ret < 0) {  \
>>>  unref_fn();\
>>> +av_freep(>list);  \
>>> +av_freep();\
>>>  return ret; \
>>>  }   \
>>>  count++;\
>>> @@ -528,6 +530,8 @@ void ff_formats_changeref(AVFilterFormats **oldref,
>>> AVFilterFormats **newref)
>>>  int ret = ref_fn(fmts, >outputs[i]->in_fmts);  \
>>>  if (ret < 0) {  \
>>>  unref_fn();\
>>> +av_freep(>list);  \
>>> +av_freep();\
>>>  return ret; \
>>>  }   \
>>>  count++;\
>>
>> This is a good effort, and I favor it. However, note that no matter
>> what is done here (ie at the API level in avfilter/formats), it will
>> not fix all possible error paths in the filters as far as I can tell.
>> The reason roughly boils down to the calls being able to free their
>> own stuff on failure, but they are not able to free other things, e.g
>> samplerates functions can't free channel layouts.
>>
>> Or put in other words, if one wants to be absolutely correct, a ton of
>> filters will need updating unfortunately.
>
> Have example of leak this one doesn't solve?

It will be more difficult to construct (one line patch not possible),
and I would rather not put in the effort for it right now.
Like I said, independent of whether this completely solves the issue
or not, it is a step in the right direction and is good with me.

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

Re: [FFmpeg-devel] [PATCH] avcodec/srtdec: Keep exact end times

[Eelco Lempsink]

Hi Rodger,

Thank you for looking into this.

> On 29 dec. 2015, at 03:41, Rodger Combs  wrote:
> 
>> On Dec 3, 2015, at 03:30, Eelco Lempsink  wrote:
>> 
>> When converting SRT to SRT (to normalize) or WebVTT the end timestamps were
>> modified compared to the original.
>> 
>> Fixes trac 4783.
>> 
>> NOTE: The FATE test 'sub-srt' fails after this patch, because the end times 
>> of
>> the Dialogue lines change from X:XX:XX.50 to X:XX:XX.49. I can argue that 
>> this
>> is semantically correct, because in the original SRT the begin and end times
>> are such that there is no (unintended) overlap between cues, but since the
>> semantics of SRT are poorly defined, I’m not sure it’s correct.
> This is incorrect. ASS doesn't produce overlap when 2 consecutive lines end 
> and start on the same timestamp, since it uses `<` instead of `<=` when 
> comparing end times.
> 
>> ---
>> libavcodec/srtdec.c  | 15 ++
>> tests/ref/fate/sub-webvttenc | 66 
>> ++--
>> 2 files changed, 43 insertions(+), 38 deletions(-)
>> 
>> diff --git a/libavcodec/srtdec.c b/libavcodec/srtdec.c
>> index 542dd35..54568ca 100644
>> --- a/libavcodec/srtdec.c
>> +++ b/libavcodec/srtdec.c
>> @@ -57,7 +57,7 @@ static int srt_decode_frame(AVCodecContext *avctx,
>> {
>>AVSubtitle *sub = data;
>>AVBPrint buffer;
>> -int ts_start, ts_end, x1 = -1, y1 = -1, x2 = -1, y2 = -1;
>> +int ts_start, ts_duration, x1 = -1, y1 = -1, x2 = -1, y2 = -1;
>>int size, ret;
>>const uint8_t *p = av_packet_get_side_data(avpkt, 
>> AV_PKT_DATA_SUBTITLE_POSITION, );
>> 
>> @@ -77,12 +77,17 @@ static int srt_decode_frame(AVCodecContext *avctx,
>>ts_start = av_rescale_q(avpkt->pts,
>>avctx->time_base,
>>(AVRational){1,100});
>> -ts_end   = av_rescale_q(avpkt->pts + avpkt->duration,
>> -avctx->time_base,
>> -(AVRational){1,100});
>> +
>> +// Floor the duration (for ASS output)
>> +ts_duration = avpkt->duration / 10;
>> +
>> +// Set an exact end display time to prevent the rounding for ASS 
>> messing it up
>> +sub->end_display_time = av_rescale_q(avpkt->duration,
>> + avctx->pkt_timebase,
>> + (AVRational){1,1000});
> Is this patch still effective if you just add this^ statement, and leave out 
> the ts_end/ts_duration changes (to preserve the ASS rounding behavior)?

Good question, it’s not effective, because this code relies on 
libavcodec/ass.c:162 to work:

> sub->end_display_time = FFMAX(sub->end_display_time, 10 * duration);

That’s why the duration is floored. The main reason is that I didn’t want to 
touch libavcodec/ass.c but keep the fix as local to SRT as possible for our use.

What would be a better way to fix this?

Best,
Eelco

> 
>> 
>>srt_to_ass(avctx, , avpkt->data, x1, y1, x2, y2);
>> -ret = ff_ass_add_rect_bprint(sub, , ts_start, ts_end-ts_start);
>> +ret = ff_ass_add_rect_bprint(sub, , ts_start, ts_duration);
>>av_bprint_finalize(, NULL);
>>if (ret < 0)
>>return ret;
>> diff --git a/tests/ref/fate/sub-webvttenc b/tests/ref/fate/sub-webvttenc
>> index dbeadb0..ba567c3 100644
>> --- a/tests/ref/fate/sub-webvttenc
>> +++ b/tests/ref/fate/sub-webvttenc
>> @@ -14,12 +14,12 @@ If you see this with the normal font, the player don't 
>> (fully) support font face
>> 00:04.500 --> 00:04.500
>> Hidden
>> 
>> -00:04.501 --> 00:07.501
>> +00:04.501 --> 00:07.500
>> This text should be small
>> This text should be normal
>> This text should be big
>> 
>> -00:07.501 --> 00:11.501
>> +00:07.501 --> 00:11.500
>> This should be an E with an accent: È
>> 日本語
>> This text should be bold, italics and underline
>> @@ -27,7 +27,7 @@ This text should be small and green
>> This text should be small and red
>> This text should be big and brown
>> 
>> -00:11.501 --> 00:14.501
>> +00:11.501 --> 00:14.500
>> This line should be bold
>> This line should be italics
>> This line should be underline
>> @@ -35,7 +35,7 @@ This line should be strikethrough
>> Both lines
>> should be underline
>> 
>> -00:14.501 --> 00:17.501
>> +00:14.501 --> 00:17.500
>>> 
>> It would be a good thing to
>> hide invalid html tags that are closed and show the text in them
>> @@ -43,110 +43,110 @@ hide invalid html tags that are closed and show the 
>> text in them
>> Show not opened tags
>> <
>> 
>> -00:17.501 --> 00:20.501
>> +00:17.501 --> 00:20.500
>> and also
>> hide invalid html tags with parameters that are closed and show the text in 
>> them
>> but show un-closed invalid html tags
>> This text should be showed underlined without problems also: 
>> 2<3,5>1,4<6
>> This shouldn't be underlined
>> 
>> -00:20.501 --> 00:21.501
>> +00:20.501 --> 00:21.500
>> This text should be in the normal position...
>> 
>> -00:21.501 --> 00:22.501
>> +00:21.501 

Re: [FFmpeg-devel] [RFC][PATCH]lavf/matroskaenc: Assume 48kHz sample rate for Opus initial padding.

[wm4]

On Tue, 5 Jan 2016 13:23:54 +0100
Carl Eugen Hoyos  wrote:

> Hi!
> 
> Attached patch may fix this issue reported for Firefox:
> https://bugzilla.mozilla.org/show_bug.cgi?id=1227153
> Completely untested but I believe the patch matches a 
> comment in libopusenc.c line 90.
> 
> Please comment, Carl Eugen

I'd consider this a broken file and not apply the patch.

Why is the codec delay only applied for opus in the first place?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] Enable dash output to work when the output isn't a local file

[Hendrik Leppkes]

On Tue, Jan 5, 2016 at 1:50 PM, Raymond Hilseth  wrote:
>
>
> On 04/01/16 17:36, "ffmpeg-devel on behalf of Hendrik Leppkes"
>  wrote:
>
>>On Mon, Jan 4, 2016 at 3:58 PM,   wrote:
>>> From: Raymond Hilseth 
>>>
>>> Signed-off-by: Raymond Hilseth 
>>> ---
>>>  libavformat/dashenc.c | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
>>> index 4509ee4..378c4e4 100644
>>> --- a/libavformat/dashenc.c
>>> +++ b/libavformat/dashenc.c
>>> @@ -549,7 +549,7 @@ static int write_manifest(AVFormatContext *s, int
>>>final)
>>>  avio_printf(out, "\n");
>>>  avio_flush(out);
>>>  avio_close(out);
>>> -return ff_rename(temp_filename, s->filename, s);
>>> +return avpriv_io_move(temp_filename, s->filename);
>>>  }
>>>
>>>  static int dash_write_header(AVFormatContext *s)
>>> @@ -856,7 +856,7 @@ static int dash_flush(AVFormatContext *s, int
>>>final, int stream)
>>>  } else {
>>>  ffurl_close(os->out);
>>>  os->out = NULL;
>>> -ret = ff_rename(temp_path, full_path, s);
>>> +ret = avpriv_io_move(temp_path, full_path);
>>>  if (ret < 0)
>>>  break;
>>>  }
>>> --
>>
>>For unknown reasons, url_move(which avpriv_io_move uses) in the "file"
>>protocol depends on unistd.h, which is not available everywhere.
>>So commiting this without making sure file.url_move is available on
>>all systems where ff_rename works would introduce a regression.
>>
>>- Hendrik
>
> Are there any supported platforms other than Windows where unistd.h isn¹t
> available?
>
> As far as I can see, the existing code for dash encoding cannot work on
> Windows, since rename on Windows will fail when the destination file
> already exists, and the manifest file will already exist after the first
> segment has been written.
>

I don't know about DASH, but HLS does the same thing and it works
perfectly fine there even on Windows.
In any case, this is not directly relevant to the patch - it should
keep calling the same functions on local files.

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

Re: [FFmpeg-devel] [RFC][PATCH]lavf/matroskaenc: Assume 48kHz sample rate for Opus initial padding.

[Hendrik Leppkes]

On Tue, Jan 5, 2016 at 2:26 PM, wm4  wrote:
> On Tue, 5 Jan 2016 13:23:54 +0100
> Carl Eugen Hoyos  wrote:
>
>> Hi!
>>
>> Attached patch may fix this issue reported for Firefox:
>> https://bugzilla.mozilla.org/show_bug.cgi?id=1227153
>> Completely untested but I believe the patch matches a
>> comment in libopusenc.c line 90.
>>
>> Please comment, Carl Eugen
>
> I'd consider this a broken file and not apply the patch.
>
> Why is the codec delay only applied for opus in the first place?

This is about the muxer, so we are the ones producing broken files, apparently.

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

[wm4]

On Mon,  4 Jan 2016 17:50:01 -0800
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.
> 
> Reviewed-by: wm4 
> Reviewed-by: Ronald S. Bultje 
> Reviewed-by: Michael Niedermayer 
> Signed-off-by: Ganesh Ajjanagadde 
> ---
>  libavformat/mp3dec.c | 13 +++--
>  libavformat/rmdec.c  |  3 ++-
>  2 files changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
> index c76b21e..57ebcc8 100644
> --- a/libavformat/mp3dec.c
> +++ b/libavformat/mp3dec.c
> @@ -372,11 +372,19 @@ 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,
> +   "initial junk detection and skipping impossible due 
> to: %s\n", av_err2str(ret));
> +goto skip_fail;
> +}
>  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,
> +   "initial junk detection and skipping impossible due 
> to: %s\n", av_err2str(ret));
> +goto skip_fail;
> +}
>  if (check(s->pb, off + i + frame_size, ) >= 0 &&
>  (header & SAME_HEADER_MASK) == (header2 & SAME_HEADER_MASK))
>  {
> @@ -387,6 +395,7 @@ static int mp3_read_header(AVFormatContext *s)
>  }
>  avio_seek(s->pb, off, SEEK_SET);
>  }
> +skip_fail:
>  
>  // the seek index is relative to the end of the xing vbr headers
>  for (i = 0; i < st->nb_index_entries; i++)
> diff --git a/libavformat/rmdec.c b/libavformat/rmdec.c
> index 4e46a3d..470e227 100644
> --- a/libavformat/rmdec.c
> +++ b/libavformat/rmdec.c
> @@ -618,7 +618,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, "seeking back impossible due to: 
> %s\n", av_err2str(ret));
>  v = avio_rb32(pb);
>  if (v == MKBETAG('M', 'L', 'T', 'I')) {
>  ret = rm_read_multi(s, s->pb, st, mime);

I maintain that this should not be done, because it makes the code
verbose for no reason. Or if you insist, do it in the ffio_ function
itself.
___
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, Jan 05, 2016 at 01:57:09PM -0800, Ganesh Ajjanagadde wrote:
> On Tue, Jan 5, 2016 at 11:01 AM, wm4  wrote:
> > On Tue, 5 Jan 2016 08:32:02 -0800
> > Ganesh Ajjanagadde  wrote:
> >
> >> On Tue, Jan 5, 2016 at 5:29 AM, wm4  wrote:
> >> > On Mon,  4 Jan 2016 17:50:01 -0800
> >> > 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.
> >> >>
> >> >> Reviewed-by: wm4 
> >> >> Reviewed-by: Ronald S. Bultje 
> >> >> Reviewed-by: Michael Niedermayer 
> >> >> Signed-off-by: Ganesh Ajjanagadde 
> >> >> ---
> >> >>  libavformat/mp3dec.c | 13 +++--
> >> >>  libavformat/rmdec.c  |  3 ++-
> >> >>  2 files changed, 13 insertions(+), 3 deletions(-)
> >> >>
> >> >> diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
> >> >> index c76b21e..57ebcc8 100644
> >> >> --- a/libavformat/mp3dec.c
> >> >> +++ b/libavformat/mp3dec.c
> >> >> @@ -372,11 +372,19 @@ 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,
> >> >> +   "initial junk detection and skipping impossible 
> >> >> due to: %s\n", av_err2str(ret));
> >> >> +goto skip_fail;
> >> >> +}
> >> >>  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,
> >> >> +   "initial junk detection and skipping impossible 
> >> >> due to: %s\n", av_err2str(ret));
> >> >> +goto skip_fail;
> >> >> +}
> >> >>  if (check(s->pb, off + i + frame_size, ) >= 0 &&
> >> >>  (header & SAME_HEADER_MASK) == (header2 & 
> >> >> SAME_HEADER_MASK))
> >> >>  {
> >> >> @@ -387,6 +395,7 @@ static int mp3_read_header(AVFormatContext *s)
> >> >>  }
> >> >>  avio_seek(s->pb, off, SEEK_SET);
> >> >>  }
> >> >> +skip_fail:
> >> >>
> >> >>  // the seek index is relative to the end of the xing vbr headers
> >> >>  for (i = 0; i < st->nb_index_entries; i++)
> >> >> diff --git a/libavformat/rmdec.c b/libavformat/rmdec.c
> >> >> index 4e46a3d..470e227 100644
> >> >> --- a/libavformat/rmdec.c
> >> >> +++ b/libavformat/rmdec.c
> >> >> @@ -618,7 +618,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, "seeking back impossible due 
> >> >> to: %s\n", av_err2str(ret));
> >> >>  v = avio_rb32(pb);
> >> >>  if (v == MKBETAG('M', 'L', 'T', 'I')) {
> >> >>  ret = rm_read_multi(s, s->pb, st, mime);
> >> >
> >> > I maintain that this should not be done, because it makes the code
> >> > verbose for no reason.
> >>
> >> Michael has pointed out that when seekback fails, one should really
> >> break out of the junk detection loop in mp3dec. Your idea fails to
> >> achieve it.
> >
> > Why? It's not really harmful. You could even argue that this will make
> > the common case (skipping jumk and finding a valid frame in a seekable
> > file) work in low memory situations, while your patch breaks it.

ffio_ensure_seekback() never fails for seekable files
it checks s->seekable and return 0 if true before attempting to
allocate anything


> 
> I assumed Michael had a good reason for it, but from what you say here
> and my examination just now, I would at a first glance agree with you.
> Ronald had concerns about repetition of messages, so if he (and
> Michael) are fine with logging within ffio_ensure_seekback, then I
> will log it there.

as the code is written currently, if ffio_ensure_seekback() fails
and that being ignored, the url_seek() to return to the begin will
fail too quite likely while forward seeks will succeed.
this would result in random amounts of data to be lost at the begin
of the file
it may print errors or warnings if such are added but nothing (like
the applications return code) would indicate a problem, the user though
could end with 

Re: [FFmpeg-devel] [PATCH 1/2] avcodec/nvenc: Include NVENC SDK header

[Roger Pack]

On 12/10/15, Philip Langdale  wrote:
> On 2015-12-09 21:34, wm4 wrote:
>> On Mon, 7 Dec 2015 19:34:20 +0100
>> Timo Rothenpieler  wrote:
>>
>>> > I don't remember if this was discussed when avisynth and other headers
>>> > where included, but what's the advantage of directly including the
>>> > header and burden the FFmpeg sources, rather than asking the user to
>>> > download them in case of need?
>>>
>>> The nvenc sdk isn't exactly a common thing that distributions provide.
>>> As distributions can now easily ship ffmpeg with nvenc support, this
>>> propably helps users who now no longer need to build ffmpeg themselves
>>> for nvenc support.
>>
>> So why would distros build with nvenc support, if they can't even do a
>> simple thing like installing a single header file?
>>
>> Are we really talking about including a huge 3rd party header because
>> distros are so lazy? What's next, do we add Windows headers to the
>> FFmpeg tree, because MinGW lags severely behind the newest definitions
>> like HEVC DXVA support?
>>
>> We could just provide a download link for the nvenc header somewhere if
>> the problem is finding the header.
>
> Admittedly, we are solving someone else's problem, but the header is
> buried inside the SDK download which is hidden behind a click-through on
> nvidia's web page. So it's not made available in a way that is readily
> consumable by an end user or by a distribution vendor.

I have had some success scripting its download in the past, FWIW :)
Cheers!
-roger-

  echo "installing nvenc [nvidia gpu assisted encoder]"
  curl 
http://developer.download.nvidia.com/assets/cuda/files/nvidia_video_sdk_6.0.1.zip
-O -L
  unzip nvidia_video_sdk_6.0.1.zip
  cp nvidia_video_sdk_6.0.1/Samples/common/inc/*
$mingw_w64_x86_64_prefix/include
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [RFC] avcodec: Add native DCA decoder based on libdcadec.

[Hendrik Leppkes]

On Tue, Jan 5, 2016 at 10:46 PM, Andreas Cadhalpun
 wrote:
> On 05.01.2016 21:38, foo86 wrote:
>> On Tue, Jan 05, 2016 at 08:45:22PM +0100, Andreas Cadhalpun wrote:
>>> On 03.01.2016 18:49, foo86 wrote:
 +// 5.3.1 - Bit stream header
 +static int parse_frame_header(DCA2CoreDecoder *s)
 +{
>>> [...]
 +// Source PCM resolution
 +s->source_pcm_res = ff_dca_bits_per_sample[pcmr_index = 
 get_bits(>gb, 3)];
>>>
>>> This can cause an out-of-bounds read if get_bits returns 7, because 
>>> ff_dca_bits_per_sample
>>> only has 7 elements.
>>
>> Fixed locally, thanks.
>
> Thanks.
>
>> P.S. To avoid resending this huge patch, I've put the fixes accumulated
>> so far in a private dcadec2 branch on github [1] (will be rebased
>> frequently against FFmpeg master).
>>
>> [1]: https://github.com/foo86/FFmpeg/tree/dcadec2
>
> OK. This decoder seems to be quite robust in handling fuzzed samples,
> so from a security point of view it should be fine to replace the
> old dca decoder with this one.
>


So that leaves us with a bunch of positive comments, on this side
anyway, and noone opposed yet.

Arguments for a switch include:
- Nearly complete coverage of all DTS features, well tested and
confirmed bit-exact (only DTS Express is missing, which is technically
its own little codec using DTS EXSS headers)
- Slightly faster (~5%)
- Active maintainer
- Andreas seal of security approval ;)

If anyone thinks we should not replace our decoder, speak now or
forever hold your peace (and bring proper arguments).
I will try to do a code review to the best of my abilities in the upcoming days.

foo86, could you work on changing the patch to replace the original instead?

After it is merged, we could think about integrating your test-suite
into the FATE system, but all in good time.

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

Re: [FFmpeg-devel] [RFC] avcodec: Add native DCA decoder based on libdcadec.

[Hendrik Leppkes]

On Wed, Jan 6, 2016 at 12:40 AM, Ganesh Ajjanagadde  wrote:
> On Tue, Jan 5, 2016 at 3:28 PM, Hendrik Leppkes  wrote:
>> On Tue, Jan 5, 2016 at 10:46 PM, Andreas Cadhalpun
>>  wrote:
>>> On 05.01.2016 21:38, foo86 wrote:
 On Tue, Jan 05, 2016 at 08:45:22PM +0100, Andreas Cadhalpun wrote:
> On 03.01.2016 18:49, foo86 wrote:
>> +// 5.3.1 - Bit stream header
>> +static int parse_frame_header(DCA2CoreDecoder *s)
>> +{
> [...]
>> +// Source PCM resolution
>> +s->source_pcm_res = ff_dca_bits_per_sample[pcmr_index = 
>> get_bits(>gb, 3)];
>
> This can cause an out-of-bounds read if get_bits returns 7, because 
> ff_dca_bits_per_sample
> only has 7 elements.

 Fixed locally, thanks.
>>>
>>> Thanks.
>>>
 P.S. To avoid resending this huge patch, I've put the fixes accumulated
 so far in a private dcadec2 branch on github [1] (will be rebased
 frequently against FFmpeg master).

 [1]: https://github.com/foo86/FFmpeg/tree/dcadec2
>>>
>>> OK. This decoder seems to be quite robust in handling fuzzed samples,
>>> so from a security point of view it should be fine to replace the
>>> old dca decoder with this one.
>>>
>>
>>
>> So that leaves us with a bunch of positive comments, on this side
>> anyway, and noone opposed yet.
>>
>> Arguments for a switch include:
>> - Nearly complete coverage of all DTS features, well tested and
>> confirmed bit-exact (only DTS Express is missing, which is technically
>> its own little codec using DTS EXSS headers)
>> - Slightly faster (~5%)
>> - Active maintainer
>> - Andreas seal of security approval ;)
>>
>> If anyone thinks we should not replace our decoder, speak now or
>> forever hold your peace (and bring proper arguments).
>> I will try to do a code review to the best of my abilities in the upcoming 
>> days.
>
> For now, I definitely think we should replace our decoder.
> Just a clarification: in the long run, isn't it a good idea to get
> this into the repo and not use an external library? For instance, if
> and when asm code gets written for this, isn't it easier to work
> within FFmpeg's codebase, which has some infrastructure for writing
> asm?
>

The whole idea of this patch is to integrate the decoder fully into
our codebase, and no longer use the external library.

PS:
What I forgot to mention in my last mail, foo86 if you have any
questions about ripping out the old dca decoder, feel free to contact
me.

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