Re: [FFmpeg-devel] [PATCH 3/9] electronicarts: prevent overflow during block alignment calculation

2017-01-07 Thread Paul B Mahol
On 1/7/17, Michael Niedermayer  wrote:
> On Fri, Jan 06, 2017 at 08:47:39PM +0100, Andreas Cadhalpun wrote:
>> Signed-off-by: Andreas Cadhalpun 
>> ---
>>  libavformat/electronicarts.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/libavformat/electronicarts.c b/libavformat/electronicarts.c
>> index 30eb723bd5..03422e5b2c 100644
>> --- a/libavformat/electronicarts.c
>> +++ b/libavformat/electronicarts.c
>> @@ -556,6 +556,7 @@ static int ea_read_header(AVFormatContext *s)
>>  st->codecpar->codec_tag = 0;   /* no tag */
>>  st->codecpar->channels  = ea->num_channels;
>>  st->codecpar->sample_rate   = ea->sample_rate;
>> +FF_RETURN_ON_OVERFLOW(s, ea->bytes > INT_MAX / 8 / 2)
>
> I think we should ask for a sample when the number of bytes per
> sample is larger than 2 or 4 or whatever max we think occurs.

No we should not as such samples are invalid.

>
> the patch is probably fine
>
> [...]
>
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> I am the wisest man alive, for I know one thing, and that is that I know
> nothing. -- Socrates
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 1/9] avutil: add FF_RETURN_ON_OVERFLOW

2017-01-07 Thread Paul B Mahol
On 1/7/17, Michael Niedermayer  wrote:
> On Fri, Jan 06, 2017 at 09:11:10PM -0300, James Almer wrote:
>> On 1/6/2017 4:46 PM, Andreas Cadhalpun wrote:
>> > Suggested-by: Rodger Combs 
>> > Signed-off-by: Andreas Cadhalpun 
>> > ---
>> >
>> > Changed the name as suggested by wm4 and the return value as suggested
>> > by Muhammad Faiz.
>> > There are also two new overflow checks at the end of the series.
>>
>> This is a good chance to introduce the gcc overflow check builtins.
>> See https://gcc.gnu.org/onlinedocs/gcc/Integer-Overflow-Builtins.html,
>> they
>> use hardware instructions when possible, like x86's Jump on Overflow.
>>
>> The idea would be to use __builtin_mul_overflow_p(). For example
>> (untested):
>>
>> #if AV_GCC_VERSION_AT_LEAST(5,1)
>> #define av_builtin_mul_overflow_p(a, b) __builtin_mul_overflow_p(a, b,
>> (int) 0)
>> #else
>> #define av_builtin_mul_overflow_p(a, b) ((a) > INT_MAX / (b)))
>> #endif
>>
>> It can also be used all across the codebase, not just for these checks.
>>
>
>> >
>> > ---
>> >  libavutil/common.h | 2 ++
>> >  1 file changed, 2 insertions(+)
>> >
>> > diff --git a/libavutil/common.h b/libavutil/common.h
>> > index 8142b31fdb..6d795a353a 100644
>> > --- a/libavutil/common.h
>> > +++ b/libavutil/common.h
>> > @@ -99,6 +99,8 @@
>> >  #define FFSWAP(type,a,b) do{type SWAP_tmp= b; b= a; a=
>> > SWAP_tmp;}while(0)
>> >  #define FF_ARRAY_ELEMS(a) (sizeof(a) / sizeof((a)[0]))
>> >
>> > +#define FF_RETURN_ON_OVERFLOW(ctx, x) if (x) {av_log(ctx, AV_LOG_ERROR,
>> > "Overflow check failed: " #x"\n"); return AVERROR(ERANGE);}
>>
>> Printing an error unconditionally seems like log bloat. We do all kinds
>> of
>> sanity checks on demuxers and simply return INVALIDDATA without printing
>> anything if they fail.
>> Maybe we should do the same here and let the demuxer choose to print an
>> error or not.
>
> error messages help debuging and thus maintaining code.
>

Not really. Expecially in this case.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 4/9] genh: prevent overflow during block alignment calculation

2017-01-07 Thread Paul B Mahol
On 1/7/17, Michael Niedermayer  wrote:
> On Fri, Jan 06, 2017 at 08:48:02PM +0100, Andreas Cadhalpun wrote:
>> Signed-off-by: Andreas Cadhalpun 
>> ---
>>  libavformat/genh.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/libavformat/genh.c b/libavformat/genh.c
>> index b683e026d1..6ce2588ed3 100644
>> --- a/libavformat/genh.c
>> +++ b/libavformat/genh.c
>> @@ -74,6 +74,7 @@ static int genh_read_header(AVFormatContext *s)
>>  case  0: st->codecpar->codec_id = AV_CODEC_ID_ADPCM_PSX;
>> break;
>>  case  1:
>>  case 11: st->codecpar->bits_per_coded_sample = 4;
>> + FF_RETURN_ON_OVERFLOW(s, st->codecpar->channels > INT_MAX /
>> 36)
>>   st->codecpar->block_align = 36 * st->codecpar->channels;
>>   st->codecpar->codec_id = AV_CODEC_ID_ADPCM_IMA_WAV;
>> break;
>>  case  2: st->codecpar->codec_id = AV_CODEC_ID_ADPCM_DTK;
>> break;
>
> i see a channels * 1024 in genh_read_packet()
> is the added check sufficient ?
>
> also i think we should ask for a sample for a file that has a
> channel count beyond normal bounds

No, we should not as such samples are invalid.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 1/9] avutil: add FF_RETURN_ON_OVERFLOW

2017-01-07 Thread Rodger Combs

> On Jan 7, 2017, at 02:36, Paul B Mahol  wrote:
> 
> On 1/7/17, Michael Niedermayer  > wrote:
>> On Fri, Jan 06, 2017 at 09:11:10PM -0300, James Almer wrote:
>>> On 1/6/2017 4:46 PM, Andreas Cadhalpun wrote:
 Suggested-by: Rodger Combs 
 Signed-off-by: Andreas Cadhalpun 
 ---
 
 Changed the name as suggested by wm4 and the return value as suggested
 by Muhammad Faiz.
 There are also two new overflow checks at the end of the series.
>>> 
>>> This is a good chance to introduce the gcc overflow check builtins.
>>> See https://gcc.gnu.org/onlinedocs/gcc/Integer-Overflow-Builtins.html,
>>> they
>>> use hardware instructions when possible, like x86's Jump on Overflow.
>>> 
>>> The idea would be to use __builtin_mul_overflow_p(). For example
>>> (untested):
>>> 
>>> #if AV_GCC_VERSION_AT_LEAST(5,1)
>>> #define av_builtin_mul_overflow_p(a, b) __builtin_mul_overflow_p(a, b,
>>> (int) 0)
>>> #else
>>> #define av_builtin_mul_overflow_p(a, b) ((a) > INT_MAX / (b)))
>>> #endif
>>> 
>>> It can also be used all across the codebase, not just for these checks.
>>> 
>> 
 
 ---
 libavutil/common.h | 2 ++
 1 file changed, 2 insertions(+)
 
 diff --git a/libavutil/common.h b/libavutil/common.h
 index 8142b31fdb..6d795a353a 100644
 --- a/libavutil/common.h
 +++ b/libavutil/common.h
 @@ -99,6 +99,8 @@
 #define FFSWAP(type,a,b) do{type SWAP_tmp= b; b= a; a=
 SWAP_tmp;}while(0)
 #define FF_ARRAY_ELEMS(a) (sizeof(a) / sizeof((a)[0]))
 
 +#define FF_RETURN_ON_OVERFLOW(ctx, x) if (x) {av_log(ctx, AV_LOG_ERROR,
 "Overflow check failed: " #x"\n"); return AVERROR(ERANGE);}
>>> 
>>> Printing an error unconditionally seems like log bloat. We do all kinds
>>> of
>>> sanity checks on demuxers and simply return INVALIDDATA without printing
>>> anything if they fail.
>>> Maybe we should do the same here and let the demuxer choose to print an
>>> error or not.
>> 
>> error messages help debuging and thus maintaining code.
>> 
> 
> Not really. Expecially in this case.

Eh, I generally prefer a print-and-return over just returning an error code in 
cases where the source is ambiguous. There are a lot of cases currently where 
in e.g. ffmpeg CLI, the code just gets bubbled up and printed, potentially with 
the name of the codec it came from, but no further information about the 
source, so it can be difficult to track down exactly where the failure 
occurred. Printing a message can be a fair bit more clear.
This seems like something it would be worth talking about and deciding on in 
general, though: as a rule, should we try to print errors about specific 
failures like this?

> ___
> 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] avcodec/dca: add support for 20-bit XLL

2017-01-07 Thread foo86
Fixes ticket #6063.
---
 libavcodec/dca_xll.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/libavcodec/dca_xll.c b/libavcodec/dca_xll.c
index 1d616c298c..1320aaf28f 100644
--- a/libavcodec/dca_xll.c
+++ b/libavcodec/dca_xll.c
@@ -143,7 +143,7 @@ static int chs_parse_header(DCAXllDecoder *s, DCAXllChSet 
*c, DCAExssAsset *asse
 
 // Storage unit width
 c->storage_bit_res = get_bits(&s->gb, 5) + 1;
-if (c->storage_bit_res != 16 && c->storage_bit_res != 24) {
+if (c->storage_bit_res != 16 && c->storage_bit_res != 20 && 
c->storage_bit_res != 24) {
 avpriv_request_sample(s->avctx, "%d-bit XLL storage resolution", 
c->storage_bit_res);
 return AVERROR_PATCHWELCOME;
 }
@@ -1415,9 +1415,12 @@ int ff_dca_xll_filter_frame(DCAXllDecoder *s, AVFrame 
*frame)
 switch (p->storage_bit_res) {
 case 16:
 avctx->sample_fmt = AV_SAMPLE_FMT_S16P;
+shift = 16 - p->pcm_bit_res;
 break;
+case 20:
 case 24:
 avctx->sample_fmt = AV_SAMPLE_FMT_S32P;
+shift = 24 - p->pcm_bit_res;
 break;
 default:
 return AVERROR(EINVAL);
@@ -1438,7 +1441,6 @@ int ff_dca_xll_filter_frame(DCAXllDecoder *s, AVFrame 
*frame)
s->output_mask);
 }
 
-shift = p->storage_bit_res - p->pcm_bit_res;
 for (i = 0; i < avctx->channels; i++) {
 int32_t *samples = s->output_samples[ch_remap[i]];
 if (frame->format == AV_SAMPLE_FMT_S16P) {
-- 
2.11.0

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


Re: [FFmpeg-devel] [PATCH] avcodec/dca: add support for 20-bit XLL

2017-01-07 Thread Paul B Mahol
On 1/7/17, foo86  wrote:
> Fixes ticket #6063.
> ---
>  libavcodec/dca_xll.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/libavcodec/dca_xll.c b/libavcodec/dca_xll.c
> index 1d616c298c..1320aaf28f 100644
> --- a/libavcodec/dca_xll.c
> +++ b/libavcodec/dca_xll.c
> @@ -143,7 +143,7 @@ static int chs_parse_header(DCAXllDecoder *s,
> DCAXllChSet *c, DCAExssAsset *asse
>
>  // Storage unit width
>  c->storage_bit_res = get_bits(&s->gb, 5) + 1;
> -if (c->storage_bit_res != 16 && c->storage_bit_res != 24) {
> +if (c->storage_bit_res != 16 && c->storage_bit_res != 20 &&
> c->storage_bit_res != 24) {
>  avpriv_request_sample(s->avctx, "%d-bit XLL storage resolution",
> c->storage_bit_res);
>  return AVERROR_PATCHWELCOME;
>  }
> @@ -1415,9 +1415,12 @@ int ff_dca_xll_filter_frame(DCAXllDecoder *s, AVFrame
> *frame)
>  switch (p->storage_bit_res) {
>  case 16:
>  avctx->sample_fmt = AV_SAMPLE_FMT_S16P;
> +shift = 16 - p->pcm_bit_res;
>  break;
> +case 20:
>  case 24:
>  avctx->sample_fmt = AV_SAMPLE_FMT_S32P;
> +shift = 24 - p->pcm_bit_res;
>  break;
>  default:
>  return AVERROR(EINVAL);
> @@ -1438,7 +1441,6 @@ int ff_dca_xll_filter_frame(DCAXllDecoder *s, AVFrame
> *frame)
> s->output_mask);
>  }
>
> -shift = p->storage_bit_res - p->pcm_bit_res;
>  for (i = 0; i < avctx->channels; i++) {
>  int32_t *samples = s->output_samples[ch_remap[i]];
>  if (frame->format == AV_SAMPLE_FMT_S16P) {
> --
> 2.11.0
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

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


Re: [FFmpeg-devel] [PATCH 1/2] libavformat/avio: Add avio_get_dyn_buf function

2017-01-07 Thread Michael Niedermayer
On Sat, Jan 07, 2017 at 03:38:48AM +, Soft Works wrote:
> > Michael wrote
> > Is the author name intended to be
> > Author: softworkz 
> 
> Yes please, if you don't mind, same as my previous commit: 
> http://git.videolan.org/?p=ffmpeg.git;a=commit;h=70c1647a3501fa6182c04c9ce66f477def64a611

applied


> 
> PS: On behalf of the Emby project, I'd like to thank you for your kind 
> assistance!
> (https://emby.media/)
> 
> Thank you very much,

thanks as well


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

If you think the mosad wants you dead since a long time then you are either
wrong or dead since a long time.


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


Re: [FFmpeg-devel] [PATCH 4/9] genh: prevent overflow during block alignment calculation

2017-01-07 Thread Ronald S. Bultje
Hi,

On Fri, Jan 6, 2017 at 8:43 PM, Michael Niedermayer 
wrote:

> On Fri, Jan 06, 2017 at 08:48:02PM +0100, Andreas Cadhalpun wrote:
> > Signed-off-by: Andreas Cadhalpun 
> > ---
> >  libavformat/genh.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/libavformat/genh.c b/libavformat/genh.c
> > index b683e026d1..6ce2588ed3 100644
> > --- a/libavformat/genh.c
> > +++ b/libavformat/genh.c
> > @@ -74,6 +74,7 @@ static int genh_read_header(AVFormatContext *s)
> >  case  0: st->codecpar->codec_id = AV_CODEC_ID_ADPCM_PSX;
> break;
> >  case  1:
> >  case 11: st->codecpar->bits_per_coded_sample = 4;
> > + FF_RETURN_ON_OVERFLOW(s, st->codecpar->channels > INT_MAX
> / 36)
> >   st->codecpar->block_align = 36 * st->codecpar->channels;
> >   st->codecpar->codec_id = AV_CODEC_ID_ADPCM_IMA_WAV;
> break;
> >  case  2: st->codecpar->codec_id = AV_CODEC_ID_ADPCM_DTK;
> break;
>
> i see a channels * 1024 in genh_read_packet()
> is the added check sufficient ?
>
> also i think we should ask for a sample for a file that has a
> channel count beyond normal bounds


Not in this code. Such generic channel sanity checks belong in utils.c, not
here, and should not be invoked by the demuxer explicitly, but always run
as integral part of read_header or add_stream or so.

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


Re: [FFmpeg-devel] [PATCH 1/3] mpeg12dec: validate color space

2017-01-07 Thread Ronald S. Bultje
Hi,

On Fri, Jan 6, 2017 at 9:35 PM, Michael Niedermayer 
wrote:

> On Fri, Jan 06, 2017 at 09:43:24PM +0100, Andreas Cadhalpun wrote:
> > On 23.12.2016 00:57, Andreas Cadhalpun wrote:
> > > Signed-off-by: Andreas Cadhalpun 
> > > ---
> > >  libavcodec/mpeg12dec.c | 4 
> > >  1 file changed, 4 insertions(+)
> > >
> > > diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
> > > index 63979079c8..d3dc67ad6a 100644
> > > --- a/libavcodec/mpeg12dec.c
> > > +++ b/libavcodec/mpeg12dec.c
> > > @@ -1470,6 +1470,10 @@ static void 
> > > mpeg_decode_sequence_display_extension(Mpeg1Context
> *s1)
> > >  s->avctx->color_primaries = get_bits(&s->gb, 8);
> > >  s->avctx->color_trc   = get_bits(&s->gb, 8);
> > >  s->avctx->colorspace  = get_bits(&s->gb, 8);
> > > +if (!av_color_space_name(s->avctx->colorspace)) {
> > > +av_log(s->avctx, AV_LOG_WARNING, "Invalid color space %d,
> setting to unspecified\n", s->avctx->colorspace);
> > > +s->avctx->colorspace = AVCOL_SPC_UNSPECIFIED;
> > > +}
> > >  }
> > >  w = get_bits(&s->gb, 14);
> > >  skip_bits(&s->gb, 1); // marker
> > >
> >
> > Ping for the series.
>
> i have no real objection to it.
> iam used to see these being exported unchanged though so it feels a
> bit odd


Doesn't this destroy exporting of newly added colorspaces that have no
explicit mention yet in libavutil? I'm not 100% sure this is a good thing.

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


Re: [FFmpeg-devel] [PATCH 2/2] avformat/matroskaenc: Regression fix for invalid MKV headers

2017-01-07 Thread James Almer
On 1/6/2017 8:33 PM, Soft Works wrote:
> Revision #4: Don't create CRC32 for preliminary headers
> 
> The following three commits created a regression by writing initially
> invalid mkv headers:
> 
> 650e17d88b63b5aca6e0a43483e89e64b0f7d2dd avformat/matroskaenc: write a
> CRC32 element on Tags
> 3bcadf822711720ff0f8d14db71ae47cdf97e652 avformat/matroskaenc: write a
> CRC32 element on Info
> ee888cfbe777cd2916a3548c750e433ab8f8e6a5 avformat/matroskaenc: postpone
> writing the Tracks master
> 
> Symptoms:
> 
> - You can no longer playback a file that is still processed by ffmpeg,
> e.g. VLC fails playback
> - You can no longer stream a file to a client while if is still being
> processed
> - Various diagnosing tools show header errors or incomplete headers
> (e.g. ffprobe, mediainfo, mkvalidator)
> 
> Note: The symptoms do not apply to completed files or ffmpeg runs that
> were interrupted with 'q'
> 
> Cause:
> 
> The mentioned commits made changes in a way that some header elements
> are only partially written in
> mkv_write_header, leaving the header in an invalid state. Only in
> mkv_write_trailer, these elements
> are finished correctly, but that does only occur at the end of the
> process.
> 
> Regression:
> 
> Before these commits were applied, mkv headers have always been valid,
> even before completion of ffmpeg.
> This has worked reliably over many versions of ffmpeg, to it was an
> obvious regression.
> 
> Bugtracker:
> 
> This issue has been recorded as #5977 which is resolved by this patch
> 
> Patch:
> 
> The patch adds a new function 'end_ebml_master_crc32_preliminary' that
> preliminarily finishes the ebl
> element without destroying the buffer. The buffer can be used to update
> the ebml element later during
> mkv_write_trailer. But most important: mkv_write_header finishes with a
> valid mkv header again.
> ---
>  libavformat/matroskaenc.c | 22 +++---
>  1 file changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> index 827d755..dd8ff8e 100644
> --- a/libavformat/matroskaenc.c
> +++ b/libavformat/matroskaenc.c
> @@ -367,6 +367,22 @@ static void end_ebml_master_crc32(AVIOContext *pb, 
> AVIOContext **dyn_cp, Matrosk
>  *dyn_cp = NULL;
>  }
>  
> +/**
> +* Complete ebml master whithout destroying the buffer, allowing for later 
> updates
> +*/
> +static void end_ebml_master_crc32_preliminary(AVIOContext *pb, AVIOContext 
> **dyn_cp, MatroskaMuxContext *mkv,
> +ebml_master master)
> +{
> +if (pb->seekable) {
> +
> +uint8_t *buf;
> +int size = avio_get_dyn_buf(*dyn_cp, &buf);
> +
> +avio_write(pb, buf, size);
> +end_ebml_master(pb, master);
> +}
> +}
> +
>  static void put_xiph_size(AVIOContext *pb, int size)
>  {
>  ffio_fill(pb, 255, size / 255);
> @@ -1309,7 +1325,7 @@ static int mkv_write_tracks(AVFormatContext *s)
>  }
>  
>  if (pb->seekable && !mkv->is_live)
> -put_ebml_void(pb, avio_tell(mkv->tracks_bc));
> +end_ebml_master_crc32_preliminary(pb, &mkv->tracks_bc, mkv, 
> mkv->tracks_master);
>  else
>  end_ebml_master_crc32(pb, &mkv->tracks_bc, mkv, mkv->tracks_master);
>  
> @@ -1554,7 +1570,7 @@ static int mkv_write_tags(AVFormatContext *s)
>  
>  if (mkv->tags.pos) {
>  if (s->pb->seekable && !mkv->is_live)
> -put_ebml_void(s->pb, avio_tell(mkv->tags_bc));
> +end_ebml_master_crc32_preliminary(s->pb, &mkv->tags_bc, mkv, 
> mkv->tags);
>  else
>  end_ebml_master_crc32(s->pb, &mkv->tags_bc, mkv, mkv->tags);
>  }
> @@ -1811,7 +1827,7 @@ static int mkv_write_header(AVFormatContext *s)
>  }
>  }
>  if (s->pb->seekable && !mkv->is_live)
> -put_ebml_void(s->pb, avio_tell(pb));
> +end_ebml_master_crc32_preliminary(s->pb, &mkv->info_bc, mkv, 
> mkv->info);
>  else
>  end_ebml_master_crc32(s->pb, &mkv->info_bc, mkv, mkv->info);
>  pb = s->pb;

Pushed, thanks.

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


Re: [FFmpeg-devel] [PATCH] libavcodec/exr: Fix blank output when data window != display window

2017-01-07 Thread Moritz Barsnick
On Fri, Jan 06, 2017 at 18:08:44 +0100, Martin Vignali wrote:
> Did you try,all the official display/data window sample ?

I did have a look at a few selected samples, and ffmpeg doesn't seem to
handle them:

http://ffmpeg.org/pipermail/ffmpeg-user/2017-January/034831.html

I'm not sure
https://github.com/openexr/openexr-images/tree/master/DisplayWindow
is the "specification", but any EXR decoder should be able to handle
all the listed file, IIUC.

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


Re: [FFmpeg-devel] [PATCH] avcodec/dca: add support for 20-bit XLL

2017-01-07 Thread James Almer
On 1/7/2017 6:26 AM, foo86 wrote:
> Fixes ticket #6063.
> ---
>  libavcodec/dca_xll.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/libavcodec/dca_xll.c b/libavcodec/dca_xll.c
> index 1d616c298c..1320aaf28f 100644
> --- a/libavcodec/dca_xll.c
> +++ b/libavcodec/dca_xll.c
> @@ -143,7 +143,7 @@ static int chs_parse_header(DCAXllDecoder *s, DCAXllChSet 
> *c, DCAExssAsset *asse
>  
>  // Storage unit width
>  c->storage_bit_res = get_bits(&s->gb, 5) + 1;
> -if (c->storage_bit_res != 16 && c->storage_bit_res != 24) {
> +if (c->storage_bit_res != 16 && c->storage_bit_res != 20 && 
> c->storage_bit_res != 24) {
>  avpriv_request_sample(s->avctx, "%d-bit XLL storage resolution", 
> c->storage_bit_res);
>  return AVERROR_PATCHWELCOME;
>  }
> @@ -1415,9 +1415,12 @@ int ff_dca_xll_filter_frame(DCAXllDecoder *s, AVFrame 
> *frame)
>  switch (p->storage_bit_res) {
>  case 16:
>  avctx->sample_fmt = AV_SAMPLE_FMT_S16P;
> +shift = 16 - p->pcm_bit_res;
>  break;
> +case 20:
>  case 24:
>  avctx->sample_fmt = AV_SAMPLE_FMT_S32P;
> +shift = 24 - p->pcm_bit_res;
>  break;
>  default:
>  return AVERROR(EINVAL);
> @@ -1438,7 +1441,6 @@ int ff_dca_xll_filter_frame(DCAXllDecoder *s, AVFrame 
> *frame)
> s->output_mask);
>  }
>  
> -shift = p->storage_bit_res - p->pcm_bit_res;
>  for (i = 0; i < avctx->channels; i++) {
>  int32_t *samples = s->output_samples[ch_remap[i]];
>  if (frame->format == AV_SAMPLE_FMT_S16P) {

I think you still don't have git access, so pushed.

Poke Michael with your public key if that's the case and you want it.

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


[FFmpeg-devel] Implementation of Huffman codes for DCA encoder

2017-01-07 Thread Даниил Чередник
Hi!.

Currently I am working on improvement quality of DTS encoder. Following
patches introduce Huffman coding.
First one - reverse data layout
[SUBBAND_SAMPLES][DCAENC_SUBBANDS][MAX_CHANNELS] to
[MAX_CHANNELS][DCAENC_SUBBANDS][SUBBAND_SAMPLES]. I need it to write
more readable code, and reduce (a bit) speed penalty.
Second one - Huffman coding for quantized audio data.

Results:

https://yadi.sk/d/rVtRuFZv37ZqYk - without huffman
https://yadi.sk/d/grTuitcu37ZqSS - with huffman

Speed degradation in my case approximately 20%.


-- 
Daniil Cherednik
From d083e669765b81026130ed50b734f184e44e9424 Mon Sep 17 00:00:00 2001
From: Daniil Cherednik 
Date: Fri, 6 Jan 2017 02:07:54 +0300
Subject: [PATCH 1/2] avcodec/dcaenc: Reverse data layout to prevent data
 copies during Huffman encoding introduction
To: ffmpeg-devel@ffmpeg.org

---
 libavcodec/dcaenc.c | 88 ++---
 1 file changed, 43 insertions(+), 45 deletions(-)

diff --git a/libavcodec/dcaenc.c b/libavcodec/dcaenc.c
index 6bb7d29..3af2a35 100644
--- a/libavcodec/dcaenc.c
+++ b/libavcodec/dcaenc.c
@@ -61,15 +61,15 @@ typedef struct DCAEncContext {
 int32_t lfe_peak_cb;
 const int8_t *channel_order_tab;  ///< channel reordering table, lfe and non lfe
 
-int32_t history[512][MAX_CHANNELS]; /* This is a circular buffer */
-int32_t subband[SUBBAND_SAMPLES][DCAENC_SUBBANDS][MAX_CHANNELS];
-int32_t quantized[SUBBAND_SAMPLES][DCAENC_SUBBANDS][MAX_CHANNELS];
-int32_t peak_cb[DCAENC_SUBBANDS][MAX_CHANNELS];
+int32_t history[MAX_CHANNELS][512]; /* This is a circular buffer */
+int32_t subband[MAX_CHANNELS][DCAENC_SUBBANDS][SUBBAND_SAMPLES];
+int32_t quantized[MAX_CHANNELS][DCAENC_SUBBANDS][SUBBAND_SAMPLES];
+int32_t peak_cb[MAX_CHANNELS][DCAENC_SUBBANDS];
 int32_t downsampled_lfe[DCA_LFE_SAMPLES];
 int32_t masking_curve_cb[SUBSUBFRAMES][256];
-int abits[DCAENC_SUBBANDS][MAX_CHANNELS];
-int scale_factor[DCAENC_SUBBANDS][MAX_CHANNELS];
-softfloat quant[DCAENC_SUBBANDS][MAX_CHANNELS];
+int abits[MAX_CHANNELS][DCAENC_SUBBANDS];
+int scale_factor[MAX_CHANNELS][DCAENC_SUBBANDS];
+softfloat quant[MAX_CHANNELS][DCAENC_SUBBANDS];
 int32_t eff_masking_curve_cb[256];
 int32_t band_masking_cb[32];
 int32_t worst_quantization_noise;
@@ -259,8 +259,7 @@ static void subband_transform(DCAEncContext *c, const int32_t *input)
 int hist_start = 0;
 const int chi = c->channel_order_tab[ch];
 
-for (i = 0; i < 512; i++)
-hist[i] = c->history[i][ch];
+memcpy(hist, &c->history[ch][0], 512 * sizeof(int32_t));
 
 for (subs = 0; subs < SUBBAND_SAMPLES; subs++) {
 int32_t accum[64];
@@ -268,8 +267,7 @@ static void subband_transform(DCAEncContext *c, const int32_t *input)
 int band;
 
 /* Calculate the convolutions at once */
-for (i = 0; i < 64; i++)
-accum[i] = 0;
+memset(accum, 0, 64 * sizeof(int32_t));
 
 for (k = 0, i = hist_start, j = 0;
 i < 512; k = (k + 1) & 63, i++, j++)
@@ -289,12 +287,13 @@ static void subband_transform(DCAEncContext *c, const int32_t *input)
 resp += mul32(accum[i], cos_t(s << 3)) >> 3;
 }
 
-c->subband[subs][band][ch] = ((band + 1) & 2) ? -resp : resp;
+c->subband[ch][band][subs] = ((band + 1) & 2) ? -resp : resp;
 }
 
 /* Copy in 32 new samples from input */
 for (i = 0; i < 32; i++)
 hist[i + hist_start] = input[(subs * 32 + i) * c->channels + chi];
+
 hist_start = (hist_start + 32) & 511;
 }
 }
@@ -309,8 +308,7 @@ static void lfe_downsample(DCAEncContext *c, const int32_t *input)
 int32_t accum;
 int hist_start = 0;
 
-for (i = 0; i < 512; i++)
-hist[i] = c->history[i][c->channels - 1];
+memcpy(hist, &c->history[c->channels - 1][0], 512 * sizeof(int32_t));
 
 for (lfes = 0; lfes < DCA_LFE_SAMPLES; lfes++) {
 /* Calculate the convolution */
@@ -516,7 +514,7 @@ static void calc_masking(DCAEncContext *c, const int32_t *input)
 const int chi = c->channel_order_tab[ch];
 
 for (i = 0, k = 128 + 256 * ssf; k < 512; i++, k++)
-data[i] = c->history[k][ch];
+data[i] = c->history[ch][k];
 for (k -= 512; i < 512; i++, k++)
 data[i] = input[k * c->channels + chi];
 adjust_jnd(c->samplerate_index, data, c->masking_curve_cb[ssf]);
@@ -541,17 +539,17 @@ static void find_peaks(DCAEncContext *c)
 {
 int band, ch;
 
-for (band = 0; band < 32; band++)
-for (ch = 0; ch < c->fullband_channels; ch++) {
+for (ch = 0; ch < c->fullband_channels; ch++)
+for (band = 0; band < 32; band++) {
 int sample;
 int32_t m = 0;
 
 for (sample = 0; sample < SUBBAND_SAMPLES; sample++

Re: [FFmpeg-devel] Implementation of Huffman codes for DCA encoder

2017-01-07 Thread Carl Eugen Hoyos
2017-01-07 16:00 GMT+01:00 Даниил Чередник :

> Currently I am working on improvement quality of DTS encoder. Following
> patches introduce Huffman coding.

Is the quality improvement so obvious that no further tests are necessary?
(Does psnr improve measurably?)

Please split the second patch so that the moving of functions is one
independent patch.

Thank you for this very welcome work!

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


Re: [FFmpeg-devel] [PATCH] avcodec/dca: add support for 20-bit XLL

2017-01-07 Thread Michael Niedermayer
On Sat, Jan 07, 2017 at 12:26:43PM +0300, foo86 wrote:
> Fixes ticket #6063.
> ---
>  libavcodec/dca_xll.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)

I think i suggested it before but
You are de facto maintaining dca, you should add yourself in a patch
to the MAINTAINERs file for dca

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

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


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


Re: [FFmpeg-devel] [PATCH] avformat/hlsenc: start_number new options

2017-01-07 Thread Bodecs Bela



2017.01.07. 0:32 keltezéssel, Steven Liu írta:

2017-01-07 0:47 GMT+08:00 Bodecs Bela :



2017.01.06. 17:33 keltezéssel, Steven Liu írta:


2017-01-07 0:22 GMT+08:00 Bodecs Bela :



2017.01.06. 16:50 keltezéssel, Steven Liu írta:

2017-01-06 22:07 GMT+08:00 Bodecs Bela :

Dear All,


in avformat/hlsenc the start_number option starts the playlist sequence
number
(#EXT-X-MEDIA-SEQUENCE) from the specified number. Unless hls_flags
single_file is set, it also specifies starting sequence numbers of
segment and subtitle filenames. Sometimes it is usefull to have unique
starting numbers at each run, but currently it is only achiveable by
setting this parameter manually.
This patch enables to set start_number parameter automatically for
practically unique numbers. If start_number is set to -1, then
the start number will be the seconds since epoch (1970-01-01 00:00:00).
If set to -2, then the start number will be based on the current
date/time value as mmddHHMMSS. e.g. 20161231235659.


thank you,

Bela Bodecs


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


Two question:


1. char b[21];   Why this is 21 ?

you are right, 15 is enough.

2. +{"start_number",  "set first number in the sequence",

OFFSET(start_sequence),AV_OPT_TYPE_INT64,  {.i64 = 0}, -2,
INT64_MAX,
E},
Why is this -2 and the help message maybe need more infomation, for
example
-2 mean -1 mean  0 mean, and default value.

yes, I have altered now but I have written verbosly into the doc

(muxers.texi), here:

+If set to -1, then the start number will be the seconds since epoch
(1970-01-01 00:00:00).
+If set to -2, then the start number will be based on the current
date/time as mmddHHMMSS. e.g. 20161231235759.
+Default value is 0.

___


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

I have enclosed a fixed version. A have changed some code, where greater

than 32 bit long sequence numbers were not handled correctly.
(av_get_frame_filename2)

thank you.
Bela Bodecs


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


+{"start_number",  "set first number in the sequence, 0 is default,

-1:
second since epoch, -2: current datetime as MMDDhhmmss, actual value
otherwise", OFFSET(start_sequence),AV_OPT_TYPE_INT64,  {.i64 = 0},
  -2,
INT64_MAX, E},

I have check this option, i think add flag to control the start_number
maybe better,
for example:
hls_flags
hls_playlist_type

maybe add a start_number_flags is better, What about you think?


Using hls_flags is not enough to specify different values for them.


NO, i am not mean use hls_flags, i mean you can creat a new flags,

start_number_flags
  generic
  epoch
  datetime

Ok, I see it. May I implement it?



I thought that there should be 3 options beside this start_number option.

hls_start_number_playlist, hls_start_number_segment and
hls_start_number_vtt
Using start_number and any of the new 3 ones would be mutualy exlusive.

This way anybody could use the old option (start_number) and it won't
break the current behaviour.
But those who want to have finer control, they may use the new options.

of course -start_number x  has the same effect as using
-hls_start_number_playlist x -hls_start_number_segment x
-hls_start_number_vtt x



___

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 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] Implementation of Huffman codes for DCA encoder

2017-01-07 Thread Rostislav Pehlivanov
On 7 January 2017 at 16:11, Carl Eugen Hoyos  wrote:

> 2017-01-07 16:00 GMT+01:00 Даниил Чередник :
>
> > Currently I am working on improvement quality of DTS encoder. Following
> > patches introduce Huffman coding.
>
> Is the quality improvement so obvious that no further tests are necessary?
> (Does psnr improve measurably?)
>
>
PSNR is pretty much useless for audio. The ear's the only metric which
works.
From the 2 samples he posted I can tell there's a big difference, and the
encoder
isn't very good right now and the patch definitely helps.


> Please split the second patch so that the moving of functions is one
> independent patch.
>
> Thank you for this very welcome work!
>
> Carl Eugen
> ___
> 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] Implementation of Huffman codes for DCA encoder

2017-01-07 Thread Carl Eugen Hoyos
2017-01-07 20:39 GMT+01:00 Rostislav Pehlivanov :
> On 7 January 2017 at 16:11, Carl Eugen Hoyos  wrote:
>
>> 2017-01-07 16:00 GMT+01:00 Даниил Чередник :
>>
>> > Currently I am working on improvement quality of DTS encoder. Following
>> > patches introduce Huffman coding.
>>
>> Is the quality improvement so obvious that no further tests are necessary?
>> (Does psnr improve measurably?)
>>
> PSNR is pretty much useless for audio.

> The ear's the only metric which works.

Not everybody's;-))

> From the 2 samples he posted I can tell there's a big difference, and the
> encoder isn't very good right now and the patch definitely helps.

Thank you!

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


Re: [FFmpeg-devel] [PATCH] lavc/cuvid: fail early if GPU can't handle given video parameters

2017-01-07 Thread Pavel Koshevoy
On Mon, Jan 2, 2017 at 2:26 PM,   wrote:
> From: Pavel Koshevoy 
>
> Evidently CUVID doesn't support decoding 422 or 444 chroma formats,
> and only a limited set of resolutions per codec are supported.
>
> Given that stream resolution and pixel format are typically known as a
> result of probing it is better to use this info during avcodec_open2
> call and fail early in case the video parameters are not supported,
> rather than failing later during avcodec_send_packet calls.
>
> This problem surfaced when trying to decode 5120x2700 h246 video on
> Geforce GT 730, or when decoding 422 mpeg2 stream on same GPU --
> avcodec_open2 succeeded but decoding failed.
> ---
>  libavcodec/cuvid.c | 58 
> +-
>  1 file changed, 53 insertions(+), 5 deletions(-)
>
> diff --git a/libavcodec/cuvid.c b/libavcodec/cuvid.c
> index 8fc713d..febdd71 100644
> --- a/libavcodec/cuvid.c
> +++ b/libavcodec/cuvid.c
> @@ -612,7 +612,11 @@ static av_cold int cuvid_decode_end(AVCodecContext 
> *avctx)
>  return 0;
>  }
>
> -static int cuvid_test_dummy_decoder(AVCodecContext *avctx, CUVIDPARSERPARAMS 
> *cuparseinfo)
> +static int cuvid_test_dummy_decoder(AVCodecContext *avctx,
> +const CUVIDPARSERPARAMS *cuparseinfo,
> +cudaVideoChromaFormat 
> probed_chroma_format,
> +int probed_width,
> +int probed_height)
>  {
>  CuvidContext *ctx = avctx->priv_data;
>  CUVIDDECODECREATEINFO cuinfo;
> @@ -622,11 +626,11 @@ static int cuvid_test_dummy_decoder(AVCodecContext 
> *avctx, CUVIDPARSERPARAMS *cu
>  memset(&cuinfo, 0, sizeof(cuinfo));
>
>  cuinfo.CodecType = cuparseinfo->CodecType;
> -cuinfo.ChromaFormat = cudaVideoChromaFormat_420;
> +cuinfo.ChromaFormat = probed_chroma_format;
>  cuinfo.OutputFormat = cudaVideoSurfaceFormat_NV12;
>
> -cuinfo.ulWidth = 1280;
> -cuinfo.ulHeight = 720;
> +cuinfo.ulWidth = probed_width;
> +cuinfo.ulHeight = probed_height;
>  cuinfo.ulTargetWidth = cuinfo.ulWidth;
>  cuinfo.ulTargetHeight = cuinfo.ulHeight;
>
> @@ -653,6 +657,36 @@ static int cuvid_test_dummy_decoder(AVCodecContext 
> *avctx, CUVIDPARSERPARAMS *cu
>  return 0;
>  }
>
> +static int convert_to_cuda_video_chroma_format(enum AVPixelFormat pix_fmt,
> +   cudaVideoChromaFormat *out)
> +{
> +const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(pix_fmt);
> +if (!(out && desc &&
> +  (desc->nb_components == 1 || desc->nb_components == 3) &&
> +  (desc->log2_chroma_w < 2 && desc->log2_chroma_h < 2)))
> +{
> +return AVERROR(EINVAL);
> +}
> +
> +if (desc->nb_components == 1)
> +{
> +*out = cudaVideoChromaFormat_Monochrome;
> +}
> +else if (desc->flags == AV_PIX_FMT_FLAG_PLANAR)
> +{
> +*out = ((desc->log2_chroma_w == 0) ? cudaVideoChromaFormat_444 :
> +(desc->log2_chroma_h == 0) ? cudaVideoChromaFormat_422 :
> +cudaVideoChromaFormat_420);
> +}
> +else
> +{
> +return AVERROR(EINVAL);
> +}
> +
> +// unfortunately, 420 is the only one that works:
> +return (*out == cudaVideoChromaFormat_420) ? 0 : AVERROR_EXTERNAL;
> +}
> +
>  static av_cold int cuvid_decode_init(AVCodecContext *avctx)
>  {
>  CuvidContext *ctx = avctx->priv_data;
> @@ -663,12 +697,23 @@ static av_cold int cuvid_decode_init(AVCodecContext 
> *avctx)
>  CUcontext cuda_ctx = NULL;
>  CUcontext dummy;
>  const AVBitStreamFilter *bsf;
> +cudaVideoChromaFormat probed_chroma_format;
> +int probed_width;
> +int probed_height;
>  int ret = 0;
>
>  enum AVPixelFormat pix_fmts[3] = { AV_PIX_FMT_CUDA,
> AV_PIX_FMT_NV12,
> AV_PIX_FMT_NONE };
>
> +ret = convert_to_cuda_video_chroma_format(avctx->pix_fmt, 
> &probed_chroma_format);
> +if (ret < 0) {
> +// pixel format is not supported:
> +return ret;
> +}
> +probed_width = avctx->coded_width ? avctx->coded_width : 1280;
> +probed_height = avctx->coded_height ? avctx->coded_height : 720;
> +
>  // Accelerated transcoding scenarios with 'ffmpeg' require that the
>  // pix_fmt be set to AV_PIX_FMT_CUDA early. The sw_pix_fmt, and the
>  // pix_fmt for non-accelerated transcoding, do not need to be correct
> @@ -824,7 +869,10 @@ static av_cold int cuvid_decode_init(AVCodecContext 
> *avctx)
>  if (ret < 0)
>  goto error;
>
> -ret = cuvid_test_dummy_decoder(avctx, &ctx->cuparseinfo);
> +ret = cuvid_test_dummy_decoder(avctx, &ctx->cuparseinfo,
> +   probed_chroma_format,
> +   probed_width,
> +   probed_height);
>  if (ret < 0)
>  goto error;
>
>

Re: [FFmpeg-devel] Implementation of Huffman codes for DCA encoder

2017-01-07 Thread Даниил Чередник
With real music and 256k bitrate encoding (the source was 44100, 16bit
stereo) I got:
Without Huffman: Best PSNR is  31.77 for shift 0
With: Best PSNR is  37.45 for shift 0

Current implementation of DCA encoder has minimal set of DTS features (no
ADPCM, no VQ, fixed amount of transmitted subbands, no transient control,
etc). So distortion is quite audible. Current set of patches introduces
Huffman encoding for quantized audio data which is equivalent of increasing
bitrate for 10-20%. Also bitstream allows to use Huffman for scale factor
indexes, and some other data. I am working on it too.

I have attached new split set of patches.

On Sat, Jan 7, 2017 at 11:50 PM, Carl Eugen Hoyos 
wrote:

> 2017-01-07 20:39 GMT+01:00 Rostislav Pehlivanov :
> > On 7 January 2017 at 16:11, Carl Eugen Hoyos  wrote:
> >
> >> 2017-01-07 16:00 GMT+01:00 Даниил Чередник :
> >>
> >> > Currently I am working on improvement quality of DTS encoder.
> Following
> >> > patches introduce Huffman coding.
> >>
> >> Is the quality improvement so obvious that no further tests are
> necessary?
> >> (Does psnr improve measurably?)
> >>
> > PSNR is pretty much useless for audio.
>
> > The ear's the only metric which works.
>
> Not everybody's;-))
>
> > From the 2 samples he posted I can tell there's a big difference, and the
> > encoder isn't very good right now and the patch definitely helps.
>
> Thank you!
>
> Carl Eugen
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>



-- 
Daniil Cherednik
From d083e669765b81026130ed50b734f184e44e9424 Mon Sep 17 00:00:00 2001
From: Daniil Cherednik 
Date: Fri, 6 Jan 2017 02:07:54 +0300
Subject: [PATCH 1/3] avcodec/dcaenc: Reverse data layout to prevent data
 copies during Huffman encoding introduction
To: ffmpeg-devel@ffmpeg.org

---
 libavcodec/dcaenc.c | 88 ++---
 1 file changed, 43 insertions(+), 45 deletions(-)

diff --git a/libavcodec/dcaenc.c b/libavcodec/dcaenc.c
index 6bb7d29..3af2a35 100644
--- a/libavcodec/dcaenc.c
+++ b/libavcodec/dcaenc.c
@@ -61,15 +61,15 @@ typedef struct DCAEncContext {
 int32_t lfe_peak_cb;
 const int8_t *channel_order_tab;  ///< channel reordering table, lfe and non lfe
 
-int32_t history[512][MAX_CHANNELS]; /* This is a circular buffer */
-int32_t subband[SUBBAND_SAMPLES][DCAENC_SUBBANDS][MAX_CHANNELS];
-int32_t quantized[SUBBAND_SAMPLES][DCAENC_SUBBANDS][MAX_CHANNELS];
-int32_t peak_cb[DCAENC_SUBBANDS][MAX_CHANNELS];
+int32_t history[MAX_CHANNELS][512]; /* This is a circular buffer */
+int32_t subband[MAX_CHANNELS][DCAENC_SUBBANDS][SUBBAND_SAMPLES];
+int32_t quantized[MAX_CHANNELS][DCAENC_SUBBANDS][SUBBAND_SAMPLES];
+int32_t peak_cb[MAX_CHANNELS][DCAENC_SUBBANDS];
 int32_t downsampled_lfe[DCA_LFE_SAMPLES];
 int32_t masking_curve_cb[SUBSUBFRAMES][256];
-int abits[DCAENC_SUBBANDS][MAX_CHANNELS];
-int scale_factor[DCAENC_SUBBANDS][MAX_CHANNELS];
-softfloat quant[DCAENC_SUBBANDS][MAX_CHANNELS];
+int abits[MAX_CHANNELS][DCAENC_SUBBANDS];
+int scale_factor[MAX_CHANNELS][DCAENC_SUBBANDS];
+softfloat quant[MAX_CHANNELS][DCAENC_SUBBANDS];
 int32_t eff_masking_curve_cb[256];
 int32_t band_masking_cb[32];
 int32_t worst_quantization_noise;
@@ -259,8 +259,7 @@ static void subband_transform(DCAEncContext *c, const int32_t *input)
 int hist_start = 0;
 const int chi = c->channel_order_tab[ch];
 
-for (i = 0; i < 512; i++)
-hist[i] = c->history[i][ch];
+memcpy(hist, &c->history[ch][0], 512 * sizeof(int32_t));
 
 for (subs = 0; subs < SUBBAND_SAMPLES; subs++) {
 int32_t accum[64];
@@ -268,8 +267,7 @@ static void subband_transform(DCAEncContext *c, const int32_t *input)
 int band;
 
 /* Calculate the convolutions at once */
-for (i = 0; i < 64; i++)
-accum[i] = 0;
+memset(accum, 0, 64 * sizeof(int32_t));
 
 for (k = 0, i = hist_start, j = 0;
 i < 512; k = (k + 1) & 63, i++, j++)
@@ -289,12 +287,13 @@ static void subband_transform(DCAEncContext *c, const int32_t *input)
 resp += mul32(accum[i], cos_t(s << 3)) >> 3;
 }
 
-c->subband[subs][band][ch] = ((band + 1) & 2) ? -resp : resp;
+c->subband[ch][band][subs] = ((band + 1) & 2) ? -resp : resp;
 }
 
 /* Copy in 32 new samples from input */
 for (i = 0; i < 32; i++)
 hist[i + hist_start] = input[(subs * 32 + i) * c->channels + chi];
+
 hist_start = (hist_start + 32) & 511;
 }
 }
@@ -309,8 +308,7 @@ static void lfe_downsample(DCAEncContext *c, const int32_t *input)
 int32_t accum;
 int hist_start = 0;
 
-for (i = 0; i < 512; i++)
-hist[i] = c->history[i][c->channels - 1];
+memcpy(hi

[FFmpeg-devel] [PATCH][WIP][RFC] Add support for MPEG-4 Simple Studio Profile

2017-01-07 Thread Kieran Kunhya
Hi,

I have added support for MPEG-4 Sstp using the available samples on trac.
Yes it doesn't pass fate, yes it's not format-patch, yes it uses printfs.
https://trac.ffmpeg.org/ticket/4447

Being MPEG-4, it depends on mpegvideo.c so has tons of yuv420p assumptions
baked in which are of course undocumented.
Here are my questions (line number refers to attached patch):

Line 35: How do I signal to this idctdsp thing that I want an idct with
32-bit coefficients AND intermediates? A lot of that code has assumptions
that intermediates will be the next size up, i.e 8-bit coeffs, 16-bit
intermediates, or 16-bit coeffs and 32-bit intermediates.

Line 906: Why do RGB samples not decode unless the GBRP format is moved to
the top of PIX_FMT. I get "[mpeg4 @ 0x7f945c029600] format change not
supported" otherwise.

Line 932: What's going on with this branch. Normal mpeg-4 video does
dequant during unpack, why is it not part of this condition?

Line 987: Are there more assumptions baked into mpegvideo.c about "square"
macroblocks, i.e ones where (width == height)?

Line 997: What is all this stuff going on with -1U, unless I remove this I
get a segfault. I do get a stripe on the left though.

Line 1055: How can I make the existing code use 32-bit coefficients
cleanly? I can't reuse block[a][b] because it's allocated in a single
malloc.

Regards,
Kieran Kunhya
-- 

Sent from my mobile device
diff --git a/libavcodec/h263dec.c b/libavcodec/h263dec.c
index d0da1d3..f09df29 100644
--- a/libavcodec/h263dec.c
+++ b/libavcodec/h263dec.c
@@ -197,6 +197,10 @@ static int decode_slice(MpegEncContext *s)
 
 ff_set_qscale(s, s->qscale);
 
+if (s->studio_profile) {
+ff_mpeg4_decode_studio_slice_header(s->avctx->priv_data);
+}
+
 if (s->avctx->hwaccel) {
 const uint8_t *start = s->gb.buffer + get_bits_count(&s->gb) / 8;
 ret = s->avctx->hwaccel->decode_slice(s->avctx, start, s->gb.buffer_end - start);
@@ -252,7 +256,7 @@ static int decode_slice(MpegEncContext *s)
 ff_dlog(s, "%d %06X\n",
 get_bits_count(&s->gb), show_bits(&s->gb, 24));
 
-ff_tlog(NULL, "Decoding MB at %dx%d\n", s->mb_x, s->mb_y);
+//printf("Decoding MB at %dx%d\n", s->mb_x, s->mb_y);
 ret = s->decode_mb(s, s->block);
 
 if (s->pict_type != AV_PICTURE_TYPE_B)
diff --git a/libavcodec/idctdsp.c b/libavcodec/idctdsp.c
index 63e9b52..396fd53 100644
--- a/libavcodec/idctdsp.c
+++ b/libavcodec/idctdsp.c
@@ -259,7 +259,8 @@ av_cold void ff_idctdsp_init(IDCTDSPContext *c, AVCodecContext *avctx)
 c->perm_type = FF_IDCT_PERM_NONE;
 } else {
 if (avctx->bits_per_raw_sample == 10 || avctx->bits_per_raw_sample == 9) {
-c->idct_put  = ff_simple_idct_put_10;
+//c->idct_put  = ff_simple_idct_put_10;
+c->idct_put  = ff_idct_float;
 c->idct_add  = ff_simple_idct_add_10;
 c->idct  = ff_simple_idct_10;
 c->perm_type = FF_IDCT_PERM_NONE;
@@ -303,8 +304,8 @@ av_cold void ff_idctdsp_init(IDCTDSPContext *c, AVCodecContext *avctx)
 ff_idctdsp_init_arm(c, avctx, high_bit_depth);
 if (ARCH_PPC)
 ff_idctdsp_init_ppc(c, avctx, high_bit_depth);
-if (ARCH_X86)
-ff_idctdsp_init_x86(c, avctx, high_bit_depth);
+//if (ARCH_X86)
+//ff_idctdsp_init_x86(c, avctx, high_bit_depth);
 if (ARCH_MIPS)
 ff_idctdsp_init_mips(c, avctx, high_bit_depth);
 
diff --git a/libavcodec/ituh263dec.c b/libavcodec/ituh263dec.c
index 5e3c0ea..85c8f76 100644
--- a/libavcodec/ituh263dec.c
+++ b/libavcodec/ituh263dec.c
@@ -207,12 +207,21 @@ static int h263_decode_gob_header(MpegEncContext *s)
 }
 
 /**
- * Decode the group of blocks / video packet header.
+ * Decode the group of blocks / video packet header / slice header (MPEG-4 Studio).
  * @return bit position of the resync_marker, or <0 if none was found
  */
 int ff_h263_resync(MpegEncContext *s){
 int left, pos, ret;
 
+//printf ("__PRETTY_FUNCTION__ = %s\n", __PRETTY_FUNCTION__);
+
+/* In MPEG-4 studio mode look for a new slice startcode
+ * and decode slice header */
+if(s->codec_id==AV_CODEC_ID_MPEG4 && s->studio_profile) {
+// FIXME search for new slice startcode if not there already
+return pos;
+}
+
 if(s->codec_id==AV_CODEC_ID_MPEG4){
 skip_bits1(&s->gb);
 align_get_bits(&s->gb);
diff --git a/libavcodec/mpeg4data.h b/libavcodec/mpeg4data.h
index b7c3fab..4756e9e 100644
--- a/libavcodec/mpeg4data.h
+++ b/libavcodec/mpeg4data.h
@@ -373,4 +373,120 @@ const uint8_t ff_mpeg4_dc_threshold[8]={
 99, 13, 15, 17, 19, 21, 23, 0
 };
 
+/* Note these are different in studio mode */
+const uint16_t ff_mpeg4_studio_dc_luma[19][2]={
+{0x0e,  6}, {0x06,  5}, {0x00,  4}, {0x02,  4},
+{0x07,  3}, {0x05,  3}, {0x03,  3}, {0x02,  3},
+{0x04,  3}, {0x06,  3}, {0x01,  4}, {0x1e

Re: [FFmpeg-devel] [PATCH] lavc/vaapi_encode_h264: disable B frame in baseline profile

2017-01-07 Thread Mark Thompson
On 06/01/17 15:19, Moritz Barsnick wrote:
> Since Michael mentioned it:
> 
> On Fri, Dec 16, 2016 at 10:21:25 +0800, Jun Zhao wrote:
> 
>> +if (avctx->max_b_frames != 0) {
>> +avctx->max_b_frames = 0;
>> +av_log(avctx, AV_LOG_WARNING, "H.264 constrained baseline "
>> +   "profile don't support encode B frame.\n");
>> +}
> 
> "H.264 constrained baseline profile doesn't support encoding with B
> frames, disabling them.\n".
> 
>> +if (avctx->max_b_frames != 0) {
>> +avctx->max_b_frames = 0;
>> +av_log(avctx, AV_LOG_WARNING, "H.264 baseline "
>> +   "profile don't support encode B frame.\n");
>> +}
> 
> "H.264 baseline profile doesn't support encoding with B frames,
> disabling them.\n".

+1 to this rephrasing (for improved grammar and clarity).  Patch otherwise fine.

Also apologies for missing the mail last month, thank you for the ping.

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


Re: [FFmpeg-devel] [PATCH] libavutil/hwcontext_qsv: Command line using hwaccel 'QSV' doesn't work

2017-01-07 Thread Mark Thompson
On 06/01/17 06:37, Huang, Zhengxu wrote:
> Hi
> 
> According to the suggestion update the patch.
> 
> thanks.
> 
> 
> From 4beadd3c84c797a56c4f375458d0a1e9d9b233c8 Mon Sep 17 00:00:00 2001
> From: Zhengxu 
> Date: Thu, 5 Jan 2017 14:48:06 +0800
> Subject: [PATCH] ffmpeg_qsv: Add an option "qsv_child_device" to choose an
>  proper node for QSV child device(vaapi or dxva2).
> 
> Reason: For some cases, such as 2 or more graphics card existing, the
> default command line may fail because ffmpeg open a wrong device node:
> ffmpeg -hwaccel qsv -c:v h264_qsv -i test.264 -c:v h264_qsv out.264
> Let user to choose the proper one by running like below:
> ffmpeg -hwaccel qsv -qsv_child_device /dev/dri/renderD128 -c:v h264_qsv \
> -i test.264 -c:v h264_qsv out.264

I think it should just be "qsv_device".  It is the device being used by qsv, 
the fact that it is a child device of another kind (dxva or vaapi) wrapped 
inside the hwcontext infrastructure isn't really relevant to the ffmpeg utility.

> 
> Signed-off-by: ChaoX A Liu 
> Signed-off-by: Huang, Zhengxu 
> Signed-off-by: Andrew, Zhang 
> ---
>  ffmpeg.h |  3 +++
>  ffmpeg_opt.c |  5 +
>  ffmpeg_qsv.c | 11 ++-
>  3 files changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/ffmpeg.h b/ffmpeg.h
> index ebe5bf0..91a 100644
> --- a/ffmpeg.h
> +++ b/ffmpeg.h
> @@ -602,6 +602,9 @@ extern const OptionDef options[];
>  extern const HWAccel hwaccels[];
>  extern int hwaccel_lax_profile_check;
>  extern AVBufferRef *hw_device_ctx;
> +#if CONFIG_QSV
> +extern char *qsv_child_device;
> +#endif
>  
>  
>  void term_init(void);
> diff --git a/ffmpeg_opt.c b/ffmpeg_opt.c
> index 6862456..7fd08a2 100644
> --- a/ffmpeg_opt.c
> +++ b/ffmpeg_opt.c
> @@ -3679,5 +3679,10 @@ const OptionDef options[] = {
>  "set VAAPI hardware device (DRM path or X11 display name)", "device" 
> },
>  #endif
>  
> +#if CONFIG_QSV
> +{ "qsv_child_device", HAS_ARG | OPT_STRING | OPT_EXPERT, { 
> &qsv_child_device },
> +"set QSV child device (DRM path or DXVA)", "device"},

An X11 display will still work here.  Also, maybe clarify that the DXVA case is 
a DirectX adapter index?

"set QSV hardware device (DirectX adapter index, DRM path or X11 display 
name)", say.

> +#endif
> +
>  { NULL, },
>  };
> diff --git a/ffmpeg_qsv.c b/ffmpeg_qsv.c
> index 68ff5bd..5a6db20 100644
> --- a/ffmpeg_qsv.c
> +++ b/ffmpeg_qsv.c
> @@ -28,6 +28,8 @@
>  
>  #include "ffmpeg.h"
>  
> +char *qsv_child_device = NULL;
> +
>  static int qsv_get_buffer(AVCodecContext *s, AVFrame *frame, int flags)
>  {
>  InputStream *ist = s->opaque;
> @@ -44,9 +46,16 @@ static void qsv_uninit(AVCodecContext *s)
>  static int qsv_device_init(InputStream *ist)
>  {
>  int err;
> +AVDictionary *dict = NULL;
> +
> +if (qsv_child_device) {
> +err = av_dict_set(&dict, "child_device", qsv_child_device, 0);
> +if (err < 0)
> +return err;
> +}
>  
>  err = av_hwdevice_ctx_create(&hw_device_ctx, AV_HWDEVICE_TYPE_QSV,
> - ist->hwaccel_device, NULL, 0);
> + ist->hwaccel_device, dict, 0);
>  if (err < 0) {
>  av_log(NULL, AV_LOG_ERROR, "Error creating a QSV device\n");
>  return err;
> -- 
> 1.8.3.1
> 

Patch otherwise ok.

Thanks,

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


Re: [FFmpeg-devel] [PATCH][WIP][RFC] Add support for MPEG-4 Simple Studio Profile

2017-01-07 Thread Michael Niedermayer
On Sat, Jan 07, 2017 at 10:35:43PM +, Kieran Kunhya wrote:
> Hi,
> 
> I have added support for MPEG-4 Sstp using the available samples on trac.
> Yes it doesn't pass fate, yes it's not format-patch, yes it uses printfs.
> https://trac.ffmpeg.org/ticket/4447
> 
> Being MPEG-4, it depends on mpegvideo.c so has tons of yuv420p assumptions
> baked in which are of course undocumented.
> Here are my questions (line number refers to attached patch):
> 
> Line 35: How do I signal to this idctdsp thing that I want an idct with
> 32-bit coefficients AND intermediates? A lot of that code has assumptions
> that intermediates will be the next size up, i.e 8-bit coeffs, 16-bit
> intermediates, or 16-bit coeffs and 32-bit intermediates.
> 

> Line 906: Why do RGB samples not decode unless the GBRP format is moved to
> the top of PIX_FMT. I get "[mpeg4 @ 0x7f945c029600] format change not
> supported" otherwise.

The format is choosen by the AVCodecContext.get_format() callback
from the list of choices a decoder presents to it.
if you present yuv420 as a choice it can pick that.
the current code will present the full list from the AVCodec as is
and the first in the list is supposed to be the best choice so it
likely will be choosen


> 
> Line 932: What's going on with this branch. Normal mpeg-4 video does
> dequant during unpack, why is it not part of this condition?

mpeg-4 uses DC/AC prediction in intra blocks, this prediction is done
before dequantization so doing dequant during block decode becomes
messy, its no longer just the non zero coded coeffs that need dequant

and as the dequant codepath is needed for encode already, using it for
intra blocks too is the cleanest solution. Doing decode+pred+dequant
in one would be probably rather ugly


> 
> Line 987: Are there more assumptions baked into mpegvideo.c about "square"
> macroblocks, i.e ones where (width == height)?

a code "audit" would tell if there are more such assumptions, i dont
know of the top of my head, its too long ago


> 
> Line 997: What is all this stuff going on with -1U, unless I remove this I
> get a segfault. I do get a stripe on the left though.

IIRC the 1U compensates the effect of ff_update_block_index()
block_size in ff_update_block_index() looks wrong after your patch

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

Republics decline into democracies and democracies degenerate into
despotisms. -- Aristotle


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


Re: [FFmpeg-devel] [PATCH] avformat/hlsenc: start_number new options

2017-01-07 Thread Steven Liu
2017-01-08 1:37 GMT+08:00 Bodecs Bela :

>
>
> 2017.01.07. 0:32 keltezéssel, Steven Liu írta:
>
>> 2017-01-07 0:47 GMT+08:00 Bodecs Bela :
>>
>>
>>> 2017.01.06. 17:33 keltezéssel, Steven Liu írta:
>>>
>>> 2017-01-07 0:22 GMT+08:00 Bodecs Bela :


 2017.01.06. 16:50 keltezéssel, Steven Liu írta:
>
> 2017-01-06 22:07 GMT+08:00 Bodecs Bela :
>
>> Dear All,
>>
>> in avformat/hlsenc the start_number option starts the playlist
>>> sequence
>>> number
>>> (#EXT-X-MEDIA-SEQUENCE) from the specified number. Unless hls_flags
>>> single_file is set, it also specifies starting sequence numbers of
>>> segment and subtitle filenames. Sometimes it is usefull to have
>>> unique
>>> starting numbers at each run, but currently it is only achiveable by
>>> setting this parameter manually.
>>> This patch enables to set start_number parameter automatically for
>>> practically unique numbers. If start_number is set to -1, then
>>> the start number will be the seconds since epoch (1970-01-01
>>> 00:00:00).
>>> If set to -2, then the start number will be based on the current
>>> date/time value as mmddHHMMSS. e.g. 20161231235659.
>>>
>>>
>>> thank you,
>>>
>>> Bela Bodecs
>>>
>>>
>>> ___
>>> ffmpeg-devel mailing list
>>> ffmpeg-devel@ffmpeg.org
>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>
>>>
>>> Two question:
>>>
>>> 1. char b[21];   Why this is 21 ?
>>
>> you are right, 15 is enough.
>>
> 2. +{"start_number",  "set first number in the sequence",
>
>> OFFSET(start_sequence),AV_OPT_TYPE_INT64,  {.i64 = 0}, -2,
>> INT64_MAX,
>> E},
>> Why is this -2 and the help message maybe need more infomation, for
>> example
>> -2 mean -1 mean  0 mean, and default value.
>>
>> yes, I have altered now but I have written verbosly into the doc
>>
> (muxers.texi), here:
>
> +If set to -1, then the start number will be the seconds since epoch
> (1970-01-01 00:00:00).
> +If set to -2, then the start number will be based on the current
> date/time as mmddHHMMSS. e.g. 20161231235759.
> +Default value is 0.
>
> ___
>
> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> I have enclosed a fixed version. A have changed some code, where
>> greater
>>
> than 32 bit long sequence numbers were not handled correctly.
> (av_get_frame_filename2)
>
> thank you.
> Bela Bodecs
>
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>
> +{"start_number",  "set first number in the sequence, 0 is default,
>
 -1:
 second since epoch, -2: current datetime as MMDDhhmmss, actual value
 otherwise", OFFSET(start_sequence),AV_OPT_TYPE_INT64,  {.i64 = 0},
   -2,
 INT64_MAX, E},

 I have check this option, i think add flag to control the start_number
 maybe better,
 for example:
 hls_flags
 hls_playlist_type

 maybe add a start_number_flags is better, What about you think?

 Using hls_flags is not enough to specify different values for them.
>>>
>>> NO, i am not mean use hls_flags, i mean you can creat a new flags,
>>
>> start_number_flags
>>   generic
>>   epoch
>>   datetime
>>
> Ok, I see it. May I implement it?
>
>
yes, of course ;-)

>
>
>> I thought that there should be 3 options beside this start_number option.
>>
>>> hls_start_number_playlist, hls_start_number_segment and
>>> hls_start_number_vtt
>>> Using start_number and any of the new 3 ones would be mutualy exlusive.
>>>
>>> This way anybody could use the old option (start_number) and it won't
>>> break the current behaviour.
>>> But those who want to have finer control, they may use the new options.
>>>
>>> of course -start_number x  has the same effect as using
>>> -hls_start_number_playlist x -hls_start_number_segment x
>>> -hls_start_number_vtt x
>>>
>>>
>>>
>>> ___
>>>
 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 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] ffserver: local OOB write with custom program name

2017-01-07 Thread Tobias Stoeckmann
When the command line for children is created, it is assumed that
my_program_name always ends with "ffserver", which doesn't have to
be true if ffserver is called through a symbolic link.

In such a case, it could be that not enough space for "ffmpeg" is
available at the end, leading to a buffer overflow.

One example would be:

$ ln -s /usr/bin/ffserver ~/f; ~/f

As this is only a local buffer overflow, i.e. is based on a weird
program call, this has NO security impact.
---
 ffserver.c | 20 +++-
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/ffserver.c b/ffserver.c
index 02a583464b..8b819b6934 100644
--- a/ffserver.c
+++ b/ffserver.c
@@ -495,20 +495,22 @@ static void start_children(FFServerStream *feed)
 return;
 }
 
-pathname = av_strdup (my_program_name);
+slash = strrchr(my_program_name, '/');
+if (!slash) {
+pathname = av_mallocz(sizeof("ffmpeg"));
+} else {
+pathname = av_mallocz(slash - my_program_name + sizeof("ffmpeg"));
+if (pathname != NULL) {
+memcpy(pathname, my_program_name, slash - my_program_name);
+}
+}
 if (!pathname) {
 http_log("Could not allocate memory for children cmd line\n");
 return;
 }
-   /* replace "ffserver" with "ffmpeg" in the path of current
-* program. Ignore user provided path */
+   /* use "ffmpeg" in the path of current program. Ignore user provided path */
 
-slash = strrchr(pathname, '/');
-if (!slash)
-slash = pathname;
-else
-slash++;
-strcpy(slash, "ffmpeg");
+strcat(pathname, "ffmpeg");
 
 for (; feed; feed = feed->next) {
 
-- 
2.11.0

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


[FFmpeg-devel] [PATCH] Never unmap unallocated space

2017-01-07 Thread Tobias Stoeckmann
The function av_file_map can use mmap to memory-map the content of a
file into address space. Just like every other alternative, mmap() could
fail due to various reasons, e.g. if not enough address space is
available.

Unfortunately, av_file_map writes the requested size even on error into
the supplied size pointer, which will eventually passed to munmap.

As the base pointer will be NULL, the first 'size' bytes of the address
space will be unmapped, which will most likely hit a lot of libraries.
If that happens, a segmentation fault is most likely to occur.

This example can trigger the issue on 32 bit systems. Adjust the seek
value if necessary (free memory < seek_size < physical RAM installed):

$ uname -m
i686
$ dd if=/dev/zero of=large bs=1 count=1 seek=30
$ ffmpeg -f lavfi -i life=filename=large
Error occurred in mmap(): Cannot allocate memory
Error initializing filter 'life' with args 'filename=large'
Segmentation fault
$ _

---
I chose to set *size back to 0 in error-cases just because each error
case already handles close(fd), too. Adding a new variable and setting
*size at the end would have introduced too much noise in this diff.

Feel free to adjust. :)
---
 libavutil/file.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/libavutil/file.c b/libavutil/file.c
index 7bdf6cde84..8e12efe3ce 100644
--- a/libavutil/file.c
+++ b/libavutil/file.c
@@ -87,6 +87,7 @@ int av_file_map(const char *filename, uint8_t **bufptr, 
size_t *size,
 err = AVERROR(errno);
 av_strerror(err, errbuf, sizeof(errbuf));
 av_log(&file_log_ctx, AV_LOG_ERROR, "Error occurred in mmap(): %s\n", 
errbuf);
+*size = 0;
 close(fd);
 return err;
 }
@@ -98,6 +99,7 @@ int av_file_map(const char *filename, uint8_t **bufptr, 
size_t *size,
 mh = CreateFileMapping(fh, NULL, PAGE_READONLY, 0, 0, NULL);
 if (!mh) {
 av_log(&file_log_ctx, AV_LOG_ERROR, "Error occurred in 
CreateFileMapping()\n");
+*size = 0;
 close(fd);
 return -1;
 }
@@ -106,6 +108,7 @@ int av_file_map(const char *filename, uint8_t **bufptr, 
size_t *size,
 CloseHandle(mh);
 if (!ptr) {
 av_log(&file_log_ctx, AV_LOG_ERROR, "Error occurred in 
MapViewOfFile()\n");
+*size = 0;
 close(fd);
 return -1;
 }
@@ -116,6 +119,7 @@ int av_file_map(const char *filename, uint8_t **bufptr, 
size_t *size,
 *bufptr = av_malloc(*size);
 if (!*bufptr) {
 av_log(&file_log_ctx, AV_LOG_ERROR, "Memory allocation error 
occurred\n");
+*size = 0;
 close(fd);
 return AVERROR(ENOMEM);
 }
-- 
2.11.0

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


Re: [FFmpeg-devel] [PATCH][WIP][RFC] Add support for MPEG-4 Simple Studio Profile

2017-01-07 Thread Kieran Kunhya
On Sat, 7 Jan 2017 at 23:43 Michael Niedermayer 
wrote:

> On Sat, Jan 07, 2017 at 10:35:43PM +, Kieran Kunhya wrote:
> > Hi,
> >
> > I have added support for MPEG-4 Sstp using the available samples on trac.
> > Yes it doesn't pass fate, yes it's not format-patch, yes it uses printfs.
> > https://trac.ffmpeg.org/ticket/4447
> >
> > Being MPEG-4, it depends on mpegvideo.c so has tons of yuv420p
> assumptions
> > baked in which are of course undocumented.
> > Here are my questions (line number refers to attached patch):
> >
> > Line 35: How do I signal to this idctdsp thing that I want an idct with
> > 32-bit coefficients AND intermediates? A lot of that code has assumptions
> > that intermediates will be the next size up, i.e 8-bit coeffs, 16-bit
> > intermediates, or 16-bit coeffs and 32-bit intermediates.
> >
>
> > Line 906: Why do RGB samples not decode unless the GBRP format is moved
> to
> > the top of PIX_FMT. I get "[mpeg4 @ 0x7f945c029600] format change not
> > supported" otherwise.
>
> The format is choosen by the AVCodecContext.get_format() callback
> from the list of choices a decoder presents to it.
> if you present yuv420 as a choice it can pick that.
> the current code will present the full list from the AVCodec as is
> and the first in the list is supposed to be the best choice so it
> likely will be choosen
>

Why does it ignore the pix_fmt I tell it to use?

>
> > Line 932: What's going on with this branch. Normal mpeg-4 video does
> > dequant during unpack, why is it not part of this condition?
>
> mpeg-4 uses DC/AC prediction in intra blocks, this prediction is done
> before dequantization so doing dequant during block decode becomes
> messy, its no longer just the non zero coded coeffs that need dequant
>
> and as the dequant codepath is needed for encode already, using it for
> intra blocks too is the cleanest solution. Doing decode+pred+dequant
> in one would be probably rather ugly
>
>
> >
> > Line 987: Are there more assumptions baked into mpegvideo.c about
> "square"
> > macroblocks, i.e ones where (width == height)?
>
> a code "audit" would tell if there are more such assumptions, i dont
> know of the top of my head, its too long ago
>
>
> >
> > Line 997: What is all this stuff going on with -1U, unless I remove this
> I
> > get a segfault. I do get a stripe on the left though.
>
> IIRC the 1U compensates the effect of ff_update_block_index()
> block_size in ff_update_block_index() looks wrong after your patch
>

Well I segfault with the -1U so what do I do? How can this happen?

Kieran


>
> [...]
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Republics decline into democracies and democracies degenerate into
> despotisms. -- Aristotle
> ___
> 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] ffserver: local OOB write with custom program name

2017-01-07 Thread Michael Niedermayer
On Fri, Jan 06, 2017 at 11:33:16PM +0100, Tobias Stoeckmann wrote:
> When the command line for children is created, it is assumed that
> my_program_name always ends with "ffserver", which doesn't have to
> be true if ffserver is called through a symbolic link.
> 
> In such a case, it could be that not enough space for "ffmpeg" is
> available at the end, leading to a buffer overflow.
> 
> One example would be:
> 
> $ ln -s /usr/bin/ffserver ~/f; ~/f
> 
> As this is only a local buffer overflow, i.e. is based on a weird
> program call, this has NO security impact.
> ---
>  ffserver.c | 20 +++-
>  1 file changed, 11 insertions(+), 9 deletions(-)

applied

thx

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

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


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


Re: [FFmpeg-devel] [PATCH][WIP][RFC] Add support for MPEG-4 Simple Studio Profile

2017-01-07 Thread Michael Niedermayer
On Sun, Jan 08, 2017 at 01:00:31AM +, Kieran Kunhya wrote:
> On Sat, 7 Jan 2017 at 23:43 Michael Niedermayer 
> wrote:
> 
> > On Sat, Jan 07, 2017 at 10:35:43PM +, Kieran Kunhya wrote:
> > > Hi,
> > >
> > > I have added support for MPEG-4 Sstp using the available samples on trac.
> > > Yes it doesn't pass fate, yes it's not format-patch, yes it uses printfs.
> > > https://trac.ffmpeg.org/ticket/4447
> > >
> > > Being MPEG-4, it depends on mpegvideo.c so has tons of yuv420p
> > assumptions
> > > baked in which are of course undocumented.
> > > Here are my questions (line number refers to attached patch):
> > >
> > > Line 35: How do I signal to this idctdsp thing that I want an idct with
> > > 32-bit coefficients AND intermediates? A lot of that code has assumptions
> > > that intermediates will be the next size up, i.e 8-bit coeffs, 16-bit
> > > intermediates, or 16-bit coeffs and 32-bit intermediates.
> > >
> >
> > > Line 906: Why do RGB samples not decode unless the GBRP format is moved
> > to
> > > the top of PIX_FMT. I get "[mpeg4 @ 0x7f945c029600] format change not
> > > supported" otherwise.
> >
> > The format is choosen by the AVCodecContext.get_format() callback
> > from the list of choices a decoder presents to it.
> > if you present yuv420 as a choice it can pick that.
> > the current code will present the full list from the AVCodec as is
> > and the first in the list is supposed to be the best choice so it
> > likely will be choosen
> >
> 
> Why does it ignore the pix_fmt I tell it to use?

its likely set by h263_get_format() calling ff_get_format()
calling avctx->get_format() / avcodec_default_get_format()

I would try to pass the correct list of pixel formats to
ff_get_format(), that is only 422 formats if its 422, only 444 formats
if its 444, only rgb formats if its rgb [it could be multiple because
of hw decoders]


> 
> >
> > > Line 932: What's going on with this branch. Normal mpeg-4 video does
> > > dequant during unpack, why is it not part of this condition?
> >
> > mpeg-4 uses DC/AC prediction in intra blocks, this prediction is done
> > before dequantization so doing dequant during block decode becomes
> > messy, its no longer just the non zero coded coeffs that need dequant
> >
> > and as the dequant codepath is needed for encode already, using it for
> > intra blocks too is the cleanest solution. Doing decode+pred+dequant
> > in one would be probably rather ugly
> >
> >
> > >
> > > Line 987: Are there more assumptions baked into mpegvideo.c about
> > "square"
> > > macroblocks, i.e ones where (width == height)?
> >
> > a code "audit" would tell if there are more such assumptions, i dont
> > know of the top of my head, its too long ago
> >
> >
> > >
> > > Line 997: What is all this stuff going on with -1U, unless I remove this
> > I
> > > get a segfault. I do get a stripe on the left though.
> >
> > IIRC the 1U compensates the effect of ff_update_block_index()
> > block_size in ff_update_block_index() looks wrong after your patch
> >
> 
> Well I segfault with the -1U so what do I do? How can this happen?

ff_update_block_index() must update dest by the correct size of the
macroblock
that is unless iam missing something, which is not impossible


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