Re: [FFmpeg-devel] [PATCH v7 1/2] lavc/svt_hevc: add libsvt hevc encoder wrapper.

[Sun, Jing A]

On  Monday, March 11, 2019 10:50 PM Vittorio Giovara vittorio.giov...@gmail.com 
wrote:

>On Mon, Mar 11, 2019 at 12:50 AM Sun, Jing A  wrote:
>I just searched my inbox again but failed to find that email of question you 
>mentioned.
>
>Yeah I often see my mail bounced with this message:
>
>Address not found
>Your message wasn't delivered to jun.z...@intel.com because the address 
>couldn't be found, or is unable to receive mail. 
 
>For reference this was the message on the mailing list 
>https://ffmpeg.org/pipermail/ffmpeg-devel/2019-March/240663.html
>
>Could you please elaborate your request? What is the preservation for and how 
>is it expected to work? 
>
>Yes of course, when you encode an HEVC stream you should be able to signal how 
>the color properties of the video buffers should be rendered. This is usually 
>conveyed with three 
>parameters, the matrix coefficients, the color primaries and the transfer 
>characteristics. Without such information, the data stored in the video may be 
>interpreted differently and often 
>incorrectly by modern video players, causing image degradation, wrong 
>rendering and off colors.

>For HEVC they are usually expressed in the stream itself, under the VUI, and 
>it is kinda expected that modern encoder allow to set them to any of the 
>applicable values.
>In ffmpeg-land, they are represented by the colorspace, color_primaries and 
>color_transfer options in AVCodecContext and carried over through the whole 
>video processing.
>-- 
>Vittorio

Hi Giovara,

SVT HEVC has the interface to enable/disable sending a vui structure in the 
HEVC bitstream, but supports no interface for setting the color properties 
before encoding yet. I will be opening an issue in SVT HEVC github asking if 
they have plans to add such feature, and will keep you posted. In the meantime, 
I think it is not blocking the first version of this plugin’s merging , is it?

In SVT HEVC user guide: "VideoUsabilityInfo - Enables or disables sending a vui 
structure in the HEVC Elementary bitstream. 0 = OFF, 1 = ON"

Regards,
SUN, Jing

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

Re: [FFmpeg-devel] [PATCH v7 1/2] lavc/svt_hevc: add libsvt hevc encoder wrapper.

[Sun, Jing A]

Hi Giovara,

SVT HEVC has the interface to enable/disable sending a vui structure in the 
HEVC bitstream, but supports no interface for setting the color properties 
before encoding yet. I will be opening an issue in SVT HEVC github asking if 
they have plans to add such feature, and will keep you posted. In the meantime, 
I think it is not blocking the first version of this plugin’s merging , is it?

VideoUsabilityInfo

Enables or disables sending a vui
structure in the HEVC Elementary
bitstream. 0 = OFF, 1 = ON


Regards,
SUN, Jing

From: Vittorio Giovara [mailto:vittorio.giov...@gmail.com]
Sent: Monday, March 11, 2019 10:50 PM
To: Sun, Jing A 
Cc: FFmpeg development discussions and patches ; 
Huang, Zhengxu ; Tmar, Hassene 
Subject: Re: [FFmpeg-devel] [PATCH v7 1/2] lavc/svt_hevc: add libsvt hevc 
encoder wrapper.



On Mon, Mar 11, 2019 at 12:50 AM Sun, Jing A 
mailto:jing.a@intel.com>> wrote:
I just searched my inbox again but failed to find that email of question you 
mentioned.

Yeah I often see my mail bounced with this message:

Address not found
Your message wasn't delivered to jun.z...@intel.com 
because the address couldn't be found, or is unable to receive mail.


For reference this was the message on the mailing list 
https://ffmpeg.org/pipermail/ffmpeg-devel/2019-March/240663.html

Could you please elaborate your request? What is the preservation for and how 
is it expected to work?

Yes of course, when you encode an HEVC stream you should be able to signal how 
the color properties of the video buffers should be rendered. This is usually 
conveyed with three parameters, the matrix coefficients, the color primaries 
and the transfer characteristics. Without such information, the data stored in 
the video may be interpreted differently and often incorrectly by modern video 
players, causing image degradation, wrong rendering and off colors.

For HEVC they are usually expressed in the stream itself, under the VUI, and it 
is kinda expected that modern encoder allow to set them to any of the 
applicable values.
In ffmpeg-land, they are represented by the colorspace, color_primaries and 
color_transfer options in AVCodecContext and carried over through the whole 
video processing.
--
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v2 5/6] vaapi_encode: Add ROI support

[Guo, Yejun]

> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> Of Mark Thompson
> Sent: Wednesday, March 13, 2019 8:18 AM
> To: ffmpeg-devel@ffmpeg.org
> Subject: [FFmpeg-devel] [PATCH v2 5/6] vaapi_encode: Add ROI support
> 
> ---
>  libavcodec/vaapi_encode.c   | 128
> 
>  libavcodec/vaapi_encode.h   |  11 +++
>  libavcodec/vaapi_encode_h264.c  |   2 +
>  libavcodec/vaapi_encode_h265.c  |   2 +
>  libavcodec/vaapi_encode_mpeg2.c |   2 +
>  libavcodec/vaapi_encode_vp8.c   |   2 +
>  libavcodec/vaapi_encode_vp9.c   |   2 +
>  7 files changed, 149 insertions(+)
> 
> diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
> index 2dda451882..03a7f5ce3e 100644
> --- a/libavcodec/vaapi_encode.c
> +++ b/libavcodec/vaapi_encode.c
> @@ -143,6 +143,7 @@ static int vaapi_encode_issue(AVCodecContext
> *avctx,
>  int err, i;
>  char data[MAX_PARAM_BUFFER_SIZE];
>  size_t bit_len;
> +AVFrameSideData *sd;
> 
>  av_log(avctx, AV_LOG_DEBUG, "Issuing encode for
> pic %"PRId64"/%"PRId64" "
> "as type %s.\n", pic->display_order, pic->encode_order,
> @@ -412,6 +413,91 @@ static int vaapi_encode_issue(AVCodecContext
> *avctx,
>  }
>  }
> 
> +sd = av_frame_get_side_data(pic->input_image,
> +AV_FRAME_DATA_REGIONS_OF_INTEREST);
> +
> +#if VA_CHECK_VERSION(1, 0, 0)
> +if (sd && ctx->roi_allowed) {
> +const AVRegionOfInterest *roi;
> +int nb_roi;
> +VAEncMiscParameterBuffer param_misc = {
> +.type = VAEncMiscParameterTypeROI,
> +};
> +VAEncMiscParameterBufferROI param_roi;
> +// VAAPI requires the second structure to immediately follow the
> +// first in the parameter buffer, but they have different natural
> +// alignment on 64-bit architectures (4 vs. 8, since the ROI
> +// structure contains a pointer).  This means we can't make a
> +// single working structure, nor can we make the buffer
> +// separately and map it because dereferencing the second pointer
> +// would be undefined.  Therefore, construct the two parts
> +// separately and combine them into a single character buffer
> +// before uploading.
> +char param_buffer[sizeof(param_misc) + sizeof(param_roi)];
> +int i, v;
> +
> +roi = (const AVRegionOfInterest*)sd->data;
> +av_assert0(roi->self_size && sd->size % roi->self_size == 0);

it is possible if the user forget to set the value of roi->self_size. 
assert is not feasible. 

> +nb_roi = sd->size / roi->self_size;
> +if (nb_roi > ctx->roi_regions) {
> +if (!ctx->roi_warned) {
> +av_log(avctx, AV_LOG_WARNING, "More ROIs set than "
> +   "supported by driver (%d > %d).\n",
> +   nb_roi, ctx->roi_regions);
> +ctx->roi_warned = 1;
> +}
> +nb_roi = ctx->roi_regions;
> +}
> +
> +pic->roi = av_mallocz_array(nb_roi, sizeof(*pic->roi));
> +if (!pic->roi) {
> +err = AVERROR(ENOMEM);
> +goto fail;
> +}

shall we add comments here to explain the list visit order?
> +for (i = 0; i < nb_roi; i++) {
> +roi = (const AVRegionOfInterest*)(sd->data + roi->self_size * i);

same comment as libx264

> +
> +v = roi->qoffset.num * ctx->roi_quant_range / roi->qoffset.den;

shall we check here if roi->qoffset-den is zero?

> +av_log(avctx, AV_LOG_DEBUG, "ROI region: (%d,%d) (%dx%d) -
> > %+d.\n",
> +   roi->x, roi->y, roi->width, roi->height, v);
> +
> +pic->roi[i] = (VAEncROI) {
> +.roi_rectangle = {
> +.x  = roi->x,
> +.y  = roi->y,
> +.width  = roi->width,
> +.height = roi->height,
> +},
> +.roi_value = av_clip_c(v, INT8_MIN, INT8_MAX),
> +};
> +}
> +
> +param_roi = (VAEncMiscParameterBufferROI) {
> +.num_roi  = nb_roi,
> +.max_delta_qp = INT8_MAX,
> +.min_delta_qp = 0,
> +.roi  = pic->roi,
> +.roi_flags.bits.roi_value_is_qp_delta = 1,
> +};
> +
> +memcpy(param_buffer, _misc, sizeof(param_misc));
> +memcpy(param_buffer + sizeof(param_misc),
> +   _roi, sizeof(param_roi));
> +
> +err = vaapi_encode_make_param_buffer(avctx, pic,
> + VAEncMiscParameterBufferType,
> + param_buffer,
> + sizeof(param_buffer));
> +if (err < 0)
> +goto fail;
> +} else
> +#endif
> +if (sd && !ctx->roi_warned) {
> +av_log(avctx, AV_LOG_WARNING, "ROI 

Re: [FFmpeg-devel] [PATCH] Added more commandline options for libaom encoder.

[Sam John]

Thank you James and Moritz.

For many of these parameters, the default value within the codec is '1'. So
we want to add the option for disabling it also.

I would like to follow the method used for "frame-parallel",  after
changing the default value to -1.

{ "frame-parallel",   "Enable frame parallel decodability features",
OFFSET(frame_parallel),  AV_OPT_TYPE_BOOL,{.i64 = -1}, -1, 1, VE},

if (frame_parallel >= 0) {
..
}

I will send the updated patch soon.




On Tue, Mar 12, 2019 at 9:14 AM Moritz Barsnick  wrote:

> On Tue, Mar 12, 2019 at 11:45:06 -0300, James Almer wrote:
> > > +{ "frame-parallel",   "Enable frame parallel decodability
> features", OFFSET(frame_parallel),  AV_OPT_TYPE_BOOL,{.i64 = -1}, -1, 1,
> VE},
> >
> > A bool with three values is weird. If this is meant to be disabled by
> > default, then just make it 0 and remove the >= 0 condition above.
>
> It's actually quite common with ffmpeg options. Such an option can only
> be assigned true or false, but if it isn't assigned, it will be e.g. -1
> and can be used to trigger a default mode, as is apparently done here
> (by not setting this option through the API, probably letting the
> library do whatever it considers default).
>
> Cheers,
> Moritz
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v2] avformat/smoothstreamingenc:add bitrate calculate

[Jun Li]

Hi Carl,
Thanks for review, is there any more issue I need to take care ?
or any process I need to follow before applying the patch? I am new to the
community, don't know too much.

Thanks
-Jun

On Mon, Mar 11, 2019 at 11:44 AM Carl Eugen Hoyos 
wrote:

> 2019-03-11 18:58 GMT+01:00, Jun Li :
>
> > Smooth is not the only one need bitrate for manifest, Dash also need this
> > value for "Bandwidth" field:
> > https://github.com/FFmpeg/FFmpeg/blob/master/libavformat/dashenc.c#L730
> > Although there is some difference, both the calculation are based on
> > fragment size, which is the result after muxing.
>
> Thank you for the explanation!
>
> Carl Eugen
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v2 4/6] libx265: Update ROI behaviour to match documentation

[Guo, Yejun]

> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> Of Mark Thompson
> Sent: Wednesday, March 13, 2019 8:18 AM
> To: ffmpeg-devel@ffmpeg.org
> Subject: [FFmpeg-devel] [PATCH v2 4/6] libx265: Update ROI behaviour to
> match documentation
> 
> Equivalent to the previous patch for libx264.
> ---
>  libavcodec/libx265.c | 42 +-
>  1 file changed, 21 insertions(+), 21 deletions(-)
> 
> diff --git a/libavcodec/libx265.c b/libavcodec/libx265.c
> index c2c1f8b9bc..a580d113b9 100644
> --- a/libavcodec/libx265.c
> +++ b/libavcodec/libx265.c
> @@ -304,27 +304,34 @@ static av_cold int
> libx265_encode_set_roi(libx265Context *ctx, const AVFrame *fr
>  int mb_size = (ctx->params->rc.qgSize == 8) ? 8 : 16;
>  int mbx = (frame->width + mb_size - 1) / mb_size;
>  int mby = (frame->height + mb_size - 1) / mb_size;
> +int qp_range = 51 + 6 * (pic->bitDepth - 8);
>  int nb_rois;
> -AVRegionOfInterest *roi;
> +const AVRegionOfInterest *roi;
>  float *qoffsets; /* will be freed after encode is 
> called. */
> +
> +roi = (const AVRegionOfInterest*)sd->data;
> +if (!roi->self_size || sd->size % roi->self_size != 0) {
> +av_log(ctx, AV_LOG_ERROR, "Invalid
> AVRegionOfInterest.self_size.\n");
> +return AVERROR(EINVAL);
> +}
> +nb_rois = sd->size / roi->self_size;
> +
>  qoffsets = av_mallocz_array(mbx * mby, sizeof(*qoffsets));
>  if (!qoffsets)
>  return AVERROR(ENOMEM);
> 
> -nb_rois = sd->size / sizeof(AVRegionOfInterest);
> -roi = (AVRegionOfInterest*)sd->data;
> -for (int count = 0; count < nb_rois; count++) {
> -int starty = av_clip(roi->y / mb_size, 0, mby);
> -int endy   = av_clip((roi->y + roi->height + mb_size - 1) / 
> mb_size, 0,
> mby);
> -int startx = av_clip(roi->x / mb_size, 0, mbx);
> -int endx   = av_clip((roi->x + roi->width  + mb_size - 1) / 
> mb_size, 0,
> mbx);
> +// This list must be iterated in reverse because the first
> +// region in the list applies when regions overlap.
> +for (int i = nb_rois - 1; i >= 0; i--) {
> +int startx, endx, starty, endy;
>  float qoffset;
> 
> -if (roi->self_size == 0) {
> -av_free(qoffsets);
> -av_log(ctx, AV_LOG_ERROR, "AVRegionOfInterest.self_size 
> must
> be set to sizeof(AVRegionOfInterest).\n");
> -return AVERROR(EINVAL);
> -}
> +roi = (const AVRegionOfInterest*)(sd->data + roi->self_size 
> * i);

same comment as libx264

> +
> +starty = av_clip(roi->y / mb_size, 0, mby);
> +endy   = av_clip((roi->y + roi->height + mb_size - 1) / 
> mb_size, 0,
> mby);
> +startx = av_clip(roi->x / mb_size, 0, mbx);
> +endx   = av_clip((roi->x + roi->width  + mb_size - 1) / 
> mb_size, 0,
> mbx);
> 
>  if (roi->qoffset.den == 0) {
>  av_free(qoffsets);
> @@ -332,18 +339,11 @@ static av_cold int
> libx265_encode_set_roi(libx265Context *ctx, const AVFrame *fr
>  return AVERROR(EINVAL);
>  }
>  qoffset = roi->qoffset.num * 1.0f / roi->qoffset.den;
> -qoffset = av_clipf(qoffset, -1.0f, 1.0f);
> -
> -/* qp range of x265 is from 0 to 51, just choose 25 as the 
> scale value,
> - * so the range of final qoffset is [-25.0, 25.0].
> - */
> -qoffset = qoffset * 25;
> +qoffset = av_clipf(qoffset * qp_range, -qp_range, +qp_range);
> 
>  for (int y = starty; y < endy; y++)
>  for (int x = startx; x < endx; x++)
>  qoffsets[x + y*mbx] = qoffset;
> -
> -roi = (AVRegionOfInterest*)((char*)roi + roi->self_size);
>  }
> 
>  pic->quantOffsets = qoffsets;
> --
> 2.19.2
> 
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v2 3/6] libx264: Update ROI behaviour to match documentation

[Guo, Yejun]

> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> Of Mark Thompson
> Sent: Wednesday, March 13, 2019 8:18 AM
> To: ffmpeg-devel@ffmpeg.org
> Subject: [FFmpeg-devel] [PATCH v2 3/6] libx264: Update ROI behaviour to
> match documentation
> 
> Fix the quantisation offset - use the whole range, and don't change the
> offset size based on bit depth.
> 
> Iterate the list in reverse order.  The first region in the list is the one
> that applies in the case of overlapping regions.
> ---
>  libavcodec/libx264.c | 51 
>  1 file changed, 28 insertions(+), 23 deletions(-)
> 
> diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c
> index 87e6fe7c94..f5b9b8b821 100644
> --- a/libavcodec/libx264.c
> +++ b/libavcodec/libx264.c
> @@ -285,16 +285,18 @@ static int X264_frame(AVCodecContext *ctx,
> AVPacket *pkt, const AVFrame *frame,
>  int nnal, i, ret;
>  x264_picture_t pic_out = {0};
>  int pict_type;
> +int bit_depth;
>  int64_t *out_opaque;
>  AVFrameSideData *sd;
> 
>  x264_picture_init( >pic );
>  x4->pic.img.i_csp   = x4->params.i_csp;
>  #if X264_BUILD >= 153
> -if (x4->params.i_bitdepth > 8)
> +bit_depth = x4->params.i_bitdepth;
>  #else
> -if (x264_bit_depth > 8)
> +bit_depth = x264_bit_depth;
>  #endif
> +if (bit_depth > 8)
>  x4->pic.img.i_csp |= X264_CSP_HIGH_DEPTH;
>  x4->pic.img.i_plane = avfmt2_num_planes(ctx->pix_fmt);
> 
> @@ -359,45 +361,48 @@ static int X264_frame(AVCodecContext *ctx,
> AVPacket *pkt, const AVFrame *frame,
>  if (frame->interlaced_frame == 0) {
>  int mbx = (frame->width + MB_SIZE - 1) / MB_SIZE;
>  int mby = (frame->height + MB_SIZE - 1) / MB_SIZE;
> +int qp_range = 51 + 6 * (bit_depth - 8);
>  int nb_rois;
> -AVRegionOfInterest* roi;
> -float* qoffsets;
> +const AVRegionOfInterest *roi;
> +float *qoffsets;

need a variable to record self_size of the first roi, see reason below.
uint32_t self_size;

> +
> +roi = (const AVRegionOfInterest*)sd->data;

self_size = roi->self_size;

> +if (!roi->self_size || sd->size % roi->self_size != 0) {

if (!self_size || sd->size % self_size != 0) {

and other cases for self_size

> +av_log(ctx, AV_LOG_ERROR, "Invalid
> AVRegionOfInterest.self_size.\n");
> +return AVERROR(EINVAL);
> +}
> +nb_rois = sd->size / roi->self_size;
> +
>  qoffsets = av_mallocz_array(mbx * mby, 
> sizeof(*qoffsets));
>  if (!qoffsets)
>  return AVERROR(ENOMEM);
> 
> -nb_rois = sd->size / sizeof(AVRegionOfInterest);
> -roi = (AVRegionOfInterest*)sd->data;
> -for (int count = 0; count < nb_rois; count++) {
> -int starty = av_clip(roi->y / MB_SIZE, 0, mby);
> -int endy   = av_clip((roi->y + roi->height + MB_SIZE 
> - 1) /
> MB_SIZE, 0, mby);
> -int startx = av_clip(roi->x / MB_SIZE, 0, mbx);
> -int endx   = av_clip((roi->x + roi->width  + MB_SIZE 
> - 1) /
> MB_SIZE, 0, mbx);
> +// This list must be iterated in reverse because the 
> first
> +// region in the list applies when regions overlap.
> +for (int i = nb_rois - 1; i >= 0; i--) {
> +int startx, endx, starty, endy;
>  float qoffset;
> 
> +roi = (const AVRegionOfInterest*)(sd->data + 
> roi->self_size * i);

here is the reason, roi->self_size is not verified when roi changes with below 
code.

> +
> +starty = av_clip(roi->y / MB_SIZE, 0, mby);
> +endy   = av_clip((roi->y + roi->height + MB_SIZE - 
> 1) / MB_SIZE,
> 0, mby);
> +startx = av_clip(roi->x / MB_SIZE, 0, mbx);
> +endx   = av_clip((roi->x + roi->width  + MB_SIZE - 
> 1) / MB_SIZE,
> 0, mbx);
> +
>  if (roi->qoffset.den == 0) {
>  av_free(qoffsets);
> -av_log(ctx, AV_LOG_ERROR,
> "AVRegionOfInterest.qoffset.den should not be zero.\n");
> +av_log(ctx, AV_LOG_ERROR,
> "AVRegionOfInterest.qoffset.den must not be zero.\n");
>  return AVERROR(EINVAL);
>  }
>  qoffset = roi->qoffset.num * 1.0f / roi->qoffset.den;
> -qoffset = av_clipf(qoffset, -1.0f, 1.0f);
> -
> -// 25 is a number that I think it is a possible 
> proper scale value.
> -   

Re: [FFmpeg-devel] [PATCHv2] avcodec/nvenc: Reconfigure resolution on-the-fly

[Oliver Collyer]

> On 12 Mar 2019, at 23:40, Mark Thompson  wrote:
> 
>> On 08/03/2019 07:44, Oliver Collyer wrote:
>> From c704e3535f866d9f89535b9df59db9ca9811bcf9 Mon Sep 17 00:00:00 2001
>> From: Oliver Collyer 
>> Date: Fri, 8 Mar 2019 07:42:41 +
>> Subject: [PATCH 1/1] avcodec/nvenc: Reconfigure resolution on-the-fly
>> 
>> ---
>> libavcodec/nvenc.c  | 31 ---
>> libavcodec/nvenc.h  |  3 +++
>> libavcodec/nvenc_h264.c |  4 
>> libavcodec/nvenc_hevc.c |  4 
>> 4 files changed, 39 insertions(+), 3 deletions(-)
> 
> Can you explain the actual intended use-case for this?
> 
> The current way to handle resolution changes (or any other stream change of 
> similar magnitude, like a pixel format change) is to flush the existing 
> encoder and then make a new one with the new parameters.  Even ignoring the 
> trivial benefit that all encoders handle this with no additional code, it has 
> the significant advantage that all of the stream metadata, including 
> parameter sets, can be handled correctly.  The handling here does rather 
> badly at this - stream metadata will be misleading, and if you take one of 
> these streams and try to put it into some container with global headers you 
> may well end up with a quite broken file.
> 

I’m not sure I follow; your logic seems contradictory here - clearly if you are 
trying to do this in a stream with global headers you’re going to run into 
trouble, but during writing to such a stream whether you 1) flush, delete and 
re-create, or 2) reconfigure the encoder is going to have the same effect iand 
won’t change anything since it’s not the encoder writing any such global stream 
headers it’s the muxer? Or did you mean something else?

I’m using it in a server app where I want to quickly and efficiently change the 
video size/bitrate of a transport stream sent over long distance either when 
the remote user requests or in response to changing network conditions, with as 
little disruption to the viewing experience as possible.

For the record when this patch is used in conjunction with encoding into an 
mpegts stream it plays fine in VLC/libVLC, which defects the video changes in 
the stream and recreates the vout/resizes the video accordingly.

> Given that, I think there should be some justification for why you might want 
> to do this rather than following the existing approach.  Some mention in the 
> API (avcodec.h) to explain what's going on might be a good idea, since that 
> is currently assuming that parameters like width/height are fixed and must be 
> set before opening the encoder.  Relatedly, if there isn't any support for 
> retrieving new metadata then it should probably display a big warning if the 
> user tries to make a stream like this with global headers, since the global 
> headers provided to the user on startup won't actually work for the whole 
> stream.
> 

I think the fact this functionality is only accessible programmatically means 
that the bar would be quite high, ie the user likely knows what they are doing, 
but I can certainly put a comment next to the width/height avcodecctx members 
along the lines of “In some limited scenarios such as when using nvenc the 
width/height can be changed during encoding and will be detected by the encoder 
causing it to reconfigure itself to the new picture sIze. Extreme care should 
be taken when doing this with a format that uses global headers as the global 
headers will no longer correspond to the adjusted picture size!”

Alternatively, maybe this is all a bit too obscure and it’s better left in my 
own customised ffmpeg branch? That would be quite ok, although the code does 
already handle dynamic bitrate and DAR changes so I figured to offer you the 
patch...

> Thanks,
> 
> - Mark
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] avcodec/vaapi_h264: skip decode if pic has no slices

[Mark Thompson]

On 09/03/2019 20:30, Peter F wrote:
> Thank you very much for your reply.
> Am Sa., 9. März 2019 um 21:09 Uhr schrieb Jan Ekström :
> 
>>> From 386c94489a86bb747b6531f727843cf259a24f5d Mon Sep 17 00:00:00 2001
>>> From: xbmc 
>> Is this author field meant to not have an actual name in it? Just verifying.
> 
> It can stay as is. The original author sometimes uses fernetmenta /
> xbmc depending on his
> local git configuration. The email is unique though. I just
> transported it upstream and fixed
> the minors.
> 
>>
>>> Date: Sat, 26 Jan 2019 19:48:35 +0100
>>> Subject: [PATCH] avcodec/vaapi_h264: skip decode if pic has no slices
>>
>> Something along the lines of "avcodec/vaapi_h264: skip decoding if no
>> slices were provided"?
>>
>> Also I would prefer if the reasoning for skipping decode on our layer
>> would be explained in further lines of the commit message, like you
>> have nicely explained it in the initial e-mail (to work-around a mesa
>> vaapi driver bug).
>> I don't remember the specifics of AVC, but are there valid VCL NAL
>> units without slices (and would such end up in this code path to begin
>> with)? I would guess that there would be no such valid VCL NAL units
>> (or if there were, they wouldn't get to this point in the decode
>> chain) - mostly just noting that this might be something we would like
>> to check to verify if this should be an error or a "normal" state.
>>
>> The general idea I'm OK with since if we already know that there's no
>> slices to decode, we might as well skip decoding as long as that
>> doesn't cause issues with the state of the underlying
>> libraries/drivers. Thus, I would mostly just wait for a reply from one
>> of the VAAPI wrapper maintainers regarding if this skip should happen
>> here or earlier in the decode process (where the buffers are being
>> allocated).

I suspect it would make sense for this to happen earlier in the decoder if it 
knows it doesn't have any slices (therefore not calling any hwaccel code at 
all), but I'm not really sure - maybe in this case it doesn't know early enough 
so you always have the start_frame call.  The current setup is going to 
allocate a reference frame but then with this change never do anything with it, 
which seems bad from an uninitialised-data perspective.  On the other hand, I 
agree that calling into the hardware decoder with no slices seems rather 
unhelpful, and it probably doesn't write anything useful to the frame either.

Still, all of the other hwaccel APIs (VDPAU, DXVA2, NVDEC) do currently do the 
same thing too, though, so it would seem nicer if this could be kept consistent 
across all of them.  If Mesa with VAAPI is the only case where this fails 
(checking Mesa with VDPAU on the same hardware might be interesting?) then 
fixing it in the driver would seem the best answer.  I might have a look at 
that later.

> From 3c4885579e86f6c002e614c4082a3bdb02d8426e Mon Sep 17 00:00:00 2001
> From: xbmc 
> Date: Sat, 26 Jan 2019 19:48:35 +0100
> Subject: [PATCH] avcodec/vaapi_h264: skip decode if pic has no slices
> 
> This fixes / workarounds https://bugs.freedesktop.org/show_bug.cgi?id=105368.
> It was hit frequently when watching h264 channels received via DVB-X.
> Corresponding kodi bug: https://github.com/xbmc/xbmc/issues/15704
> ---
>  libavcodec/vaapi_h264.c | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/libavcodec/vaapi_h264.c b/libavcodec/vaapi_h264.c
> index 5854587a25..f12fdc457a 100644
> --- a/libavcodec/vaapi_h264.c
> +++ b/libavcodec/vaapi_h264.c
> @@ -317,6 +317,11 @@ static int vaapi_h264_end_frame(AVCodecContext *avctx)
>  H264SliceContext *sl = >slice_ctx[0];
>  int ret;
>  
> +if (pic->nb_slices == 0) {
> +ret = AVERROR_INVALIDDATA;
> +goto finish;
> +}
> +
>  ret = ff_vaapi_decode_issue(avctx, pic);
>  if (ret < 0)
>  goto finish;
> -- 
> 2.20.1

This pastes the parameter buffers you have already sent on to the front of the 
next frame, which is going to do something pretty weird - I think you want to 
call ff_vaapi_decode_cancel() before returning.

Have you tried this on the i965 and iHD drivers and made sure it doesn't do 
anything nasty in the same case there?  (Or if you can provide a sample file 
I'll have a go myself.)

Thanks,

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

[FFmpeg-devel] [PATCH v2 5/6] vaapi_encode: Add ROI support

[Mark Thompson]

---
 libavcodec/vaapi_encode.c   | 128 
 libavcodec/vaapi_encode.h   |  11 +++
 libavcodec/vaapi_encode_h264.c  |   2 +
 libavcodec/vaapi_encode_h265.c  |   2 +
 libavcodec/vaapi_encode_mpeg2.c |   2 +
 libavcodec/vaapi_encode_vp8.c   |   2 +
 libavcodec/vaapi_encode_vp9.c   |   2 +
 7 files changed, 149 insertions(+)

diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
index 2dda451882..03a7f5ce3e 100644
--- a/libavcodec/vaapi_encode.c
+++ b/libavcodec/vaapi_encode.c
@@ -143,6 +143,7 @@ static int vaapi_encode_issue(AVCodecContext *avctx,
 int err, i;
 char data[MAX_PARAM_BUFFER_SIZE];
 size_t bit_len;
+AVFrameSideData *sd;
 
 av_log(avctx, AV_LOG_DEBUG, "Issuing encode for pic %"PRId64"/%"PRId64" "
"as type %s.\n", pic->display_order, pic->encode_order,
@@ -412,6 +413,91 @@ static int vaapi_encode_issue(AVCodecContext *avctx,
 }
 }
 
+sd = av_frame_get_side_data(pic->input_image,
+AV_FRAME_DATA_REGIONS_OF_INTEREST);
+
+#if VA_CHECK_VERSION(1, 0, 0)
+if (sd && ctx->roi_allowed) {
+const AVRegionOfInterest *roi;
+int nb_roi;
+VAEncMiscParameterBuffer param_misc = {
+.type = VAEncMiscParameterTypeROI,
+};
+VAEncMiscParameterBufferROI param_roi;
+// VAAPI requires the second structure to immediately follow the
+// first in the parameter buffer, but they have different natural
+// alignment on 64-bit architectures (4 vs. 8, since the ROI
+// structure contains a pointer).  This means we can't make a
+// single working structure, nor can we make the buffer
+// separately and map it because dereferencing the second pointer
+// would be undefined.  Therefore, construct the two parts
+// separately and combine them into a single character buffer
+// before uploading.
+char param_buffer[sizeof(param_misc) + sizeof(param_roi)];
+int i, v;
+
+roi = (const AVRegionOfInterest*)sd->data;
+av_assert0(roi->self_size && sd->size % roi->self_size == 0);
+nb_roi = sd->size / roi->self_size;
+if (nb_roi > ctx->roi_regions) {
+if (!ctx->roi_warned) {
+av_log(avctx, AV_LOG_WARNING, "More ROIs set than "
+   "supported by driver (%d > %d).\n",
+   nb_roi, ctx->roi_regions);
+ctx->roi_warned = 1;
+}
+nb_roi = ctx->roi_regions;
+}
+
+pic->roi = av_mallocz_array(nb_roi, sizeof(*pic->roi));
+if (!pic->roi) {
+err = AVERROR(ENOMEM);
+goto fail;
+}
+for (i = 0; i < nb_roi; i++) {
+roi = (const AVRegionOfInterest*)(sd->data + roi->self_size * i);
+
+v = roi->qoffset.num * ctx->roi_quant_range / roi->qoffset.den;
+av_log(avctx, AV_LOG_DEBUG, "ROI region: (%d,%d) (%dx%d) -> 
%+d.\n",
+   roi->x, roi->y, roi->width, roi->height, v);
+
+pic->roi[i] = (VAEncROI) {
+.roi_rectangle = {
+.x  = roi->x,
+.y  = roi->y,
+.width  = roi->width,
+.height = roi->height,
+},
+.roi_value = av_clip_c(v, INT8_MIN, INT8_MAX),
+};
+}
+
+param_roi = (VAEncMiscParameterBufferROI) {
+.num_roi  = nb_roi,
+.max_delta_qp = INT8_MAX,
+.min_delta_qp = 0,
+.roi  = pic->roi,
+.roi_flags.bits.roi_value_is_qp_delta = 1,
+};
+
+memcpy(param_buffer, _misc, sizeof(param_misc));
+memcpy(param_buffer + sizeof(param_misc),
+   _roi, sizeof(param_roi));
+
+err = vaapi_encode_make_param_buffer(avctx, pic,
+ VAEncMiscParameterBufferType,
+ param_buffer,
+ sizeof(param_buffer));
+if (err < 0)
+goto fail;
+} else
+#endif
+if (sd && !ctx->roi_warned) {
+av_log(avctx, AV_LOG_WARNING, "ROI side data on input "
+   "frames ignored due to lack of driver support.\n");
+ctx->roi_warned = 1;
+}
+
 vas = vaBeginPicture(ctx->hwctx->display, ctx->va_context,
  pic->input_surface);
 if (vas != VA_STATUS_SUCCESS) {
@@ -477,6 +563,7 @@ fail_at_end:
 av_freep(>codec_picture_params);
 av_freep(>param_buffers);
 av_freep(>slices);
+av_freep(>roi);
 av_frame_free(>recon_image);
 av_buffer_unref(>output_buffer_ref);
 pic->output_buffer = VA_INVALID_ID;
@@ -611,6 +698,7 @@ static int vaapi_encode_free(AVCodecContext *avctx,
 
 av_freep(>priv_data);
 av_freep(>codec_picture_params);
+av_freep(>roi);
 
 av_free(pic);
 
@@ 

[FFmpeg-devel] [PATCH v2 6/6] lavfi: addroi filter

[Mark Thompson]

This can be used to add region of interest side data to video frames.
---
Now using the x,y,w,h style to match other filters.


 libavfilter/Makefile |   1 +
 libavfilter/allfilters.c |   1 +
 libavfilter/vf_addroi.c  | 265 +++
 3 files changed, 267 insertions(+)
 create mode 100644 libavfilter/vf_addroi.c

diff --git a/libavfilter/Makefile b/libavfilter/Makefile
index fef6ec5c55..31ae738a50 100644
--- a/libavfilter/Makefile
+++ b/libavfilter/Makefile
@@ -149,6 +149,7 @@ OBJS-$(CONFIG_SINE_FILTER)   += asrc_sine.o
 OBJS-$(CONFIG_ANULLSINK_FILTER)  += asink_anullsink.o
 
 # video filters
+OBJS-$(CONFIG_ADDROI_FILTER) += vf_addroi.o
 OBJS-$(CONFIG_ALPHAEXTRACT_FILTER)   += vf_extractplanes.o
 OBJS-$(CONFIG_ALPHAMERGE_FILTER) += vf_alphamerge.o
 OBJS-$(CONFIG_AMPLIFY_FILTER)+= vf_amplify.o
diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
index c51ae0f3c7..52413c8f0d 100644
--- a/libavfilter/allfilters.c
+++ b/libavfilter/allfilters.c
@@ -140,6 +140,7 @@ extern AVFilter ff_asrc_sine;
 
 extern AVFilter ff_asink_anullsink;
 
+extern AVFilter ff_vf_addroi;
 extern AVFilter ff_vf_alphaextract;
 extern AVFilter ff_vf_alphamerge;
 extern AVFilter ff_vf_amplify;
diff --git a/libavfilter/vf_addroi.c b/libavfilter/vf_addroi.c
new file mode 100644
index 00..8bca5e7371
--- /dev/null
+++ b/libavfilter/vf_addroi.c
@@ -0,0 +1,265 @@
+/*
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include "libavutil/eval.h"
+#include "libavutil/opt.h"
+#include "avfilter.h"
+#include "internal.h"
+
+enum {
+X, Y, W, H,
+};
+static const char *addroi_param_names[] = {
+"x", "y", "w", "h",
+};
+
+enum {
+VAR_IW,
+VAR_IH,
+NB_VARS,
+};
+static const char *const addroi_var_names[] = {
+"iw",
+"ih",
+};
+
+typedef struct AddROIContext {
+const AVClass *class;
+
+char *region_str[4];
+
+AVExpr *region_expr[4];
+double vars[NB_VARS];
+
+int region[4];
+AVRational qoffset;
+
+int clear;
+} AddROIContext;
+
+static int addroi_config_input(AVFilterLink *inlink)
+{
+AVFilterContext *avctx = inlink->dst;
+AddROIContext *ctx = avctx->priv;
+int i;
+double val;
+
+ctx->vars[VAR_IW] = inlink->w;
+ctx->vars[VAR_IH] = inlink->h;
+
+for (i = 0; i < 4; i++) {
+int max_value;
+switch (i) {
+case X: max_value = inlink->w;  break;
+case Y: max_value = inlink->h;  break;
+case W: max_value = inlink->w - ctx->region[X]; break;
+case H: max_value = inlink->h - ctx->region[Y]; break;
+}
+
+val = av_expr_eval(ctx->region_expr[i], ctx->vars, NULL);
+if (val < 0.0) {
+av_log(avctx, AV_LOG_WARNING, "Calculated value %g for %s is "
+   "less than zero - using zero instead.\n", val,
+   addroi_param_names[i]);
+val = 0.0;
+} else if (val > max_value) {
+av_log(avctx, AV_LOG_WARNING, "Calculated value %g for %s is "
+   "greater than maximum allowed value %d - "
+   "using %d instead.\n", val, addroi_param_names[i],
+   max_value, max_value);
+val = max_value;
+}
+ctx->region[i] = val;
+}
+
+return 0;
+}
+
+static int addroi_filter_frame(AVFilterLink *inlink, AVFrame *frame)
+{
+AVFilterContext *avctx = inlink->dst;
+AVFilterLink  *outlink = avctx->outputs[0];
+AddROIContext *ctx = avctx->priv;
+AVRegionOfInterest *roi;
+AVFrameSideData *sd;
+int err;
+
+if (ctx->clear) {
+av_frame_remove_side_data(frame, AV_FRAME_DATA_REGIONS_OF_INTEREST);
+sd = NULL;
+} else {
+sd = av_frame_get_side_data(frame, AV_FRAME_DATA_REGIONS_OF_INTEREST);
+}
+if (sd) {
+const AVRegionOfInterest *old_roi;
+AVBufferRef *roi_ref;
+int nb_roi, i;
+
+old_roi = (const AVRegionOfInterest*)sd->data;
+nb_roi = sd->size / old_roi->self_size + 1;
+
+roi_ref = av_buffer_alloc(sizeof(*roi) * nb_roi);
+if (!roi_ref) {
+err = AVERROR(ENOMEM);
+goto fail;
+ 

[FFmpeg-devel] [PATCH v2 4/6] libx265: Update ROI behaviour to match documentation

[Mark Thompson]

Equivalent to the previous patch for libx264.
---
 libavcodec/libx265.c | 42 +-
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/libavcodec/libx265.c b/libavcodec/libx265.c
index c2c1f8b9bc..a580d113b9 100644
--- a/libavcodec/libx265.c
+++ b/libavcodec/libx265.c
@@ -304,27 +304,34 @@ static av_cold int libx265_encode_set_roi(libx265Context 
*ctx, const AVFrame *fr
 int mb_size = (ctx->params->rc.qgSize == 8) ? 8 : 16;
 int mbx = (frame->width + mb_size - 1) / mb_size;
 int mby = (frame->height + mb_size - 1) / mb_size;
+int qp_range = 51 + 6 * (pic->bitDepth - 8);
 int nb_rois;
-AVRegionOfInterest *roi;
+const AVRegionOfInterest *roi;
 float *qoffsets; /* will be freed after encode is called. 
*/
+
+roi = (const AVRegionOfInterest*)sd->data;
+if (!roi->self_size || sd->size % roi->self_size != 0) {
+av_log(ctx, AV_LOG_ERROR, "Invalid 
AVRegionOfInterest.self_size.\n");
+return AVERROR(EINVAL);
+}
+nb_rois = sd->size / roi->self_size;
+
 qoffsets = av_mallocz_array(mbx * mby, sizeof(*qoffsets));
 if (!qoffsets)
 return AVERROR(ENOMEM);
 
-nb_rois = sd->size / sizeof(AVRegionOfInterest);
-roi = (AVRegionOfInterest*)sd->data;
-for (int count = 0; count < nb_rois; count++) {
-int starty = av_clip(roi->y / mb_size, 0, mby);
-int endy   = av_clip((roi->y + roi->height + mb_size - 1) / 
mb_size, 0, mby);
-int startx = av_clip(roi->x / mb_size, 0, mbx);
-int endx   = av_clip((roi->x + roi->width  + mb_size - 1) / 
mb_size, 0, mbx);
+// This list must be iterated in reverse because the first
+// region in the list applies when regions overlap.
+for (int i = nb_rois - 1; i >= 0; i--) {
+int startx, endx, starty, endy;
 float qoffset;
 
-if (roi->self_size == 0) {
-av_free(qoffsets);
-av_log(ctx, AV_LOG_ERROR, "AVRegionOfInterest.self_size 
must be set to sizeof(AVRegionOfInterest).\n");
-return AVERROR(EINVAL);
-}
+roi = (const AVRegionOfInterest*)(sd->data + roi->self_size * 
i);
+
+starty = av_clip(roi->y / mb_size, 0, mby);
+endy   = av_clip((roi->y + roi->height + mb_size - 1) / 
mb_size, 0, mby);
+startx = av_clip(roi->x / mb_size, 0, mbx);
+endx   = av_clip((roi->x + roi->width  + mb_size - 1) / 
mb_size, 0, mbx);
 
 if (roi->qoffset.den == 0) {
 av_free(qoffsets);
@@ -332,18 +339,11 @@ static av_cold int libx265_encode_set_roi(libx265Context 
*ctx, const AVFrame *fr
 return AVERROR(EINVAL);
 }
 qoffset = roi->qoffset.num * 1.0f / roi->qoffset.den;
-qoffset = av_clipf(qoffset, -1.0f, 1.0f);
-
-/* qp range of x265 is from 0 to 51, just choose 25 as the 
scale value,
- * so the range of final qoffset is [-25.0, 25.0].
- */
-qoffset = qoffset * 25;
+qoffset = av_clipf(qoffset * qp_range, -qp_range, +qp_range);
 
 for (int y = starty; y < endy; y++)
 for (int x = startx; x < endx; x++)
 qoffsets[x + y*mbx] = qoffset;
-
-roi = (AVRegionOfInterest*)((char*)roi + roi->self_size);
 }
 
 pic->quantOffsets = qoffsets;
-- 
2.19.2

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

[FFmpeg-devel] [PATCH v2 3/6] libx264: Update ROI behaviour to match documentation

[Mark Thompson]

Fix the quantisation offset - use the whole range, and don't change the
offset size based on bit depth.

Iterate the list in reverse order.  The first region in the list is the one
that applies in the case of overlapping regions.
---
 libavcodec/libx264.c | 51 
 1 file changed, 28 insertions(+), 23 deletions(-)

diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c
index 87e6fe7c94..f5b9b8b821 100644
--- a/libavcodec/libx264.c
+++ b/libavcodec/libx264.c
@@ -285,16 +285,18 @@ static int X264_frame(AVCodecContext *ctx, AVPacket *pkt, 
const AVFrame *frame,
 int nnal, i, ret;
 x264_picture_t pic_out = {0};
 int pict_type;
+int bit_depth;
 int64_t *out_opaque;
 AVFrameSideData *sd;
 
 x264_picture_init( >pic );
 x4->pic.img.i_csp   = x4->params.i_csp;
 #if X264_BUILD >= 153
-if (x4->params.i_bitdepth > 8)
+bit_depth = x4->params.i_bitdepth;
 #else
-if (x264_bit_depth > 8)
+bit_depth = x264_bit_depth;
 #endif
+if (bit_depth > 8)
 x4->pic.img.i_csp |= X264_CSP_HIGH_DEPTH;
 x4->pic.img.i_plane = avfmt2_num_planes(ctx->pix_fmt);
 
@@ -359,45 +361,48 @@ static int X264_frame(AVCodecContext *ctx, AVPacket *pkt, 
const AVFrame *frame,
 if (frame->interlaced_frame == 0) {
 int mbx = (frame->width + MB_SIZE - 1) / MB_SIZE;
 int mby = (frame->height + MB_SIZE - 1) / MB_SIZE;
+int qp_range = 51 + 6 * (bit_depth - 8);
 int nb_rois;
-AVRegionOfInterest* roi;
-float* qoffsets;
+const AVRegionOfInterest *roi;
+float *qoffsets;
+
+roi = (const AVRegionOfInterest*)sd->data;
+if (!roi->self_size || sd->size % roi->self_size != 0) {
+av_log(ctx, AV_LOG_ERROR, "Invalid 
AVRegionOfInterest.self_size.\n");
+return AVERROR(EINVAL);
+}
+nb_rois = sd->size / roi->self_size;
+
 qoffsets = av_mallocz_array(mbx * mby, sizeof(*qoffsets));
 if (!qoffsets)
 return AVERROR(ENOMEM);
 
-nb_rois = sd->size / sizeof(AVRegionOfInterest);
-roi = (AVRegionOfInterest*)sd->data;
-for (int count = 0; count < nb_rois; count++) {
-int starty = av_clip(roi->y / MB_SIZE, 0, mby);
-int endy   = av_clip((roi->y + roi->height + MB_SIZE - 
1) / MB_SIZE, 0, mby);
-int startx = av_clip(roi->x / MB_SIZE, 0, mbx);
-int endx   = av_clip((roi->x + roi->width  + MB_SIZE - 
1) / MB_SIZE, 0, mbx);
+// This list must be iterated in reverse because the first
+// region in the list applies when regions overlap.
+for (int i = nb_rois - 1; i >= 0; i--) {
+int startx, endx, starty, endy;
 float qoffset;
 
+roi = (const AVRegionOfInterest*)(sd->data + 
roi->self_size * i);
+
+starty = av_clip(roi->y / MB_SIZE, 0, mby);
+endy   = av_clip((roi->y + roi->height + MB_SIZE - 1) 
/ MB_SIZE, 0, mby);
+startx = av_clip(roi->x / MB_SIZE, 0, mbx);
+endx   = av_clip((roi->x + roi->width  + MB_SIZE - 1) 
/ MB_SIZE, 0, mbx);
+
 if (roi->qoffset.den == 0) {
 av_free(qoffsets);
-av_log(ctx, AV_LOG_ERROR, 
"AVRegionOfInterest.qoffset.den should not be zero.\n");
+av_log(ctx, AV_LOG_ERROR, 
"AVRegionOfInterest.qoffset.den must not be zero.\n");
 return AVERROR(EINVAL);
 }
 qoffset = roi->qoffset.num * 1.0f / roi->qoffset.den;
-qoffset = av_clipf(qoffset, -1.0f, 1.0f);
-
-// 25 is a number that I think it is a possible proper 
scale value.
-qoffset = qoffset * 25;
+qoffset = av_clipf(qoffset * qp_range, -qp_range, 
+qp_range);
 
 for (int y = starty; y < endy; y++) {
 for (int x = startx; x < endx; x++) {
 qoffsets[x + y*mbx] = qoffset;
 }
 }
-
-if (roi->self_size == 0) {
-av_free(qoffsets);
-av_log(ctx, AV_LOG_ERROR, 
"AVRegionOfInterest.self_size should be set to sizeof(AVRegionOfInterest).\n");
-return AVERROR(EINVAL);
-}
-roi = (AVRegionOfInterest*)((char*)roi + 
roi->self_size);
 

[FFmpeg-devel] [PATCH v2 2/6] lavu/frame: Expand ROI documentation

[Mark Thompson]

Clarify and add examples for the behaviour of the quantisation offset,
and define how multiple ranges should be handled.
---
 libavutil/frame.h | 46 ++
 1 file changed, 34 insertions(+), 12 deletions(-)

diff --git a/libavutil/frame.h b/libavutil/frame.h
index cc3b78d8b6..4933538ad4 100644
--- a/libavutil/frame.h
+++ b/libavutil/frame.h
@@ -207,22 +207,21 @@ typedef struct AVFrameSideData {
 } AVFrameSideData;
 
 /**
- * Structure to hold Region Of Interest.
+ * Structure describing a single Region Of Interest.
  *
- * self_size specifies the size of this data structure. This value
- * should be set to sizeof(AVRegionOfInterest). EINVAL is returned if 
self_size is zero.
+ * When multiple regions are defined in a single side-data block, they
+ * should be ordered from most to least important - some encoders are only
+ * capable of supporting a limited number of distinct regions, so will have
+ * to truncate the list.
  *
- * If the regions overlap, the last value in the list will be used.
- *
- * qoffset is quant offset, and base rule here:
- * returns EINVAL if AVRational.den is zero.
- * the value (num/den) range is [-1.0, 1.0], clamp to +-1.0 if out of range.
- * 0 means no picture quality change,
- * negative offset asks for better quality (and the best with value -1.0),
- * positive offset asks for worse quality (and the worst with value 1.0).
- * How to explain/implement the different quilaity requirement is encoder 
dependent.
+ * When overlapping regions are defined, the first region containing a given
+ * area of the frame applies.
  */
 typedef struct AVRegionOfInterest {
+/**
+ * Must be set to the size of this data structure (that is,
+ * sizeof(AVRegionOfInterest)).
+ */
 uint32_t self_size;
 /**
  * x/y coordinates of the top-left corner and width/height of the
@@ -236,6 +235,29 @@ typedef struct AVRegionOfInterest {
 int y;
 int width;
 int height;
+/**
+ * Quantisation offset.
+ *
+ * Must be in the range -1 to +1.  A value of zero indicates no quality
+ * change.  A negative value asks for better quality (less quantisation),
+ * while a positive value asks for worse quality (greater quantisation).
+ *
+ * The range is calibrated so that the extreme values indicate the
+ * largest possible offset - if the rest of the frame is encoded with the
+ * worst possible quality, an offset of -1 indicates that this region
+ * should be encoded with the best possible quality anyway.  Intermediate
+ * values are then interpolated in some codec-dependent way.
+ *
+ * For example, in 10-bit H.264 the quantisation parameter varies between
+ * -12 and 51.  A typical qoffset value of -1/10 therefore indicates that
+ * this region should be encoded with a QP around one-tenth of the full
+ * range better than the rest of the frame.  So, if most of the frame
+ * were to be encoded with a QP of around 30, this region would get a QP
+ * of around 24 (an offset of approximately -1/10 * (51 - -12) = -6.3).
+ * An extreme value of -1 would indicate that this region should be
+ * encoded with the best possible quality regardless of the treatment of
+ * the rest of the frame - that is, should be encoded at a QP of -12.
+ */
 AVRational qoffset;
 } AVRegionOfInterest;
 
-- 
2.19.2

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

[FFmpeg-devel] [PATCH v2 1/6] lavu/frame: Change region of interest rectangle definition

[Mark Thompson]

The definition here previously matched the way cropping worked, but the
actual uses in libavcodec instead treated the values as the coordinates
of the top-left and bottom-right corners.

The most common way of defining rectangles in FFmpeg appears to be as
coordinates of the top-left corner combined with a width and height; so,
change to using that instead.  This is technically both an API and an ABI
break, but since the structure was added recently and the previous
definition was never actually used correctly I believe that this should
not cause any problems.
---
This seems to be by far the most common choice of rectangle definition, so I 
think the most consistent one to use.

If you think the break is a step too far (if there are active users then maybe 
it's left a bit too long since the API was added) then please do say and I'll 
revert back to the version keeping the currently-documented cropping behaviour.

(Actual version bump to add.)

- Mark


 doc/APIchanges   |  3 +++
 libavcodec/libx264.c |  8 
 libavcodec/libx265.c |  8 
 libavutil/frame.h| 20 
 4 files changed, 23 insertions(+), 16 deletions(-)

diff --git a/doc/APIchanges b/doc/APIchanges
index 784a5e5bd2..4e3b5d6b5a 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -15,6 +15,9 @@ libavutil: 2017-10-21
 
 API changes, most recent first:
 
+2019-03-XX - XX - lavc 56.XX.100 - frame.h
+  Change definition of AVRegionOfInterest.
+
 2019-01-27 - XX - lavc 58.46.100 - avcodec.h
   Add discard_damaged_percentage
 
diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c
index a3493f393d..87e6fe7c94 100644
--- a/libavcodec/libx264.c
+++ b/libavcodec/libx264.c
@@ -369,10 +369,10 @@ static int X264_frame(AVCodecContext *ctx, AVPacket *pkt, 
const AVFrame *frame,
 nb_rois = sd->size / sizeof(AVRegionOfInterest);
 roi = (AVRegionOfInterest*)sd->data;
 for (int count = 0; count < nb_rois; count++) {
-int starty = FFMIN(mby, roi->top / MB_SIZE);
-int endy   = FFMIN(mby, (roi->bottom + MB_SIZE - 1)/ 
MB_SIZE);
-int startx = FFMIN(mbx, roi->left / MB_SIZE);
-int endx   = FFMIN(mbx, (roi->right + MB_SIZE - 1)/ 
MB_SIZE);
+int starty = av_clip(roi->y / MB_SIZE, 0, mby);
+int endy   = av_clip((roi->y + roi->height + MB_SIZE - 
1) / MB_SIZE, 0, mby);
+int startx = av_clip(roi->x / MB_SIZE, 0, mbx);
+int endx   = av_clip((roi->x + roi->width  + MB_SIZE - 
1) / MB_SIZE, 0, mbx);
 float qoffset;
 
 if (roi->qoffset.den == 0) {
diff --git a/libavcodec/libx265.c b/libavcodec/libx265.c
index fe39f45241..c2c1f8b9bc 100644
--- a/libavcodec/libx265.c
+++ b/libavcodec/libx265.c
@@ -314,10 +314,10 @@ static av_cold int libx265_encode_set_roi(libx265Context 
*ctx, const AVFrame *fr
 nb_rois = sd->size / sizeof(AVRegionOfInterest);
 roi = (AVRegionOfInterest*)sd->data;
 for (int count = 0; count < nb_rois; count++) {
-int starty = FFMIN(mby, roi->top / mb_size);
-int endy   = FFMIN(mby, (roi->bottom + mb_size - 1)/ mb_size);
-int startx = FFMIN(mbx, roi->left / mb_size);
-int endx   = FFMIN(mbx, (roi->right + mb_size - 1)/ mb_size);
+int starty = av_clip(roi->y / mb_size, 0, mby);
+int endy   = av_clip((roi->y + roi->height + mb_size - 1) / 
mb_size, 0, mby);
+int startx = av_clip(roi->x / mb_size, 0, mbx);
+int endx   = av_clip((roi->x + roi->width  + mb_size - 1) / 
mb_size, 0, mbx);
 float qoffset;
 
 if (roi->self_size == 0) {
diff --git a/libavutil/frame.h b/libavutil/frame.h
index 8aa3e88367..cc3b78d8b6 100644
--- a/libavutil/frame.h
+++ b/libavutil/frame.h
@@ -212,10 +212,6 @@ typedef struct AVFrameSideData {
  * self_size specifies the size of this data structure. This value
  * should be set to sizeof(AVRegionOfInterest). EINVAL is returned if 
self_size is zero.
  *
- * Number of pixels to discard from the top/bottom/left/right border of
- * the frame to obtain the region of interest of the frame.
- * They are encoder dependent and will be extended internally
- * if the codec requires an alignment.
  * If the regions overlap, the last value in the list will be used.
  *
  * qoffset is quant offset, and base rule here:
@@ -228,10 +224,18 @@ typedef struct AVFrameSideData {
  */
 typedef struct AVRegionOfInterest {
 uint32_t self_size;
-int top;
-int bottom;
-int left;
-int right;
+/**
+ * x/y coordinates of the top-left corner and width/height of the
+ * rectangle defining this region of interest.
+ *
+ * The constraints on a region are encoder dependent, so the region
+   

Re: [FFmpeg-devel] [PATCHv2] avcodec/nvenc: Reconfigure resolution on-the-fly

[Mark Thompson]

On 08/03/2019 07:44, Oliver Collyer wrote:
> From c704e3535f866d9f89535b9df59db9ca9811bcf9 Mon Sep 17 00:00:00 2001
> From: Oliver Collyer 
> Date: Fri, 8 Mar 2019 07:42:41 +
> Subject: [PATCH 1/1] avcodec/nvenc: Reconfigure resolution on-the-fly
> 
> ---
>  libavcodec/nvenc.c  | 31 ---
>  libavcodec/nvenc.h  |  3 +++
>  libavcodec/nvenc_h264.c |  4 
>  libavcodec/nvenc_hevc.c |  4 
>  4 files changed, 39 insertions(+), 3 deletions(-)

Can you explain the actual intended use-case for this?

The current way to handle resolution changes (or any other stream change of 
similar magnitude, like a pixel format change) is to flush the existing encoder 
and then make a new one with the new parameters.  Even ignoring the trivial 
benefit that all encoders handle this with no additional code, it has the 
significant advantage that all of the stream metadata, including parameter 
sets, can be handled correctly.  The handling here does rather badly at this - 
stream metadata will be misleading, and if you take one of these streams and 
try to put it into some container with global headers you may well end up with 
a quite broken file.

Given that, I think there should be some justification for why you might want 
to do this rather than following the existing approach.  Some mention in the 
API (avcodec.h) to explain what's going on might be a good idea, since that is 
currently assuming that parameters like width/height are fixed and must be set 
before opening the encoder.  Relatedly, if there isn't any support for 
retrieving new metadata then it should probably display a big warning if the 
user tries to make a stream like this with global headers, since the global 
headers provided to the user on startup won't actually work for the whole 
stream.

Thanks,

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

Re: [FFmpeg-devel] [PATCH v1] avformat/rtpdec.h remove unused variable

[Steven Liu]

> On Mar 13, 2019, at 05:26, Jun Li  wrote:
> 
> Looks like the variable 'cur_timestamp' is not used anywhere.
> So remove this variable.
> 
> Signed-off-by: Jun Li 
> ---
> libavformat/rtpdec.h | 1 -
> 1 file changed, 1 deletion(-)
> 
> diff --git a/libavformat/rtpdec.h b/libavformat/rtpdec.h
> index 5a47d6f79d..9144edbe8b 100644
> --- a/libavformat/rtpdec.h
> +++ b/libavformat/rtpdec.h
> @@ -154,7 +154,6 @@ struct RTPDemuxContext {
> uint16_t seq;
> uint32_t timestamp;
> uint32_t base_timestamp;
> -uint32_t cur_timestamp;
> int64_t  unwrapped_timestamp;
> int64_t  range_start_offset;
> int max_payload_size;
> -- 
> 2.17.1
> 
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

LGTM

Thanks
Steven





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

Re: [FFmpeg-devel] [PATCH] libdav1d: Add support for reading hdr10 metadata

[James Almer]

On 3/5/2019 3:19 PM, Vittorio Giovara wrote:
> ---
>  configure |  2 +-
>  libavcodec/libdav1d.c | 30 +-
>  2 files changed, 30 insertions(+), 2 deletions(-)
> 
> diff --git a/configure b/configure
> index dcead3a300..a5cef4bc09 100755
> --- a/configure
> +++ b/configure
> @@ -6142,7 +6142,7 @@ enabled libcelt   && require libcelt 
> celt/celt.h celt_decode -lcelt0 &&
> die "ERROR: libcelt must be installed and 
> version must be >= 0.11.0."; }
>  enabled libcaca   && require_pkg_config libcaca caca caca.h 
> caca_create_canvas
>  enabled libcodec2 && require libcodec2 codec2/codec2.h codec2_create 
> -lcodec2
> -enabled libdav1d  && require_pkg_config libdav1d "dav1d >= 0.1.0" 
> "dav1d/dav1d.h" dav1d_version
> +enabled libdav1d  && require_pkg_config libdav1d "dav1d >= 0.2.0" 
> "dav1d/dav1d.h" dav1d_version
>  enabled libdavs2  && require_pkg_config libdavs2 "davs2 >= 1.6.0" 
> davs2.h davs2_decoder_open
>  enabled libdc1394 && require_pkg_config libdc1394 libdc1394-2 
> dc1394/dc1394.h dc1394_new
>  enabled libdrm&& require_pkg_config libdrm libdrm xf86drm.h 
> drmGetVersion
> diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c
> index ed02da4ebf..355dd184f4 100644
> --- a/libavcodec/libdav1d.c
> +++ b/libavcodec/libdav1d.c
> @@ -22,6 +22,7 @@
>  #include 
>  
>  #include "libavutil/avassert.h"
> +#include "libavutil/mastering_display_metadata.h"
>  #include "libavutil/opt.h"
>  
>  #include "avcodec.h"
> @@ -90,7 +91,7 @@ static int libdav1d_receive_frame(AVCodecContext *c, 
> AVFrame *frame)
>  Libdav1dContext *dav1d = c->priv_data;
>  Dav1dData *data = >data;
>  Dav1dPicture *p;
> -int res;
> +int i, res;
>  
>  if (!data->sz) {
>  AVPacket pkt = { 0 };
> @@ -206,6 +207,33 @@ FF_ENABLE_DEPRECATION_WARNINGS
>  return AVERROR_INVALIDDATA;
>  }
>  
> +if (p->mastering_display) {
> +AVMasteringDisplayMetadata *mastering = 
> av_mastering_display_metadata_create_side_data(frame);
> +if (!mastering)
> +return AVERROR(ENOMEM);
> +
> +for (i = 0; i < 3; i++) {
> +mastering->display_primaries[i][0] = 
> av_make_q(p->mastering_display->primaries[i][0], 1 << 16);
> +mastering->display_primaries[i][1] = 
> av_make_q(p->mastering_display->primaries[i][1], 1 << 16);
> +}
> +mastering->white_point[0] = 
> av_make_q(p->mastering_display->white_point[0], 1 << 16);
> +mastering->white_point[1] = 
> av_make_q(p->mastering_display->white_point[1], 1 << 16);
> +
> +mastering->max_luminance = 
> av_make_q(p->mastering_display->max_luminance, 1 << 8);
> +mastering->min_luminance = 
> av_make_q(p->mastering_display->min_luminance, 1 << 14);
> +
> +mastering->has_primaries = 1;
> +mastering->has_luminance = 1;
> +}
> +if (p->content_light) {
> +AVContentLightMetadata *light = 
> av_content_light_metadata_create_side_data(frame);
> +if (!light)
> +return AVERROR(ENOMEM);
> +
> +light->MaxCLL = p->content_light->max_content_light_level;
> +light->MaxFALL = p->content_light->max_frame_average_light_level;
> +}
> +
>  return 0;
>  }

Pushed alongside my patches.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/1] avcodec/aacenc: Validate and log bitrate changes made during encoding

[Oliver Collyer]

> On 12 Mar 2019, at 21:57, Rostislav Pehlivanov  wrote:
> 
>> On Sat, 9 Mar 2019 at 12:56, Oliver Collyer  wrote:
>> 
>> Although accenc appears able to cope with dynamic bitrate changes, this
>> patch makes sure that any such changes are validated in the same manner as
>> the initial bitrate and also adds logging of such changes in verbose mode.
>> 
>> ___
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> 
> I think its better to leave the bitrate default bitrate setting in init,

Ok.

> remove the last_bit_rate variable entirely and always do limiting at the
> start of the encode_frame function. Don't bother with printing out bitrate
> changes, users can assume the encoder will start using it.

Ok but It’s only in verbose mode, I saw other encoders (such as nvenc) doing 
similar when bitrate changes so I modelled it on that.

I would have thought that is exactly the sort of thing verbose exists for.

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

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

Re: [FFmpeg-devel] [PATCH 1/1] avcodec/aacenc: Validate and log bitrate changes made during encoding

[Rostislav Pehlivanov]

On Sat, 9 Mar 2019 at 12:56, Oliver Collyer  wrote:

> Although accenc appears able to cope with dynamic bitrate changes, this
> patch makes sure that any such changes are validated in the same manner as
> the initial bitrate and also adds logging of such changes in verbose mode.
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel


I think its better to leave the bitrate default bitrate setting in init,
remove the last_bit_rate variable entirely and always do limiting at the
start of the encode_frame function. Don't bother with printing out bitrate
changes, users can assume the encoder will start using it.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 05/10] avformat/matroskadec: Remove non-incremental parsing of clusters

[Andreas Rheinhardt]

Michael Niedermayer:
> 
> thanks for the detailed analysis, and yes i agree this is a bug in
> existing code
> 

Does this mean that the fact that this patch affects seeking does not
hinder its merging? Or does the seeking issue have to be fixed first?
In the latter case, I'd omitt this patch for now and modify the other
ones.

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

[FFmpeg-devel] [PATCH] Revised ff_v210_planar_unpack AVX2

[Michael Stoner]

Replaced VSHUFPS with VPBLENDD to relieve port 5 bottleneck
AVX2 is now 1.4x faster than AVX

Tested on Broadwell CPU, Ubuntu 18.10 x86_64

~/FFmpeg$ tests/checkasm/checkasm --bench --test=v210dec
benchmarking with native FFmpeg timers
nop: 94.1
checkasm: using random seed 3963743306
SSSE3:
 - v210dec.v210_unpack [OK]
 AVX:
  - v210dec.v210_unpack [OK]
  AVX2:
   - v210dec.v210_unpack [OK]
   checkasm: all 3 tests passed
   v210_unpack_c: 1625.2
   v210_unpack_ssse3: 604.2
   v210_unpack_avx: 592.2
   v210_unpack_avx2: 422.2

---
 libavcodec/v210dec.c   | 10 +-
 libavcodec/x86/v210-init.c |  8 +
 libavcodec/x86/v210.asm| 72 +-
 3 files changed, 73 insertions(+), 17 deletions(-)

diff --git a/libavcodec/v210dec.c b/libavcodec/v210dec.c
index ddc5dbe8be..26954c0df3 100644
--- a/libavcodec/v210dec.c
+++ b/libavcodec/v210dec.c
@@ -119,7 +119,7 @@ static int decode_frame(AVCodecContext *avctx, void *data, 
int *got_frame,
 const uint32_t *src = (const uint32_t*)psrc;
 uint32_t val;
 
-w = (avctx->width / 6) * 6;
+w = (avctx->width / 12) * 12;
 s->unpack_frame(src, y, u, v, w);
 
 y += w;
@@ -127,6 +127,14 @@ static int decode_frame(AVCodecContext *avctx, void *data, 
int *got_frame,
 v += w >> 1;
 src += (w << 1) / 3;
 
+if (w < avctx->width - 5) {
+   READ_PIXELS(u, y, v);
+   READ_PIXELS(y, u, y);
+   READ_PIXELS(v, y, u);
+   READ_PIXELS(y, v, y);
+w += 6;
+}
+
 if (w < avctx->width - 1) {
 READ_PIXELS(u, y, v);
 
diff --git a/libavcodec/x86/v210-init.c b/libavcodec/x86/v210-init.c
index d64dbca1a8..cb9a6cbd6a 100644
--- a/libavcodec/x86/v210-init.c
+++ b/libavcodec/x86/v210-init.c
@@ -21,9 +21,11 @@
 
 extern void ff_v210_planar_unpack_unaligned_ssse3(const uint32_t *src, 
uint16_t *y, uint16_t *u, uint16_t *v, int width);
 extern void ff_v210_planar_unpack_unaligned_avx(const uint32_t *src, uint16_t 
*y, uint16_t *u, uint16_t *v, int width);
+extern void ff_v210_planar_unpack_unaligned_avx2(const uint32_t *src, uint16_t 
*y, uint16_t *u, uint16_t *v, int width);
 
 extern void ff_v210_planar_unpack_aligned_ssse3(const uint32_t *src, uint16_t 
*y, uint16_t *u, uint16_t *v, int width);
 extern void ff_v210_planar_unpack_aligned_avx(const uint32_t *src, uint16_t 
*y, uint16_t *u, uint16_t *v, int width);
+extern void ff_v210_planar_unpack_aligned_avx2(const uint32_t *src, uint16_t 
*y, uint16_t *u, uint16_t *v, int width);
 
 av_cold void ff_v210_x86_init(V210DecContext *s)
 {
@@ -36,6 +38,9 @@ av_cold void ff_v210_x86_init(V210DecContext *s)
 
 if (HAVE_AVX_EXTERNAL && cpu_flags & AV_CPU_FLAG_AVX)
 s->unpack_frame = ff_v210_planar_unpack_aligned_avx;
+
+if (HAVE_AVX2_EXTERNAL && cpu_flags & AV_CPU_FLAG_AVX2)
+s->unpack_frame = ff_v210_planar_unpack_aligned_avx2;
 }
 else {
 if (cpu_flags & AV_CPU_FLAG_SSSE3)
@@ -43,6 +48,9 @@ av_cold void ff_v210_x86_init(V210DecContext *s)
 
 if (HAVE_AVX_EXTERNAL && cpu_flags & AV_CPU_FLAG_AVX)
 s->unpack_frame = ff_v210_planar_unpack_unaligned_avx;
+
+if (HAVE_AVX2_EXTERNAL && cpu_flags & AV_CPU_FLAG_AVX2)
+s->unpack_frame = ff_v210_planar_unpack_unaligned_avx2;
 }
 #endif
 }
diff --git a/libavcodec/x86/v210.asm b/libavcodec/x86/v210.asm
index c24c765e5b..706712313d 100644
--- a/libavcodec/x86/v210.asm
+++ b/libavcodec/x86/v210.asm
@@ -22,9 +22,14 @@
 
 %include "libavutil/x86/x86util.asm"
 
-SECTION_RODATA
+SECTION_RODATA 32
+
+; for AVX2 version only
+v210_luma_permute: dd 0,1,2,4,5,6,7,7  ; 32-byte alignment required
+v210_chroma_shuf2: db 0,1,2,3,4,5,8,9,10,11,12,13,-1,-1,-1,-1
+v210_luma_shuf_avx2: db 0,1,4,5,6,7,8,9,12,13,14,15,-1,-1,-1,-1
+v210_chroma_shuf_avx2: db 0,1,4,5,10,11,-1,-1,2,3,8,9,12,13,-1,-1
 
-v210_mask: times 4 dd 0x3ff
 v210_mult: dw 64,4,64,4,64,4,64,4
 v210_luma_shuf: db 8,9,0,1,2,3,12,13,4,5,6,7,-1,-1,-1,-1
 v210_chroma_shuf: db 0,1,8,9,6,7,-1,-1,2,3,4,5,12,13,-1,-1
@@ -34,40 +39,65 @@ SECTION .text
 %macro v210_planar_unpack 1
 
 ; v210_planar_unpack(const uint32_t *src, uint16_t *y, uint16_t *u, uint16_t 
*v, int width)
-cglobal v210_planar_unpack_%1, 5, 5, 7
+cglobal v210_planar_unpack_%1, 5, 5, 8
 movsxdifnidn r4, r4d
 lear1, [r1+2*r4]
 addr2, r4
 addr3, r4
 negr4
 
-mova   m3, [v210_mult]
-mova   m4, [v210_mask]
-mova   m5, [v210_luma_shuf]
-mova   m6, [v210_chroma_shuf]
+VBROADCASTI128   m3, [v210_mult]
+VBROADCASTI128   m5, [v210_chroma_shuf]
+
+%if cpuflag(avx2)
+VBROADCASTI128   m4, [v210_luma_shuf_avx2]
+VBROADCASTI128   m5, [v210_chroma_shuf_avx2]
+mova m6, [v210_luma_permute]
+VBROADCASTI128   m7, [v210_chroma_shuf2]
+%else
+VBROADCASTI128   m4, [v210_luma_shuf]
+VBROADCASTI128   m5, [v210_chroma_shuf]

[FFmpeg-devel] [PATCH v1] avformat/rtpdec.h remove unused variable

[Jun Li]

Looks like the variable 'cur_timestamp' is not used anywhere.
So remove this variable.

Signed-off-by: Jun Li 
---
 libavformat/rtpdec.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/libavformat/rtpdec.h b/libavformat/rtpdec.h
index 5a47d6f79d..9144edbe8b 100644
--- a/libavformat/rtpdec.h
+++ b/libavformat/rtpdec.h
@@ -154,7 +154,6 @@ struct RTPDemuxContext {
 uint16_t seq;
 uint32_t timestamp;
 uint32_t base_timestamp;
-uint32_t cur_timestamp;
 int64_t  unwrapped_timestamp;
 int64_t  range_start_offset;
 int max_payload_size;
-- 
2.17.1

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

Re: [FFmpeg-devel] [PATCH] Revised ff_v210_planar_unpack AVX2

[Mike Stoner]

I am submitting another patch.  Please disregard this one.

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

Re: [FFmpeg-devel] [PATCH v2 2/2] fate/mxf: add mxf user comments tests

[Michael Niedermayer]

On Mon, Mar 11, 2019 at 01:22:38PM -0700, mindm...@gmail.com wrote:
> From: Mark Reid 
> 
> ---
>  tests/fate/mxf.mak  | 15 ++-
>  tests/ref/fate/mxf-d10-user-comments|  1 +
>  tests/ref/fate/mxf-opatom-user-comments |  1 +
>  tests/ref/fate/mxf-user-comments|  1 +
>  4 files changed, 17 insertions(+), 1 deletion(-)
>  create mode 100644 tests/ref/fate/mxf-d10-user-comments
>  create mode 100644 tests/ref/fate/mxf-opatom-user-comments
>  create mode 100644 tests/ref/fate/mxf-user-comments

tested on x86-64 and mips qemu

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

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


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

Re: [FFmpeg-devel] [PATCH 1/5] avfilter/af_astats: add support for selecting measured statistics

[Marton Balint]

On Tue, 5 Mar 2019, Marton Balint wrote:


set_metadata with many entries is not very efficient, and with small audio
frames the performance loss is noticable. Also with this very simple
calculations (like peak) can be even further optimized.

Unfoturnately there are some small differences in metadata and av_log info
output, so factorizing calculations and output might not worth the hassle.



Ping for the series.

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

Re: [FFmpeg-devel] [PATCH 05/10] avformat/matroskadec: Remove non-incremental parsing of clusters

[Michael Niedermayer]

On Tue, Mar 12, 2019 at 05:05:00AM +, Andreas Rheinhardt wrote:
> Michael Niedermayer:
> > On Sun, Mar 10, 2019 at 11:03:00PM +, Andreas Rheinhardt wrote:
> >> Michael Niedermayer:
> >>> On Fri, Mar 08, 2019 at 10:25:59AM +0100, Andreas Rheinhardt wrote:
>  When the new incremental parser was introduced, the old parser was
>  kept, because the new parser was unable to handle the way SSA packets
>  are put into Matroska. But since 2014 (since
>  c7d8dbad14ed5fa3c217a4fc1790021d6c0b6416) this is no longer needed, so
>  that the old parser can be completely removed.
> 
>  Signed-off-by: Andreas Rheinhardt 
>  ---
>   libavformat/matroskadec.c | 72 ++-
>   1 file changed, 10 insertions(+), 62 deletions(-)
> >>>
> >>> This affects seeking:
> >>>
> >>> libavformat/tests/seek 
> >>> issues/388/Matrix.Reloaded.Trailer-640x346-XviD-1.0beta2-HE_AAC_subtitled.mkv
> >>>  -duration 400 >oldseek
> >>>
> >>> The file appears to be available here:
> >>> https://samples.ffmpeg.org/Matroska/matrix/Matrix.Reloaded.Trailer-640x346-XviD-1.0beta2-HE_AAC_subtitled.mkv
> >>>
> >>> --- oldseek   2019-03-08 23:08:21.380042329 +0100
> >>> +++ newseek   2019-03-08 23:08:02.048041745 +0100
> >>> @@ -8,7 +8,7 @@
> >>>  ret: 0 st:13 flags:1 dts: 86.75 pts: 86.75 pos: -1 
> >>> size: 50436
> >>>  ret:-1 st: 1 flags:0  ts: 250.577000
> >>>  ret: 0 st: 1 flags:1  ts: 13.471000
> >>> -ret: 0 st:13 flags:1 dts: 1.776000 pts: 1.776000 pos: -1 
> >>> size: 50436
> >>> +ret: 0 st:13 flags:1 dts: 0.00 pts: 0.00 pos: -1 
> >>> size: 50436
> >>>  ret:-1 st: 2 flags:0  ts: 176.365000
> >>>  ret: 0 st: 2 flags:1  ts: 339.259000
> >>>  ret: 0 st:13 flags:1 dts: 145.08 pts: 145.08 pos: -1 
> >>> size: 50436
> >>>
> >> This is not unexpected. The reason is that the old parser always
> >> parses a whole cluster at once while the new parser parses clusters
> >> incrementally, i.e. one block (I use block as a general term for
> >> SimpleBlock or BlockGroup) at a time. The goal of incremental parsing
> >> was reduced latency (and with my patch #6 one also achieves a
> >> reduction of memory used and an increase in performance as well).
> >>
> >> With the old parser, the very first cluster gets parsed during
> >> avformat_find_stream_info() and index entries were created for all the
> >> keyframes contained therein (the cues only contain entries for the
> >> main video track, so this only affects the other tracks). The 1.776
> >> pts corresponds to the block with the highest timestamp for track #1
> >> (or track #2 in Matroska's counting) in the first cluster (with a
> >> timestamp of 1776ms).
> >>
> >> With the incremental parser, only one block of the audio track gets
> >> parsed during avformat_find_stream_info() (it has a timestamp of 0)
> >> and so only one index entry gets created for it.
> >>
> >> So when the seek based upon the audio track is performed, the used
> >> index point has a timestamp of either 0ms or 1776ms. Then the
> >> current_dts is updated based upon this timestamp.
> >>
> >> Now this file has an attached picture which gets translated into its
> >> own track and upon every seek this picture will be put into the
> >> raw_packet_buffer from which it will be read by ff_read_packet as the
> >> first packet after each seek. Given that this packet doesn't have
> >> timestamps, the timestamp will be set equal to current_dts (in
> >> compute_pkt_fields()). Hence the discrepancy.
> >>
> >> Btw: Because of things like this, a fate test had to be changed during
> >> the initial introduction of incremental parsing (in
> >> 8336eb6f85e4b94b9c198b16bd0ac4178f4dba86).
> > 
> > Sounds like undesirable behavior if not a bug
> > 
> > The seek request is to 13.471000 and the result should not depend on
> > what has been parsed or not prior to the seek request.
> > also if a packet can be produced for 1.776000 which adequatly fullfills
> > the seek reuest that is better than a more distant one as that would
> > increase latency experienced by the user
> > 
> If all one cares about is that the returned packet is near to the
> desired timestamp, then matroska_read_seek succeeds in two cases:
> a) The currently available index entries for the desired track are so
> fine-grained that one can use this index to seek accurately. This
> includes the standard case that one seeks according to a video track
> for which the file provides cues (for the keyframes).
> b) The desired timestamp is so big that it is beyond the last index
> entry or the returned index entry is the last index entry. In this
> case the file is read from the last index entry onward, so that a very
> precise timestamp can be found.
> 
> If one only seeks wrt the same track and if said track either has a
> good index or no index, then this works well: It's clear that it works
> in case there is a 

Re: [FFmpeg-devel] [PATCH] Added more commandline options for libaom encoder.

[Moritz Barsnick]

On Tue, Mar 12, 2019 at 11:45:06 -0300, James Almer wrote:
> > +{ "frame-parallel",   "Enable frame parallel decodability features", 
> > OFFSET(frame_parallel),  AV_OPT_TYPE_BOOL,{.i64 = -1}, -1, 1, VE},
> 
> A bool with three values is weird. If this is meant to be disabled by
> default, then just make it 0 and remove the >= 0 condition above.

It's actually quite common with ffmpeg options. Such an option can only
be assigned true or false, but if it isn't assigned, it will be e.g. -1
and can be used to trigger a default mode, as is apparently done here
(by not setting this option through the API, probably letting the
library do whatever it considers default).

Cheers,
Moritz
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] Added more commandline options for libaom encoder.

[James Almer]

On 3/8/2019 10:21 PM, Sam John wrote:
> The following are the newly added options:
> arnr_max_frames, arnr_strength, aq_mode, denoise_noise_level, 
> denoise_block_size,
> rc_undershoot_pct, rc_overshoot_pct, minsection_pct, maxsection_pct, 
> frame_parallel,
> enable_cdef, and enable_global_motion.
> 
> Also added macros for compiling for aom 1.0.0.
> ---
>  libavcodec/libaomenc.c | 86 --
>  1 file changed, 83 insertions(+), 3 deletions(-)
> 
> diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c
> index faec61cacd..8e0ba7241e 100644
> --- a/libavcodec/libaomenc.c
> +++ b/libavcodec/libaomenc.c
> @@ -66,36 +66,65 @@ typedef struct AOMEncoderContext {
>  struct FrameListData *coded_frame_list;
>  int cpu_used;
>  int auto_alt_ref;
> +int arnr_max_frames;
> +int arnr_strength;
> +int aq_mode;
>  int lag_in_frames;
>  int error_resilient;
>  int crf;
>  int static_thresh;
>  int drop_threshold;
> +int denoise_noise_level;
> +int denoise_block_size;
>  uint64_t sse[4];
>  int have_sse; /**< true if we have pending sse[] */
>  uint64_t frame_number;
> +int rc_undershoot_pct;
> +int rc_overshoot_pct;
> +int minsection_pct;
> +int maxsection_pct;
> +int frame_parallel;
>  int tile_cols, tile_rows;
>  int tile_cols_log2, tile_rows_log2;
>  aom_superblock_size_t superblock_size;
>  int uniform_tiles;
>  int row_mt;
> +int enable_cdef;
> +int enable_global_motion;
>  } AOMContext;
>  
>  static const char *const ctlidstr[] = {
>  [AOME_SET_CPUUSED]  = "AOME_SET_CPUUSED",
>  [AOME_SET_CQ_LEVEL] = "AOME_SET_CQ_LEVEL",
>  [AOME_SET_ENABLEAUTOALTREF] = "AOME_SET_ENABLEAUTOALTREF",
> +[AOME_SET_ARNR_MAXFRAMES]   = "AOME_SET_ARNR_MAXFRAMES",
> +[AOME_SET_ARNR_STRENGTH]= "AOME_SET_ARNR_STRENGTH",
>  [AOME_SET_STATIC_THRESHOLD] = "AOME_SET_STATIC_THRESHOLD",
>  [AV1E_SET_COLOR_RANGE]  = "AV1E_SET_COLOR_RANGE",
>  [AV1E_SET_COLOR_PRIMARIES]  = "AV1E_SET_COLOR_PRIMARIES",
>  [AV1E_SET_MATRIX_COEFFICIENTS] = "AV1E_SET_MATRIX_COEFFICIENTS",
>  [AV1E_SET_TRANSFER_CHARACTERISTICS] = 
> "AV1E_SET_TRANSFER_CHARACTERISTICS",
> +[AV1E_SET_AQ_MODE]  = "AV1E_SET_AQ_MODE",
> +[AV1E_SET_FRAME_PARALLEL_DECODING] = "AV1E_SET_FRAME_PARALLEL_DECODING",
>  [AV1E_SET_SUPERBLOCK_SIZE]  = "AV1E_SET_SUPERBLOCK_SIZE",
>  [AV1E_SET_TILE_COLUMNS] = "AV1E_SET_TILE_COLUMNS",
>  [AV1E_SET_TILE_ROWS]= "AV1E_SET_TILE_ROWS",
>  #ifdef AOM_CTRL_AV1E_SET_ROW_MT
>  [AV1E_SET_ROW_MT]   = "AV1E_SET_ROW_MT",
>  #endif
> +#ifdef AV1E_SET_DENOISE_NOISE_LEVEL
> +[AV1E_SET_DENOISE_NOISE_LEVEL] =  "AV1E_SET_DENOISE_NOISE_LEVEL",
> +#endif
> +#ifdef AV1E_SET_DENOISE_BLOCK_SIZE
> +[AV1E_SET_DENOISE_BLOCK_SIZE] =   "AV1E_SET_DENOISE_BLOCK_SIZE",
> +#endif
> +#ifdef AV1E_SET_MAX_REFERENCE_FRAMES
> +[AV1E_SET_MAX_REFERENCE_FRAMES] = "AV1E_SET_MAX_REFERENCE_FRAMES",
> +#endif
> +#ifdef AV1E_SET_ENABLE_GLOBAL_MOTION
> +[AV1E_SET_ENABLE_GLOBAL_MOTION] = "AV1E_SET_ENABLE_GLOBAL_MOTION",
> +#endif
> +[AV1E_SET_ENABLE_CDEF]  = "AV1E_SET_ENABLE_CDEF",
>  };
>  
>  static av_cold void log_encoder_error(AVCodecContext *avctx, const char 
> *desc)
> @@ -567,10 +596,14 @@ static av_cold int aom_init(AVCodecContext *avctx,
>  
>  // 0-100 (0 => CBR, 100 => VBR)
>  enccfg.rc_2pass_vbr_bias_pct   = round(avctx->qcompress * 100);
> -if (avctx->bit_rate)
> +if (ctx->minsection_pct >= 0)
> +enccfg.rc_2pass_vbr_minsection_pct = ctx->minsection_pct;
> +else if (avctx->bit_rate)
>  enccfg.rc_2pass_vbr_minsection_pct =
>  avctx->rc_min_rate * 100LL / avctx->bit_rate;
> -if (avctx->rc_max_rate)
> +if (ctx->maxsection_pct >= 0)
> +enccfg.rc_2pass_vbr_maxsection_pct = ctx->maxsection_pct;
> +else if (avctx->rc_max_rate)
>  enccfg.rc_2pass_vbr_maxsection_pct =
>  avctx->rc_max_rate * 100LL / avctx->bit_rate;
>  
> @@ -582,6 +615,11 @@ static av_cold int aom_init(AVCodecContext *avctx,
>  avctx->rc_initial_buffer_occupancy * 1000LL / avctx->bit_rate;
>  enccfg.rc_buf_optimal_sz = enccfg.rc_buf_sz * 5 / 6;
>  
> +if (ctx->rc_undershoot_pct >= 0)
> +enccfg.rc_undershoot_pct = ctx->rc_undershoot_pct;
> +if (ctx->rc_overshoot_pct >= 0)
> +enccfg.rc_overshoot_pct = ctx->rc_overshoot_pct;
> +
>  // _enc_init() will balk if kf_min_dist differs from max w/AOM_KF_AUTO
>  if (avctx->keyint_min >= 0 && avctx->keyint_min == avctx->gop_size)
>  enccfg.kf_min_dist = avctx->keyint_min;
> @@ -643,7 +681,12 @@ static av_cold int aom_init(AVCodecContext *avctx,
>  codecctl_int(avctx, AOME_SET_CPUUSED, ctx->cpu_used);
>  if (ctx->auto_alt_ref >= 0)
>  codecctl_int(avctx, AOME_SET_ENABLEAUTOALTREF, ctx->auto_alt_ref);
> -
> +if (ctx->arnr_max_frames 

[FFmpeg-devel] [RFC] fate-samples entry in MAINTAINERS

[Michael Niedermayer]

Hi

I think we should add a entry for fate-samples to MAINTAINERS
so that people who need a file uploaded know with whom to talk
if the file is not being uploaded from a post to the ML

if people agree. Who can i add to that entry ? (i dont want to
just dump everyone who has access to it as people did not explicitly
volunteer to handle it)

thx

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I do not agree with what you have to say, but I'll defend to the death your
right to say it. -- Voltaire


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

Re: [FFmpeg-devel] [PATCH] libavcodec/zmbvenc: Add support for RGB formats

[Tomas Härdin]

tis 2019-03-12 klockan 10:27 + skrev Matthew Fearnley:
> > On 11 Mar 2019, at 10:37, Tomas Härdin  wrote:
> > 
> > 
> > There's some justification for adding sub-8bpp, like BMP. bmp.c
> > converts all of them except GRAY8 to PAL8. Bitdepths besides 1, 4
> > and 8
> > don't work at all.
> > 
> > One way to at least allow both the bmp and zmbv encoders to do sub-
> > 8bpp 
> > from PAL8 would be to keep track of the maximum number of colors in
> > some appropriate struct.
> > 
> > Adding proper sub-8bpp support would involve a lot of libsws
> > headache I
> > suspect.
> 
> It occurs to me that adding sub-8bpp has some implications:
> 
> My current understanding (I could be wrong) is that FFmpeg tends to
> detect the pix_fmt based on the first frame. If FFmpeg detects the
> first frame as e.g. PAL4, and chooses that as its output, that means
> the rest of the video will have to be encodable as PAL4, otherwise it
> (obviously) won’t be encoded properly.
> 
> So adding a PAL4 format puts a new constraint on encoders (inside and
> outside FFmpeg) to not encode frames in a way that looks like PAL4,
> unless the whole video will be encodable that way.

Yes, FFmpeg will probe the initial format of the video and audio.
Nothing says these are constant. There are FATE samples specifically
for files that change resolution. Since ZMBV is a DOS capture codec,
and DOS programs frequently change resolution and colordepth, this is
indeed something we have to think about. Example: DOS boots into mode
3h, 80x25 16-color text. A recording may start in this mode, then
switch to mode 13h (320x200 256 colors graphics), if I understand the
format correctly.

> If FFmpeg supports PAL8 only, then it can be tempting to optimise
> videos to encode as sub-8bpp whenever possible, knowing they will
> always (in FFmpeg at least) decode to PAL8. But this could break
> format detection for tools outside FFmpeg, if they choose to add sub-
> 8bpp support.
> 
> The safest thing FFmpeg can do is to always decode sub-8bpp to PAL8,
> and to emit PAL8 frames as exactly 8bpp (where applicable). It could
> still offer encoding formats for PAL1/2/4, but these formats could
> only be detected by scanning the whole video.
> 
> The suggestion of bits_per_raw_sample sounds interesting. What would
> that look like in practice?

The decoder would set bits_per_raw_sample on every keyframe. It and
resolution might change during the course of a ZMBV file, as explained
above.

I think frivolously changing the bitdepth is a bad idea, at least
inside the encoder. If the encoder is fed with changing bitdepths then
it's a different story.

> > Just a small thing to be clear: ZMBV_ENABLE_24BPP is not defined
> > anywhere, so we're free to do however we want with it. It's not
> > going
> > to break anyone's workflow unless they were foolish enough to
> > encode
> > 24-bit ZMBVs outside of the non-existing spec.
> 
> True. But they might be understandably puzzled if they encode as 24-
> bit, and then find the channels swapped when they decode.

Yeah, that's why we'd need to get this nailed down

> Thanks for writing the email to the DOSBox crew.
> If they choose a channel order, then we have good grounds for fixing
> the encoder (if need be), and implementing the decoder in the same
> way.
> It occurs to me that they might (in theory) also want to specify 2/4
> byte alignment on RGB, like with the MVs.  My gut says there’d be
> very little benefit though, and it would only be seen with strange
> video / block widths.
> 
> It also occurs to me this will may warrant a version bump in the
> format, to give an easy error case for decoders that don’t expect it.
> Particularly if our decoder has to redefine its channel order.

Are there any decoders besides ours and dosbox's?

It also turns out the creator of this codec is Harekiet, who hangs out
in #revision on IRCnet

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

Re: [FFmpeg-devel] [PATCH] configure: enable ffnvcodec, nvenc, nvdec for ppc64

[Timo Rothenpieler]

On 12/03/2019 09:52, Ruta Gadkari wrote:


On 11/03/2019 15:51, Carl Eugen Hoyos wrote:

2019-03-11 11:16 GMT+01:00, Timo Rothenpieler :

On 11/03/2019 09:40, Carl Eugen Hoyos wrote:

2019-03-11 6:36 GMT+01:00, Ruta Gadkari :


Please find attached the patch, it enables ffnvcodec and nvenc,
nvdec, cuvid for PPC64 architecture.


Is it supported on both little and big endian?


Good question.



Assuming it is only supported on little-endian ppc64, the check should be fixed 
accordingly.


The support is present only on little endian.
Fixed the check, please find attached the updated patch.


Looks good to me now.



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

Re: [FFmpeg-devel] [PATCH] libavcodec/zmbvenc: Add support for RGB formats

[Matthew Fearnley]

> On 11 Mar 2019, at 10:37, Tomas Härdin  wrote:
> 
> fre 2019-03-08 klockan 21:39 + skrev Matthew Fearnley:
 On Fri, 8 Mar 2019 at 18:07, Carl Eugen Hoyos  wrote:
 2019-03-08 15:04 GMT+01:00, Tomas Härdin :
 
 Maybe we could coordinate 1/2/4/24-bit support with the
>>> 
>>> I believe FFmpeg cannot support 1/2/4 bit for encoding.
>>> 
>> 
>> As far as I can see, FFmpeg has very limited support for bit depths less
>> than 8.  I think there are basically two formats (plus variants), with
>> fixed "palettes":
>> 1bpp: MONO_BLACK / MONO_WHITE (black/white only)
>> 4bpp: RGB4[_BYTE], BGR4[_BYTE] (RGB 1:2:1, 16 colours - I presume red/blue
>> would be 0,255; green would be 0,85,170,255)
>> 
>> If the ZMBV formats were defined, these might be worth encoder adding
>> support for.
>> (Practically speaking though, it would be a slight pain, because the
>> encoder would do the work in 8bpp and pack/unpack as needed.)
>> But with PAL8 being the only format allowing a free palette, all sub-8bpp
>> formats would have to decode to that, so they wouldn't round-trip.
> 
> There's some justification for adding sub-8bpp, like BMP. bmp.c
> converts all of them except GRAY8 to PAL8. Bitdepths besides 1, 4 and 8
> don't work at all.
> 
> One way to at least allow both the bmp and zmbv encoders to do sub-8bpp 
> from PAL8 would be to keep track of the maximum number of colors in
> some appropriate struct.
> 
> Adding proper sub-8bpp support would involve a lot of libsws headache I
> suspect.
It occurs to me that adding sub-8bpp has some implications:

My current understanding (I could be wrong) is that FFmpeg tends to detect the 
pix_fmt based on the first frame. If FFmpeg detects the first frame as e.g. 
PAL4, and chooses that as its output, that means the rest of the video will 
have to be encodable as PAL4, otherwise it (obviously) won’t be encoded 
properly.

So adding a PAL4 format puts a new constraint on encoders (inside and outside 
FFmpeg) to not encode frames in a way that looks like PAL4, unless the whole 
video will be encodable that way.

If FFmpeg supports PAL8 only, then it can be tempting to optimise videos to 
encode as sub-8bpp whenever possible, knowing they will always (in FFmpeg at 
least) decode to PAL8. But this could break format detection for tools outside 
FFmpeg, if they choose to add sub-8bpp support.

The safest thing FFmpeg can do is to always decode sub-8bpp to PAL8, and to 
emit PAL8 frames as exactly 8bpp (where applicable). It could still offer 
encoding formats for PAL1/2/4, but these formats could only be detected by 
scanning the whole video.

The suggestion of bits_per_raw_sample sounds interesting. What would that look 
like in practice?

>> (It should be possible to implement decoding to pal8 if
>>> that doesn't work yet and if samples exist.)
>>> 
>> 
>> No samples or specifications exist that I know of, so I don't plan to
>> submit any patches to the decoder unless/until there is something to work
>> with there.
>> 
>>> dosbox devs? And maybe we should do something about
 the RGB24 thing in the decoder..
>> 
>> Yeah, I think talking with the DOSBox devs sounds like a potentially good
>> idea.
>> 
>>> 
>>> Do I understand correctly that no existing implementation
>>> supports 24bit rgb? If that is correct, I believe FFmpeg
>>> shouldn't add it (but this may only be me).
>>> 
>> 
>> I agree that FFmpeg shouldn't add support for any formats that haven't been
>> defined.
>> As far as I know, the specifics of the 24-bit RGB format havn't been
>> discussed anywhere, and there are no samples I know of.
>> 
>> A likely specification of 24-bit is trivial enough to add support for, that
>> I was originally planning to add it with an #ifdef (like in the decoder).
>> But it wouldn't do to have contradictory channel orders proposed in the
>> decoder and encoder, so I will leave that for now unless DOSBox will commit
>> to one.
>> 
>> I presume that FFmpeg generally doesn't like to set standards in media
>> formats, only to implement existing ones.
>> My personal feelings in this case would be to provide support that's
>> disabled at compile-time if an official specification can be agreed, and to
>> have support included by default if an independent implementation - or at
>> least independent samples - are available that agree with the specification.
> 
> Just a small thing to be clear: ZMBV_ENABLE_24BPP is not defined
> anywhere, so we're free to do however we want with it. It's not going
> to break anyone's workflow unless they were foolish enough to encode
> 24-bit ZMBVs outside of the non-existing spec.
True. But they might be understandably puzzled if they encode as 24-bit, and 
then find the channels swapped when they decode.

Thanks for writing the email to the DOSBox crew.
If they choose a channel order, then we have good grounds for fixing the 
encoder (if need be), and implementing the decoder in the same way.
It occurs to me that they might (in theory) also want to 

Re: [FFmpeg-devel] [PATCH] configure: enable ffnvcodec, nvenc, nvdec for ppc64

[Ruta Gadkari]

On 11/03/2019 15:51, Carl Eugen Hoyos wrote:
> 2019-03-11 11:16 GMT+01:00, Timo Rothenpieler :
>> On 11/03/2019 09:40, Carl Eugen Hoyos wrote:
>>> 2019-03-11 6:36 GMT+01:00, Ruta Gadkari :
>>>
 Please find attached the patch, it enables ffnvcodec and nvenc, 
 nvdec, cuvid for PPC64 architecture.
>>>
>>> Is it supported on both little and big endian?
>>
>> Good question.

> Assuming it is only supported on little-endian ppc64, the check should be 
> fixed accordingly.

The support is present only on little endian.
Fixed the check, please find attached the updated patch.

 This email message is for the sole use of the intended
 recipient(s) and may contain confidential information.
>>>
>>> Please remove this nonsense from future emails.
>>
>> While I agree that it's pointless on mails to public lists, I don't 
>> think there is anything they can do against their mail server adding it.
>> And I prefer having that at the end of every mail over not being able 
>> to identify patches from @nvidia.com as such as easily.

> They could add "nvidia" to their gmail / hotmail / ... account names.


Thanks
Ruta 


0001-configure-enable-ffnvcodec-nvenc-nvdec-for-ppc6le.patch
Description: 0001-configure-enable-ffnvcodec-nvenc-nvdec-for-ppc6le.patch
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH]lavf/matroska: Also support WebVTT in Matroska

[Andreas Rheinhardt]

Carl Eugen Hoyos:
> Hi!
> 
> Attached patch is supposed to fix HandBrake issue 1964, FFmpeg
> currently fails to identify WebVTT in Matroska (only webm is supported
> as container). I don't have the original sample to test though.
> 
> Please comment, Carl Eugen
The way WebVTT is stored in Matroska is different from the way it is
stored in WebM: The Matroska way uses an optional BlockAdditional to
store the information that would be contained at the beginning of the
subtitle payload (the actual data of the Block inside the BlockGroup)
in WebM, namely the stuff that ends up as
AV_PKT_DATA_WEBVTT_IDENTIFIER and AV_PKT_DATA_WEBVTT_SETTINGS side
data. It also contains another form of information, namely WebVTT
Comment Blocks (for which no side data has been defined yet). Your
patch would export this data as part of a general
AV_PKT_DATA_MATROSKA_BLOCKADDITIONAL and not as its components. This
is suboptimal.

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