[FFmpeg-devel] [PATCH 1/2] avcodec/h264_slice: Fix overflow in slice offset

2017-08-03 Thread Michael Niedermayer 
Fixes: runtime error: signed integer overflow: 1610612736 * 2 cannot be 
represented in type 'int'
Fixes: 2817/clusterfuzz-testcase-minimized-5289691240726528

Found-by: continuous fuzzing process 
https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer 
---
 libavcodec/h264_slice.c | 16 +---
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c
index 2fb89b189d..4e7eba4adb 100644
--- a/libavcodec/h264_slice.c
+++ b/libavcodec/h264_slice.c
@@ -1854,17 +1854,19 @@ static int h264_slice_header_parse(const H264Context 
*h, H264SliceContext *sl,
 sl->deblocking_filter ^= 1;  // 1<->0
 
 if (sl->deblocking_filter) {
-sl->slice_alpha_c0_offset = get_se_golomb(>gb) * 2;
-sl->slice_beta_offset = get_se_golomb(>gb) * 2;
-if (sl->slice_alpha_c0_offset >  12 ||
-sl->slice_alpha_c0_offset < -12 ||
-sl->slice_beta_offset >  12 ||
-sl->slice_beta_offset < -12) {
+int slice_alpha_c0_offset_div2 = get_se_golomb(>gb);
+int slice_beta_offset_div2 = get_se_golomb(>gb);
+if (slice_alpha_c0_offset_div2 >  6 ||
+slice_alpha_c0_offset_div2 < -6 ||
+slice_beta_offset_div2 >  6 ||
+slice_beta_offset_div2 < -6) {
 av_log(h->avctx, AV_LOG_ERROR,
"deblocking filter parameters %d %d out of range\n",
-   sl->slice_alpha_c0_offset, sl->slice_beta_offset);
+   slice_alpha_c0_offset_div2, slice_beta_offset_div2);
 return AVERROR_INVALIDDATA;
 }
+sl->slice_alpha_c0_offset = slice_alpha_c0_offset_div2 * 2;
+sl->slice_beta_offset = slice_beta_offset_div2 * 2;
 }
 }
 
-- 
2.13.0

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

[FFmpeg-devel] [PATCH 2/2] avcodec/aacdec_fixed: fix invalid shift in predict()

2017-08-03 Thread Michael Niedermayer 
Fixes: runtime error: shift exponent -2 is negative
Fixes: 2818/clusterfuzz-testcase-minimized-5062943676825600

Found-by: continuous fuzzing process 
https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer 
---
 libavcodec/aacdec_fixed.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/libavcodec/aacdec_fixed.c b/libavcodec/aacdec_fixed.c
index ccfef7f652..f6a533010f 100644
--- a/libavcodec/aacdec_fixed.c
+++ b/libavcodec/aacdec_fixed.c
@@ -305,8 +305,12 @@ static av_always_inline void predict(PredictorState *ps, 
int *coef,
 if (output_enable) {
 int shift = 28 - pv.exp;
 
-if (shift < 31)
-*coef += (pv.mant + (1 << (shift - 1))) >> shift;
+if (shift < 31) {
+if (shift > 0) {
+*coef += (pv.mant + (1 << (shift - 1))) >> shift;
+} else
+*coef += pv.mant << -shift;
+}
 }
 
 e0 = av_int2sf(*coef, 2);
-- 
2.13.0

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

Re: [FFmpeg-devel] [PATCH] JPEG2000 encoding with variable codeblock size

2017-08-03 Thread Michael Niedermayer 
On Thu, Aug 03, 2017 at 03:58:22PM +0200, france...@bltitalia.com wrote:
> From: Francesco Cuzzocrea 
> 
> Hi to all
> I've made some simple changes tha allow encoding with variable codeblock
> size.  Default value are the same as previous (16X16) but now setting them
> to 64x128 make generated codestream compatible with Analog Devices ADV212
> video codec.
> 
> 
> ---
>  libavcodec/j2kenc.c | 22 +-
>  1 file changed, 13 insertions(+), 9 deletions(-)
> 
> diff --git a/libavcodec/j2kenc.c b/libavcodec/j2kenc.c
> index c8d3861..1bd4fbd 100644
> --- a/libavcodec/j2kenc.c
> +++ b/libavcodec/j2kenc.c
> @@ -1178,17 +1178,21 @@ static int j2kenc_destroy(AVCodecContext *avctx)
>  // taken from the libopenjpeg wraper so it matches
>  
>  #define OFFSET(x) offsetof(Jpeg2000EncoderContext, x)
> +#define OFFSET1(x) offsetof(Jpeg2000CodingStyle, x)
> +
> +
>  #define VE AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_ENCODING_PARAM
>  static const AVOption options[] = {
> -{ "format","Codec Format",  OFFSET(format),
> AV_OPT_TYPE_INT,   { .i64 = CODEC_JP2   }, CODEC_J2K, CODEC_JP2,   VE, 
> "format"  },
> -{ "j2k",   NULL,0, 
> AV_OPT_TYPE_CONST, { .i64 = CODEC_J2K   }, 0, 0,   VE, 
> "format"  },
> -{ "jp2",   NULL,0, 
> AV_OPT_TYPE_CONST, { .i64 = CODEC_JP2   }, 0, 0,   VE, 
> "format"  },
> -{ "tile_width","Tile Width",OFFSET(tile_width),
> AV_OPT_TYPE_INT,   { .i64 = 256 }, 1, 1<<30,   VE, },
> -{ "tile_height",   "Tile Height",   OFFSET(tile_height),   
> AV_OPT_TYPE_INT,   { .i64 = 256 }, 1, 1<<30,   VE, },
> -{ "pred",  "DWT Type",  OFFSET(pred),  
> AV_OPT_TYPE_INT,   { .i64 = 0   }, 0, 1,   VE, "pred" 
>},
> -{ "dwt97int",  NULL,0, 
> AV_OPT_TYPE_CONST, { .i64 = 0   }, INT_MIN, INT_MAX,   VE, "pred" 
>},
> -{ "dwt53", NULL,0, 
> AV_OPT_TYPE_CONST, { .i64 = 0   }, INT_MIN, INT_MAX,   VE, "pred" 
>},
> -
> +{ "format",   "Codec Format",   OFFSET(format),  
>  AV_OPT_TYPE_INT,   { .i64 = CODEC_JP2   }, CODEC_J2K, 
> CODEC_JP2,   VE, "format"  },
> +{ "j2k",   NULL,0,   
>  AV_OPT_TYPE_CONST, { .i64 = CODEC_J2K   }, 0, 0, 
>   VE, "format"  },
> +{ "jp2",   NULL,0,   
>  AV_OPT_TYPE_CONST, { .i64 = CODEC_JP2   }, 0, 0, 
>   VE, "format"  },
> +{ "tile_width","Tile Width",OFFSET(tile_width),  
>  AV_OPT_TYPE_INT,   { .i64 = 256 }, 1, 1<<30, 
>   VE, },
> +{ "tile_height",   "Tile Height",   OFFSET(tile_height), 
>  AV_OPT_TYPE_INT,   { .i64 = 256 }, 1, 1<<30, 
>   VE, },
> +{ "pred",  "DWT Type",  OFFSET(pred),
>  AV_OPT_TYPE_INT,   { .i64 = 0   }, 0, 1, 
>   VE, "pred"},
> +{ "dwt97int",  NULL,0,   
>  AV_OPT_TYPE_CONST, { .i64 = 0   }, INT_MIN, INT_MAX, 
>   VE, "pred"},
> +{ "dwt53", NULL,0,   
>  AV_OPT_TYPE_CONST, { .i64 = 0   }, INT_MIN, INT_MAX, 
>   VE, "pred"},

> +{ "log2_cblk_width",   "Codeblock Width",   
> (OFFSET(codsty)+OFFSET1(log2_cblk_width)),AV_OPT_TYPE_INT,   { .i64 = 4   
> }, 1, 1<<30,   VE, },
> +{ "log2_cblk_height",  "Codeblock Height",  
> (OFFSET(codsty)+OFFSET1(log2_cblk_height)),   AV_OPT_TYPE_INT,   { .i64 = 4   
> }, 1, 1<<30,   VE, },

OFFSET(codsty.log2_cblk_width) is more robust than adding offsets
the maximum also is certainly not 2^(2^30)

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

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


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

Re: [FFmpeg-devel] [PATCH v2] avfilter/vf_ssim: fix temp size calculation

2017-08-03 Thread Muhammad Faiz 
On Thu, Aug 3, 2017 at 1:53 PM, Tobias Rapp  wrote:
> On 03.08.2017 03:03, Muhammad Faiz wrote:
>>
>> Also use av_mallocz_array.
>> Fix Ticket6519.
>>
>> Signed-off-by: Muhammad Faiz 
>> ---
>>  libavfilter/vf_ssim.c | 8 +---
>>  1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/libavfilter/vf_ssim.c b/libavfilter/vf_ssim.c
>> index c3c204268f..d8c049177c 100644
>> --- a/libavfilter/vf_ssim.c
>> +++ b/libavfilter/vf_ssim.c
>> @@ -219,6 +219,8 @@ static float ssim_endn_8bit(const int (*sum0)[4],
>> const int (*sum1)[4], int widt
>>  return ssim;
>>  }
>>
>> +#define SUM_LEN(w) (((w) >> 2) + 3)
>> +
>>  static float ssim_plane_16bit(SSIMDSPContext *dsp,
>>uint8_t *main, int main_stride,
>>uint8_t *ref, int ref_stride,
>> @@ -228,7 +230,7 @@ static float ssim_plane_16bit(SSIMDSPContext *dsp,
>>  int z = 0, y;
>>  float ssim = 0.0;
>>  int64_t (*sum0)[4] = temp;
>> -int64_t (*sum1)[4] = sum0 + (width >> 2) + 3;
>> +int64_t (*sum1)[4] = sum0 + SUM_LEN(width);
>>
>>  width >>= 2;
>>  height >>= 2;
>> @@ -256,7 +258,7 @@ static float ssim_plane(SSIMDSPContext *dsp,
>>  int z = 0, y;
>>  float ssim = 0.0;
>>  int (*sum0)[4] = temp;
>> -int (*sum1)[4] = sum0 + (width >> 2) + 3;
>> +int (*sum1)[4] = sum0 + SUM_LEN(width);
>>
>>  width >>= 2;
>>  height >>= 2;
>> @@ -402,7 +404,7 @@ static int config_input_ref(AVFilterLink *inlink)
>>  for (i = 0; i < s->nb_components; i++)
>>  s->coefs[i] = (double) s->planeheight[i] * s->planewidth[i] /
>> sum;
>>
>> -s->temp = av_malloc_array((2 * inlink->w + 12), sizeof(*s->temp) * (1
>> + (desc->comp[0].depth > 8)));
>> +s->temp = av_mallocz_array(2 * SUM_LEN(inlink->w),
>> (desc->comp[0].depth > 8) ? sizeof(int64_t[4]) : sizeof(int[4]));
>>  if (!s->temp)
>>  return AVERROR(ENOMEM);
>>  s->max = (1 << desc->comp[0].depth) - 1;
>>
>
> Fixes the crashing command and runs without reports in Valgrind.

Applied.

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

Re: [FFmpeg-devel] [Patch] nvenc : Add P016 support

2017-08-03 Thread Philip Langdale 
On Wed, 26 Jul 2017 09:14:04 +
Philip Langdale  wrote:

> On 2017-07-25 08:33, Yogender Gupta wrote:
> >>> I can push, but to answer Timo's question: I did not add P016 to 
> >>> nvenc because it is technically not correct to pass 12bit content 
> >>> through a 10bit surface - that will lead to truncation, rather
> >>> than dithering. If you are telling us that nvenc supports 12bit
> >>> >> input and will correctly dither, then great. Otherwise, it's
> >>> >> more correct 
> >>> to not support and force dithering to happen through swscale. You 
> >>> could also write cuda accelerated dithering.  
> > 
> > I have dropped the NVENC patch for now (we will build a P016 to P010
> > CUDA dithering filter). Attached the other patches, also taking into
> > account some code formatting comments.
> > 
> > Thanks,
> > Yogender  
> 
> Hi Yogender,
> 
> That's great.
> 
> Your patches look fine, although they need micro version bumps in 
> libavfilter and libavutil.
> I've added those to my local copy of the patches, but I know realise
> I can't push either,
> as I'm travelling for another week. I'll push them when I get back.

Pushed.

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

Re: [FFmpeg-devel] [PATCH] hwupload_cuda : Add 10/16 bit format support

2017-08-03 Thread Philip Langdale 
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On Wed, 19 Jul 2017 17:46:30 +0200
Michael Niedermayer  wrote:

> On Tue, Jul 18, 2017 at 10:34:20AM +, Yogender Gupta wrote:
> > Added 10/16bit formats to hwupload_cuda.
> > 
> > Thanks,
> > Yogender
> > 
> > ---
> > This email message is for the sole use of the intended recipient(s)
> > and may contain confidential information.  Any unauthorized review,
> > use, disclosure or distribution is prohibited.  If you are not the
> > intended recipient, please contact the sender by reply email and
> > destroy all copies of the original message.
> > ---
> >   
> 
> >  vf_hwupload_cuda.c |2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 49188f1bd1da5d94a1236c4da55a2609e7906f36
> > 0001-hwupload_cuda-Add-10-16-bit-format-support.patch From
> > 0c780f59188edcef25a43aa6e6f375b51b87c22b Mon Sep 17 00:00:00 2001
> > From: Yogender Gupta  Date: Tue, 18 Jul 2017
> > 16:01:02 +0530 Subject: [PATCH] hwupload_cuda : Add 10/16 bit
> > format support
> > 
> > ---
> >  libavfilter/vf_hwupload_cuda.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/libavfilter/vf_hwupload_cuda.c
> > b/libavfilter/vf_hwupload_cuda.c index ef98233..fe872d8 100644
> > --- a/libavfilter/vf_hwupload_cuda.c
> > +++ b/libavfilter/vf_hwupload_cuda.c
> > @@ -58,7 +58,7 @@ static int
> > cudaupload_query_formats(AVFilterContext *ctx) 
> >  static const enum AVPixelFormat input_pix_fmts[] = {
> >  AV_PIX_FMT_NV12, AV_PIX_FMT_YUV420P, AV_PIX_FMT_YUV444P,
> > -AV_PIX_FMT_NONE,
> > +AV_PIX_FMT_P010, AV_PIX_FMT_P016, AV_PIX_FMT_YUV444P16,
> > AV_PIX_FMT_NONE,  
> 
> I suggest to keep AV_PIX_FMT_NONE, on the last line alone
> that way patches adding formats at the end dont need to change it
> 
> [...]
> 

Pushed with that changed.

Thanks,

- --phil
-BEGIN PGP SIGNATURE-

iEYEARECAAYFAlmDpNcACgkQ+NaxlGp1aC6HVACeLECFdRfj+32cl9Jy6mr5TPdE
HiYAnRZhC6WDwYp+C+Q4yuOIqM/AZLxw
=QaYA
-END PGP SIGNATURE-
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [DISCUSSION] Motion Estimation API/Library

2017-08-03 Thread Ronald S. Bultje 
Hi,

On Thu, Aug 3, 2017 at 5:32 PM, Davinder Singh  wrote:

> On Tue, Aug 1, 2017 at 7:40 AM Davinder Singh  wrote:
>
> > [...]
> >
>
> Keeping where the code lives, aside,
>
> main thing is API, so we need to talk about it.


You're assuming that we know in advance what the API will have to do. I
don't think that's the case.

For example, what block sizes does it operate on? me_cc doesn't work for
64x64 blocks in HEVC, VP9 (and AV1?).

Or what if we don't use VLC coding, but some arithmetic variation? Like all
H.264, VP8-9, HEVC (and AV1?). Where perhaps the arithmetic coding can
depend not just on intra/inter, but also on transform size, chroma/luma,
etc.

What if qscale is not uniform and allows per-coefficient matrix variations?
How many sub-types of such variations will exist?

My point is: designing it in advance for everything like this is insane.
Just copy what is there, and prepare for the API to change _all the time_
while we're molding it to fit in other problem shapes.

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

Re: [FFmpeg-devel] [DISCUSSION] Motion Estimation API/Library

2017-08-03 Thread Davinder Singh 
On Tue, Aug 1, 2017 at 7:40 AM Davinder Singh  wrote:

> [...]
>

Keeping where the code lives, aside,

main thing is API, so we need to talk about it.
-- 
Davinder Singh
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH]v6 Opus Pyramid Vector Quantization Search in x86 SIMD asm

2017-08-03 Thread Ivan Kalvachev 
On 8/2/17, Henrik Gramner  wrote:
> On Tue, Aug 1, 2017 at 11:46 PM, Ivan Kalvachev 
> wrote:
>> On 7/31/17, Henrik Gramner  wrote:
>>> Use rN instead of rNq for numbered registers (q suffix is used for
>>> named args only due to preprocessor limitations).
>>
>> Is this documented?
>
> Not sure, but there's probably a bunch small things like this that
> aren't really well documented.
>
>>> Use the same "standard" vertical alignment rules as most existing
>>> code, e.g. instructions indented by 4 spaces and operands aligned on
>>> the first comma.
>>
>> What do you mean operands aligned on the first comma?
>> The longest operand I had is "xmm0" , counting comma and space
>> I get 6 position alignment for operands.
>> Now, with "xm7" i can lower this to 5 position. Should I do that?
>> (I don't have "xm15" ).
>>
>
> 1234_1234_1234_123
> VBROADCASTSS ym1, xm1
> BLENDVPS  m1, m2, m3
>
> is the most commonly used alignment.

I see that a lot of .asm files use different alignments.
I'll try to pick something similar that I find good looking.

I also see that different function/macro can have
different position for the first comma.
This could be quite useful, to visually separate
the macros.


>>> Unless aligning every single loop entry helps a lot I'd avoid it since
>>> it does waste a bit of icache.
>>
>> I'll redo my benchmarks, but I have placed these aligns for a reason.
>> At some point removed debug instructions started making my code
>> slower. So I've placed align to avoid random slowdowns.
>
> Ok.
>
>>> Explicitly using the carry flag as a branch condition is a bit weird.
>>> Generally using jg/jl/jge/jle tends to be clearer.
>>
>> I use it to take advantage of the so called MacroFusion.
>> It allows the merge of cmp and jxx, as long as the branch
>> checks only one flag, so all signed branches are to be avoided
>> (as stated by intel manual).
>> The newer the cpu, the more opcodes (add/sub)
>> could be merged and less limitations.
>
> Every µarch that can do macro-op fusion with add/sub can do so with
> both signed and unsigned branch conditions.
> There's actually only a single µarch that can fuse cmp with unsigned
> but not signed branch conditions and that's more than a decade old
> (Conroe), but if you want to check if (unsigned)a < (unsigned)b 'jb'
> is preferred of 'jc' (both produce the exact same opcode).

One is enough reason for me :}
Since the same thing works just as well or better on newer CPU's.

Also, using above/bellow on arithmetic operations
is a lot more confusing than carry/borrow.
You can see that I use "jc" and "jnc" after "sub".

I'll keep it as it is, unless you have solid reason to change it.

>>> Assembler-specific ifdeffery should be avoided in individual files.
>>> Something equivalent already exists in x86inc actually but I don't
>>> remember if it was merged to FFmpeg from upstream (x264) yet.
>>
>> Definitely not merged.
>>
>> I hear a lot about the improved x86inc,
>> maybe it is time for you to merge it in FFmpeg?
>
> Now that I think about it I believe that part wasn't merged because
> smartalign is broken on some older nasm versions (which is another
> reason for avoiding things like this in individual files) and FFmpeg
> doesn't enforce any specific version requirements. I guess it could be
> worked around with some version checking ifdeffery if we know which
> versions are affected.

I don't see anything relevant in here:
http://repo.or.cz/nasm.git/history/HEAD:/macros/smartalign.mac
I also didn't notice anything similar in the changelog.

I don't think this is relevant reason for not merging avx2 support. ;)

>>> Isn't this just an overly complicated way of expressing "dd 0*4, 1*4,
>>> 2*4, 3*4, 4*4, 5*4, 6*4, 7*4"?
>>
>> Yes.
>> Do you know a way that works with "times 8"?
>> I've done it this way to make it easy to change the size of the constant.
>>
>> Do you request I change it?
>
> I'd prefer just using that simple one-liner I posted. Replacing the
> numbers if you want to change things later is trivial.

It might be trivial, but it also makes it easier to misspell something.
I'd prefer to keep it as it is.

>>> Is reordering moves for the non-AVX path worth the additional
>>> complexity? Microoptimizations that only affect legacy hardware are
>>> IMO a bit questionable.
>>
>> Yes, I'm on "legacy" hardware and the improvement is reliably measurable.
>
> Ok, in that case you should also use shufps/addps instead of
> vshufps/vaddps in the AVX branch for cosmetic reasons though (x86inc
> performs automatic VEX-encoding).

Only on instructions it knows...
that brings us again to the AVX2.

I had it mixed before, but I changed it to be consistent.
Some new avx instruction are not overloaded or
available without their "v-" prefix.
e.g. x86util uses vpbroadcastw in SPLATW.

It makes more sense to use "v" prefix for
code that is guaranteed to be avx.

>>> The AVX1 integer warning is

Re: [FFmpeg-devel] [PATCH] swscale/utils.c: Fix incorrect error when calling sws_setColorspaceDetails

2017-08-03 Thread Michael Niedermayer 
On Thu, Aug 03, 2017 at 12:06:34PM +0100, Mark Boorer wrote:
> I can confirm that range works as expected, but I couldn't actually see any
> difference when setting brightness, contrast or saturation. It looks as
> though all of those calculations only take place inside the yuv2rgb lookup
> table, so when converting from yuv->yuv they won't get applied.
> 
> In my case I am not using the brightness/contrast/saturation settings, only
> the range.

Please correct me if iam wrong but IIUC before your patch setting
brightness errors out while afterwards it gives a wrong result


[...]
-- 
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] [WIP] libcodec2 wrapper + de/muxer in FFmpeg

2017-08-03 Thread Reimar Döffinger 
On Thu, Aug 03, 2017 at 12:40:04AM +0200, Tomas Härdin wrote:

> +//statically assert the size of avpriv_codec2_header
> +//putting it here because all codec2 things depend on codec2utils
> +switch(0) {
> +case 0:
> +case sizeof(avpriv_codec2_header) == 7: //if false then the compiler 
> will complain
> +break;
> +}

I was pretty sure we had some kind of static assert already,
based on negative array size in a struct (so it can work outside
functions)...
But doesn't matter due to next comments...

> +*((avpriv_codec2_header*)avctx->extradata) = 
> avpriv_codec2_make_header(mode);

I am pretty sure this is a strict aliasing violation.
Even ignoring the ugliness and performance issues with
returning structs and assigning them causing lots of
pointless copies.
Also, since all struct elements are bytes, it isn't
really any more readable than just assigning to the
byte array, with the names you not have in the struct
in comments instead.
And that is without going into all the ways someone could
change that struct (e.g. in case of new features) that
would completely break this kind of code due to endianness,
alignment, ...
Even just supporting both the current and a potential future version
that changes any of the fields would be hard with this kind of code.

> +if (avctx->extradata_size != sizeof(avpriv_codec2_header)) {
> +av_log(avctx, AV_LOG_ERROR, "must have exactly %zu bytes of 
> extradata (got %i)\n",
> +   sizeof(avpriv_codec2_header), avctx->extradata_size);
> +}

I would think at least if it is less you wouldn't want to just
continue? Even if extradata is required to be padded (is it?),
blindly selecting 0 as mode doesn't seem very likely to be right.

> +output = (int16_t *)frame->data[0];

The codec2 documentation seems pretty non-existant, so I
assume you are right that this is always in native endianness.
However from what I can see codec2 actually uses the "short" type,
not int16_t.
While it shouldn't make a difference in practice, if casting anyway
I'd suggest using the type matching the API.

> +if (nframes > 0) {
> +*got_frame_ptr = 1;
> +}

Not just *got_frame_ptr = nframes > 0; ?

> +int16_t *samples = (int16_t *)frame->data[0];

You are casting the const away.
Did they just forget to add the const to the API?
If so, can you suggest it to be added?
Otherwise if it's intentional you need to make a copy.

> +int ret;
> +
> +if ((ret = ff_alloc_packet2(avctx, avpkt, avctx->block_align, 0)) < 0) {

If you merge the declaration and assignment it you would get
for free not having the assignment inside the if, which I
still think is just horrible style. :)

> +//file starts wih 0xC0DEC2
> +if (p->buf[0] == 0xC0 && p->buf[1] == 0xDE && p->buf[2] == 0xC2) {
> +return AVPROBE_SCORE_MAX;
> +}

As mentioned, try to find a few more bits and reduce the score.
If only these 3 bytes match, I would suggest a rather low score.
(I doubt there are enough useful bits in this header to allow
reaching MAX score, it's a shame they didn't invest a byte or
2 more, especially considering that while 0xC0DEC2 might look
clever it doesn't exactly get maximum entropy out of those 3 bytes).

> +AVStream *st;
> +
> +if (!(st = avformat_new_stream(s, NULL)) ||

Can merge allocation and declaration again.

> +//Read roughly 0.1 seconds worth of data.
> +n = st->codecpar->bit_rate / ((int)(8/0.1) * block_align) + 1;

That doesn't seem overly readable to me...
// Read about 1/10th of a second worth of data
st->codecpar->bit_rate / 10 / 8 / block_align + 1

Seems not really worse and doesn't have doubles and casts...
Otherwise
blocks_per_second = st->codecpar->bit_rate / 8 / block_align;
n = blocks_per_second / 10;
might be even clearer.

> +if (av_new_packet(pkt, size) < 0)
> +return AVERROR(ENOMEM);
> +//try to read desired number of bytes, recompute n from to actual number 
> of bytes read
> +pkt->pos= avio_tell(s->pb);
> +pkt->stream_index = 0;
> +ret = ffio_read_partial(s->pb, pkt->data, size);
> +if (ret < 0) {
> +av_packet_unref(pkt);
> +return ret;
> +}
> +av_shrink_packet(pkt, ret);

Ouch, why are you not just using av_get_packet?

> +avio_write(s->pb, st->codecpar->extradata, sizeof(avpriv_codec2_header));

Error check?

> +if (!(st = avformat_new_stream(s, NULL)) ||
> +ff_alloc_extradata(st->codecpar, sizeof(avpriv_codec2_header))) {
> +return AVERROR(ENOMEM);

Here and in the other read_header, this would preferably preserve the error that
ff_alloc_extradata returned.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v5 3/3] Add myself as speedhq maintainer, per request.

2017-08-03 Thread James Almer 
On 8/3/2017 4:31 AM, Steinar H. Gunderson wrote:
> ---
>  MAINTAINERS | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index ae0e08d121..ce5e1dae08 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -233,6 +233,7 @@ Codecs:
>smvjpegdec.c  Ash Hughes
>snow* Michael Niedermayer, Loren Merritt
>sonic.c   Alex Beregszaszi
> +  speedhq.c Steinar H. Gunderson
>srt*  Aurelien Jacobs
>sunrast.c Ivo van Poorten
>svq3.cMichael Niedermayer
> @@ -598,6 +599,7 @@ Reynaldo H. Verdejo Pinochet  6E27 CD34 170C C78E 4D4F 
> 5F40 C18E 077F 3114 452A
>  Robert Swain  EE7A 56EA 4A81 A7B5 2001 A521 67FA 362D A2FC 
> 3E71
>  Sascha Sommer 38A0 F88B 868E 9D3A 97D4 D6A0 E823 706F 1E07 
> 0D3C
>  Stefano Sabatini  0D0B AD6B 5330 BBAD D3D6 6A0C 719C 2839 FC43 
> 2D5F
> +Steinar H. Gunderson  C2E9 004F F028 C18E 4EAD DB83 7F61 7561 7797 
> 8F76
>  Stephan Hilb  4F38 0B3A 5F39 B99B F505 E562 8D5C 5554 4E17 
> 8863
>  Tiancheng "Timothy" Gu9456 AFC0 814A 8139 E994 8351 7FE6 B095 B582 
> B0D4
>  Tim Nicholson 38CF DB09 3ED0 F607 8B67 6CED 0C0B FC44 8B0B 
> FC83
> 

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

Re: [FFmpeg-devel] [PATCH v5 2/3] speedhq: add FATE tests

2017-08-03 Thread James Almer 
On 8/3/2017 4:31 AM, Steinar H. Gunderson wrote:
> Also add simple FATE tests, based on output produced by the NDI SDK.
> ---
>  tests/Makefile | 1 +
>  tests/fate/speedhq.mak | 9 +
>  tests/ref/fate/speedhq-422 | 6 ++
>  tests/ref/fate/speedhq-422-singlefield | 6 ++
>  4 files changed, 22 insertions(+)
>  create mode 100644 tests/fate/speedhq.mak
>  create mode 100644 tests/ref/fate/speedhq-422
>  create mode 100644 tests/ref/fate/speedhq-422-singlefield
> 
> diff --git a/tests/Makefile b/tests/Makefile
> index ab83ae855d..f9b9cf4188 100644
> --- a/tests/Makefile
> +++ b/tests/Makefile
> @@ -164,6 +164,7 @@ include $(SRC_PATH)/tests/fate/qt.mak
>  include $(SRC_PATH)/tests/fate/qtrle.mak
>  include $(SRC_PATH)/tests/fate/real.mak
>  include $(SRC_PATH)/tests/fate/screen.mak
> +include $(SRC_PATH)/tests/fate/speedhq.mak
>  include $(SRC_PATH)/tests/fate/source.mak
>  include $(SRC_PATH)/tests/fate/subtitles.mak
>  include $(SRC_PATH)/tests/fate/utvideo.mak
> diff --git a/tests/fate/speedhq.mak b/tests/fate/speedhq.mak
> new file mode 100644
> index 00..7b6b03ea0d
> --- /dev/null
> +++ b/tests/fate/speedhq.mak
> @@ -0,0 +1,9 @@
> +FATE_SPEEDHQ = fate-speedhq-422   \
> +   fate-speedhq-422-singlefield   \
> +
> +FATE_SAMPLES_FFMPEG-$(CONFIG_SPEEDHQ_DECODER) += fate-speedhq-422 
> fate-speedhq-422-singlefield
> +
> +fate-speedhq: $(FATE_SPEEDHQ)
> +
> +fate-speedhq-422: CMD = framecrc -flags +bitexact -f rawvideo 
> -codec speedhq -vtag SHQ2 -video_size 112x64 -i 
> $(TARGET_SAMPLES)/speedhq/progressive.shq2 -pix_fmt yuv422p
> +fate-speedhq-422-singlefield: CMD = framecrc -flags +bitexact -f rawvideo 
> -codec speedhq -vtag SHQ2 -video_size 112x32 -i 
> $(TARGET_SAMPLES)/speedhq/singlefield.shq2 -pix_fmt yuv422p
> diff --git a/tests/ref/fate/speedhq-422 b/tests/ref/fate/speedhq-422
> new file mode 100644
> index 00..7bb0d2388d
> --- /dev/null
> +++ b/tests/ref/fate/speedhq-422
> @@ -0,0 +1,6 @@
> +#tb 0: 1/25
> +#media_type 0: video
> +#codec_id 0: rawvideo
> +#dimensions 0: 112x64
> +#sar 0: 0/1
> +0,  0,  0,1,14336, 0x9bb6dc6d
> diff --git a/tests/ref/fate/speedhq-422-singlefield 
> b/tests/ref/fate/speedhq-422-singlefield
> new file mode 100644
> index 00..343c52645c
> --- /dev/null
> +++ b/tests/ref/fate/speedhq-422-singlefield
> @@ -0,0 +1,6 @@
> +#tb 0: 1/25
> +#media_type 0: video
> +#codec_id 0: rawvideo
> +#dimensions 0: 112x32
> +#sar 0: 0/1
> +0,  0,  0,1, 7168, 0x75de4109
> 

Applied all the changes from my previous review you apparently missed,
and pushed.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v5 1/3] speedhq: fix behavior of single-field decoding

2017-08-03 Thread James Almer 
On 8/3/2017 4:22 AM, Steinar H. Gunderson wrote:
> The height convention for decoding frames with only a single field made sense
> for compatibility with legacy decoders, but doesn't really match the 
> convention
> used by NDI, which is the primary (only?) user. Thus, change it to simply
> assuming that if the two fields overlap, the frame is meant to be a single
> field and the frame height matches the field height.
> ---
>  libavcodec/speedhq.c | 9 ++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/libavcodec/speedhq.c b/libavcodec/speedhq.c
> index 60efb0222b..47b1e4dc7a 100644
> --- a/libavcodec/speedhq.c
> +++ b/libavcodec/speedhq.c
> @@ -450,10 +450,13 @@ static int speedhq_decode_frame(AVCodecContext *avctx,
>  if (second_field_offset == 4) {
>  /*
>   * Overlapping first and second fields is used to signal
> - * encoding only a single field (the second field then comes
> - * as a separate, later frame).
> + * encoding only a single field. In this case, "height"
> + * is ambiguous; it could mean either the height of the
> + * frame as a whole, or of the field. The former would make
> + * more sense for compatibility with legacy decoders,
> + * but this matches the convention used in NDI, which is
> + * the primary user of this trick.
>   */
> -frame->height >>= 1;
>  if ((ret = decode_speedhq_field(s, buf, buf_size, frame, 0, 4, 
> buf_size, 1)) < 0)
>  return ret;
>  } else {
> 

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

Re: [FFmpeg-devel] [PATCH] JPEG2000 encoding with variable codeblock size

2017-08-03 Thread Aaron Boxer 
On Aug 3, 2017 9:58 AM,  wrote:

From: Francesco Cuzzocrea 

Hi to all
I've made some simple changes tha allow encoding with variable codeblock
size.  Default value are the same as previous (16X16) but now setting them
to 64x128 make generated codestream compatible with Analog Devices ADV212
video codec.


Interesting - as far as I  aware, part 1 of standard only allows codeblock
of max size 4096, so 64x128 would not be legal.



---
 libavcodec/j2kenc.c | 22 +-
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/libavcodec/j2kenc.c b/libavcodec/j2kenc.c
index c8d3861..1bd4fbd 100644
--- a/libavcodec/j2kenc.c
+++ b/libavcodec/j2kenc.c
@@ -1178,17 +1178,21 @@ static int j2kenc_destroy(AVCodecContext *avctx)
 // taken from the libopenjpeg wraper so it matches

 #define OFFSET(x) offsetof(Jpeg2000EncoderContext, x)
+#define OFFSET1(x) offsetof(Jpeg2000CodingStyle, x)
+
+
 #define VE AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_ENCODING_PARAM
 static const AVOption options[] = {
-{ "format","Codec Format",  OFFSET(format),
AV_OPT_TYPE_INT,   { .i64 = CODEC_JP2   }, CODEC_J2K, CODEC_JP2,   VE,
"format"  },
-{ "j2k",   NULL,0,
 AV_OPT_TYPE_CONST, { .i64 = CODEC_J2K   }, 0, 0,   VE,
"format"  },
-{ "jp2",   NULL,0,
 AV_OPT_TYPE_CONST, { .i64 = CODEC_JP2   }, 0, 0,   VE,
"format"  },
-{ "tile_width","Tile Width",OFFSET(tile_width),
AV_OPT_TYPE_INT,   { .i64 = 256 }, 1, 1<<30,   VE, },
-{ "tile_height",   "Tile Height",   OFFSET(tile_height),
 AV_OPT_TYPE_INT,   { .i64 = 256 }, 1, 1<<30,   VE, },
-{ "pred",  "DWT Type",  OFFSET(pred),
AV_OPT_TYPE_INT,   { .i64 = 0   }, 0, 1,   VE,
"pred"},
-{ "dwt97int",  NULL,0,
 AV_OPT_TYPE_CONST, { .i64 = 0   }, INT_MIN, INT_MAX,   VE,
"pred"},
-{ "dwt53", NULL,0,
 AV_OPT_TYPE_CONST, { .i64 = 0   }, INT_MIN, INT_MAX,   VE,
"pred"},
-
+{ "format",   "Codec Format",   OFFSET(format),
   AV_OPT_TYPE_INT,   { .i64 = CODEC_JP2   }, CODEC_J2K,
CODEC_JP2,   VE, "format"  },
+{ "j2k",   NULL,0,
AV_OPT_TYPE_CONST, { .i64 = CODEC_J2K   }, 0,
 0,   VE, "format"  },
+{ "jp2",   NULL,0,
AV_OPT_TYPE_CONST, { .i64 = CODEC_JP2   }, 0,
 0,   VE, "format"  },
+{ "tile_width","Tile Width",OFFSET(tile_width),
   AV_OPT_TYPE_INT,   { .i64 = 256 }, 1,
 1<<30,   VE, },
+{ "tile_height",   "Tile Height",   OFFSET(tile_height),
AV_OPT_TYPE_INT,   { .i64 = 256 }, 1,
 1<<30,   VE, },
+{ "pred",  "DWT Type",  OFFSET(pred),
   AV_OPT_TYPE_INT,   { .i64 = 0   }, 0,
 1,   VE, "pred"},
+{ "dwt97int",  NULL,0,
AV_OPT_TYPE_CONST, { .i64 = 0   }, INT_MIN,
INT_MAX,   VE, "pred"},
+{ "dwt53", NULL,0,
AV_OPT_TYPE_CONST, { .i64 = 0   }, INT_MIN,
INT_MAX,   VE, "pred"},
+{ "log2_cblk_width",   "Codeblock Width",
 (OFFSET(codsty)+OFFSET1(log2_cblk_width)),AV_OPT_TYPE_INT,   { .i64 =
4   }, 1, 1<<30,   VE, },
+{ "log2_cblk_height",  "Codeblock Height",
(OFFSET(codsty)+OFFSET1(log2_cblk_height)),   AV_OPT_TYPE_INT,   { .i64 =
4   }, 1, 1<<30,   VE, },
 { NULL }
 };

--
2.1.4

___
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] Implement NewTek NDI support

2017-08-03 Thread Steinar H. Gunderson 
On Thu, Aug 03, 2017 at 08:21:09AM +0300, Maksym Veremeyenko wrote:
>> You should reject bottom field first formats, as NDI explicitly
>> disallows them.
> even if it disallowed, why to drop? may be warning message would be enough?

FWIW, the documentation may say that formats are always TFF, but on the wire,
it seems the bottom field actually goes first.

(I guess this is a bug that isn't noticed because there's actually no way
from the API to get at the individual fields)

> +NDI is very picky about the formats it supports. Pixel format is always
> +uyvy422.

Or bgra (internally, uyvya4224, I suppose).

> +recv_create_desc.allow_video_fields = ctx->allow_video_fields;

I'm not convinced you ever want to set this to false, as what's happening if
you do is simply that the SDK applies a vertical blur to incoming interlaced
frames. FFmpeg has much better deinterlacers if that's what you want.

/* Steinar */
-- 
Homepage: https://www.sesse.net/
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avformat/utils: fix memory leak in avformat_free_context

2017-08-03 Thread Steven Siloti 
On Mon, Jul 31, 2017 at 11:08 AM, Steven Siloti 
wrote:

> The pointer to the packet queue is stored in the internal structure
> so the queue needs to be flushed before internal is freed.
>
> Signed-off-by: Steven Siloti 
> ---
>  libavformat/utils.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index 38d247c6cd..58283616dc 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -4333,8 +4333,8 @@ void avformat_free_context(AVFormatContext *s)
>  av_dict_free(>metadata);
>  av_dict_free(>internal->id3v2_meta);
>  av_freep(>streams);
> -av_freep(>internal);
>  flush_packet_queue(s);
> +av_freep(>internal);
>  av_free(s);
>  }
>
> --
> 2.13.0.windows.1
>
> Is there something wrong with this patch? It seems like a straightforward
fix to me. Perhaps you would prefer it as an attachment. If so, attached.


0001-avformat-utils-fix-memory-leak-in-avformat_free_cont.patch
Description: Binary data
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [WIP] libcodec2 wrapper + de/muxer in FFmpeg

2017-08-03 Thread Hendrik Leppkes 
On Thu, Aug 3, 2017 at 5:45 PM, Tomas Härdin  wrote:
>> > +} else {
>> > +if (avctx->extradata_size != sizeof(avpriv_codec2_header))
>> > {
>> > +av_log(avctx, AV_LOG_ERROR, "must have exactly %zu
>> > bytes of extradata (got %i)\n",
>> > +   sizeof(avpriv_codec2_header), avctx-
>> > >extradata_size);
>> return AVERROR_INVALIDDATA?
>
> Good catch, fixed. Should AVERROR_INVALIDDATA be preferred over
> AVERROR(EINVAL) perhaps? I've seen both used almost interchangably.
>

Generally, AVERROR_INVALIDDATA is for invalid bitstreams (which
extradata would be a part of), AVERROR(EINVAL) for invalid
user-provided options/settings.
Not all code follows that quite strictly, but thats the usual
definition we use around here.

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

Re: [FFmpeg-devel] [WIP] libcodec2 wrapper + de/muxer in FFmpeg

2017-08-03 Thread Tomas Härdin 
On Thu, 2017-08-03 at 13:00 +0200, Nicolas George wrote:
> Le sextidi 16 thermidor, an CCXXV, Tomas Härdin a écrit :
> > With both the raw demuxer and the encoder you need to specify the
> > desired mode, like this:
> The encoder could default to one the two.

Default on-the-air is 1300 (aka FreeDV 1600), so that seems like a
reasonable default

> > Some remarks:
> > 
> > * I had to make the ffmpeg CLI not complain about the 700 modes,
> > since
> > it thinks encoding at below 1 kbps is a user error
> It is just a warning. I am not really a fan of a special case like
> that,
> and it would be better split into a separate patch I think.

There's other codec-specific special cases in ffmpeg.c, like the one
for AV_CODEC_ID_VP9 which doesn't even have an explanation. Separate
patch is a good idea however.

> > * I have duplicated some minor functions in libcodec2 in
> > libavcodec/codec2utils.*. This avoid having to link libcodec2 just
> > for
> > remuxing, and in case someone writes a native decoder for
> > libavcodec
> The license allows it, but you neglected to give copyright
> attribution.

Alright, I can fix that. Although the only thing lifted straight are
the mode #defines

> > * Not sure if codec2utils should go in libavutil, libavcodec felt
> > good
> > enough
> Definitely libavcodec.

Cool.

> > 
> > * The demuxer tries to read up to 0.1 seconds worth of frames to
> > speed
> > things up a little without making seeking too broken. 3 frames = 12
> > bytes for the 700 bit/s modes, which decode to 960 samples
> I do not like the sound of that, but I will see the code.
> 
> > 
> > * The decoder is able to deal with multiple frames at a time, the
> > encoder does not need to
> ???

The encoder is given frame_size number of samples each time. Are there
times where this is not the case? I haven't set
AV_CODEC_CAP_SMALL_LAST_FRAME.

> > Feel free to bikeshed around whether extradata should be the entire
> > .c2
> > header or just the mode byte. It really only matters if we go with
> > RIFF
> > or ISOM mappings (.wav .avi .mov and friends), which I decided to
> > leave
> > out for now.
> It matters for inclusion in any format: Matroska, Nut, etc. Is
> anybody
> considering normalization?

I raised the possibility on [Freetel-codec2], either by coding mode as
part of the TwoCC (0xC208 for mode 8 for examples) or via extradata. I
imagine version + mode + flags in extradata would not be unreasonable.

> > +libcodec2_decoder_deps="libcodec2"
> > +libcodec2_decoder_select="codec2utils"
> > +libcodec2_encoder_deps="libcodec2"
> > +libcodec2_encoder_select="codec2utils"
> This and the similar hunks are not necessary, see below the comments
> about the Makefile.

I presume you mean codec2utils and not the libcodec2 deps.

> > +OBJS-$(CONFIG_CODEC2UTILS) += codec2utils.o
> You do not need a separate configuration option, you can just depend
> on
> the actual visible option:
> 
> +OBJS-$(CONFIG_CODEC2_DEMUXER)   += codec2utils.o
> +OBJS-$(CONFIG_CODEC2_MUXER) += codec2utils.o
> 
> Having the same file several times will not cause it to be included
> several times.
> 
> > +OBJS-$(CONFIG_LIBCODEC2_DECODER)  += libcodec2.o
> > +OBJS-$(CONFIG_LIBCODEC2_ENCODER)  += libcodec2.o
> +OBJS-$(CONFIG_LIBCODEC2_DECODER)  += libcodec2.o
> codec2utils.o
> +OBJS-$(CONFIG_LIBCODEC2_ENCODER)  += libcodec2.o
> codec2utils.o

Fixed.

> > +++ b/libavcodec/codec2utils.c
> > @@ -0,0 +1,118 @@
> > +/*
> > + * codec2 utility functions
> > 
> > + * Copyright (c) 2017 Tomas Härdin
> Missing copyright attribution if the imported code is non-trivial
> (better err on the side of more attribution).

Guess I'll nick the comment from the relevant files in libcodec2.


> > 
> > +int avpriv_codec2_mode_from_str(void *logctx, const char *modestr)
> > {
> Please normalize the coding style. Same below.

I presume you mean putting the function opening braces on their own
lines like the rest of FFmpeg. Fixed.

> > 
> > +//statically assert the size of avpriv_codec2_header
> > +//putting it here because all codec2 things depend on
> > codec2utils
> > +switch(0) {
> > +case 0:
> > +case sizeof(avpriv_codec2_header) == 7: //if false then the
> > compiler will complain
> > +break;
> > +}
> I see how it works. This is a nice trick. Want to make it an official
> macro FF_ASSERT_STATIC()?

Sure. Could go in avutil.h or something. Maybe even inside a static
function with a fake name like:

#define FF_ASSERT_STATIC(x, name) static void name(){switch(0){...}}

Didn't do that in this patchset however.

> > +int avpriv_codec2_mode_bit_rate(void *logctx, int mode) {
> > 
> > +int ret = 8 * 8000 * avpriv_codec2_mode_block_align(logctx,
> > mode) / avpriv_codec2_mode_frame_size(logctx, mode);
> > +if (ret <= 0) {
> > +av_log(logctx, AV_LOG_WARNING, "unknown codec2 mode %i,
> > can't estimate bitrate\n", mode);
> I bet you did not test the invalid case.

[FFmpeg-devel] [PATCH v2] swscale: fix gbrap16 alpha channel issues

2017-08-03 Thread James Cowgill 
Fixes filter-pixfmts-scale test failing on big-endian systems due to
alpSrc not being cast to (const int32_t**).

Also fixes distortions in the output alpha channel values by copying the
alpha channel code from the rgba64 case found elsewhere in output.c.

Fixes ticket 6555.

Signed-off-by: James Cowgill 
---
 v2
 
 Move declaration of A inside the loop and don't bother initializing it since
 the initial value would never be read.

 libswscale/output.c | 16 
 tests/ref/fate/filter-pixfmts-scale |  4 ++--
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/libswscale/output.c b/libswscale/output.c
index 9774e9f327..f30bce8dd3 100644
--- a/libswscale/output.c
+++ b/libswscale/output.c
@@ -2026,24 +2026,24 @@ yuv2gbrp16_full_X_c(SwsContext *c, const int16_t 
*lumFilter,
 const int16_t **lumSrcx, int lumFilterSize,
 const int16_t *chrFilter, const int16_t **chrUSrcx,
 const int16_t **chrVSrcx, int chrFilterSize,
-const int16_t **alpSrc, uint8_t **dest,
+const int16_t **alpSrcx, uint8_t **dest,
 int dstW, int y)
 {
 const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(c->dstFormat);
 int i;
-int hasAlpha = (desc->flags & AV_PIX_FMT_FLAG_ALPHA) && alpSrc;
+int hasAlpha = (desc->flags & AV_PIX_FMT_FLAG_ALPHA) && alpSrcx;
 uint16_t **dest16 = (uint16_t**)dest;
 const int32_t **lumSrc  = (const int32_t**)lumSrcx;
 const int32_t **chrUSrc = (const int32_t**)chrUSrcx;
 const int32_t **chrVSrc = (const int32_t**)chrVSrcx;
-int A = 0; // init to silence warning
+const int32_t **alpSrc  = (const int32_t**)alpSrcx;
 
 for (i = 0; i < dstW; i++) {
 int j;
 int Y = -0x4000;
 int U = -(128 << 23);
 int V = -(128 << 23);
-int R, G, B;
+int R, G, B, A;
 
 for (j = 0; j < lumFilterSize; j++)
 Y += lumSrc[j][i] * (unsigned)lumFilter[j];
@@ -2059,13 +2059,13 @@ yuv2gbrp16_full_X_c(SwsContext *c, const int16_t 
*lumFilter,
 V >>= 14;
 
 if (hasAlpha) {
-A = 1 << 18;
+A = -0x4000;
 
 for (j = 0; j < lumFilterSize; j++)
 A += alpSrc[j][i] * lumFilter[j];
 
-if (A & 0xF800)
-A =  av_clip_uintp2(A, 27);
+A >>= 1;
+A += 0x20002000;
 }
 
 Y -= c->yuv2rgb_y_offset;
@@ -2083,7 +2083,7 @@ yuv2gbrp16_full_X_c(SwsContext *c, const int16_t 
*lumFilter,
 dest16[1][i] = B >> 14;
 dest16[2][i] = R >> 14;
 if (hasAlpha)
-dest16[3][i] = A >> 11;
+dest16[3][i] = av_clip_uintp2(A, 30) >> 14;
 }
 if ((!isBE(c->dstFormat)) != (!HAVE_BIGENDIAN)) {
 for (i = 0; i < dstW; i++) {
diff --git a/tests/ref/fate/filter-pixfmts-scale 
b/tests/ref/fate/filter-pixfmts-scale
index 9b601b71da..dcc34bd4d1 100644
--- a/tests/ref/fate/filter-pixfmts-scale
+++ b/tests/ref/fate/filter-pixfmts-scale
@@ -23,8 +23,8 @@ gbrap10be   6d89abb9248006c3e9017545e9474654
 gbrap10le   cf974e23f485a10740f5de74a5c8c3df
 gbrap12be   1d9b57766ba9c2192403f43967cb9af0
 gbrap12le   bb1ba1c157717db3dd612a76d38a018e
-gbrap16be   81542b96575d1fe3b239d23899f5ece3
-gbrap16le   6feb8b9da131917abe867e0eaaf07b90
+gbrap16be   c72b935a6e57a8e1c37bff08c2db55b1
+gbrap16le   13eb0e62b1ac9c1c86c81521eaefab5f
 gbrpdc3387f925f972c61aae7eb23cdc19f0
 gbrp10be0277d4c3a8498d75e2783fb81379e481
 gbrp10lef3d70f8ab845c3c9b8f7452e4a6e285a
-- 
2.13.3

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

Re: [FFmpeg-devel] [PATCHv3 4/4] libavcodec: v4l2: add support for v4l2 mem2mem codecs

2017-08-03 Thread Jorge Ramirez 

On 08/03/2017 01:53 AM, Mark Thompson wrote:

On 02/08/17 08:32, Jorge Ramirez-Ortiz wrote:

...
diff --git a/compat/v4l2/v4l2-common.h b/compat/v4l2/v4l2-common.h
diff --git a/compat/v4l2/videodev2.h b/compat/v4l2/videodev2.h

These are discussed in other threads.  I don't really have any comment either 
way.


ok. will be ammended in v4.




diff --git a/configure b/configure
index ed94de0..650c8fb 100755
--- a/configure
+++ b/configure
@@ -10,7 +10,6 @@
  # Prevent locale nonsense from breaking basic text processing.
  LC_ALL=C
  export LC_ALL
-

Spurious change?

ack



  # make sure we are running under a compatible shell
  # try to make this part work with most shells
  
@@ -149,6 +148,7 @@ Component options:

--disable-pixelutils disable pixel utils in libavutil
  
  Individual component options:

+  --disable-v4l2_m2m   disable V4L2 mem2mem code [autodetect]

s/_/-/ here.

ack




--disable-everything disable all components listed below
--disable-encoder=NAME   disable encoder NAME
--enable-encoder=NAMEenable encoder NAME
@@ -1432,6 +1432,7 @@ AVCODEC_COMPONENTS="
  
  AVDEVICE_COMPONENTS="

  indevs
+v4l2_m2m
  outdevs
  "
  AVFILTER_COMPONENTS="
@@ -2269,11 +2270,12 @@ map 'eval ${v}_inline_deps=inline_asm' 
$ARCH_EXT_LIST_ARM
  
  loongson2_deps="mips"

  loongson3_deps="mips"
-v4l2_deps_any="linux_videodev2_h sys_videoio_h"
+v4l2_m2m_select="v4l2"
  mipsfpu_deps="mips"
  mipsdsp_deps="mips"
  mipsdspr2_deps="mips"
  mips32r2_deps="mips"
+vc1_v4l2m2m_decoder_deps="v4l2_m2m"
  mips32r5_deps="mips"
  mips32r6_deps="mips"
  mips64r2_deps="mips"
@@ -2284,6 +2286,9 @@ mmi_deps="mips"
  altivec_deps="ppc"
  dcbzl_deps="ppc"
  ldbrx_deps="ppc"
+vp8_v4l2m2m_decoder_deps="v4l2_m2m"
+vp8_v4l2m2m_encoder_deps="v4l2_m2m"
+vp9_v4l2m2m_decoder_deps="v4l2_m2m"

These seem to be placed randomly in the file.  There is a hardware codec 
section at around line 2800, put all of these declarations there.


thanks for the guideline. will do.



  ppc4xx_deps="ppc"
  vsx_deps="altivec"
  power8_deps="vsx"
@@ -2437,15 +2442,22 @@ h261_decoder_select="mpegvideo"
  h261_encoder_select="aandcttables mpegvideoenc"
  h263_decoder_select="h263_parser h263dsp mpegvideo qpeldsp"
  h263_encoder_select="aandcttables h263dsp mpegvideoenc"
+h263_v4l2m2m_decoder_deps="v4l2_m2m"
+h263_v4l2m2m_encoder_deps="v4l2_m2m"
  h263i_decoder_select="h263_decoder"
  h263p_decoder_select="h263_decoder"
  h263p_encoder_select="h263_encoder"
  h264_decoder_select="cabac golomb h264chroma h264dsp h264parse h264pred h264qpel 
videodsp"
  h264_decoder_suggest="error_resilience"
+h264_v4l2m2m_decoder_deps="v4l2_m2m"
+h264_v4l2m2m_encoder_deps="v4l2_m2m"
  hap_decoder_select="snappy texturedsp"
  hap_encoder_deps="libsnappy"
  hap_encoder_select="texturedspenc"
  hevc_decoder_select="bswapdsp cabac golomb hevcparse videodsp"
+hevc_encoder_select="hevc_v4l2m2m"
+hevc_v4l2m2m_decoder_deps="v4l2_m2m"
+hevc_v4l2m2m_encoder_deps="v4l2_m2m"
  huffyuv_decoder_select="bswapdsp huffyuvdsp llviddsp"
  huffyuv_encoder_select="bswapdsp huffman huffyuvencdsp llvidencdsp"
  iac_decoder_select="imc_decoder"
@@ -2482,6 +2494,7 @@ mpc7_decoder_select="bswapdsp mpegaudiodsp"
  mpc8_decoder_select="mpegaudiodsp"
  mpeg_xvmc_decoder_deps="X11_extensions_XvMClib_h"
  mpeg_xvmc_decoder_select="mpeg2video_decoder"
+mpeg1_v4l2m2m_decoder_deps="v4l2_m2m"
  mpegvideo_decoder_select="mpegvideo"
  mpeg1video_decoder_select="mpegvideo"
  mpeg1video_encoder_select="aandcttables mpegvideoenc h263dsp"
@@ -2489,6 +2502,8 @@ mpeg2video_decoder_select="mpegvideo"
  mpeg2video_encoder_select="aandcttables mpegvideoenc h263dsp"
  mpeg4_decoder_select="h263_decoder mpeg4video_parser"
  mpeg4_encoder_select="h263_encoder"
+mpeg4_v4l2m2m_decoder_deps="v4l2_m2m"
+mpeg4_v4l2m2m_encoder_deps="v4l2_m2m"
  msa1_decoder_select="mss34dsp"
  mscc_decoder_select="zlib"
  msmpeg4v1_decoder_select="h263_decoder"
@@ -3042,7 +3057,6 @@ qtkit_indev_select="qtkit"
  sdl2_outdev_deps="sdl2"
  sndio_indev_deps="sndio"
  sndio_outdev_deps="sndio"
-v4l_indev_deps="linux_videodev_h"
  v4l2_indev_select="v4l2"
  v4l2_outdev_select="v4l2"
  vfwcap_indev_deps="vfw32 vfwcap_defines"
@@ -3592,7 +3606,7 @@ done
  enable_weak audiotoolbox
  
  # Enable hwaccels by default.

-enable_weak d3d11va dxva2 vaapi vda vdpau videotoolbox_hwaccel xvmc
+enable_weak d3d11va dxva2 vaapi v4l2_m2m vda vdpau videotoolbox_hwaccel xvmc
  enable_weak xlib
  
  enable_weak cuda cuvid nvenc vda_framework videotoolbox videotoolbox_encoder

@@ -6058,12 +6072,10 @@ pod2man --help > /dev/null 2>&1 && enable pod2man   
|| disable pod2man
  rsync --help 2> /dev/null | grep -q 'contimeout' && enable rsync_contimeout 
|| disable rsync_contimeout
  
  check_header linux/fb.h

-check_header linux/videodev.h
-check_header linux/videodev2.h
-check_code cc linux/videodev2.h "struct v4l2_frmsizeenum vfse; vfse.discrete.width = 
0;" && enable_safe struct_v4l2_frmivalenum_discrete
  
  check_header

Re: [FFmpeg-devel] [PATCH] swscale: fix gbrap16 alpha channel issues

2017-08-03 Thread James Cowgill 
Hi,

On 02/08/17 23:21, Michael Niedermayer wrote:
> On Wed, Aug 02, 2017 at 03:32:04PM +0100, James Cowgill wrote:
>> Hi,
>>
>> On 02/08/17 14:18, Michael Niedermayer wrote:
>>> On Tue, Aug 01, 2017 at 02:46:22PM +0100, James Cowgill wrote:
 Fixes filter-pixfmts-scale test failing on big-endian systems due to
 alpSrc not being cast to (const int32_t**).

 Also fixes distortions in the output alpha channel values by copying the
 alpha channel code from the rgba64 case found elsewhere in output.c.

 Fixes ticket 6555.

 Signed-off-by: James Cowgill 
 ---
  libswscale/output.c | 15 ---
  tests/ref/fate/filter-pixfmts-scale |  4 ++--
  2 files changed, 10 insertions(+), 9 deletions(-)

 diff --git a/libswscale/output.c b/libswscale/output.c
 index 9774e9f327..8e5ec0a256 100644
 --- a/libswscale/output.c
 +++ b/libswscale/output.c
 @@ -2026,17 +2026,18 @@ yuv2gbrp16_full_X_c(SwsContext *c, const int16_t 
 *lumFilter,
  const int16_t **lumSrcx, int lumFilterSize,
  const int16_t *chrFilter, const int16_t **chrUSrcx,
  const int16_t **chrVSrcx, int chrFilterSize,
 -const int16_t **alpSrc, uint8_t **dest,
 +const int16_t **alpSrcx, uint8_t **dest,
  int dstW, int y)
  {
  const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(c->dstFormat);
  int i;
 -int hasAlpha = (desc->flags & AV_PIX_FMT_FLAG_ALPHA) && alpSrc;
 +int hasAlpha = (desc->flags & AV_PIX_FMT_FLAG_ALPHA) && alpSrcx;
  uint16_t **dest16 = (uint16_t**)dest;
  const int32_t **lumSrc  = (const int32_t**)lumSrcx;
  const int32_t **chrUSrc = (const int32_t**)chrUSrcx;
  const int32_t **chrVSrc = (const int32_t**)chrVSrcx;
 -int A = 0; // init to silence warning
 +const int32_t **alpSrc  = (const int32_t**)alpSrcx;
>>>
 +int A = 0x << 14;
>>>
>>> unused value
>>
>> The initial value of A is unused in the old code, but not in the new code.
> 
> IIRC all uses are under hasAlpha and it is writen to in that case first

Sorry, you're right. I think I was looking at the code from yuv2rgba64.
I'll send a v2.

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

[FFmpeg-devel] [PATCH] JPEG2000 encoding with variable codeblock size

2017-08-03 Thread francesco 
From: Francesco Cuzzocrea 

Hi to all
I've made some simple changes tha allow encoding with variable codeblock
size.  Default value are the same as previous (16X16) but now setting them
to 64x128 make generated codestream compatible with Analog Devices ADV212
video codec.


---
 libavcodec/j2kenc.c | 22 +-
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/libavcodec/j2kenc.c b/libavcodec/j2kenc.c
index c8d3861..1bd4fbd 100644
--- a/libavcodec/j2kenc.c
+++ b/libavcodec/j2kenc.c
@@ -1178,17 +1178,21 @@ static int j2kenc_destroy(AVCodecContext *avctx)
 // taken from the libopenjpeg wraper so it matches
 
 #define OFFSET(x) offsetof(Jpeg2000EncoderContext, x)
+#define OFFSET1(x) offsetof(Jpeg2000CodingStyle, x)
+
+
 #define VE AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_ENCODING_PARAM
 static const AVOption options[] = {
-{ "format","Codec Format",  OFFSET(format),
AV_OPT_TYPE_INT,   { .i64 = CODEC_JP2   }, CODEC_J2K, CODEC_JP2,   VE, "format" 
 },
-{ "j2k",   NULL,0, 
AV_OPT_TYPE_CONST, { .i64 = CODEC_J2K   }, 0, 0,   VE, "format" 
 },
-{ "jp2",   NULL,0, 
AV_OPT_TYPE_CONST, { .i64 = CODEC_JP2   }, 0, 0,   VE, "format" 
 },
-{ "tile_width","Tile Width",OFFSET(tile_width),
AV_OPT_TYPE_INT,   { .i64 = 256 }, 1, 1<<30,   VE, },
-{ "tile_height",   "Tile Height",   OFFSET(tile_height),   
AV_OPT_TYPE_INT,   { .i64 = 256 }, 1, 1<<30,   VE, },
-{ "pred",  "DWT Type",  OFFSET(pred),  
AV_OPT_TYPE_INT,   { .i64 = 0   }, 0, 1,   VE, "pred"   
 },
-{ "dwt97int",  NULL,0, 
AV_OPT_TYPE_CONST, { .i64 = 0   }, INT_MIN, INT_MAX,   VE, "pred"   
 },
-{ "dwt53", NULL,0, 
AV_OPT_TYPE_CONST, { .i64 = 0   }, INT_MIN, INT_MAX,   VE, "pred"   
 },
-
+{ "format",   "Codec Format",   OFFSET(format),
   AV_OPT_TYPE_INT,   { .i64 = CODEC_JP2   }, CODEC_J2K, CODEC_JP2, 
  VE, "format"  },
+{ "j2k",   NULL,0, 
   AV_OPT_TYPE_CONST, { .i64 = CODEC_J2K   }, 0, 0, 
  VE, "format"  },
+{ "jp2",   NULL,0, 
   AV_OPT_TYPE_CONST, { .i64 = CODEC_JP2   }, 0, 0, 
  VE, "format"  },
+{ "tile_width","Tile Width",OFFSET(tile_width),
   AV_OPT_TYPE_INT,   { .i64 = 256 }, 1, 1<<30, 
  VE, },
+{ "tile_height",   "Tile Height",   OFFSET(tile_height),   
   AV_OPT_TYPE_INT,   { .i64 = 256 }, 1, 1<<30, 
  VE, },
+{ "pred",  "DWT Type",  OFFSET(pred),  
   AV_OPT_TYPE_INT,   { .i64 = 0   }, 0, 1, 
  VE, "pred"},
+{ "dwt97int",  NULL,0, 
   AV_OPT_TYPE_CONST, { .i64 = 0   }, INT_MIN, INT_MAX, 
  VE, "pred"},
+{ "dwt53", NULL,0, 
   AV_OPT_TYPE_CONST, { .i64 = 0   }, INT_MIN, INT_MAX, 
  VE, "pred"},
+{ "log2_cblk_width",   "Codeblock Width",   
(OFFSET(codsty)+OFFSET1(log2_cblk_width)),AV_OPT_TYPE_INT,   { .i64 = 4 
  }, 1, 1<<30,   VE, },
+{ "log2_cblk_height",  "Codeblock Height",  
(OFFSET(codsty)+OFFSET1(log2_cblk_height)),   AV_OPT_TYPE_INT,   { .i64 = 4 
  }, 1, 1<<30,   VE, },
 { NULL }
 };
 
-- 
2.1.4

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

Re: [FFmpeg-devel] [PATCH 1/2] avcodec/h264idct_template: Fix integer overflow in ff_h264_idct_add()

2017-08-03 Thread Michael Niedermayer 
On Wed, Aug 02, 2017 at 01:28:17AM +0200, Michael Niedermayer wrote:
> Fixes: runtime error: signed integer overflow: 26215360 + 2121330944 cannot 
> be represented in type 'int'
> Fixes: 2809/clusterfuzz-testcase-minimized-4785181833560064
> 
> Found-by: continuous fuzzing process 
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> ---
>  libavcodec/h264idct_template.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)

patchset applied

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

The real ebay dictionary, page 1
"Used only once"- "Some unspecified defect prevented a second use"
"In good condition" - "Can be repaird by experienced expert"
"As is" - "You wouldnt want it even if you were payed for it, if you knew ..."


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

Re: [FFmpeg-devel] [PATCH] swscale/utils.c: Fix incorrect error when calling sws_setColorspaceDetails

2017-08-03 Thread Mark Boorer 
I can confirm that range works as expected, but I couldn't actually see any
difference when setting brightness, contrast or saturation. It looks as
though all of those calculations only take place inside the yuv2rgb lookup
table, so when converting from yuv->yuv they won't get applied.

In my case I am not using the brightness/contrast/saturation settings, only
the range.

On Thu, Aug 3, 2017 at 2:52 AM, Michael Niedermayer 
wrote:

> On Thu, Aug 03, 2017 at 12:18:01AM +0100, Mark Boorer wrote:
> > Fixes: When converting yuv -> yuv and changing any of srcRange,
> > dstRange, brightness, contrast, or saturation the context would not be
> > re-initialized as required.
> > ---
> >  libswscale/utils.c | 3 +++
> >  1 file changed, 3 insertions(+)
>
> have you confirmed that with this patch these parameters also work ?
>
> [...]
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> "I am not trying to be anyone's saviour, I'm trying to think about the
>  future and not be sad" - Elon Musk
>
>
> ___
> 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] [WIP] libcodec2 wrapper + de/muxer in FFmpeg

2017-08-03 Thread Nicolas George 
Le sextidi 16 thermidor, an CCXXV, Tomas Härdin a écrit :
> Posting this to both [ffmpeg-devel] and [Freetel-codec2] in hopes of
> maximum feedback.
> 
> I've been spending the last few days getting codec2 (http://freedv.org/
> ) hooked up in libavcodec, and a set of muxers and demuxers for both
> raw codec2 streams and the recently created .c2 format. codec2 is very
> low bitrate (3200 bit/s down to 700 bit/s) speech codec developed for
> digital voice in amateur radio, but is now finding use in other
> applications where compressing large amounts of human speech is useful
> (audiobooks or podcasts for example)
> 
> Sample: http://www.nivex.net/images/tmp/report2074.c2
> 
> With both the raw demuxer and the encoder you need to specify the
> desired mode, like this:

The encoder could default to one the two.

> Some remarks:
> 
> * I had to make the ffmpeg CLI not complain about the 700 modes, since
> it thinks encoding at below 1 kbps is a user error

It is just a warning. I am not really a fan of a special case like that,
and it would be better split into a separate patch I think.

> * I have duplicated some minor functions in libcodec2 in
> libavcodec/codec2utils.*. This avoid having to link libcodec2 just for
> remuxing, and in case someone writes a native decoder for libavcodec

The license allows it, but you neglected to give copyright attribution.

> * Not sure if codec2utils should go in libavutil, libavcodec felt good
> enough

Definitely libavcodec.

> * The demuxer tries to read up to 0.1 seconds worth of frames to speed
> things up a little without making seeking too broken. 3 frames = 12
> bytes for the 700 bit/s modes, which decode to 960 samples

I do not like the sound of that, but I will see the code.

> * The decoder is able to deal with multiple frames at a time, the
> encoder does not need to

???

> Feel free to bikeshed around whether extradata should be the entire .c2
> header or just the mode byte. It really only matters if we go with RIFF
> or ISOM mappings (.wav .avi .mov and friends), which I decided to leave
> out for now.

It matters for inclusion in any format: Matroska, Nut, etc. Is anybody
considering normalization?

> From 569a252536ea224bcd44f55f0f5102ce1aa4ec77 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Tomas=20H=C3=A4rdin?= 
> Date: Wed, 2 Aug 2017 22:17:19 +0200
> Subject: [PATCH] Add codec2 muxers, demuxers and en/decoder via libcodec2
> 
> ---
>  Changelog|   2 +
>  configure|  12 +++
>  doc/general.texi |  13 +++
>  ffmpeg.c |   3 +-
>  libavcodec/Makefile  |   3 +
>  libavcodec/allcodecs.c   |   1 +
>  libavcodec/avcodec.h |   1 +
>  libavcodec/codec2utils.c | 118 +++
>  libavcodec/codec2utils.h |  58 +++
>  libavcodec/codec_desc.c  |   7 ++
>  libavcodec/libcodec2.c   | 190 
>  libavcodec/version.h |   2 +-
>  libavformat/Makefile |   4 +
>  libavformat/allformats.c |   2 +
>  libavformat/codec2.c | 244 
> +++
>  libavformat/rawenc.c |  14 +++
>  libavformat/utils.c  |   1 +
>  libavformat/version.h|   2 +-
>  18 files changed, 674 insertions(+), 3 deletions(-)
>  create mode 100644 libavcodec/codec2utils.c
>  create mode 100644 libavcodec/codec2utils.h
>  create mode 100644 libavcodec/libcodec2.c
>  create mode 100644 libavformat/codec2.c
> 
> diff --git a/Changelog b/Changelog
> index 187ae7950a..e28da7dcc4 100644
> --- a/Changelog
> +++ b/Changelog
> @@ -29,6 +29,8 @@ version :
>  - limiter video filter
>  - libvmaf video filter
>  - Dolby E decoder and SMPTE 337M demuxer
> +- codec2 en/decoding via libcodec2
> +- muxer/demuxer for raw codec2 files and .c2 files
>  
>  version 3.3:
>  - CrystalHD decoder moved to new decode API
> diff --git a/configure b/configure
> index 66c7b948e4..05af25cb22 100755
> --- a/configure
> +++ b/configure
> @@ -220,6 +220,7 @@ External library support:
>--enable-libcaca enable textual display using libcaca [no]
>--enable-libcelt enable CELT decoding via libcelt [no]
>--enable-libcdio enable audio CD grabbing with libcdio [no]
> +  --enable-libcodec2   enable codec2 en/decoding using libcodec2 [no]
>--enable-libdc1394   enable IIDC-1394 grabbing using libdc1394
> and libraw1394 [no]
>--enable-libfdk-aac  enable AAC de/encoding via libfdk-aac [no]
> @@ -1540,6 +1541,7 @@ EXTERNAL_LIBRARY_LIST="
>  libbs2b
>  libcaca
>  libcelt
> +libcodec2
>  libdc1394
>  libflite
>  libfontconfig
> @@ -2087,6 +2089,7 @@ CONFIG_EXTRA="
>  blockdsp
>  bswapdsp
>  cabac
> +codec2utils
>  dirac_parse
>  dvprofile
>  exif
> @@ -2863,6 +2866,10 @@ pcm_mulaw_at_encoder_select="audio_frame_queue"
>  chromaprint_muxer_deps="chromaprint"
>  h264_videotoolbox_encoder_deps="videotoolbox_encoder pthreads"
>

Re: [FFmpeg-devel] [PATCHv2 4/4] libavcodec: v4l2: add support for v4l2 mem2mem codecs

2017-08-03 Thread Jorge Ramirez 

On 08/03/2017 11:22 AM, Nicolas George wrote:

I still hope you can be convinced to understand proper design
principles. But if that cannot be achieved,then authority it is.


if it is ok with you, let's move on.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCHv2 4/4] libavcodec: v4l2: add support for v4l2 mem2mem codecs

2017-08-03 Thread Nicolas George 
Le quintidi 15 thermidor, an CCXXV, Jorge Ramirez a écrit :
> how can I propose that "every single application duplicate headers" without
> knowing their use cases? that sort of generalization seems silly to me.
> there is no silver bullet to these kind of problems as you probably know.

You said yourself: the same reasons lead to the same conclusions. Since
all reasons you have given in this discussion apply equally to all
libraries and all applications, therefore the same conclusions should
apply too.

Since this is completely ridiculous, you have to understand that there
is something flawed in your reasoning.

> >If the new v4l APIs are so unstable that they break compatibility at
> >every single kernel version, then maybe they are not yet mature enough
> >for programs like FFmpeg.
> nope, non sequitur.

Good.

> since when open for extension and closed for modification is unstable?

Have you noticed that my sentence starts with "if"? You told the
condition was not meant, and I assume you know how an if/else clause
works, so you know that you should have answered to the next paragraph
and not this one:

> >And if they are, we can just use the installed headers.

Unfortunately, you neglected to answer that.

> it depends...duplicating the Universe might serve its purpose

Yes, indeed: to shift a little burden from your shoulders into somebody
else's, hugely inflating it at the same time as Reimar explained. You
will forgive us for not being receptive.

>   (you seem to
> go blindly against duplication...dont you have two lungs?)

I hope burning these straw men keeps you warm during these cold summer
days.

> anyway, if you and other maintainers want to simply use the "authority" card
> fine by me.

I still hope you can be convinced to understand proper design
principles. But if that cannot be achieved,then authority it is.

> 1. reduction in the frequency of the maintenance tasks.
> When they need to be performed (ie new format or fourcc or whatever), the
> user will be updating for the _future_ since it will import many other
> updates.
> -> You can't say the same about maintaining configure.

I can say the same about any other library and any other application
using them. You have not proven, as Hendrik requested, that V4L was
exceptional.

> 2. the build environment is always sane no matter where you build.
> This translates in more extensive testing since it enables building on more
> environments.
> -> You can't say the same about checking against whatever API was installed
> in the build environment (could be 8 years old)

I can say the same about any other library and any other application
using them. You have not proven, as Hendrik requested, that V4L was
exceptional.

> 3. you can build encoders natively on a 14.04 server running a 3.3 kernel
> and execute them on a target running a recent kernel
> -> that can't be done the way you propose

I can say the same about any other library and any other application
using them. You have not proven, as Hendrik requested, that V4L was
exceptional.

> And since the kernel guarantees that will not break userspace, there is zero
> risk to the users if we import the V4L2 API just as other projects have
> done.
> (do you have this guarantee with all the other APIs that you are using?)

Libraries following a disciplined development cycle do guarantee that
indeed.

Since you have not proven that V4L was special, we will assume it is not
and treat it as any other library. configure checks and no headers
duplication please.

> unfortunately  we will _always_ have that with the v4linux codecs.
> Only at runtime the user will discover if the codecs can be run in their
> platform

That can be said of any device: sample rates supported by ALSA devices,
resolutions supported by broadcast cards, etc. It is entirely normal.

Regards,

-- 
  Nicolas George


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

Re: [FFmpeg-devel] [WIP] libcodec2 wrapper + de/muxer in FFmpeg

2017-08-03 Thread Tomas Härdin 
On Thu, 2017-08-03 at 01:07 +0200, Carl Eugen Hoyos wrote:
> 2017-08-03 0:40 GMT+02:00 Tomas Härdin :
> 
> > 
> > Decoding a .c2 file is straightforward however, thanks to the
> > header:
> > 
> >   ffmpeg -i report2074.c2 out.wav
> The probe score is too high.
> I suspect you should also check for the major version,
> the score should be MAX/2 if the first 32bit are ok,
> significantly less for 24bit.

OK, didn't know that. Major being bumped would imply a new API, and
we're not likely to link two versions of the same library. I suspect it
would primarily impact the decoder, not the demuxer. But checking major
== 0 makes sense for now. libcodec2 does not yet expose version in its
headers, but should soon since I pointed that out to them :)

/Tomas



signature.asc
Description: This is a digitally signed message part
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] Implement NewTek NDI support

2017-08-03 Thread Steinar H. Gunderson 
On Thu, Aug 03, 2017 at 08:21:09AM +0300, Maksym Veremeyenko wrote:
>> You should reject bottom field first formats, as NDI explicitly
>> disallows them.
> even if it disallowed, why to drop? may be warning message would be enough?

FWIW, the documentation may say that formats are always TFF, but on the wire,
it seems the bottom field actually goes first.

(I guess this is a bug that isn't noticed because there's actually no way
from the API to get at the individual fields)

> +NDI is very picky about the formats it supports. Pixel format is always
> +uyvy422.

Or bgra (internally, uyvya4224, I suppose).

> +recv_create_desc.allow_video_fields = ctx->allow_video_fields;

I'm not convinced you ever want to set this to false, as what's happening if
you do is simply that the SDK applies a vertical blur to incoming interlaced
frames. FFmpeg has much better deinterlacers if that's what you want.

/* Steinar */
-- 
Homepage: https://www.sesse.net/
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH v5 2/3] speedhq: add FATE tests

2017-08-03 Thread Steinar H. Gunderson 
Also add simple FATE tests, based on output produced by the NDI SDK.
---
 tests/Makefile | 1 +
 tests/fate/speedhq.mak | 9 +
 tests/ref/fate/speedhq-422 | 6 ++
 tests/ref/fate/speedhq-422-singlefield | 6 ++
 4 files changed, 22 insertions(+)
 create mode 100644 tests/fate/speedhq.mak
 create mode 100644 tests/ref/fate/speedhq-422
 create mode 100644 tests/ref/fate/speedhq-422-singlefield

diff --git a/tests/Makefile b/tests/Makefile
index ab83ae855d..f9b9cf4188 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -164,6 +164,7 @@ include $(SRC_PATH)/tests/fate/qt.mak
 include $(SRC_PATH)/tests/fate/qtrle.mak
 include $(SRC_PATH)/tests/fate/real.mak
 include $(SRC_PATH)/tests/fate/screen.mak
+include $(SRC_PATH)/tests/fate/speedhq.mak
 include $(SRC_PATH)/tests/fate/source.mak
 include $(SRC_PATH)/tests/fate/subtitles.mak
 include $(SRC_PATH)/tests/fate/utvideo.mak
diff --git a/tests/fate/speedhq.mak b/tests/fate/speedhq.mak
new file mode 100644
index 00..7b6b03ea0d
--- /dev/null
+++ b/tests/fate/speedhq.mak
@@ -0,0 +1,9 @@
+FATE_SPEEDHQ = fate-speedhq-422   \
+   fate-speedhq-422-singlefield   \
+
+FATE_SAMPLES_FFMPEG-$(CONFIG_SPEEDHQ_DECODER) += fate-speedhq-422 
fate-speedhq-422-singlefield
+
+fate-speedhq: $(FATE_SPEEDHQ)
+
+fate-speedhq-422: CMD = framecrc -flags +bitexact -f rawvideo 
-codec speedhq -vtag SHQ2 -video_size 112x64 -i 
$(TARGET_SAMPLES)/speedhq/progressive.shq2 -pix_fmt yuv422p
+fate-speedhq-422-singlefield: CMD = framecrc -flags +bitexact -f rawvideo 
-codec speedhq -vtag SHQ2 -video_size 112x32 -i 
$(TARGET_SAMPLES)/speedhq/singlefield.shq2 -pix_fmt yuv422p
diff --git a/tests/ref/fate/speedhq-422 b/tests/ref/fate/speedhq-422
new file mode 100644
index 00..7bb0d2388d
--- /dev/null
+++ b/tests/ref/fate/speedhq-422
@@ -0,0 +1,6 @@
+#tb 0: 1/25
+#media_type 0: video
+#codec_id 0: rawvideo
+#dimensions 0: 112x64
+#sar 0: 0/1
+0,  0,  0,1,14336, 0x9bb6dc6d
diff --git a/tests/ref/fate/speedhq-422-singlefield 
b/tests/ref/fate/speedhq-422-singlefield
new file mode 100644
index 00..343c52645c
--- /dev/null
+++ b/tests/ref/fate/speedhq-422-singlefield
@@ -0,0 +1,6 @@
+#tb 0: 1/25
+#media_type 0: video
+#codec_id 0: rawvideo
+#dimensions 0: 112x32
+#sar 0: 0/1
+0,  0,  0,1, 7168, 0x75de4109
-- 
2.13.3

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

[FFmpeg-devel] [PATCH v5 3/3] Add myself as speedhq maintainer, per request.

2017-08-03 Thread Steinar H. Gunderson 
---
 MAINTAINERS | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index ae0e08d121..ce5e1dae08 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -233,6 +233,7 @@ Codecs:
   smvjpegdec.c  Ash Hughes
   snow* Michael Niedermayer, Loren Merritt
   sonic.c   Alex Beregszaszi
+  speedhq.c Steinar H. Gunderson
   srt*  Aurelien Jacobs
   sunrast.c Ivo van Poorten
   svq3.cMichael Niedermayer
@@ -598,6 +599,7 @@ Reynaldo H. Verdejo Pinochet  6E27 CD34 170C C78E 4D4F 5F40 
C18E 077F 3114 452A
 Robert Swain  EE7A 56EA 4A81 A7B5 2001 A521 67FA 362D A2FC 3E71
 Sascha Sommer 38A0 F88B 868E 9D3A 97D4 D6A0 E823 706F 1E07 0D3C
 Stefano Sabatini  0D0B AD6B 5330 BBAD D3D6 6A0C 719C 2839 FC43 2D5F
+Steinar H. Gunderson  C2E9 004F F028 C18E 4EAD DB83 7F61 7561 7797 8F76
 Stephan Hilb  4F38 0B3A 5F39 B99B F505 E562 8D5C 5554 4E17 8863
 Tiancheng "Timothy" Gu9456 AFC0 814A 8139 E994 8351 7FE6 B095 B582 B0D4
 Tim Nicholson 38CF DB09 3ED0 F607 8B67 6CED 0C0B FC44 8B0B FC83
-- 
2.13.3

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

[FFmpeg-devel] [PATCH v5 1/3] speedhq: fix behavior of single-field decoding

2017-08-03 Thread Steinar H. Gunderson 
The height convention for decoding frames with only a single field made sense
for compatibility with legacy decoders, but doesn't really match the convention
used by NDI, which is the primary (only?) user. Thus, change it to simply
assuming that if the two fields overlap, the frame is meant to be a single
field and the frame height matches the field height.
---
 libavcodec/speedhq.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/libavcodec/speedhq.c b/libavcodec/speedhq.c
index 60efb0222b..47b1e4dc7a 100644
--- a/libavcodec/speedhq.c
+++ b/libavcodec/speedhq.c
@@ -450,10 +450,13 @@ static int speedhq_decode_frame(AVCodecContext *avctx,
 if (second_field_offset == 4) {
 /*
  * Overlapping first and second fields is used to signal
- * encoding only a single field (the second field then comes
- * as a separate, later frame).
+ * encoding only a single field. In this case, "height"
+ * is ambiguous; it could mean either the height of the
+ * frame as a whole, or of the field. The former would make
+ * more sense for compatibility with legacy decoders,
+ * but this matches the convention used in NDI, which is
+ * the primary user of this trick.
  */
-frame->height >>= 1;
 if ((ret = decode_speedhq_field(s, buf, buf_size, frame, 0, 4, 
buf_size, 1)) < 0)
 return ret;
 } else {
-- 
2.13.3

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

Re: [FFmpeg-devel] [PATCH v4 2/3] speedhq: add FATE tests

2017-08-03 Thread Steinar H. Gunderson 
On Thu, Aug 03, 2017 at 02:24:17AM +0200, Michael Niedermayer wrote:
> doesnt work
> make -j12 fate-speedhq
> make: *** No rule to make target `fate-speedhq-422', needed by 
> `fate-speedhq'.  Stop.
> 
> make fate-speedhq-422-singlefield
> make: *** No rule to make target `fate-speedhq-422-singlefield'.  Stop.
> 
> make fate-list | grep speedhq

*sigh*

At this point, it would probably be nice just to find some documentation on
how this is supposed to work, because clearly, I can't get it right by trial
and error. :-)

/* Steinar */
-- 
Homepage: https://www.sesse.net/
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v2] avfilter/vf_ssim: fix temp size calculation

2017-08-03 Thread Tobias Rapp 

On 03.08.2017 03:03, Muhammad Faiz wrote:

Also use av_mallocz_array.
Fix Ticket6519.

Signed-off-by: Muhammad Faiz 
---
 libavfilter/vf_ssim.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/libavfilter/vf_ssim.c b/libavfilter/vf_ssim.c
index c3c204268f..d8c049177c 100644
--- a/libavfilter/vf_ssim.c
+++ b/libavfilter/vf_ssim.c
@@ -219,6 +219,8 @@ static float ssim_endn_8bit(const int (*sum0)[4], const int 
(*sum1)[4], int widt
 return ssim;
 }

+#define SUM_LEN(w) (((w) >> 2) + 3)
+
 static float ssim_plane_16bit(SSIMDSPContext *dsp,
   uint8_t *main, int main_stride,
   uint8_t *ref, int ref_stride,
@@ -228,7 +230,7 @@ static float ssim_plane_16bit(SSIMDSPContext *dsp,
 int z = 0, y;
 float ssim = 0.0;
 int64_t (*sum0)[4] = temp;
-int64_t (*sum1)[4] = sum0 + (width >> 2) + 3;
+int64_t (*sum1)[4] = sum0 + SUM_LEN(width);

 width >>= 2;
 height >>= 2;
@@ -256,7 +258,7 @@ static float ssim_plane(SSIMDSPContext *dsp,
 int z = 0, y;
 float ssim = 0.0;
 int (*sum0)[4] = temp;
-int (*sum1)[4] = sum0 + (width >> 2) + 3;
+int (*sum1)[4] = sum0 + SUM_LEN(width);

 width >>= 2;
 height >>= 2;
@@ -402,7 +404,7 @@ static int config_input_ref(AVFilterLink *inlink)
 for (i = 0; i < s->nb_components; i++)
 s->coefs[i] = (double) s->planeheight[i] * s->planewidth[i] / sum;

-s->temp = av_malloc_array((2 * inlink->w + 12), sizeof(*s->temp) * (1 + 
(desc->comp[0].depth > 8)));
+s->temp = av_mallocz_array(2 * SUM_LEN(inlink->w), (desc->comp[0].depth > 
8) ? sizeof(int64_t[4]) : sizeof(int[4]));
 if (!s->temp)
 return AVERROR(ENOMEM);
 s->max = (1 << desc->comp[0].depth) - 1;



Fixes the crashing command and runs without reports in Valgrind.

Thanks,
Tobias

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