Re: [FFmpeg-devel] mpegaudio_parser question

2018-06-17 Thread Ronald S. Bultje
Hi Karsten,

I believe that the parsers are seek-unaware, so you're expected to delete
and recreate (or reinit) the parsers after each seek.

Ronald

On Sun, Jun 17, 2018 at 10:56 AM, Karsten Otto  wrote:

> Hi list,
>
> I have a question about mpegaudio_parser. I see that it keeps any packet
> data before a frame header, instead of discarding it. This makes sense,
> because it usually can combine this data with the leftovers from a previous
> packet to complete a frame. But the parser also does this when just
> starting up, i.e. after a seek. I had expected it to discard the preceding
> data in this case. So, is this expected behaviour? Am I using it wrong?
>
> Background: I am trying to add seek capability to libavformat/aadec.
> Unfortunately, when the format contains MP3 content, its frames do *not*
> align with the internal crypto chunk boundaries. So when seeking to a valid
> chunk start, it usually lands in the middle of an MP3 frame. Given the
> behaviour of the mpegaudio_parser, there is always some audible glitch. I
> can work around this by adding my own MP3 header detection, but that would
> not be very DRY. So, what is the proper way of handling this case? Any
> suggestions?
>
> Thanks,
> Karsten
> ___
> 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 3/4 v3] avcodec/vc1: rewrite vc1_decode_i_blocks to align with VC-1 spec

2018-06-17 Thread Michael Niedermayer
On Tue, Jun 12, 2018 at 11:34:58AM +0200, Jerome Borsboom wrote:
> Change vc1_decode_i_blocks to use vc1_put_blocks_clamped and
> ff_vc1_i_loop_filter.
> 
> Signed-off-by: Jerome Borsboom 
> ---
> The v3 patch should resolve the crashing that was seen on corrupted source 
> files.

This (commit 77a3dfb328df535fb98d43ed2204fc6a42d6dd5e) broke fate
that is fate-mss2-wmv
fate-suite//mss2/msscreencodec.wmv with this shows a large green column and 
checksums
changed

--- ./tests/ref/fate/mss2-wmv   2018-06-15 22:31:04.412555973 +0200
+++ tests/data/fate/mss2-wmv2018-06-18 00:54:56.484378249 +0200
@@ -36,70 +36,70 @@
 0, 36, 36,1,   230400, 0x08bb41ee
 0, 37, 37,1,   230400, 0x43ccbd29
 0, 38, 38,1,   230400, 0x4ee3
-0, 39, 39,1,   230400, 0xbfd2ef29
-0, 40, 40,1,   230400, 0x6504545f
-0, 41, 41,1,   230400, 0x8fb86901
+0, 39, 39,1,   230400, 0x527879e5
+0, 40, 40,1,   230400, 0x1cc7e329
+0, 41, 41,1,   230400, 0xde1706ab
 0, 42, 42,1,   230400, 0xc95f0917
-0, 43, 43,1,   230400, 0x21f6a54b
-0, 44, 44,1,   230400, 0xf808106b
-0, 45, 45,1,   230400, 0x34150020
-0, 46, 46,1,   230400, 0x50fdfe89
+0, 43, 43,1,   230400, 0x81353456
+0, 44, 44,1,   230400, 0x8a15a752
+0, 45, 45,1,   230400, 0xede88dbb
+0, 46, 46,1,   230400, 0xb22fa577
 0, 47, 47,1,   230400, 0x920b7708
-0, 48, 48,1,   230400, 0xed64fcc4
-0, 49, 49,1,   230400, 0x6291a170
-0, 50, 50,1,   230400, 0x20524643
+0, 48, 48,1,   230400, 0x581c924c
+0, 49, 49,1,   230400, 0x18952c56
+0, 50, 50,1,   230400, 0x45d9e3f3
 0, 51, 51,1,   230400, 0x92aafecd
-0, 52, 52,1,   230400, 0xf00ee14d
-0, 53, 53,1,   230400, 0xfa3113ea
-0, 54, 54,1,   230400, 0x99c06df1
-0, 55, 55,1,   230400, 0x625c6918
+0, 52, 52,1,   230400, 0x1f789647
+0, 53, 53,1,   230400, 0x12ba8c93
+0, 54, 54,1,   230400, 0x6aabb970
+0, 55, 55,1,   230400, 0x1b3e003d
 0, 56, 56,1,   230400, 0xb277b25e
-0, 57, 57,1,   230400, 0x2e913006
-0, 58, 58,1,   230400, 0x3f6f1d99
-0, 59, 59,1,   230400, 0x100ab60f
+0, 57, 57,1,   230400, 0x511eb729
+0, 58, 58,1,   230400, 0x7328a657
+0, 59, 59,1,   230400, 0xfa2d5eed
 0, 60, 60,1,   230400, 0x9b73d0bf
-0, 61, 61,1,   230400, 0xda0df2ce
-0, 62, 62,1,   230400, 0x67f7ca24
-0, 63, 63,1,   230400, 0xbde9b3d0
-0, 64, 64,1,   230400, 0x92e14d07
+0, 61, 61,1,   230400, 0xd5698c41
+0, 62, 62,1,   230400, 0xa9c332d7
+0, 63, 63,1,   230400, 0x22451f10
+0, 64, 64,1,   230400, 0x6fc0b571
 0, 65, 65,1,   230400, 0x9426c3d9
-0, 66, 66,1,   230400, 0x6104be70
-0, 67, 67,1,   230400, 0xc4d1078a
-0, 68, 68,1,   230400, 0x89426a42
-0, 69, 69,1,   230400, 0x5271324a
+0, 66, 66,1,   230400, 0x375932bf
+0, 67, 67,1,   230400, 0xaf3f9d2e
+0, 68, 68,1,   230400, 0xefced725
+0, 69, 69,1,   230400, 0xaa85d8b3
 0, 70, 70,1,   230400, 0x1cb1c735
-0, 71, 71,1,   230400, 0x4249b8c6
-0, 72, 72,1,   230400, 0x4b88cad3
-0, 73, 73,1,   230400, 0x76af545d
+0, 71, 71,1,   230400, 0xe2805640
+0, 72, 72,1,   230400, 0xe6a22093
+0, 73, 73,1,   230400, 0xa95ddab7
 0, 74, 74,1,   230400, 0xfe47e3c4
-0, 75, 75,1,   230400, 0xa2e0e721
-0, 76, 76,1,   230400, 0xde974a42
-0, 77, 77,1,   230400, 0x87bf38ba
-0, 78, 78,1,   230400, 0xd52318fd
-0, 79, 79,1,   230400, 0x0bbb1526
+0, 75, 75,1,   230400, 0xed3970ea
+0, 76, 76,1,   230400, 0x2ba6ee17
+0,   

Re: [FFmpeg-devel] [PATCH] avcodec/qsvenc: fix version detection on cygwin

2018-06-17 Thread Timo Rothenpieler



Am 17.06.2018 um 17:48 schrieb Mark Thompson:

On 15/06/18 15:52, Timo Rothenpieler wrote:

---
  libavcodec/qsvenc.h | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/qsvenc.h b/libavcodec/qsvenc.h
index d48272224c..bb175c5df8 100644
--- a/libavcodec/qsvenc.h
+++ b/libavcodec/qsvenc.h
@@ -45,7 +45,7 @@
  #define QSV_HAVE_LA_DS  QSV_VERSION_ATLEAST(1, 8)
  #define QSV_HAVE_LA_HRD QSV_VERSION_ATLEAST(1, 11)
  
-#if defined(_WIN32)

+#if defined(_WIN32) || defined(__CYGWIN__)
  #define QSV_HAVE_AVBR   QSV_VERSION_ATLEAST(1, 3)
  #define QSV_HAVE_ICQQSV_VERSION_ATLEAST(1, 8)
  #define QSV_HAVE_VCMQSV_VERSION_ATLEAST(1, 8)



Probably ok.

Does something actually go wrong here on Cygwin, or do you just end up without 
those features?  (This stuff should all be tested at runtime, but I can't ask 
you to rewrite it to do that...)


Cygwin is WIN32 for all intents and purposes of QSV.

Without this change, it goes into the, what I assume is, the very 
Limited Linux branch, and most useful features like -global_quality ICQ 
and other common rate control modes are disabled.



Are you going to want similar checks at the other instances of #if _WIN32 in 
that code?


The only other two instances of _WIN32 in the QSV code only select a 
default for the HEVC de/encoder plugin.
In my case, the non-WIN32 one worked, the WIN32 one wasn't even known, 
so I decided not to touch that code.




smime.p7s
Description: S/MIME Cryptographic Signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 3/4] ffprobe: print mb_types frame side data

2018-06-17 Thread Stefano Sabatini
On date Sunday 2018-06-17 06:21:16 +0200, Ramiro Polla encoded:
> ---
>  fftools/ffprobe.c | 19 +++
>  1 file changed, 19 insertions(+)

[...]

Missing changes to doc/ffprobe.xsd I think.
-- 
FFmpeg = Fast and Foolish Mysterious Picky Efficient Guide
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 2/4] mpegutils: split debug function that prints mb_type so it may be used by ffprobe

2018-06-17 Thread Michael Niedermayer
On Sun, Jun 17, 2018 at 06:21:15AM +0200, Ramiro Polla wrote:
> ---
>  libavcodec/mpegutils.c | 115 
> +
>  libavcodec/mpegutils.h |   7 +++
>  2 files changed, 76 insertions(+), 46 deletions(-)
> 
> diff --git a/libavcodec/mpegutils.c b/libavcodec/mpegutils.c
> index 0fbe5f8c9d..12c2468797 100644
> --- a/libavcodec/mpegutils.c
> +++ b/libavcodec/mpegutils.c
> @@ -100,6 +100,72 @@ void ff_draw_horiz_band(AVCodecContext *avctx,
>  }
>  }
>  
> +int ff_mb_type_str(char *str, int size, int mb_type)
> +{
> +char *ptr = str;
> +
> +if (size <= 0)
> +return 0;
> +
> +if (--size <= 0)
> +goto end;
> +
> +// Type & MV direction
> +if (IS_PCM(mb_type))
> +*ptr++ = 'P';
> +else if (IS_INTRA(mb_type) && IS_ACPRED(mb_type))
> +*ptr++ = 'A';
> +else if (IS_INTRA4x4(mb_type))
> +*ptr++ = 'i';
> +else if (IS_INTRA16x16(mb_type))
> +*ptr++ = 'I';
> +else if (IS_DIRECT(mb_type) && IS_SKIP(mb_type))
> +*ptr++ = 'd';
> +else if (IS_DIRECT(mb_type))
> +*ptr++ = 'D';
> +else if (IS_GMC(mb_type) && IS_SKIP(mb_type))
> +*ptr++ = 'g';
> +else if (IS_GMC(mb_type))
> +*ptr++ = 'G';
> +else if (IS_SKIP(mb_type))
> +*ptr++ = 'S';
> +else if (!USES_LIST(mb_type, 1))
> +*ptr++ = '>';
> +else if (!USES_LIST(mb_type, 0))
> +*ptr++ = '<';
> +else {
> +av_assert2(USES_LIST(mb_type, 0) && USES_LIST(mb_type, 1));
> +*ptr++ = 'X';
> +}
> +
> +if (--size <= 0)
> +goto end;
> +
> +// segmentation
> +if (IS_8X8(mb_type))
> +*ptr++ = '+';
> +else if (IS_16X8(mb_type))
> +*ptr++ = '-';
> +else if (IS_8X16(mb_type))
> +*ptr++ = '|';
> +else if (IS_INTRA(mb_type) || IS_16X16(mb_type))
> +*ptr++ = ' ';
> +else
> +*ptr++ = '?';
> +
> +if (--size <= 0)
> +goto end;
> +
> +if (IS_INTERLACED(mb_type))
> +*ptr++ = '=';
> +else
> +*ptr++ = ' ';
> +
> +end:
> +*ptr = '\0';
> +return ptr - str;
> +}
> +
>  void ff_print_debug_info2(AVCodecContext *avctx, AVFrame *pict, uint8_t 
> *mbskip_table,
>   uint32_t *mbtype_table, int8_t *qscale_table, 
> int16_t (*motion_val[2])[2],
>   int *low_delay,
> @@ -231,52 +297,9 @@ void ff_print_debug_info2(AVCodecContext *avctx, AVFrame 
> *pict, uint8_t *mbskip_
> qscale_table[x + y * mb_stride]);
>  }
>  if (avctx->debug & FF_DEBUG_MB_TYPE) {
> -int mb_type = mbtype_table[x + y * mb_stride];
> -// Type & MV direction
> -if (IS_PCM(mb_type))
> -av_log(avctx, AV_LOG_DEBUG, "P");
> -else if (IS_INTRA(mb_type) && IS_ACPRED(mb_type))
> -av_log(avctx, AV_LOG_DEBUG, "A");
> -else if (IS_INTRA4x4(mb_type))
> -av_log(avctx, AV_LOG_DEBUG, "i");
> -else if (IS_INTRA16x16(mb_type))
> -av_log(avctx, AV_LOG_DEBUG, "I");
> -else if (IS_DIRECT(mb_type) && IS_SKIP(mb_type))
> -av_log(avctx, AV_LOG_DEBUG, "d");
> -else if (IS_DIRECT(mb_type))
> -av_log(avctx, AV_LOG_DEBUG, "D");
> -else if (IS_GMC(mb_type) && IS_SKIP(mb_type))
> -av_log(avctx, AV_LOG_DEBUG, "g");
> -else if (IS_GMC(mb_type))
> -av_log(avctx, AV_LOG_DEBUG, "G");
> -else if (IS_SKIP(mb_type))
> -av_log(avctx, AV_LOG_DEBUG, "S");
> -else if (!USES_LIST(mb_type, 1))
> -av_log(avctx, AV_LOG_DEBUG, ">");
> -else if (!USES_LIST(mb_type, 0))
> -av_log(avctx, AV_LOG_DEBUG, "<");
> -else {
> -av_assert2(USES_LIST(mb_type, 0) && 
> USES_LIST(mb_type, 1));
> -av_log(avctx, AV_LOG_DEBUG, "X");
> -}
> -
> -// segmentation
> -if (IS_8X8(mb_type))
> -av_log(avctx, AV_LOG_DEBUG, "+");
> -else if (IS_16X8(mb_type))
> -av_log(avctx, AV_LOG_DEBUG, "-");
> -else if (IS_8X16(mb_type))
> -av_log(avctx, AV_LOG_DEBUG, "|");
> -else if (IS_INTRA(mb_type) || IS_16X16(mb_type))
> -av_log(avctx, AV_LOG_DEBUG, " ");
> -else
> -av_log(avctx, AV_LOG_DEBUG, "?");
> -
> -
> -if (IS_INTERLACED(mb_type))
> -av_log(avctx, AV_LOG_DEBUG, "=");
> -else

Re: [FFmpeg-devel] [PATCH] lavfi: add helper macro for OpenCL error handling.

2018-06-17 Thread Mark Thompson
On 15/06/18 03:36, Dan Yaschenko wrote:
> On 12 June 2018 at 10:20, Ruiling Song  wrote:
> 
>> Signed-off-by: Ruiling Song 
>> ---
>> I am not sure whether do you think this would be useful?
>> the main purpose is to make OpenCL error check code simpler.
>> If we think this is good, I can go to replace current
>> OpenCL filters to use this macro.
> 
> 
>> for example:
>> if (cle != CL_SUCCESS) {
>> av_log(avctx, AV_LOG_ERROR, "Failed to enqueue kernel: %d.\n",
>>cle);
>> err = AVERROR(EIO);
>> goto fail;
>> }
>> can be replaced with:
>> OCL_FAIL_ON_ERR(avctx, cle, AVERROR(EIO), "Failed to enqueue kernel:
>> %d.\n", cle);
>>
>> Thanks!
>> Ruiling
>>  libavfilter/opencl.h | 11 +++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/libavfilter/opencl.h b/libavfilter/opencl.h
>> index c0a4519..c33df1c 100644
>> --- a/libavfilter/opencl.h
>> +++ b/libavfilter/opencl.h
>> @@ -97,5 +97,16 @@ int ff_opencl_filter_work_size_from_image(AVFilterContext
>> *avctx,
>>size_t *work_size,
>>AVFrame *frame, int plane,
>>int block_alignment);
>> +/**
>> + * A helper macro to handle OpenCL error. It will assign errcode to
>> + * variable err, log error msg, and jump to fail label on error.
>> + */
>> +#define OCL_FAIL_ON_ERR(logctx, cle, errcode, ...) do {\
>> +if (cle != CL_SUCCESS) {\
>> +av_log(logctx, AV_LOG_ERROR, __VA_ARGS__);\
>> +err = errcode;\
>> +goto fail;\
>> +}\
>> +} while(0)
>>
>>  #endif /* AVFILTER_OPENCL_H */
>> --
>> 2.7.4
>>
> 
> Hi!
> I like your idea, but still don't know which one is better.
> My idea relies on fact, that there are only few OpenCL functions which are
> used multiple times in filters:
> setKernelArg, clCreateKernel(in case when there are multiple kernels) and
> maybe
> clEnqueueNDRangeKernel. So that is why my purpose is totally wrap them and
> significantly reduce code,
> but yes, there are some restrictions, like you can't use kernel_arg++ when
> setting kernel arguments.
> And still most of cl-error checking statements appear after using
> cl-functions listed above.

The specific one for kernel args is the largest source of extra boilerplate, so 
I've applied it already.  (Even with the more general setup it would still make 
sense for a separate macro to exist to do the kernel arguments.)

This more general case seems reasonable to me if you'd like to implement it.  
To make it slightly simpler, it's probably ok to assume that the first two 
arguments have the fixed names used in all current filters ("avctx" for the 
AVFilterContext, "cle" for the CL error code we want to check).  I'd probably 
also make it "CL_FAIL_ON_ERROR" rather than "OCL_FAIL_ON_ERR" to better match 
existing style.

Thanks,

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


Re: [FFmpeg-devel] [PATCH] libavfilter/opencl.h: Add macro for setting opencl kernel

2018-06-17 Thread Mark Thompson
On 15/06/18 03:55, Danil Iashchenko wrote:
> ---
> 
> Hi!
> I like your idea with OCL_FAIL_ON_ERR(), but still do not know which one is 
> better.
> My idea relies on fact, that there are only few OpenCL functions which are 
> used multiple times in filters:
> clSetKernelArg, clCreateKernel(in case when there are multiple kernels) and 
> maybe
> clEnqueueNDRangeKernel. So that is why my purpose is totally wrap them and 
> significantly reduce code,
> but yes, there are some restrictions, like you can not use kernel_arg++ when 
> setting kernel arguments. 
> And still most of cl-error checking statements appear after using 
> cl-functions listed above.

I've applied this one, since it does simplify the most repetitive operation of 
setting the kernel arguments.

(I'll answer the more general suggestion in the other thread.)

Thanks,

- Mark


>  libavfilter/opencl.h| 15 ++
>  libavfilter/vf_convolution_opencl.c | 43 
>  libavfilter/vf_overlay_opencl.c | 44 +++-
>  libavfilter/vf_unsharp_opencl.c | 57 
> ++---
>  4 files changed, 46 insertions(+), 113 deletions(-)
> 
> diff --git a/libavfilter/opencl.h b/libavfilter/opencl.h
> index c0a4519..7441b11 100644
> --- a/libavfilter/opencl.h
> +++ b/libavfilter/opencl.h
> @@ -46,6 +46,21 @@ typedef struct OpenCLFilterContext {
>  intoutput_height;
>  } OpenCLFilterContext;
>  
> +
> +/**
> + * set argument to specific Kernel.
> + * This macro relies on usage of local label "fail" and variables:
> + * avctx, cle and err.
> + */
> +#define CL_SET_KERNEL_ARG(kernel, arg_num, type, arg)  \
> +cle = clSetKernelArg(kernel, arg_num, sizeof(type), arg);  \
> +if (cle != CL_SUCCESS) {   \
> +av_log(avctx, AV_LOG_ERROR, "Failed to set kernel "\
> +   "argument %d: error %d.\n", arg_num, cle);  \
> +err = AVERROR(EIO);\
> +goto fail; \
> +}
> +
>  /**
>   * Return that all inputs and outputs support only AV_PIX_FMT_OPENCL.
>   */
> diff --git a/libavfilter/vf_convolution_opencl.c 
> b/libavfilter/vf_convolution_opencl.c
> index 2df51e0..4d0ecf8 100644
> --- a/libavfilter/vf_convolution_opencl.c
> +++ b/libavfilter/vf_convolution_opencl.c
> @@ -204,43 +204,12 @@ static int convolution_opencl_filter_frame(AVFilterLink 
> *inlink, AVFrame *input)
>  if (!dst)
>  break;
>  
> -cle = clSetKernelArg(ctx->kernel, 0, sizeof(cl_mem), );
> -if (cle != CL_SUCCESS) {
> -av_log(avctx, AV_LOG_ERROR, "Failed to set kernel "
> -   "destination image argument: %d.\n", cle);
> -goto fail;
> -}
> -cle = clSetKernelArg(ctx->kernel, 1, sizeof(cl_mem), );
> -if (cle != CL_SUCCESS) {
> -av_log(avctx, AV_LOG_ERROR, "Failed to set kernel "
> -   "source image argument: %d.\n", cle);
> -goto fail;
> -}
> -cle = clSetKernelArg(ctx->kernel, 2, sizeof(cl_int), >dims[p]);
> -if (cle != CL_SUCCESS) {
> -av_log(avctx, AV_LOG_ERROR, "Failed to set kernel "
> -   "matrix size argument: %d.\n", cle);
> -goto fail;
> -}
> -cle = clSetKernelArg(ctx->kernel, 3, sizeof(cl_mem), 
> >matrix[p]);
> -if (cle != CL_SUCCESS) {
> -av_log(avctx, AV_LOG_ERROR, "Failed to set kernel "
> -   "matrix argument: %d.\n", cle);
> -goto fail;
> -}
> -cle = clSetKernelArg(ctx->kernel, 4, sizeof(cl_float), 
> >rdivs[p]);
> -if (cle != CL_SUCCESS) {
> -av_log(avctx, AV_LOG_ERROR, "Failed to set kernel "
> -   "rdiv argument: %d.\n", cle);
> -goto fail;
> -}
> -cle = clSetKernelArg(ctx->kernel, 5, sizeof(cl_float), 
> >biases[p]);
> -if (cle != CL_SUCCESS) {
> -av_log(avctx, AV_LOG_ERROR, "Failed to set kernel "
> -   "bias argument: %d.\n", cle);
> -goto fail;
> -}
> -
> +CL_SET_KERNEL_ARG(ctx->kernel, 0, cl_mem,   );
> +CL_SET_KERNEL_ARG(ctx->kernel, 1, cl_mem,   );
> +CL_SET_KERNEL_ARG(ctx->kernel, 2, cl_int,   >dims[p]);
> +CL_SET_KERNEL_ARG(ctx->kernel, 3, cl_mem,   >matrix[p]);
> +CL_SET_KERNEL_ARG(ctx->kernel, 4, cl_float, >rdivs[p]);
> +CL_SET_KERNEL_ARG(ctx->kernel, 5, cl_float, >biases[p]);
>  
>  err = ff_opencl_filter_work_size_from_image(avctx, global_work, 
> output, p, 0);
>  if (err < 0)
> diff --git a/libavfilter/vf_overlay_opencl.c b/libavfilter/vf_overlay_opencl.c
> index b43050d..556ce35 100644
> --- a/libavfilter/vf_overlay_opencl.c
> +++ b/libavfilter/vf_overlay_opencl.c
> @@ -167,47 +167,39 @@ static int overlay_opencl_blend(FFFrameSync *fs)
>

Re: [FFmpeg-devel] [PATCH] avdevice/v4l2enc: add video4linux2 name alias

2018-06-17 Thread Carl Eugen Hoyos
2018-06-17 19:10 GMT+02:00, Lou Logan :
> On Sun, Jun 17, 2018, at 8:25 AM, Carl Eugen Hoyos wrote:
>>
>> Wouldn't it make more sense to remove the extremely
>> long name at the next bump if consistency is needed
>> here?
>
> I don't think it matters too much to also have the alias video4linux2, or at
> least I can't think of any good reasons to remove it.
>
> One reason for this trivial patch was so the v4l2 in/out-devs can be listed
> on one line in "ffmpeg -devices" or "ffmpeg -formats" like (all?) the
> others. One user reported that he was unaware of the outdev because he only
> saw the indev listed (possibly looking only for video4linux2).

Thank you for the explanation.

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


Re: [FFmpeg-devel] [PATCH] avdevice/v4l2enc: add video4linux2 name alias

2018-06-17 Thread Lou Logan
On Sun, Jun 17, 2018, at 8:25 AM, Carl Eugen Hoyos wrote:
>
> Wouldn't it make more sense to remove the extremely
> long name at the next bump if consistency is needed
> here?

I don't think it matters too much to also have the alias video4linux2, or at 
least I can't think of any good reasons to remove it.

One reason for this trivial patch was so the v4l2 in/out-devs can be listed on 
one line in "ffmpeg -devices" or "ffmpeg -formats" like (all?) the others. One 
user reported that he was unaware of the outdev because he only saw the indev 
listed (possibly looking only for video4linux2).
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avcodec/mips: [loongson] optimize vp8 decoding in vp8dsp.

2018-06-17 Thread Carl Eugen Hoyos
2018-06-14 13:56 GMT+02:00, yinshiyou...@loongson.cn :
>>
>> 2018-06-14 9:36 GMT+02:00, Shiyou Yin :
>> > From: gxw 
>> >
>> > Optimize vp8 decoding with mmi in function:
>> > 1. ff_vp8_h_loop_filter8uv_mmi.
>> > 2. ff_vp8_v_loop_filter8uv_mmi.
>> > 3. ff_vp8_h_loop_filter16_mmi.
>> > 4. ff_vp8_v_loop_filter16_mmi.
>>
>> Please add some numbers about the speedup, overall and / or
>> function-wise.
>
> Ok,I will enrich the commit message.By the way, May I ask two questions.
> 1. what should I do after making some changes for the patch,Should I
> resent a new mail with 'git send-email' or what.

Send a new patch that applies against current FFmpeg git head.

> 2. How should I know if my patch have passed the review process.

If it passes, it will get applied.

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


Re: [FFmpeg-devel] [PATCH]lavfi/avfiltergraph: Do not return ENOMEM if filter is missing

2018-06-17 Thread Nicolas George
Mark Thompson (2018-06-17):
> There is AVERROR_FILTER_NOT_FOUND - while it isn't currently returned
> from lavfi, it does sound exactly right for this.

It is not exactly right for this, because this can be ENOMEM too.

Furthermore, this error code is really wrong, because this function does
not search for a filter, it takes it as an argument, and the argument
should not be NULL. Therefore, in this particular instance, the correct
fix would be to replace the initial check in ff_filter_alloc() by an
assert.

> (An alternative would be to check beforehand whether the filter name
> exists?  Not sure if that's really any better.)

It is not a filter name, it is a filter that was already checked.
Calling avfilter_graph_create_filter() with a NULL filter makes no sense
and is not allowed by the documented API. Adding a check before is
necessary.

Regards,

-- 
  Nicolas George


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


Re: [FFmpeg-devel] [PATCH] avdevice/v4l2enc: add video4linux2 name alias

2018-06-17 Thread Carl Eugen Hoyos
2018-06-14 23:39 GMT+02:00, Lou Logan :

>  AVOutputFormat ff_v4l2_muxer = {
> -.name   = "v4l2",
> +.name   = "video4linux2,v4l2",

Wouldn't it make more sense to remove the extremely
long name at the next bump if consistency is needed
here?

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


Re: [FFmpeg-devel] [PATCH]lavfi/avfiltergraph: Do not return ENOMEM if filter is missing

2018-06-17 Thread Mark Thompson
On 16/06/18 13:39, Carl Eugen Hoyos wrote:
> 2018-02-05 3:05 GMT+01:00, James Almer :
>> On 2/4/2018 10:33 PM, Carl Eugen Hoyos wrote:
>>> Hi!
>>>
>>> OOM is unlikely as a failure for avfilter_graph_alloc_filter(), the
>>> patch avoids a surprising error if a filter was not found.
>>>
>>> Please comment, Carl Eugen
>>>
>>>
>>> 0001-lavfi-avfiltergraph-Do-not-return-ENOMEM-if-filterch.patch
>>>
>>>
>>> From 6587726a5e96570bb54e49ccf0b7fd6d94b929c8 Mon Sep 17 00:00:00 2001
>>> From: Carl Eugen Hoyos 
>>> Date: Mon, 5 Feb 2018 01:43:08 +0100
>>> Subject: [PATCH] lavfi/avfiltergraph: Do not return ENOMEM if filterchain
>>>  init failed.
>>>
>>> A more likely reason is that the filter was not found.
>>> ---
>>>  libavfilter/avfiltergraph.c |2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/libavfilter/avfiltergraph.c b/libavfilter/avfiltergraph.c
>>> index 4cc6892..7ccd895 100644
>>> --- a/libavfilter/avfiltergraph.c
>>> +++ b/libavfilter/avfiltergraph.c
>>> @@ -147,7 +147,7 @@ int avfilter_graph_create_filter(AVFilterContext
>>> **filt_ctx, const AVFilter *fil
>>>
>>>  *filt_ctx = avfilter_graph_alloc_filter(graph_ctx, filt, name);
>>>  if (!*filt_ctx)
>>> -return AVERROR(ENOMEM);
>>> +return -1;
>>
>> -1 is not acceptable for a public function that states it returns an
>> AVERROR value on failure.
>> If the issue is that avfilter_graph_alloc_filter() may fail because of
>> both OOM and an invalid value for filter, then I'm more inclined to use
>> AVERROR_UNKNOWN here instead.
> 
> New patch attached.
> 
> Please comment, Carl Eugen

There is AVERROR_FILTER_NOT_FOUND - while it isn't currently returned from 
lavfi, it does sound exactly right for this.

I'd prefer for it to use a function which can return an error code and 
propagate that (IMO the return-a-pointer style should only be used for 
functions which can only fail by OOM), but given the circumstances what you've 
written there looks fine.

(An alternative would be to check beforehand whether the filter name exists?  
Not sure if that's really any better.)

Thanks,

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


Re: [FFmpeg-devel] [PATCH] avcodec/qsvenc: fix version detection on cygwin

2018-06-17 Thread Mark Thompson
On 15/06/18 15:52, Timo Rothenpieler wrote:
> ---
>  libavcodec/qsvenc.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavcodec/qsvenc.h b/libavcodec/qsvenc.h
> index d48272224c..bb175c5df8 100644
> --- a/libavcodec/qsvenc.h
> +++ b/libavcodec/qsvenc.h
> @@ -45,7 +45,7 @@
>  #define QSV_HAVE_LA_DS  QSV_VERSION_ATLEAST(1, 8)
>  #define QSV_HAVE_LA_HRD QSV_VERSION_ATLEAST(1, 11)
>  
> -#if defined(_WIN32)
> +#if defined(_WIN32) || defined(__CYGWIN__)
>  #define QSV_HAVE_AVBR   QSV_VERSION_ATLEAST(1, 3)
>  #define QSV_HAVE_ICQQSV_VERSION_ATLEAST(1, 8)
>  #define QSV_HAVE_VCMQSV_VERSION_ATLEAST(1, 8)
> 

Probably ok.

Does something actually go wrong here on Cygwin, or do you just end up without 
those features?  (This stuff should all be tested at runtime, but I can't ask 
you to rewrite it to do that...)

Are you going to want similar checks at the other instances of #if _WIN32 in 
that code?

Thanks,

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


Re: [FFmpeg-devel] [PATCH] avcodec/libx265: apply lavc maxrate and bufsize

2018-06-17 Thread Mark Thompson
On 13/06/18 14:38, Gyan Doshi wrote:
> Default for both parameters in both libs is 0.
> 
> Regards,
> Gyan
> 
> From f875734cc5a1652162156118864db406b192526b Mon Sep 17 00:00:00 2001
> From: Gyan Doshi 
> Date: Wed, 13 Jun 2018 19:01:17 +0530
> Subject: [PATCH] avcodec/libx265: apply lavc maxrate and bufsize
> 
> CLI options -maxrate and -bufsize are now picked up by the x265 wrapper
> ---
>  libavcodec/libx265.c | 3 +++
>  libavcodec/version.h | 2 +-
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/libavcodec/libx265.c b/libavcodec/libx265.c
> index bb457dfe5a..6c6fd88602 100644
> --- a/libavcodec/libx265.c
> +++ b/libavcodec/libx265.c
> @@ -205,6 +205,9 @@ static av_cold int libx265_encode_init(AVCodecContext 
> *avctx)
>  ctx->params->rc.rateControlMode = X265_RC_ABR;
>  }
>  
> +ctx->params->rc.vbvBufferSize = avctx->rc_buffer_size / 1000;
> +ctx->params->rc.vbvMaxBitrate = avctx->rc_max_rate/ 1000;
> +
>  if (!(avctx->flags & AV_CODEC_FLAG_GLOBAL_HEADER))
>  ctx->params->bRepeatHeaders = 1;
>  
> diff --git a/libavcodec/version.h b/libavcodec/version.h
> index 5e71b48816..219f56c37d 100644
> --- a/libavcodec/version.h
> +++ b/libavcodec/version.h
> @@ -29,7 +29,7 @@
>  
>  #define LIBAVCODEC_VERSION_MAJOR  58
>  #define LIBAVCODEC_VERSION_MINOR  20
> -#define LIBAVCODEC_VERSION_MICRO 101
> +#define LIBAVCODEC_VERSION_MICRO 102
>  
>  #define LIBAVCODEC_VERSION_INT  AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
> LIBAVCODEC_VERSION_MINOR, \
> -- 
> 2.12.2.windows.2

Given you have these two, maybe set vbvBufferInit from 
rc_initial_buffer_occupancy as well?

LGTM in either case.

Thanks,

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


Re: [FFmpeg-devel] [PATCH 1/2] avcodec/vaapi: slice_vertical_position starts from zero for the second field

2018-06-17 Thread Mark Thompson
On 15/06/18 06:44, Xiang, Haihao wrote:
> 
> I may confirm that the error is returned from the old version of libva and 
> vaapi
> driver w/wo this patch, the command below works well for me if using the new
> version of libva and vaapi driver.  

Trying others:

* The Mesa driver running on an AMD Polaris returns success but the output data 
is wrong for interlaced input.  I think it just isn't supported.

* The iHD driver running on Skylake GT3 always falls over when given an 
interlaced stream - the decode issue succeeds, but it returns "Failed to sync 
surface 0x5: 23 (internal decoding error)." when trying to get the result.

I tested on different machines with the current i965 driver and this looks 
good, so I will apply it.

However, since it only works on recent i965 I think that means something should 
be added to VAAPI indicate that support so that we can check it here.  The 
other cases (including old i965) all say they support the profile and therefore 
pass the point where we commit to hardware decode, but then they all fail later 
in different ways.  Maybe that should be a new config attribute, kindof like 
VAConfigAttribEncInterlaced for decode?

Thanks,

- Mark


>> Is your VAAPI library and VAAPI driver new enough? You need at least
>> libva-2.1.0 (VA-API version 1.1.0) and intel-vaapi-driver-2.1.0 for
>> interlaced VC-1 decoding. From the output, I think you are using an
>> older version and the error is just the libva library bailing out for
>> not supporting interlaced VC-1.
>>
>>
>> Regards,
>> Jerome
>>
>>> are these 2 patches enough or something else ?
>>> It feels like iam missing something but
>>>
>>> i tried
>>> ./ffmpeg -hwaccel vaapi -i SA10180.vc1 -pix_fmt yuv420p -f framecrc
>>> crcpatch12
>>>
>>> but ffmpeg spews errors at me:
>>>
>>> ibva info: VA-API version 0.39.0
>>> libva info: va_getDriverName() returns 0
>>> libva info: Trying to open /usr/lib/x86_64-linux-gnu/dri/i965_drv_video.so
>>> libva info: Found init function __vaDriverInit_0_39
>>> libva info: va_openDriver() returns 0
>>> Stream mapping:
>>>   Stream #0:0 -> #0:0 (vc1 (native) -> rawvideo (native))
>>> Press [q] to stop, [?] for help
>>> Output #0, framecrc, to 'crcpatch12':
>>>   Metadata:
>>> encoder : Lavf58.17.100
>>> Stream #0:0: Video: rawvideo (I420 / 0x30323449), yuv420p, 720x480 [SAR
>>> 1:1 DAR 3:2], q=2-31, 103680 kb/s, 25 fps, 25 tbn, 25 tbc
>>> Metadata:
>>>   encoder : Lavc58.20.102 rawvideo
>>> [vc1 @ 0x3d430c0] Failed to end picture decode issue: 23 (unknown libva
>>> error / description missing).
>>> Error while decoding stream #0:0: Input/output error
>>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] mpegaudio_parser question

2018-06-17 Thread Karsten Otto
Hi list,

I have a question about mpegaudio_parser. I see that it keeps any packet data 
before a frame header, instead of discarding it. This makes sense, because it 
usually can combine this data with the leftovers from a previous packet to 
complete a frame. But the parser also does this when just starting up, i.e. 
after a seek. I had expected it to discard the preceding data in this case. So, 
is this expected behaviour? Am I using it wrong?

Background: I am trying to add seek capability to libavformat/aadec. 
Unfortunately, when the format contains MP3 content, its frames do *not* align 
with the internal crypto chunk boundaries. So when seeking to a valid chunk 
start, it usually lands in the middle of an MP3 frame. Given the behaviour of 
the mpegaudio_parser, there is always some audible glitch. I can work around 
this by adding my own MP3 header detection, but that would not be very DRY. So, 
what is the proper way of handling this case? Any suggestions?

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


Re: [FFmpeg-devel] [PATCH 1/4] lavu/frame: add mb_types side data

2018-06-17 Thread Hendrik Leppkes
On Sun, Jun 17, 2018 at 2:45 PM Rostislav Pehlivanov
 wrote:
>
> On 17 June 2018 at 05:21, Ramiro Polla  wrote:
>
> > ---
> >  libavcodec/avcodec.h   |  4 
> >  libavcodec/mpegutils.c | 20 
> >  libavcodec/options_table.h |  1 +
> >  libavutil/frame.c  |  1 +
> >  libavutil/frame.h  |  9 +
> >  5 files changed, 35 insertions(+)
> >
> > diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> > index c90166deb6..7fe4fc9347 100644
> > --- a/libavcodec/avcodec.h
> > +++ b/libavcodec/avcodec.h
> > @@ -929,6 +929,10 @@ typedef struct RcOverride{
> >   */
> >  #define AV_CODEC_FLAG2_SHOW_ALL   (1 << 22)
> >  /**
> > + * Export macroblock types through frame side data
> > + */
> > +#define AV_CODEC_FLAG2_EXPORT_MB_TYPES (1 << 27)
> > +/**
> >   * Export motion vectors through frame side data
> >   */
> >  #define AV_CODEC_FLAG2_EXPORT_MVS (1 << 28)
> > diff --git a/libavcodec/mpegutils.c b/libavcodec/mpegutils.c
> > index 3f94540616..0fbe5f8c9d 100644
> > --- a/libavcodec/mpegutils.c
> > +++ b/libavcodec/mpegutils.c
> > @@ -188,6 +188,26 @@ void ff_print_debug_info2(AVCodecContext *avctx,
> > AVFrame *pict, uint8_t *mbskip_
> >  av_freep();
> >  }
> >
> > +if ((avctx->flags2 & AV_CODEC_FLAG2_EXPORT_MB_TYPES) &&
> > mbtype_table) {
> > +int size = (2 + mb_height * mb_width) * sizeof(uint32_t);
> > +int mb_x, mb_y;
> > +
> > +AVFrameSideData *sd;
> > +uint32_t *out;
> > +
> > +sd = av_frame_new_side_data(pict, AV_FRAME_DATA_MB_TYPES, size);
> > +if (!sd)
> > +return;
> > +
> > +out = (uint32_t *) sd->data;
> > +*out++ = mb_height;
> > +*out++ = mb_width;
> > +
> > +for (mb_y = 0; mb_y < mb_height; mb_y++)
> > +for (mb_x = 0; mb_x < mb_width; mb_x++)
> > +*out++ = mbtype_table[mb_x + mb_y * mb_stride];
> > +}
> > +
> >  /* TODO: export all the following to make them accessible for users
> > (and filters) */
> >  if (avctx->hwaccel || !mbtype_table)
> >  return;
> > diff --git a/libavcodec/options_table.h b/libavcodec/options_table.h
> > index 099261e168..25c84de321 100644
> > --- a/libavcodec/options_table.h
> > +++ b/libavcodec/options_table.h
> > @@ -76,6 +76,7 @@ static const AVOption avcodec_options[] = {
> >  {"export_mvs", "export motion vectors through frame side data", 0,
> > AV_OPT_TYPE_CONST, {.i64 = AV_CODEC_FLAG2_EXPORT_MVS}, INT_MIN, INT_MAX,
> > V|D, "flags2"},
> >  {"skip_manual", "do not skip samples and export skip information as frame
> > side data", 0, AV_OPT_TYPE_CONST, {.i64 = AV_CODEC_FLAG2_SKIP_MANUAL},
> > INT_MIN, INT_MAX, V|D, "flags2"},
> >  {"ass_ro_flush_noop", "do not reset ASS ReadOrder field on flush", 0,
> > AV_OPT_TYPE_CONST, {.i64 = AV_CODEC_FLAG2_RO_FLUSH_NOOP}, INT_MIN, INT_MAX,
> > S|D, "flags2"},
> > +{"export_mb_types", "export macroblock types through frame side data", 0,
> > AV_OPT_TYPE_CONST, {.i64 = AV_CODEC_FLAG2_EXPORT_MB_TYPES}, INT_MIN,
> > INT_MAX, V|D, "flags2"},
> >  {"time_base", NULL, OFFSET(time_base), AV_OPT_TYPE_RATIONAL, {.dbl = 0},
> > 0, INT_MAX},
> >  {"g", "set the group of picture (GOP) size", OFFSET(gop_size),
> > AV_OPT_TYPE_INT, {.i64 = 12 }, INT_MIN, INT_MAX, V|E},
> >  {"ar", "set audio sampling rate (in Hz)", OFFSET(sample_rate),
> > AV_OPT_TYPE_INT, {.i64 = DEFAULT }, 0, INT_MAX, A|D|E},
> > diff --git a/libavutil/frame.c b/libavutil/frame.c
> > index deb9b6f334..577d4f6e6d 100644
> > --- a/libavutil/frame.c
> > +++ b/libavutil/frame.c
> > @@ -834,6 +834,7 @@ const char *av_frame_side_data_name(enum
> > AVFrameSideDataType type)
> >  case AV_FRAME_DATA_ICC_PROFILE: return "ICC profile";
> >  case AV_FRAME_DATA_QP_TABLE_PROPERTIES: return "QP table
> > properties";
> >  case AV_FRAME_DATA_QP_TABLE_DATA:   return "QP table
> > data";
> > +case AV_FRAME_DATA_MB_TYPES:return "Macroblock
> > types";
> >  }
> >  return NULL;
> >  }
> > diff --git a/libavutil/frame.h b/libavutil/frame.h
> > index 9d57d6ce66..ce1231b03b 100644
> > --- a/libavutil/frame.h
> > +++ b/libavutil/frame.h
> > @@ -158,6 +158,15 @@ enum AVFrameSideDataType {
> >   */
> >  AV_FRAME_DATA_QP_TABLE_DATA,
> >  #endif
> > +
> > +/**
> > + * Macroblock types exported by some codecs (on demand through the
> > + * export_mb_types flag set in the libavcodec AVCodecContext flags2
> > option).
> > + * The data is composed by a header consisting of uint32_t mb_height
> > and
> > + * uint32_t mb_width, followed by a uint32_t
> > mb_types[mb_height][mb_width]
> > + * array.
> > + */
> > +AV_FRAME_DATA_MB_TYPES,
> >  };
> >
> >  enum AVActiveFormatDescription {
> > --
> > 2.11.0
> >
> > ___
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
>
> NAK
>
> I'd 

Re: [FFmpeg-devel] [PATCH v2 35/36] vaapi_encode_h265: Set level based on stream if not set by user

2018-06-17 Thread Mark Thompson
On 15/06/18 05:22, Xiang, Haihao wrote:
> On Fri, 2018-06-08 at 00:43 +0100, Mark Thompson wrote:
>> Sets the level based on the stream properties if it is not explicitly
>> set by the user.  Also add a tier option to set general_tier_flag, since
>> that affects the level choice.
>> ---
>>  doc/encoders.texi  |  4 
>>  libavcodec/vaapi_encode_h265.c | 34 +++---
>>  2 files changed, 35 insertions(+), 3 deletions(-)
>>
>> ...
>> diff --git a/libavcodec/vaapi_encode_h265.c b/libavcodec/vaapi_encode_h265.c
>> index 2cee19be68..100f8338d7 100644
>> --- a/libavcodec/vaapi_encode_h265.c
>> +++ b/libavcodec/vaapi_encode_h265.c
>> @@ -30,6 +30,7 @@
>>  #include "avcodec.h"
>>  #include "cbs.h"
>>  #include "cbs_h265.h"
>> +#include "h265_profile_level.h"
>>  #include "hevc.h"
>>  #include "hevc_sei.h"
>>  #include "internal.h"
>> @@ -48,6 +49,7 @@ typedef struct VAAPIEncodeH265Context {
>>  int qp;
>>  int aud;
>>  int profile;
>> +int tier;
>>  int level;
>>  int sei;
>>  
>> @@ -315,7 +317,7 @@ static int
>> vaapi_encode_h265_init_sequence_params(AVCodecContext *avctx)
>>  ptl->general_profile_space = 0;
>>  ptl->general_profile_space = 0;
>>  ptl->general_profile_idc   = avctx->profile;
>> -ptl->general_tier_flag = 0;
>> +ptl->general_tier_flag = priv->tier;
>>  
>>  if (chroma_format == 1) {
>>  ptl->general_profile_compatibility_flag[1] = bit_depth ==  8;
>> @@ -340,7 +342,25 @@ static int
>> vaapi_encode_h265_init_sequence_params(AVCodecContext *avctx)
>>  
>>  ptl->general_lower_bit_rate_constraint_flag = 1;
>>  
>> -ptl->general_level_idc = avctx->level;
>> +if (avctx->level != FF_LEVEL_UNKNOWN) {
>> +ptl->general_level_idc = avctx->level;
> 
> Fist check whether the level idc is legal? User may set a level which can not 
> be found in h265_levels. 

The user should be able to specify an arbitrary value explicitly.  As for H.264 
I'll add a check that this fits in the 8-bit field, though.

>> +} else {
>> +const H265LevelDescriptor *level;
>> +
>> +level = ff_h265_guess_level(ptl, avctx->bit_rate,
>> +ctx->surface_width, ctx->surface_height,
>> +1, 1, 1, (ctx->b_per_p > 0) + 1);
>> +if (level) {
>> +av_log(avctx, AV_LOG_VERBOSE, "Using level %s.\n", level->name);
>> +ptl->general_level_idc = level->level_idc;
>> +} else {
>> +av_log(avctx, AV_LOG_VERBOSE, "Stream will not conform to "
>> +   "any normal level; using level 8.5.\n");
>> +ptl->general_level_idc = 255;
>> +// The tier flag must be set in level 8.5.
>> +ptl->general_tier_flag = 1;
>> +}
>> +}
>>  
>>  vps->vps_sub_layer_ordering_info_present_flag = 0;
>>  vps->vps_max_dec_pic_buffering_minus1[0]  = (ctx->b_per_p > 0) + 1;
>> @@ -1146,9 +1166,17 @@ static const AVOption vaapi_encode_h265_options[] = {
>>  { PROFILE("rext",   FF_PROFILE_HEVC_REXT) },
>>  #undef PROFILE
>>  
>> +{ "tier", "Set tier (general_tier_flag)",
>> +  OFFSET(tier), AV_OPT_TYPE_INT,
>> +  { .i64 = 0 }, 0, 1, FLAGS, "tier" },
>> +{ "main", NULL, 0, AV_OPT_TYPE_CONST,
>> +  { .i64 = 0 }, 0, 0, FLAGS, "tier" },
>> +{ "high", NULL, 0, AV_OPT_TYPE_CONST,
>> +  { .i64 = 1 }, 0, 0, FLAGS, "tier" },
>> +
>>  { "level", "Set level (general_level_idc)",
>>OFFSET(level), AV_OPT_TYPE_INT,
>> -  { .i64 = 153 }, 0x00, 0xff, FLAGS, "level" },
>> +  { .i64 = FF_LEVEL_UNKNOWN }, FF_LEVEL_UNKNOWN, 0xff, FLAGS, "level" },
>>  
>>  #define LEVEL(name, value) name, NULL, 0, AV_OPT_TYPE_CONST, \
>>{ .i64 = value }, 0, 0, FLAGS, "level"

Thanks,

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


Re: [FFmpeg-devel] [PATCH v2 33/36] vaapi_encode_h265: Improve profile support

2018-06-17 Thread Mark Thompson
On 15/06/18 04:59, Xiang, Haihao wrote:
> On Fri, 2018-06-08 at 00:43 +0100, Mark Thompson wrote:
>> Set profile compatibility/constraint flags properly (including the
>> constraint flags used for RExt profiles, as all streams we can currently
>> generate are RExt-compatible), and use that to add support for the "Main
>> Intra" and "Main 10 Intra" RExt subprofiles (for which we can re-use the
>> existing Main and Main10 VAAPI profiles).
>> ---
>>  libavcodec/Makefile|  2 +-
>>  libavcodec/vaapi_encode_h265.c | 71 +--
>> ---
>>  2 files changed, 57 insertions(+), 16 deletions(-)
>>
>> ...
>> diff --git a/libavcodec/vaapi_encode_h265.c b/libavcodec/vaapi_encode_h265.c
>> index 97bb9cef6c..2cee19be68 100644
>> --- a/libavcodec/vaapi_encode_h265.c
>> +++ b/libavcodec/vaapi_encode_h265.c
>> ...
>> @@ -289,19 +312,35 @@ static int
>> vaapi_encode_h265_init_sequence_params(AVCodecContext *avctx)
>>  vps->vps_max_sub_layers_minus1 = 0;
>>  vps->vps_temporal_id_nesting_flag  = 1;
>>  
>> -vps->profile_tier_level = (H265RawProfileTierLevel) {
>> -.general_profile_space = 0,
>> -.general_profile_idc   = avctx->profile,
>> -.general_tier_flag = 0,
>> +ptl->general_profile_space = 0;
>> +ptl->general_profile_space = 0;
> 
> A duplicate line.

Removed.

>> +ptl->general_profile_idc   = avctx->profile;
>> +ptl->general_tier_flag = 0;
>>  
>> -.general_progressive_source_flag= 1,
>> -.general_interlaced_source_flag = 0,
>> -.general_non_packed_constraint_flag = 1,
>> -.general_frame_only_constraint_flag = 1,
>> +if (chroma_format == 1) {
>> +ptl->general_profile_compatibility_flag[1] = bit_depth ==  8;
>> +ptl->general_profile_compatibility_flag[2] = bit_depth <= 10;
>> +}
>> +ptl->general_profile_compatibility_flag[4] = 1;
>>  
>> -.general_level_idc = avctx->level,
>> -};
>> -
>> vps->profile_tier_level.general_profile_compatibility_flag[avctx->profile 
>> & 31] = 1;
>> +ptl->general_progressive_source_flag= 1;
>> +ptl->general_interlaced_source_flag = 0;
>> +ptl->general_non_packed_constraint_flag = 1;
>> +ptl->general_frame_only_constraint_flag = 1;
>> +
>> +ptl->general_max_12bit_constraint_flag = bit_depth <= 12;
>> +ptl->general_max_10bit_constraint_flag = bit_depth <= 10;
>> +ptl->general_max_8bit_constraint_flag  = bit_depth ==  8;
>> +
>> +ptl->general_max_422chroma_constraint_flag  = chroma_format <= 2;
>> +ptl->general_max_420chroma_constraint_flag  = chroma_format <= 1;
>> +ptl->general_max_monochrome_constraint_flag = chroma_format == 0;
>> +
>> +ptl->general_intra_constraint_flag = ctx->gop_size == 1;
>> +
>> +ptl->general_lower_bit_rate_constraint_flag = 1;
>> +
>> +ptl->general_level_idc = avctx->level;
>>  
>>  vps->vps_sub_layer_ordering_info_present_flag = 0;
>>  vps->vps_max_dec_pic_buffering_minus1[0]  = (ctx->b_per_p > 0) + 1;
>> @@ -343,7 +382,7 @@ static int
>> vaapi_encode_h265_init_sequence_params(AVCodecContext *avctx)
>>  
>>  sps->sps_seq_parameter_set_id = 0;
>>  
>> -sps->chroma_format_idc  = 1; // YUV 4:2:0.
>> +sps->chroma_format_idc  = chroma_format;
>>  sps->separate_colour_plane_flag = 0;
>>  
>>  sps->pic_width_in_luma_samples  = ctx->surface_width;
>> @@ -362,9 +401,8 @@ static int
>> vaapi_encode_h265_init_sequence_params(AVCodecContext *avctx)
>>  sps->conformance_window_flag = 0;
>>  }
>>  
>> -sps->bit_depth_luma_minus8 =
>> -avctx->profile == FF_PROFILE_HEVC_MAIN_10 ? 2 : 0;
>> -sps->bit_depth_chroma_minus8 = sps->bit_depth_luma_minus8;
>> +sps->bit_depth_luma_minus8   = bit_depth - 8;
>> +sps->bit_depth_chroma_minus8 = bit_depth - 8;
>>  
>>  sps->log2_max_pic_order_cnt_lsb_minus4 = 8;
>>  
>> ...

Thanks,

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


Re: [FFmpeg-devel] [PATCH v2 32/36] cbs_h264: Infer default VUI values if VUI parameters are not present

2018-06-17 Thread Mark Thompson
On 15/06/18 04:46, Xiang, Haihao wrote:
> On Fri, 2018-06-08 at 00:43 +0100, Mark Thompson wrote:
>> ---
>>  libavcodec/cbs_h264_syntax_template.c | 42
>> +++
>>  1 file changed, 42 insertions(+)
>>
>> diff --git a/libavcodec/cbs_h264_syntax_template.c
>> b/libavcodec/cbs_h264_syntax_template.c
>> index f53c02467e..03f2a15b0b 100644
>> --- a/libavcodec/cbs_h264_syntax_template.c
>> +++ b/libavcodec/cbs_h264_syntax_template.c
>> @@ -211,6 +211,46 @@ static int FUNC(vui_parameters)(CodedBitstreamContext
>> *ctx, RWContext *rw,
>>  return 0;
>>  }
>>  
>> +static int FUNC(vui_parameters_default)(CodedBitstreamContext *ctx,
>> +RWContext *rw, H264RawVUI *current,
>> +H264RawSPS *sps)
>> +{
>> +infer(aspect_ratio_idc, 0);
>> +
>> +infer(video_format, 5);
>> +infer(video_full_range_flag,0);
>> +infer(colour_primaries, 2);
>> +infer(transfer_characteristics, 2);
>> +infer(matrix_coefficients,  2);
>> +
>> +infer(chroma_sample_loc_type_top_field,0);
>> +infer(chroma_sample_loc_type_bottom_field, 0);
>> +
>> +infer(fixed_frame_rate_flag, 0);
>> +infer(low_delay_hrd_flag,1);
>> +
>> +infer(pic_struct_present_flag, 0);
>> +
>> +infer(motion_vectors_over_pic_boundaries_flag, 1);
>> +infer(max_bytes_per_pic_denom, 2);
>> +infer(max_bits_per_mb_denom,   1);
>> +infer(log2_max_mv_length_horizontal, 15);
>> +infer(log2_max_mv_length_vertical,   15);
> 
> Both log2_max_mv_length_horizontal and log2_max_mv_length_vertical should be 
> 16

No - see previous mail.

>> +
>> +if ((sps->profile_idc ==  44 || sps->profile_idc ==  86 ||
>> + sps->profile_idc == 100 || sps->profile_idc == 110 ||
>> + sps->profile_idc == 122 || sps->profile_idc == 244) &&
>> +sps->constraint_set3_flag) {
>> +infer(max_num_reorder_frames,  0);
>> +infer(max_dec_frame_buffering, 0);
>> +} else {
>> +infer(max_num_reorder_frames,  H264_MAX_DPB_FRAMES);
>> +infer(max_dec_frame_buffering, H264_MAX_DPB_FRAMES);
>> +}
>> +
>> +return 0;
>> +}
>> +
>>  static int FUNC(sps)(CodedBitstreamContext *ctx, RWContext *rw,
>>   H264RawSPS *current)
>>  {
>> @@ -315,6 +355,8 @@ static int FUNC(sps)(CodedBitstreamContext *ctx, 
>> RWContext
>> *rw,
>>  flag(vui_parameters_present_flag);
>>  if (current->vui_parameters_present_flag)
>>  CHECK(FUNC(vui_parameters)(ctx, rw, >vui, current));
>> +else
>> +CHECK(FUNC(vui_parameters_default)(ctx, rw, >vui, 
>> current));
>>  
>>  CHECK(FUNC(rbsp_trailing_bits)(ctx, rw));
>>  
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH v2 30/36] cbs_h264: Fix range and default value of max mv lengths

2018-06-17 Thread Mark Thompson
On 15/06/18 04:01, Xiang, Haihao wrote:
> On Fri, 2018-06-08 at 00:43 +0100, Mark Thompson wrote:
>> The max and default values are 15, not 16.
> 
> I guessed you mixed both h264 and h265. The h264 doc i have specifies the 
> range
> for log2_max_mv_length_vertical/log2_max_mv_length_horizontal is [0, 16] and 
> the
> value of log2_max_mv_length_vertical/log2_max_mv_length_horizontal should be
> inferred to 16 when log2_max_mv_length_vertical/log2_max_mv_length_horizontal 
> is
> not present.

Try the 2016/10 version (or later).

I was initially thinking the previous 16 value was an error on my part, but 
Andreas Rheinhardt noted that it was 16 in older standards and therefore we 
need to continue to accept 16 as an input value.

>> ---
>>  libavcodec/cbs_h264_syntax_template.c | 8 
>>  libavcodec/vaapi_encode_h264.c| 4 ++--
>>  2 files changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/libavcodec/cbs_h264_syntax_template.c
>> b/libavcodec/cbs_h264_syntax_template.c
>> index 027b555db6..21edcb799e 100644
>> --- a/libavcodec/cbs_h264_syntax_template.c
>> +++ b/libavcodec/cbs_h264_syntax_template.c
>> @@ -185,16 +185,16 @@ static int FUNC(vui_parameters)(CodedBitstreamContext
>> *ctx, RWContext *rw,
>>  flag(motion_vectors_over_pic_boundaries_flag);
>>  ue(max_bytes_per_pic_denom, 0, 16);
>>  ue(max_bits_per_mb_denom,   0, 16);
>> -ue(log2_max_mv_length_horizontal, 0, 16);
>> -ue(log2_max_mv_length_vertical,   0, 16);
>> +ue(log2_max_mv_length_horizontal, 0, 15);
>> +ue(log2_max_mv_length_vertical,   0, 15);

So, removed this change to the bounds but kept the new inferred values below.

>>  ue(max_num_reorder_frames,  0, H264_MAX_DPB_FRAMES);
>>  ue(max_dec_frame_buffering, 0, H264_MAX_DPB_FRAMES);
>>  } else {
>>  infer(motion_vectors_over_pic_boundaries_flag, 1);
>>  infer(max_bytes_per_pic_denom, 2);
>>  infer(max_bits_per_mb_denom,   1);
>> -infer(log2_max_mv_length_horizontal, 16);
>> -infer(log2_max_mv_length_vertical,   16);
>> +infer(log2_max_mv_length_horizontal, 15);
>> +infer(log2_max_mv_length_vertical,   15);
>>  
>>  if ((sps->profile_idc ==  44 || sps->profile_idc ==  86 ||
>>   sps->profile_idc == 110 || sps->profile_idc == 110 ||
>> diff --git a/libavcodec/vaapi_encode_h264.c b/libavcodec/vaapi_encode_h264.c
>> index 4034053dc0..0d7780110c 100644
>> --- a/libavcodec/vaapi_encode_h264.c
>> +++ b/libavcodec/vaapi_encode_h264.c
>> @@ -491,8 +491,8 @@ static int
>> vaapi_encode_h264_init_sequence_params(AVCodecContext *avctx)
>>  
>>  sps->vui.bitstream_restriction_flag= 1;
>>  sps->vui.motion_vectors_over_pic_boundaries_flag = 1;
>> -sps->vui.log2_max_mv_length_horizontal = 16;
>> -sps->vui.log2_max_mv_length_vertical   = 16;
>> +sps->vui.log2_max_mv_length_horizontal = 15;
>> +sps->vui.log2_max_mv_length_vertical   = 15;
>>  sps->vui.max_num_reorder_frames= (ctx->b_per_p > 0);
>>  sps->vui.max_dec_frame_buffering   = sps->max_num_ref_frames;
>>  

Thanks,

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


Re: [FFmpeg-devel] [PATCH v2 29/36] h264_metadata: Add option to set the level of the stream

2018-06-17 Thread Mark Thompson
On 15/06/18 03:49, Xiang, Haihao wrote:
> On Fri, 2018-06-08 at 00:43 +0100, Mark Thompson wrote:
>> ---
>>  doc/bitstream_filters.texi |  9 +
>>  libavcodec/h264_metadata_bsf.c | 90
>> ++
>>  2 files changed, 99 insertions(+)
>>
>> ...
>> diff --git a/libavcodec/h264_metadata_bsf.c b/libavcodec/h264_metadata_bsf.c
>> index 90ad4aad98..cb1a835fb8 100644
>> --- a/libavcodec/h264_metadata_bsf.c
>> +++ b/libavcodec/h264_metadata_bsf.c
>> ...
>> @@ -683,6 +743,36 @@ static const AVOption h264_metadata_options[] = {
>>  0, AV_OPT_TYPE_CONST,
>>  { .i64 = FLIP_VERTICAL },   .flags = FLAGS, .unit = "flip" },
>>  
>> +{ "level", "Set level (table A-1)",
>> +OFFSET(level), AV_OPT_TYPE_INT,
>> +{ .i64 = LEVEL_UNSET }, LEVEL_UNSET, 0xff, FLAGS, "level" },
>> +{ "auto", "Attempt to guess level from stream properties",
>> +0, AV_OPT_TYPE_CONST,
>> +{ .i64 = LEVEL_AUTO }, 0, 0, FLAGS, "level" },
> 
> Could you please use the same code style for AV_OPT_TYPE_CONST ?
> 
>> +#define LEVEL(name, value) name, NULL, 0, AV_OPT_TYPE_CONST, \
>> +{ .i64 = value }, 0, 0, FLAGS, "level"
> 
> Also here.

Both changed to use the designated initialiser.

>> +{ LEVEL("1",   10) },
>> +{ LEVEL("1b",   9) },
>> +{ LEVEL("1.1", 11) },
>> +{ LEVEL("1.2", 12) },
>> +{ LEVEL("1.3", 13) },
>> +{ LEVEL("2",   20) },
>> +{ LEVEL("2.1", 21) },
>> +{ LEVEL("2.2", 22) },
>> +{ LEVEL("3",   30) },
>> +{ LEVEL("3.1", 31) },
>> +{ LEVEL("3.2", 32) },
>> +{ LEVEL("4",   40) },
>> +{ LEVEL("4.1", 41) },
>> +{ LEVEL("4.2", 42) },
>> +{ LEVEL("5",   50) },
>> +{ LEVEL("5.1", 51) },
>> +{ LEVEL("5.2", 52) },
>> +{ LEVEL("6",   60) },
>> +{ LEVEL("6.1", 61) },
>> +{ LEVEL("6.2", 62) },
>> +#undef LEVEL
>> +
>>  { NULL }
>>  };
>>  

Thanks,

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


Re: [FFmpeg-devel] [PATCH v2 28/36] vaapi_encode_h264: Set level based on stream if not set by user

2018-06-17 Thread Mark Thompson
On 15/06/18 03:24, Xiang, Haihao wrote:
> On Fri, 2018-06-08 at 00:43 +0100, Mark Thompson wrote:
>> ---
>>  libavcodec/vaapi_encode_h264.c | 34 ++
>>  1 file changed, 30 insertions(+), 4 deletions(-)
>>
>> diff --git a/libavcodec/vaapi_encode_h264.c b/libavcodec/vaapi_encode_h264.c
>> index 82166d4457..4034053dc0 100644
>> --- a/libavcodec/vaapi_encode_h264.c
>> +++ b/libavcodec/vaapi_encode_h264.c
>> @@ -30,6 +30,7 @@
>>  #include "cbs.h"
>>  #include "cbs_h264.h"
>>  #include "h264.h"
>> +#include "h264_levels.h"
>>  #include "h264_sei.h"
>>  #include "internal.h"
>>  #include "vaapi_encode.h"
>> @@ -294,6 +295,7 @@ static int
>> vaapi_encode_h264_init_sequence_params(AVCodecContext *avctx)
>>  H264RawPPS*pps = >raw_pps;
>>  VAEncSequenceParameterBufferH264 *vseq = ctx->codec_sequence_params;
>>  VAEncPictureParameterBufferH264  *vpic = ctx->codec_picture_params;
>> +int dpb_frames;
>>  
>>  memset(>current_access_unit, 0,
>> sizeof(priv->current_access_unit));
>> @@ -319,7 +321,32 @@ static int
>> vaapi_encode_h264_init_sequence_params(AVCodecContext *avctx)
>>  sps->constraint_set5_flag = ctx->b_per_p == 0;
>>  }
>>  
>> -sps->level_idc = avctx->level;
>> +if (ctx->gop_size == 1)
>> +dpb_frames = 0;
>> +else
>> +dpb_frames = 1 + (ctx->b_per_p > 0);
>> +
>> +if (avctx->level != FF_LEVEL_UNKNOWN) {
>> +sps->level_idc = avctx->level;
> 
> Is avctx->level always legal? if not, I think some checks on avctx->level 
> should
> be added.

I'll add a check that it fits in the 8-bit field.  I don't think it should be 
checked any more than that when the user has specified an explicit value.

>> +} else {
>> +const H264LevelDescriptor *level;
>> +
>> +level = ff_h264_guess_level(sps->profile_idc,
>> +avctx->bit_rate,
>> +priv->mb_width  * 16,
>> +priv->mb_height * 16,
>> +dpb_frames);
>> +if (level) {
>> +av_log(avctx, AV_LOG_VERBOSE, "Using level %s.\n", level->name);
>> +if (level->constraint_set3_flag)
>> +sps->constraint_set3_flag = 1;
>> +sps->level_idc = level->level_idc;
>> +} else {
>> +av_log(avctx, AV_LOG_WARNING, "Stream will not conform "
>> +   "to any level: using level 6.2.\n");
>> +sps->level_idc = 62;
>> +}
>> +}
>>  
>>  sps->seq_parameter_set_id = 0;
>>  sps->chroma_format_idc= 1;
>> @@ -329,8 +356,7 @@ static int
>> vaapi_encode_h264_init_sequence_params(AVCodecContext *avctx)
>>  sps->log2_max_pic_order_cnt_lsb_minus4 =
>>  av_clip(av_log2(ctx->b_per_p + 1) - 2, 0, 12);
>>  
>> -sps->max_num_ref_frames =
>> -ctx->gop_size == 1 ? 0 : 1 + (ctx->b_per_p > 0);
>> +sps->max_num_ref_frames = dpb_frames;
>>  
>>  sps->pic_width_in_mbs_minus1= priv->mb_width  - 1;
>>  sps->pic_height_in_map_units_minus1 = priv->mb_height - 1;
>> @@ -1005,7 +1031,7 @@ static const AVOption vaapi_encode_h264_options[] = {
>>  
>>  { "level", "Set level (level_idc)",
>>OFFSET(level), AV_OPT_TYPE_INT,
>> -  { .i64 = 51 }, 0x00, 0xff, FLAGS, "level" },
>> +  { .i64 = FF_LEVEL_UNKNOWN }, FF_LEVEL_UNKNOWN, 0xff, FLAGS, "level" },
>>  
>>  #define LEVEL(name, value) name, NULL, 0, AV_OPT_TYPE_CONST, \
>>{ .i64 = value }, 0, 0, FLAGS, "level"

Thanks,

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


Re: [FFmpeg-devel] [PATCH v2 25/36] vaapi_encode_mjpeg: Use CBS to store parameters and write headers

2018-06-17 Thread Mark Thompson
On 15/06/18 02:37, Xiang, Haihao wrote:
> On Fri, 2018-06-08 at 00:43 +0100, Mark Thompson wrote:
>> Also adds greyscale, 4:2:2, 4:4:4 and RGB support.
>> ---
>>  configure   |   2 +-
>>  doc/encoders.texi   |  17 +-
>>  libavcodec/vaapi_encode_mjpeg.c | 529 +
>> ---
>>  3 files changed, 347 insertions(+), 201 deletions(-)
>>
>> ...
>> diff --git a/libavcodec/vaapi_encode_mjpeg.c 
>> b/libavcodec/vaapi_encode_mjpeg.c
>> index f76645425a..2f79070e58 100644
>> --- a/libavcodec/vaapi_encode_mjpeg.c
>> +++ b/libavcodec/vaapi_encode_mjpeg.c
>> ...
>> +static int vaapi_encode_mjpeg_write_image_header(AVCodecContext *avctx,
>> + VAAPIEncodePicture *pic,
>> + VAAPIEncodeSlice *slice,
>> + char *data, size_t
>> *data_len)
>>  {
>> -int i, mt;
>> -
>> -++src_lengths;
>> +VAAPIEncodeMJPEGContext *priv = avctx->priv_data;
>> +CodedBitstreamFragment  *frag = >current_fragment;
>> +int err;
>> +
>> +if (priv->jfif) {
>> +err = ff_cbs_insert_unit_content(priv->cbc, frag, -1,
>> + JPEG_MARKER_APPN + 0,
>> + >jfif_header, NULL);
>> +if (err < 0)
>> +goto fail;
>> +}
>>  
>> -mt = 0;
>> -for (i = 0; i < 16; i++)
>> -mt += (dst_lengths[i] = src_lengths[i]);
>> +err = ff_cbs_insert_unit_content(priv->cbc, frag, -1,
>> + JPEG_MARKER_DQT,
>> + >quant_tables, NULL);
>> +if (err < 0)
>> +goto fail;
>> +
>> +err = ff_cbs_insert_unit_content(priv->cbc, frag, -1,
>> + JPEG_MARKER_SOF0,
>> + >frame_header, NULL);
>> +if (err < 0)
>> +goto fail;
>> +
>> +if (priv->huffman) {
>> +err = ff_cbs_insert_unit_content(priv->cbc, frag, -1,
>> + JPEG_MARKER_DHT,
>> + >huffman_tables, NULL);
>> +if (err < 0)
>> +goto fail;
>> +}
>>  
>> -for (i = 0; i < mt; i++)
>> -dst_values[i] = src_values[i];
>> -}
>> +err = ff_cbs_insert_unit_content(priv->cbc, frag, -1,
>> + JPEG_MARKER_SOS,
>> + >scan, NULL);
>> +if (err < 0)
>> +goto fail;
>>  
>> -static av_cold void vaapi_encode_mjpeg_init_tables(AVCodecContext *avctx)
>> -{
>> -VAAPIEncodeMJPEGContext  *priv = avctx->priv_data;
>> -VAQMatrixBufferJPEG *quant = >quant_tables;
>> -VAHuffmanTableBufferJPEGBaseline *huff = >huffman_tables;
>> -int i;
>> -
>> -quant->load_lum_quantiser_matrix = 1;
>> -quant->load_chroma_quantiser_matrix = 1;
>> +err = ff_cbs_write_fragment_data(priv->cbc, frag);
>> +if (err < 0) {
>> +av_log(avctx, AV_LOG_ERROR, "Failed to write image header.\n");
>> +return err;
> 
> Should it be 'goto fail' ? Some new units have been inserted to the fragment 
> so
> ff_cbs_fragment_uninit() should be called to release resources.

Yep, fixed.

>> +}
>>  
>> -for (i = 0; i < 64; i++) {
>> -quant->lum_quantiser_matrix[i] =
>> -vaapi_encode_mjpeg_quant_luminance[i];
>> -quant->chroma_quantiser_matrix[i] =
>> -vaapi_encode_mjpeg_quant_chrominance[i];
>> +if (*data_len < 8 * frag->data_size) {
>> +av_log(avctx, AV_LOG_ERROR, "Image header too large: "
>> +   "%zu < %zu.\n", *data_len, 8 * frag->data_size);
>> +err = AVERROR(ENOSPC);
>> +goto fail;
> 
> Could you change the last parameter name of this function to bit_len or
> bit_data_len? I think it is more readable and user don't need to think why 
> frag-
>> data_size is multiplied by 8.

I don't think I agree?  It matches the naming and style used in all of the 
other codecs in these write-header functions - compare for example 
.

>>  }
>>  
>> -huff->load_huffman_table[0] = 1;
>> -vaapi_encode_mjpeg_copy_huffman(huff->huffman_table[0].num_dc_codes,
>> -huff->huffman_table[0].dc_values,
>> -avpriv_mjpeg_bits_dc_luminance,
>> -avpriv_mjpeg_val_dc);
>> -vaapi_encode_mjpeg_copy_huffman(huff->huffman_table[0].num_ac_codes,
>> -huff->huffman_table[0].ac_values,
>> -avpriv_mjpeg_bits_ac_luminance,
>> -avpriv_mjpeg_val_ac_luminance);
>> -memset(huff->huffman_table[0].pad, 0, 

Re: [FFmpeg-devel] [PATCH v2 24/36] lavc/cbs: Add JPEG support

2018-06-17 Thread Mark Thompson
On 15/06/18 01:52, Xiang, Haihao wrote:
> On Fri, 2018-06-08 at 00:43 +0100, Mark Thompson wrote:
>> ---
>>  configure |   2 +
>>  libavcodec/Makefile   |   1 +
>>  libavcodec/cbs.c  |   6 +
>>  libavcodec/cbs_internal.h |   1 +
>>  libavcodec/cbs_jpeg.c | 513
>> ++
>>  libavcodec/cbs_jpeg.h | 128 +
>>  libavcodec/cbs_jpeg_syntax_template.c | 191 +
>>  7 files changed, 842 insertions(+)
>>  create mode 100644 libavcodec/cbs_jpeg.c
>>  create mode 100644 libavcodec/cbs_jpeg.h
>>  create mode 100644 libavcodec/cbs_jpeg_syntax_template.c
>>
>> ...
>> diff --git a/libavcodec/cbs_jpeg.c b/libavcodec/cbs_jpeg.c
>> new file mode 100644
>> index 00..365db73394
>> --- /dev/null
>> +++ b/libavcodec/cbs_jpeg.c
>> ...
>> +static int cbs_jpeg_read_unit(CodedBitstreamContext *ctx,
>> +  CodedBitstreamUnit *unit)
>> +{
>> +GetBitContext gbc;
>> +int err;
>> +
>> +err = init_get_bits(, unit->data, 8 * unit->data_size);
>> +if (err < 0)
>> +return err;
>> +
>> +if (unit->type >= JPEG_MARKER_SOF0 &&
>> +unit->type <= JPEG_MARKER_SOF3) {
>> +err = ff_cbs_alloc_unit_content(ctx, unit,
>> +sizeof(JPEGRawFrameHeader),
>> +NULL);
>> +if (err < 0)
>> +return err;
>> +
>> +err = cbs_jpeg_read_frame_header(ctx, , unit->content);
>> +if (err < 0)
>> +return err;
>> +
>> +} else if (unit->type >= JPEG_MARKER_APPN &&
>> +   unit->type <= JPEG_MARKER_APPN + 15) {
>> +err = ff_cbs_alloc_unit_content(ctx, unit,
>> +sizeof(JPEGRawApplicationData),
>> +_jpeg_free_application_data);
>> +if (err < 0)
>> +return err;
>> +
>> +err = cbs_jpeg_read_application_data(ctx, , unit->content);
>> +if (err < 0)
>> +return err;
>> +
>> +} else if (unit->type == JPEG_MARKER_SOS) {
>> +JPEGRawScan *scan;
>> +int pos;
>> +
>> +err = ff_cbs_alloc_unit_content(ctx, unit,
>> +sizeof(JPEGRawScan),
>> +_jpeg_free_scan);
>> +if (err < 0)
>> +return err;
>> +scan = unit->content;
>> +
>> +err = cbs_jpeg_read_scan_header(ctx, , >header);
>> +if (err < 0)
>> +return err;
>> +
>> +pos = get_bits_count();
>> +av_assert0(pos % 8 == 0);
>> +if (pos > 0) {
>> +scan->data_size = unit->data_size - pos / 8;
>> +scan->data_ref  = av_buffer_ref(unit->data_ref);
>> +if (!scan->data_ref)
>> +return AVERROR(ENOMEM);
>> +scan->data = unit->data + pos / 8;
>> +}
>> +
>> +} else {
>> +switch (unit->type) {
>> +#define SEGMENT(marker, type, func) \
>> +case JPEG_MARKER_ ## marker: \
>> +{ \
>> +err = ff_cbs_alloc_unit_content(ctx, unit, \
>> +sizeof(type), NULL); \
>> +if (err < 0) \
>> +return err; \
>> +err = cbs_jpeg_read_ ## func(ctx, , unit->content); \
>> +if (err < 0) \
>> +return err; \
>> +} \
>> +break
>> +SEGMENT(DQT, JPEGRawQuantisationTableSpecification, dqt);
>> +SEGMENT(DHT, JPEGRawHuffmanTableSpecification,  dht);
>> +SEGMENT(COM, JPEGRawComment,comment);
> 
> A free function should be provided for JPEGRawComment as JPEGRawComment 
> includes
> a AVBufferRef pointer. 

Yep, fixed.

>> +#undef SEGMENT
>> +default:
>> +return AVERROR(ENOSYS);
>> +}
>> +}
>> +
>> +return 0;
>> +}
>> ...

Thanks,

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


Re: [FFmpeg-devel] [PATCH v2] lavc/qsv: suppress code scan complain

2018-06-17 Thread Mark Thompson
On 14/06/18 10:50, Carl Eugen Hoyos wrote:
> 2018-06-14 5:08 GMT+02:00, Li, Zhong :
>>> -Original Message-
>>> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
>>> Of Carl Eugen Hoyos
>>> Sent: Wednesday, June 13, 2018 6:11 PM
>>> To: FFmpeg development discussions and patches
>>> 
>>> Subject: Re: [FFmpeg-devel] [PATCH v2] lavc/qsv: suppress code scan
>>> complain
>>>
>>> 2018-06-13 12:05 GMT+02:00, Li, Zhong :
> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On
>>> Behalf
> Of Carl Eugen Hoyos
> Sent: Wednesday, June 13, 2018 5:42 PM
> To: FFmpeg development discussions and patches
> 
> Subject: Re: [FFmpeg-devel] [PATCH v2] lavc/qsv: suppress code scan
> complain
>
> 2018-05-24 11:39 GMT+02:00, Li, Zhong :
>>> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On
> Behalf
>>> Of Carl Eugen Hoyos
>>> Sent: Thursday, May 24, 2018 5:33 PM
>>> To: FFmpeg development discussions and patches
>>> 
>>> Subject: Re: [FFmpeg-devel] [PATCH v2] lavc/qsv: suppress code
>>> scan complain
>>>
>>> 2018-05-24 10:35 GMT+02:00, Li, Zhong :
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org]
>>> On
>>> Behalf
> Of Carl Eugen Hoyos
> Sent: Wednesday, May 23, 2018 8:32 PM
> To: FFmpeg development discussions and patches
> 
> Subject: Re: [FFmpeg-devel] [PATCH v2] lavc/qsv: suppress code
> scan complain
>
> 2018-05-23 12:46 GMT+02:00, Zhong Li :
>> Suppress the complain "variables 'type' is used but maybe
>> uninitialized".
>> ---
>>  libavcodec/qsv.c | 5 -
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/libavcodec/qsv.c b/libavcodec/qsv.c index
>> 45e1c25..3ff4f2c 100644
>> --- a/libavcodec/qsv.c
>> +++ b/libavcodec/qsv.c
>> @@ -31,6 +31,7 @@
>>  #include "libavutil/hwcontext.h"
>>  #include "libavutil/hwcontext_qsv.h"
>>  #include "libavutil/imgutils.h"
>> +#include "libavutil/avassert.h"
>>
>>  #include "avcodec.h"
>>  #include "qsv_internal.h"
>> @@ -197,7 +198,7 @@ int
> ff_qsv_find_surface_idx(QSVFramesContext
> *ctx,
>> QSVFrame *frame)
>>
>>  enum AVPictureType ff_qsv_map_pictype(int mfx_pic_type)  {
>> -enum AVPictureType type;
>> +enum AVPictureType type = AV_PICTURE_TYPE_NONE;
>>  switch (mfx_pic_type & 0x7) {
>>  case MFX_FRAMETYPE_I:
>>  if (mfx_pic_type & MFX_FRAMETYPE_S) @@ -214,6
> +215,8
> @@ enum
>> AVPictureType ff_qsv_map_pictype(int mfx_pic_type)
>>  else
>>  type = AV_PICTURE_TYPE_P;
>>  break;
>> +default:
>> +av_assert0(0);
>
> I didn't test but I would have expected that exactly one of
> these changes is sufficient to silence the warning, no?

 Thanks for review. It is not a compile warning and just found by
 Coverity Scan, I've double-confirmed this patch is useful to
 suppress the code scan complain.
>>>
>>> Of course, I understand.
>>>
>>> My question was if one of the two changes (ie either the variable
>>> initialization or the assert) isn't enough to suppress the code
>>> scan complain.
>>
>> I've confirmed that, running scan again. The complain is
>> disappeared now.
>
> Then why did you push both changes instead of only one of them?

  This one has been reviewed by you
>>>
>>> My two questions were:
>>> Isn't it enough to only change the variable initialization (and change
>>> nothing
>>> in the switch) to silence the complain?
>>> Isn't it enough to only add the default case to the switch (and change
>>> nothing in the variable initialization) to silence the complain?
>>>
>>> You committed changing both the variable initialization and added a
>>> default
>>> case, one should have been enough.
>>
>> Ok, I misunderstood you doubted any one of these two changes can silence the
>> complain.
>> Yes, one is enough for the complain. (the first version of this patch is
>> just to change the variable initialization. Mark suggested to add the
>> assert() to check the input is valid).
> 
>> So maybe you would like to keep the assert() and remove the variable
>> initialization?
> 
> That would have been my original suggestion, yes.

That was what I meant too.

Generally you don't want an initialisation when it isn't needed, because it 
makes warnings less useful.  To suppress the warning the assert (which the 
compiler knows will not return on failure) should be sufficient.

- Mark
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org

Re: [FFmpeg-devel] [PATCH v2 16/36] vaapi_encode: Clean up rate control configuration

2018-06-17 Thread Mark Thompson
On 14/06/18 08:22, Li, Zhong wrote:
>> -Original Message-
>> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
>> Of Xiang, Haihao
>> Sent: Thursday, June 14, 2018 2:08 PM
>> To: ffmpeg-devel@ffmpeg.org
>> Subject: Re: [FFmpeg-devel] [PATCH v2 16/36] vaapi_encode: Clean up rate
>> control configuration
>>
>> On Wed, 2018-06-13 at 23:42 +0100, Mark Thompson wrote:
>>> On 13/06/18 08:03, Xiang, Haihao wrote:
 On Fri, 2018-06-08 at 00:43 +0100, Mark Thompson wrote:
> Query which modes are supported and select between VBR and CBR
> based on that - this removes all of the codec-specific rate
> control mode selection code.
> ---
>  doc/encoders.texi   |   2 -
>  libavcodec/vaapi_encode.c   | 173
>> ---
> 
> -
>  libavcodec/vaapi_encode.h   |   6 +-
>  libavcodec/vaapi_encode_h264.c  |  18 +
> libavcodec/vaapi_encode_h265.c  |  14 +---
>  libavcodec/vaapi_encode_mjpeg.c |   3 +-
>  libavcodec/vaapi_encode_mpeg2.c |   9 +--
>  libavcodec/vaapi_encode_vp8.c   |  13 +--
>  libavcodec/vaapi_encode_vp9.c   |  13 +--
>  9 files changed, 137 insertions(+), 114 deletions(-)
>
> ...
> diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
> index f4c063734c..5de5483454 100644
> --- a/libavcodec/vaapi_encode.c
> +++ b/libavcodec/vaapi_encode.c
> ...
> +if (avctx->flags & AV_CODEC_FLAG_QSCALE ||
> +avctx->bit_rate <= 0) {

This condition ^

> +if (rc_attr.value & VA_RC_CQP) {
> +av_log(avctx, AV_LOG_VERBOSE, "Using
>> constant-quality
> mode.\n");
> +ctx->va_rc_mode = VA_RC_CQP;
> +return 0;
> +} else {
> +av_log(avctx, AV_LOG_ERROR, "Driver does not
>> support "
> +   "constant-quality mode (%#x).\n",
>> rc_attr.value);
> +return AVERROR(EINVAL);
> +}
> +}
> ...
> +} else if (avctx->rc_max_rate == avctx->bit_rate) {
> +if (!(rc_attr.value & VA_RC_CBR)) {
> +av_log(avctx, AV_LOG_WARNING, "Driver does not
>> support "
> +   "CBR mode (%#x), using VBR mode
>> instead.\n",
> +   rc_attr.value);
> +ctx->va_rc_mode = VA_RC_VBR;
> +} else {
> +ctx->va_rc_mode = VA_RC_CBR;
> +}
>
> -if (ctx->va_rc_mode == VA_RC_CBR) {
>  rc_bits_per_second   = avctx->bit_rate;
>  rc_target_percentage = 100;
> -rc_window_size   = 1000;
> +
>  } else {
> -if (avctx->rc_max_rate < avctx->bit_rate) {
> -// Max rate is unset or invalid, just use the normal
>> bitrate.
> +if (rc_attr.value & VA_RC_VBR) {
> +ctx->va_rc_mode = VA_RC_VBR;

 Is it better to take it as CBR when avctx->rc_max_rate is 0 and CBR
 is supported by driver?
>>>
>>> I don't think so?  VBR with the specified target is probably what you
>>> want in most cases, and I think anyone with specific constraints that
>>> want constant bitrate should expect to set maxrate to achieve that.
>>>
>>
>> I agree VBR is probably what an user wants in most case, however target
>> percent set to 50% is not suitable for most case. To get a specific target
>> percent, user should set both target bitrate and max bitrate, so it is
>> reasonable to ask user must set both target bitrate and max bitrate for VBR
>> cases, and for CBR user may set target bitrate only.
> 
> How about set the max_rate to be a very larger number such as INT_MAX if user 
> hasn't set it? 
> User may don't set max_rate on purpose, expecting better quality with 
> unlimited bitrate fluctuation (common requirement for local video files).
> Double of target_bit_rate is too strict IMHO. And I haven't such a limitation 
> in x264 ABR mode.

This unconstrained setup you describe was my intent (as you say, it's usually 
what you want for local files), but unfortunately the API doesn't really let us 
do it - the target bitrate is specified as an integer percentage of the max 
bitrate.  50% was an arbitrary value picked to not have a strong constraint but 
also not be small enough that we need to think about rounding/overflow problems.

We could try to pick a large value with the right properties (for example: if 
target < 2^32 / 100 then choose maxrate = target * 100, percentage = 1; 
otherwise choose percentage = 2^32 * 100 / bitrate, maxrate = bitrate * 100 / 
percentage), but that would be complex to test and I don't think the drivers 
handle this sort of setup very well.

>> I just saw it is also VBR in QSV when max bitrate is not set so I'm fine to 
>> keep
>> consistency with QSV for this case.
> 
> What will happen if user set a max_rate but without a setting for 
> target_bitrate? 
> The mode will be VBR (if 

Re: [FFmpeg-devel] [PATCH v2 19/36] vaapi_encode: Clean up the packed header configuration

2018-06-17 Thread Mark Thompson
On 14/06/18 08:15, Xiang, Haihao wrote:
> On Fri, 2018-06-08 at 00:43 +0100, Mark Thompson wrote:
>> Add a larger warning more clearly explaining the consequences of missing
>> packed header support in the driver.  Also only write the extradata if the
>> user actually requests it via the GLOBAL_HEADER flag.
>> ---
>>  libavcodec/vaapi_encode.c   | 118 +
>> ---
>>  libavcodec/vaapi_encode.h   |   7 ++-
>>  libavcodec/vaapi_encode_h264.c  |   2 +-
>>  libavcodec/vaapi_encode_h265.c  |   2 +-
>>  libavcodec/vaapi_encode_mjpeg.c |   2 +-
>>  libavcodec/vaapi_encode_mpeg2.c |   4 +-
>>  libavcodec/vaapi_encode_vp8.c   |   6 +-
>>  libavcodec/vaapi_encode_vp9.c   |   6 +-
>>  8 files changed, 79 insertions(+), 68 deletions(-)
>>
>> diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
>> index af9224c98f..14d1846ea3 100644
>> --- a/libavcodec/vaapi_encode.c
>> +++ b/libavcodec/vaapi_encode.c
>> @@ -1210,60 +1210,6 @@ fail:
>>  return err;
>>  }
>>  
>> -static av_cold int vaapi_encode_config_attributes(AVCodecContext *avctx)
>> -{
>> -VAAPIEncodeContext *ctx = avctx->priv_data;
>> -VAStatus vas;
>> -int i;
>> -
>> -VAConfigAttrib attr[] = {
>> -{ VAConfigAttribEncPackedHeaders },
>> -};
>> -
>> -vas = vaGetConfigAttributes(ctx->hwctx->display,
>> -ctx->va_profile, ctx->va_entrypoint,
>> -attr, FF_ARRAY_ELEMS(attr));
>> -if (vas != VA_STATUS_SUCCESS) {
>> -av_log(avctx, AV_LOG_ERROR, "Failed to fetch config "
>> -   "attributes: %d (%s).\n", vas, vaErrorStr(vas));
>> -return AVERROR(EINVAL);
>> -}
>> -
>> -for (i = 0; i < FF_ARRAY_ELEMS(attr); i++) {
>> -if (attr[i].value == VA_ATTRIB_NOT_SUPPORTED) {
>> -// Unfortunately we have to treat this as "don't know" and hope
>> -// for the best, because the Intel MJPEG encoder returns this
>> -// for all the interesting attributes.
>> -av_log(avctx, AV_LOG_DEBUG, "Attribute (%d) is not 
>> supported.\n",
>> -   attr[i].type);
>> -continue;
>> -}
>> -switch (attr[i].type) {
>> -case VAConfigAttribEncPackedHeaders:
>> -if (ctx->va_packed_headers & ~attr[i].value) {
>> -// This isn't fatal, but packed headers are always
>> -// preferable because they are under our control.
>> -// When absent, the driver is generating them and some
>> -// features may not work (e.g. VUI or SEI in H.264).
>> -av_log(avctx, AV_LOG_WARNING, "Warning: some packed "
>> -   "headers are not supported (want %#x, got %#x).\n",
>> -   ctx->va_packed_headers, attr[i].value);
>> -ctx->va_packed_headers &= attr[i].value;
>> -}
>> -ctx->config_attributes[ctx->nb_config_attributes++] =
>> -(VAConfigAttrib) {
>> -.type  = VAConfigAttribEncPackedHeaders,
>> -.value = ctx->va_packed_headers,
>> -};
>> -break;
>> -default:
>> -av_assert0(0 && "Unexpected config attribute.");
>> -}
>> -}
>> -
>> -return 0;
>> -}
>> -
>>  static av_cold int vaapi_encode_init_rate_control(AVCodecContext *avctx)
>>  {
>>  VAAPIEncodeContext *ctx = avctx->priv_data;
>> @@ -1494,6 +1440,65 @@ static av_cold int
>> vaapi_encode_init_gop_structure(AVCodecContext *avctx)
>>  return 0;
>>  }
>>  
>> +static av_cold int vaapi_encode_init_packed_headers(AVCodecContext *avctx)
>> +{
>> +VAAPIEncodeContext *ctx = avctx->priv_data;
>> +VAStatus vas;
>> +VAConfigAttrib attr = { VAConfigAttribEncPackedHeaders };
>> +
>> +vas = vaGetConfigAttributes(ctx->hwctx->display,
>> +ctx->va_profile,
>> +ctx->va_entrypoint,
>> +, 1);
>> +if (vas != VA_STATUS_SUCCESS) {
>> +av_log(avctx, AV_LOG_ERROR, "Failed to query packed headers "
>> +   "attribute: %d (%s).\n", vas, vaErrorStr(vas));
>> +return AVERROR_EXTERNAL;
>> +}
>> +
>> +if (attr.value == VA_ATTRIB_NOT_SUPPORTED) {
>> +if (ctx->packed_headers) {
>> +av_log(avctx, AV_LOG_WARNING, "Driver does not support any "
>> +   "packed headers (wanted %#x).\n", ctx->packed_headers);
>> +} else {
>> +av_log(avctx, AV_LOG_VERBOSE, "Driver does not support any "
>> +   "packed headers (none wanted).\n");
>> +}
>> +ctx->va_packed_headers = 0;
>> +} else {
>> +if (ctx->packed_headers & ~attr.value) {
>> +av_log(avctx, AV_LOG_WARNING, "Driver does not support some "
>> +   "wanted packed headers (wanted %#x, found %#x).\n",
>> +   

Re: [FFmpeg-devel] [PATCH v2 16/36] vaapi_encode: Clean up rate control configuration

2018-06-17 Thread Mark Thompson
On 14/06/18 07:07, Xiang, Haihao wrote:
> On Wed, 2018-06-13 at 23:42 +0100, Mark Thompson wrote:
>> On 13/06/18 08:03, Xiang, Haihao wrote:
>>> On Fri, 2018-06-08 at 00:43 +0100, Mark Thompson wrote:
 Query which modes are supported and select between VBR and CBR based
 on that - this removes all of the codec-specific rate control mode
 selection code.
 ---
  doc/encoders.texi   |   2 -
  libavcodec/vaapi_encode.c   | 173 ---
 
 -
  libavcodec/vaapi_encode.h   |   6 +-
  libavcodec/vaapi_encode_h264.c  |  18 +
  libavcodec/vaapi_encode_h265.c  |  14 +---
  libavcodec/vaapi_encode_mjpeg.c |   3 +-
  libavcodec/vaapi_encode_mpeg2.c |   9 +--
  libavcodec/vaapi_encode_vp8.c   |  13 +--
  libavcodec/vaapi_encode_vp9.c   |  13 +--
  9 files changed, 137 insertions(+), 114 deletions(-)

 ...
 diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
 index f4c063734c..5de5483454 100644
 --- a/libavcodec/vaapi_encode.c
 +++ b/libavcodec/vaapi_encode.c
 ...
 @@ -1313,44 +1286,144 @@ static av_cold int
 vaapi_encode_config_attributes(AVCodecContext *avctx)
  static av_cold int vaapi_encode_init_rate_control(AVCodecContext *avctx)
  {
  VAAPIEncodeContext *ctx = avctx->priv_data;
 -int rc_bits_per_second;
 -int rc_target_percentage;
 -int rc_window_size;
 -int hrd_buffer_size;
 -int hrd_initial_buffer_fullness;
 +int64_t rc_bits_per_second;
 +int rc_target_percentage;
 +int rc_window_size;
 +int64_t hrd_buffer_size;
 +int64_t hrd_initial_buffer_fullness;
  int fr_num, fr_den;
 +VAConfigAttrib rc_attr = { VAConfigAttribRateControl };
 +VAStatus vas;
 +
 +vas = vaGetConfigAttributes(ctx->hwctx->display,
 +ctx->va_profile, ctx->va_entrypoint,
 +_attr, 1);
 +if (vas != VA_STATUS_SUCCESS) {
 +av_log(avctx, AV_LOG_ERROR, "Failed to query rate control "
 +   "config attribute: %d (%s).\n", vas, vaErrorStr(vas));
 +return AVERROR_EXTERNAL;
 +}
  
 -if (avctx->bit_rate > INT32_MAX) {
 -av_log(avctx, AV_LOG_ERROR, "Target bitrate of 2^31 bps or "
 -   "higher is not supported.\n");
 +if (rc_attr.value == VA_ATTRIB_NOT_SUPPORTED) {
 +av_log(avctx, AV_LOG_VERBOSE, "Driver does not report any "
 +   "supported rate control modes: assuming constant-
 quality.\n");
>>>
>>> How about logging it as warning message?
>>
>> What would a user do about it?
>>
> 
> User may know what is not supported in the driver and he/she might get wrong
> result, he/she may file an issue to the driver.

If it's broken then the verbose logging provides the answer.  If it isn't 
broken, as in

>> (Note that it gets logged for JPEG encoding on all versions of the i965 
>> driver
>> except the most recent, so if it were a warning it would be seen by everyone
>> using those versions.)

then the warning is unhelpful.

 +ctx->va_rc_mode = VA_RC_CQP;
 +return 0;
 +}
 +if (avctx->flags & AV_CODEC_FLAG_QSCALE ||
 +avctx->bit_rate <= 0) {
 +if (rc_attr.value & VA_RC_CQP) {
 +av_log(avctx, AV_LOG_VERBOSE, "Using constant-quality
 mode.\n");
 +ctx->va_rc_mode = VA_RC_CQP;
 +return 0;
 +} else {
 +av_log(avctx, AV_LOG_ERROR, "Driver does not support "
 +   "constant-quality mode (%#x).\n", rc_attr.value);
 +return AVERROR(EINVAL);
 +}
 +}
 +
 +if (!(rc_attr.value & (VA_RC_CBR | VA_RC_VBR))) {
 +av_log(avctx, AV_LOG_ERROR, "Driver does not support any "
 +   "bitrate-targetted rate control modes.\n");
  return AVERROR(EINVAL);
  }
  
  if (avctx->rc_buffer_size)
  hrd_buffer_size = avctx->rc_buffer_size;
 +else if (avctx->rc_max_rate > 0)
 +hrd_buffer_size = avctx->rc_max_rate;
  else
  hrd_buffer_size = avctx->bit_rate;
 -if (avctx->rc_initial_buffer_occupancy)
 +if (avctx->rc_initial_buffer_occupancy) {
 +if (avctx->rc_initial_buffer_occupancy > hrd_buffer_size) {
 +av_log(avctx, AV_LOG_ERROR, "Invalid RC buffer settings: "
 +   "must have initial buffer size (%d) < "
 +   "buffer size (%"PRId64").\n",
 +   avctx->rc_initial_buffer_occupancy, hrd_buffer_size);
 +return AVERROR(EINVAL);
 +}
  hrd_initial_buffer_fullness = avctx->rc_initial_buffer_occupancy;
 -else
 +} else {
   

Re: [FFmpeg-devel] [PATCH v2 18/36] vaapi_encode: Clean up the GOP structure configuration

2018-06-17 Thread Mark Thompson
On 14/06/18 07:48, Xiang, Haihao wrote:
> On Fri, 2018-06-08 at 00:43 +0100, Mark Thompson wrote:
>> Choose what types of reference frames will be used based on what types
>> are available, and make the intra-only mode explicit (GOP size one,
>> which must be used for MJPEG).
>> ---
>>  libavcodec/vaapi_encode.c   | 83 
>> +++-
>> -
>>  libavcodec/vaapi_encode.h   |  1 +
>>  libavcodec/vaapi_encode_h264.c  |  4 +-
>>  libavcodec/vaapi_encode_h265.c  |  4 +-
>>  libavcodec/vaapi_encode_mjpeg.c |  1 +
>>  libavcodec/vaapi_encode_mpeg2.c |  2 +-
>>  libavcodec/vaapi_encode_vp8.c   |  7 +---
>>  libavcodec/vaapi_encode_vp9.c   |  7 ++--
>>  8 files changed, 68 insertions(+), 41 deletions(-)
>>
>> diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
>> index 0e806a29e3..af9224c98f 100644
>> --- a/libavcodec/vaapi_encode.c
>> +++ b/libavcodec/vaapi_encode.c
>> @@ -680,7 +680,7 @@ static int vaapi_encode_get_next(AVCodecContext *avctx,
>>  return AVERROR(ENOMEM);
>>  
>>  if (ctx->input_order == 0 || ctx->force_idr ||
>> -ctx->gop_counter >= avctx->gop_size) {
>> +ctx->gop_counter >= ctx->gop_size) {
>>  pic->type = PICTURE_TYPE_IDR;
>>  ctx->force_idr = 0;
>>  ctx->gop_counter = 1;
>> @@ -703,7 +703,7 @@ static int vaapi_encode_get_next(AVCodecContext *avctx,
>>  // encode-after it, but not exceeding the GOP size.
>>  
>>  for (i = 0; i < ctx->b_per_p &&
>> - ctx->gop_counter < avctx->gop_size; i++) {
>> + ctx->gop_counter < ctx->gop_size; i++) {
>>  pic = vaapi_encode_alloc(avctx);
>>  if (!pic)
>>  goto fail;
>> @@ -1217,7 +1217,6 @@ static av_cold int
>> vaapi_encode_config_attributes(AVCodecContext *avctx)
>>  int i;
>>  
>>  VAConfigAttrib attr[] = {
>> -{ VAConfigAttribEncMaxRefFrames  },
>>  { VAConfigAttribEncPackedHeaders },
>>  };
>>  
>> @@ -1240,24 +1239,6 @@ static av_cold int
>> vaapi_encode_config_attributes(AVCodecContext *avctx)
>>  continue;
>>  }
>>  switch (attr[i].type) {
>> -case VAConfigAttribEncMaxRefFrames:
>> -{
>> -unsigned int ref_l0 = attr[i].value & 0x;
>> -unsigned int ref_l1 = (attr[i].value >> 16) & 0x;
>> -
>> -if (avctx->gop_size > 1 && ref_l0 < 1) {
>> -av_log(avctx, AV_LOG_ERROR, "P frames are not "
>> -   "supported (%#x).\n", attr[i].value);
>> -return AVERROR(EINVAL);
>> -}
>> -if (avctx->max_b_frames > 0 && ref_l1 < 1) {
>> -av_log(avctx, AV_LOG_WARNING, "B frames are not "
>> -   "supported (%#x) by the underlying driver.\n",
>> -   attr[i].value);
>> -avctx->max_b_frames = 0;
>> -}
>> -}
>> -break;
>>  case VAConfigAttribEncPackedHeaders:
>>  if (ctx->va_packed_headers & ~attr[i].value) {
>>  // This isn't fatal, but packed headers are always
>> @@ -1465,6 +1446,54 @@ static av_cold int
>> vaapi_encode_init_rate_control(AVCodecContext *avctx)
>>  return 0;
>>  }
>>  
>> +static av_cold int vaapi_encode_init_gop_structure(AVCodecContext *avctx)
>> +{
>> +VAAPIEncodeContext *ctx = avctx->priv_data;
>> +VAStatus vas;
>> +VAConfigAttrib attr = { VAConfigAttribEncMaxRefFrames };
>> +uint32_t ref_l0, ref_l1;
>> +
>> +vas = vaGetConfigAttributes(ctx->hwctx->display,
>> +ctx->va_profile,
>> +ctx->va_entrypoint,
>> +, 1);
>> +if (vas != VA_STATUS_SUCCESS) {
>> +av_log(avctx, AV_LOG_ERROR, "Failed to query reference frames "
>> +   "attribute: %d (%s).\n", vas, vaErrorStr(vas));
>> +return AVERROR_EXTERNAL;
>> +}
>> +
>> +if (attr.value == VA_ATTRIB_NOT_SUPPORTED) {
>> +ref_l0 = ref_l1 = 0;
>> +} else {
>> +ref_l0 = attr.value   & 0x;
>> +ref_l1 = attr.value >> 16 & 0x;
>> +}
>> +
>> +if (avctx->gop_size <= 1) {
>> +av_log(avctx, AV_LOG_VERBOSE, "Using intra frames only.\n");
>> +ctx->gop_size = 1;
>> +} else if (ref_l0 < 1) {
>> +av_log(avctx, AV_LOG_ERROR, "Driver does not support any "
>> +   "reference frames.\n");
>> +return AVERROR(EINVAL);
>> +} else if (ref_l1 < 1 || avctx->max_b_frames < 1) {
>> +av_log(avctx, AV_LOG_VERBOSE, "Using intra and P-frames "
>> +   "(supported references: %d / %d).\n", ref_l0, ref_l1);
>> +ctx->gop_size = avctx->gop_size;
>> +ctx->p_per_i  = INT_MAX;
> 
> How about removing p_per_i? I see p_per_i is used only in the following 'else
> if' statement and I don't think ctx->p_counter can be INT_MAX in reality. 
> 
> } else if (ctx->p_counter 

Re: [FFmpeg-devel] [PATCH v2 11/36] vaapi_encode: Choose profiles dynamically

2018-06-17 Thread Mark Thompson
On 14/06/18 05:51, Xiang, Haihao wrote:
> On Wed, 2018-06-13 at 23:27 +0100, Mark Thompson wrote:
>> On 12/06/18 08:22, Xiang, Haihao wrote:
>>> On Fri, 2018-06-08 at 00:43 +0100, Mark Thompson wrote:
 Previously there was one fixed choice for each codec (e.g. H.265 -> Main
 profile), and using anything else then required an explicit option from
 the user.  This changes to selecting the profile based on the input format
 and the set of profiles actually supported by the driver (e.g. P010 input
 will choose Main 10 profile for H.265 if the driver supports it).

 The entrypoint and render target format are also chosen dynamically in the
 same way, removing those explicit selections from the per-codec code.
 ---
  doc/encoders.texi   |   3 +
  libavcodec/vaapi_encode.c   | 271 ---
 
 -
  libavcodec/vaapi_encode.h   |  43 +--
  libavcodec/vaapi_encode_h264.c  |  45 ++-
  libavcodec/vaapi_encode_h265.c  |  35 ++
  libavcodec/vaapi_encode_mjpeg.c |  13 +-
  libavcodec/vaapi_encode_mpeg2.c |  36 ++
  libavcodec/vaapi_encode_vp8.c   |  11 +-
  libavcodec/vaapi_encode_vp9.c   |  34 ++---
  9 files changed, 310 insertions(+), 181 deletions(-)

 ...
 +static av_cold int vaapi_encode_profile_entrypoint(AVCodecContext *avctx)
  {
 -VAAPIEncodeContext *ctx = avctx->priv_data;
 +VAAPIEncodeContext  *ctx = avctx->priv_data;
 +VAProfile*va_profiles= NULL;
 +VAEntrypoint *va_entrypoints = NULL;
  VAStatus vas;
 -int i, n, err;
 -VAProfile*profiles= NULL;
 -VAEntrypoint *entrypoints = NULL;
 -VAConfigAttrib attr[] = {
 -{ VAConfigAttribRTFormat },
 -{ VAConfigAttribRateControl  },
 -{ VAConfigAttribEncMaxRefFrames  },
 -{ VAConfigAttribEncPackedHeaders },
 -};
 +const VAEntrypoint *usable_entrypoints;
 +const VAAPIEncodeProfile *profile;
 +const AVPixFmtDescriptor *desc;
 +VAConfigAttrib rt_format_attr;
 +const VAAPIEncodeRTFormat *rt_format;
 +int i, j, n, depth, err;
 +
 +
 +if (ctx->low_power) {
 +#if VA_CHECK_VERSION(0, 39, 2)
 +usable_entrypoints = vaapi_encode_entrypoints_low_power;
 +#else
 +av_log(avctx, AV_LOG_ERROR, "Low-power encoding is not "
 +   "supported with this VAAPI version.\n");
>>>
>>> Is it possible to report the minimal VAAPI version in the log in case user
>>> doesn't know the requirement on vaapi version 0.39.2?
>>
>> The VAAPI version is pretty much useless to users (without reading any source
>> code, how would you find what VAAPI version 0.39.2 means?).
>>
> 
> Ok, I agree it is useless to user.
> 
>> Maybe libva version?  That's still not quite right, though, because it's more
>> "not supported in this build" (and will continue to not be supported if you
>> upgrade libva but don't rebuild with the new headers).
>>
> 
> May we ask user to rebuild ffmpeg once user upgrades libva in the log?

On further thought, I don't think a recommendation is useful here.  You end up 
having to suggest upgrading everything (libva, driver, ffmpeg), and even then 
it still may not be there (if you are using a driver or codec which doesn't 
expose this option).

So, I think it's most sensible to keep the simple text as originally written.  
If you have a string opinion then feel free to suggest your own patch for how 
this should work.  (And it should probably be consistent with other similar 
cases, such as the quality-speed tradeoff option.)

 +return AVERROR(EINVAL);
 +#endif
 +} else {
 +usable_entrypoints = vaapi_encode_entrypoints_normal;
 +}
 +
 +desc = av_pix_fmt_desc_get(ctx->input_frames->sw_format);
 +if (!desc) {
 +av_log(avctx, AV_LOG_ERROR, "Invalid input pixfmt (%d).\n",
 +   ctx->input_frames->sw_format);
 +return AVERROR(EINVAL);
 +}
 +depth = desc->comp[0].depth;
 +for (i = 1; i < desc->nb_components; i++) {
 +if (desc->comp[i].depth != depth) {
 +av_log(avctx, AV_LOG_ERROR, "Invalid input pixfmt (%s).\n",
 +   desc->name);
 +return AVERROR(EINVAL);
 +}
 +}
 +av_log(avctx, AV_LOG_VERBOSE, "Input surface format is %s.\n",
 +   desc->name);
  
  n = vaMaxNumProfiles(ctx->hwctx->display);
 -profiles = av_malloc_array(n, sizeof(VAProfile));
 -if (!profiles) {
 +va_profiles = av_malloc_array(n, sizeof(VAProfile));
 +if (!va_profiles) {
  err = AVERROR(ENOMEM);
  goto fail;
  }
 -vas = vaQueryConfigProfiles(ctx->hwctx->display, profiles, );
 +vas = 

Re: [FFmpeg-devel] [PATCH 1/4] lavu/frame: add mb_types side data

2018-06-17 Thread Rostislav Pehlivanov
On 17 June 2018 at 05:21, Ramiro Polla  wrote:

> ---
>  libavcodec/avcodec.h   |  4 
>  libavcodec/mpegutils.c | 20 
>  libavcodec/options_table.h |  1 +
>  libavutil/frame.c  |  1 +
>  libavutil/frame.h  |  9 +
>  5 files changed, 35 insertions(+)
>
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index c90166deb6..7fe4fc9347 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -929,6 +929,10 @@ typedef struct RcOverride{
>   */
>  #define AV_CODEC_FLAG2_SHOW_ALL   (1 << 22)
>  /**
> + * Export macroblock types through frame side data
> + */
> +#define AV_CODEC_FLAG2_EXPORT_MB_TYPES (1 << 27)
> +/**
>   * Export motion vectors through frame side data
>   */
>  #define AV_CODEC_FLAG2_EXPORT_MVS (1 << 28)
> diff --git a/libavcodec/mpegutils.c b/libavcodec/mpegutils.c
> index 3f94540616..0fbe5f8c9d 100644
> --- a/libavcodec/mpegutils.c
> +++ b/libavcodec/mpegutils.c
> @@ -188,6 +188,26 @@ void ff_print_debug_info2(AVCodecContext *avctx,
> AVFrame *pict, uint8_t *mbskip_
>  av_freep();
>  }
>
> +if ((avctx->flags2 & AV_CODEC_FLAG2_EXPORT_MB_TYPES) &&
> mbtype_table) {
> +int size = (2 + mb_height * mb_width) * sizeof(uint32_t);
> +int mb_x, mb_y;
> +
> +AVFrameSideData *sd;
> +uint32_t *out;
> +
> +sd = av_frame_new_side_data(pict, AV_FRAME_DATA_MB_TYPES, size);
> +if (!sd)
> +return;
> +
> +out = (uint32_t *) sd->data;
> +*out++ = mb_height;
> +*out++ = mb_width;
> +
> +for (mb_y = 0; mb_y < mb_height; mb_y++)
> +for (mb_x = 0; mb_x < mb_width; mb_x++)
> +*out++ = mbtype_table[mb_x + mb_y * mb_stride];
> +}
> +
>  /* TODO: export all the following to make them accessible for users
> (and filters) */
>  if (avctx->hwaccel || !mbtype_table)
>  return;
> diff --git a/libavcodec/options_table.h b/libavcodec/options_table.h
> index 099261e168..25c84de321 100644
> --- a/libavcodec/options_table.h
> +++ b/libavcodec/options_table.h
> @@ -76,6 +76,7 @@ static const AVOption avcodec_options[] = {
>  {"export_mvs", "export motion vectors through frame side data", 0,
> AV_OPT_TYPE_CONST, {.i64 = AV_CODEC_FLAG2_EXPORT_MVS}, INT_MIN, INT_MAX,
> V|D, "flags2"},
>  {"skip_manual", "do not skip samples and export skip information as frame
> side data", 0, AV_OPT_TYPE_CONST, {.i64 = AV_CODEC_FLAG2_SKIP_MANUAL},
> INT_MIN, INT_MAX, V|D, "flags2"},
>  {"ass_ro_flush_noop", "do not reset ASS ReadOrder field on flush", 0,
> AV_OPT_TYPE_CONST, {.i64 = AV_CODEC_FLAG2_RO_FLUSH_NOOP}, INT_MIN, INT_MAX,
> S|D, "flags2"},
> +{"export_mb_types", "export macroblock types through frame side data", 0,
> AV_OPT_TYPE_CONST, {.i64 = AV_CODEC_FLAG2_EXPORT_MB_TYPES}, INT_MIN,
> INT_MAX, V|D, "flags2"},
>  {"time_base", NULL, OFFSET(time_base), AV_OPT_TYPE_RATIONAL, {.dbl = 0},
> 0, INT_MAX},
>  {"g", "set the group of picture (GOP) size", OFFSET(gop_size),
> AV_OPT_TYPE_INT, {.i64 = 12 }, INT_MIN, INT_MAX, V|E},
>  {"ar", "set audio sampling rate (in Hz)", OFFSET(sample_rate),
> AV_OPT_TYPE_INT, {.i64 = DEFAULT }, 0, INT_MAX, A|D|E},
> diff --git a/libavutil/frame.c b/libavutil/frame.c
> index deb9b6f334..577d4f6e6d 100644
> --- a/libavutil/frame.c
> +++ b/libavutil/frame.c
> @@ -834,6 +834,7 @@ const char *av_frame_side_data_name(enum
> AVFrameSideDataType type)
>  case AV_FRAME_DATA_ICC_PROFILE: return "ICC profile";
>  case AV_FRAME_DATA_QP_TABLE_PROPERTIES: return "QP table
> properties";
>  case AV_FRAME_DATA_QP_TABLE_DATA:   return "QP table
> data";
> +case AV_FRAME_DATA_MB_TYPES:return "Macroblock
> types";
>  }
>  return NULL;
>  }
> diff --git a/libavutil/frame.h b/libavutil/frame.h
> index 9d57d6ce66..ce1231b03b 100644
> --- a/libavutil/frame.h
> +++ b/libavutil/frame.h
> @@ -158,6 +158,15 @@ enum AVFrameSideDataType {
>   */
>  AV_FRAME_DATA_QP_TABLE_DATA,
>  #endif
> +
> +/**
> + * Macroblock types exported by some codecs (on demand through the
> + * export_mb_types flag set in the libavcodec AVCodecContext flags2
> option).
> + * The data is composed by a header consisting of uint32_t mb_height
> and
> + * uint32_t mb_width, followed by a uint32_t
> mb_types[mb_height][mb_width]
> + * array.
> + */
> +AV_FRAME_DATA_MB_TYPES,
>  };
>
>  enum AVActiveFormatDescription {
> --
> 2.11.0
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

NAK

I'd really rather have a json or xml output which would be codec
independent rather than this codec specific format. Which could then be fed
into an analyzer (like the AV1 analyzer). We discussed this before.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org

Re: [FFmpeg-devel] Fw: [PATCH] Refactor two near-identical clauses.

2018-06-17 Thread Shlomi Fish
On Sun, 17 Jun 2018 03:05:27 +0200
Michael Niedermayer  wrote:

> On Tue, Jun 12, 2018 at 12:53:20PM +0300, Shlomi Fish wrote:
> > This message did not arrive to the list after three submissions.
> > 
> > Begin forwarded message:
> > 
> > Date: Tue, 12 Jun 2018 12:42:52 +0300
> > From: Shlomi Fish 
> > To: ffmpeg-devel@ffmpeg.org
> > Cc: Shlomi Fish 
> > Subject: [PATCH] Refactor two near-identical clauses.
> > 
> > 
> > Placed under the Expat licence . All tests pass.
> > ---
> >  libavfilter/vf_weave.c | 33 ++---
> >  1 file changed, 14 insertions(+), 19 deletions(-)
> > 
> > diff --git a/libavfilter/vf_weave.c b/libavfilter/vf_weave.c
> > index 037f5d1cf2..be371201e1 100644
> > --- a/libavfilter/vf_weave.c
> > +++ b/libavfilter/vf_weave.c
> > @@ -23,6 +23,7 @@
> >  #include "libavutil/pixdesc.h"
> >  #include "avfilter.h"
> >  #include "internal.h"
> > +#include 
> >  
> >  typedef struct WeaveContext {
> >  const AVClass *class;
> > @@ -84,6 +85,8 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *in)
> >  AVFilterLink *outlink = ctx->outputs[0];
> >  AVFrame *out;
> >  int i;
> > +bool weave;
> > +int field1, field2;
> >  
> >  if (!s->prev) {
> >  s->prev = in;
> > @@ -98,26 +101,18 @@ static int filter_frame(AVFilterLink *inlink, AVFrame
> > *in) }
> >  av_frame_copy_props(out, in);
> >  
> > +weave = (s->double_weave && !(inlink->frame_count_out & 1));
> > +field1 = s->first_field * weave;
> > +field2 = s->first_field * !weave;
> >  for (i = 0; i < s->nb_planes; i++) {
> > -if (s->double_weave && !(inlink->frame_count_out & 1)) {
> > -av_image_copy_plane(out->data[i] + out->linesize[i] *
> > s->first_field,  
> 
> this seems to be corrupted by line breaks
> 

Well, the git send-email email was silently dropped three times... See:

http://www.shlomifish.org/Files/files/code/0001-Refactor-two-near-identical-clauses.patch

also attached here. Email has sadly become unreliable.

> [...]
> 


>From b6678799848297cd7079085035259baf6d8c54f0 Mon Sep 17 00:00:00 2001
From: Shlomi Fish 
Date: Fri, 25 May 2018 23:44:54 +0300
Subject: [PATCH] Refactor two near-identical clauses.

Placed under the Expat licence . All tests pass.
---
 libavfilter/vf_weave.c | 33 ++---
 1 file changed, 14 insertions(+), 19 deletions(-)

diff --git a/libavfilter/vf_weave.c b/libavfilter/vf_weave.c
index 037f5d1cf2..be371201e1 100644
--- a/libavfilter/vf_weave.c
+++ b/libavfilter/vf_weave.c
@@ -23,6 +23,7 @@
 #include "libavutil/pixdesc.h"
 #include "avfilter.h"
 #include "internal.h"
+#include 
 
 typedef struct WeaveContext {
 const AVClass *class;
@@ -84,6 +85,8 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *in)
 AVFilterLink *outlink = ctx->outputs[0];
 AVFrame *out;
 int i;
+bool weave;
+int field1, field2;
 
 if (!s->prev) {
 s->prev = in;
@@ -98,26 +101,18 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *in)
 }
 av_frame_copy_props(out, in);
 
+weave = (s->double_weave && !(inlink->frame_count_out & 1));
+field1 = s->first_field * weave;
+field2 = s->first_field * !weave;
 for (i = 0; i < s->nb_planes; i++) {
-if (s->double_weave && !(inlink->frame_count_out & 1)) {
-av_image_copy_plane(out->data[i] + out->linesize[i] * s->first_field,
-out->linesize[i] * 2,
-in->data[i], in->linesize[i],
-s->linesize[i], s->planeheight[i]);
-av_image_copy_plane(out->data[i] + out->linesize[i] * !s->first_field,
-out->linesize[i] * 2,
-s->prev->data[i], s->prev->linesize[i],
-s->linesize[i], s->planeheight[i]);
-} else {
-av_image_copy_plane(out->data[i] + out->linesize[i] * !s->first_field,
-out->linesize[i] * 2,
-in->data[i], in->linesize[i],
-s->linesize[i], s->planeheight[i]);
-av_image_copy_plane(out->data[i] + out->linesize[i] * s->first_field,
-out->linesize[i] * 2,
-s->prev->data[i], s->prev->linesize[i],
-s->linesize[i], s->planeheight[i]);
-}
+av_image_copy_plane(out->data[i] + out->linesize[i] * field1,
+out->linesize[i] * 2,
+in->data[i], in->linesize[i],
+s->linesize[i], s->planeheight[i]);
+av_image_copy_plane(out->data[i] + out->linesize[i] * field2,
+out->linesize[i] * 2,
+s->prev->data[i], s->prev->linesize[i],
+s->linesize[i], s->planeheight[i]);
 }
 
 out->pts = 

Re: [FFmpeg-devel] [PATCH 2/4] mpegutils: split debug function that prints mb_type so it may be used by ffprobe

2018-06-17 Thread Carl Eugen Hoyos
2018-06-17 6:21 GMT+02:00, Ramiro Polla :

> +int ff_mb_type_str(char *str, int size, int mb_type)

I don't think you can use "ff_" functions from a tool, you
have to make this part of the API afaict.

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


[FFmpeg-devel] Conversion to audible AAX(encrypted) format.

2018-06-17 Thread Tejasvi Singh Tomar
Feature of converting audio to AAX format is highly useful in some cases. In my 
case I need to transfer personal audio files for listening on Kindle e-reader, 
which only accepts Audible files. The AAX files are to be encrypted with 
information of activation_bytes of individual Audible accounts (similar to 
already present decryption method: 
https://www.ffmpeg.org/ffmpeg-all.html#Audible-AAX). I hope if method of 
decryption is public then there should be no big deal in doing the reverse 
process.

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


Re: [FFmpeg-devel] [PATCH] avcodec/libx265: apply lavc maxrate and bufsize

2018-06-17 Thread Gyan Doshi

On 15-06-2018 09:50 PM, Gyan Doshi wrote:



On 13-06-2018 07:08 PM, Gyan Doshi wrote:

Default for both parameters in both libs is 0.


Ping.


Will push in a day.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel