Re: [libav-devel] [PATCH] avpacket: Initialize the allocated padding area in side data

2018-02-01 Thread Luca Barbato 

On 01/02/2018 14:02, Martin Storsjö wrote:

This makes sure that consumers of the side data actually can
rely on the padding as intended, without having the callers of
av_packet_new_side_data to explicitly zero initialize it.
---
  libavcodec/avpacket.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
index 93e9eb6ae7..c705df3d59 100644
--- a/libavcodec/avpacket.c
+++ b/libavcodec/avpacket.c
@@ -271,6 +271,7 @@ uint8_t *av_packet_new_side_data(AVPacket *pkt, enum 
AVPacketSideDataType type,
  data = av_malloc(size + AV_INPUT_BUFFER_PADDING_SIZE);
  if (!data)
  return NULL;
+memset(data + size, 0, AV_INPUT_BUFFER_PADDING_SIZE);
  
  ret = av_packet_add_side_data(pkt, type, data, size);

  if (ret < 0) {



Fine for me.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] avpacket: Initialize the allocated padding area in side data

2018-02-01 Thread Martin Storsjö 

On Thu, 1 Feb 2018, wm4 wrote:


On Thu,  1 Feb 2018 15:02:37 +0200
Martin Storsjö  wrote:


This makes sure that consumers of the side data actually can
rely on the padding as intended, without having the callers of
av_packet_new_side_data to explicitly zero initialize it.
---
 libavcodec/avpacket.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
index 93e9eb6ae7..c705df3d59 100644
--- a/libavcodec/avpacket.c
+++ b/libavcodec/avpacket.c
@@ -271,6 +271,7 @@ uint8_t *av_packet_new_side_data(AVPacket *pkt, enum 
AVPacketSideDataType type,
 data = av_malloc(size + AV_INPUT_BUFFER_PADDING_SIZE);
 if (!data)
 return NULL;
+memset(data + size, 0, AV_INPUT_BUFFER_PADDING_SIZE);

 ret = av_packet_add_side_data(pkt, type, data, size);
 if (ret < 0) {


+1, though I also have trouble believing that just clearing the entire
thing (including side data contents) would cause any kind of performance
trouble. (I.e. using av_mallocz.)


Yes, that'd probably also be just as nice. The upside with this approach 
is that tools more easily can point out if you as a caller forgot to 
initialize the data you actually asked to have allocated for you, but I 
don't have a very strong opinion either way.


// Martin
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] avpacket: Initialize the allocated padding area in side data

2018-02-01 Thread wm4 
On Thu,  1 Feb 2018 15:02:37 +0200
Martin Storsjö  wrote:

> This makes sure that consumers of the side data actually can
> rely on the padding as intended, without having the callers of
> av_packet_new_side_data to explicitly zero initialize it.
> ---
>  libavcodec/avpacket.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
> index 93e9eb6ae7..c705df3d59 100644
> --- a/libavcodec/avpacket.c
> +++ b/libavcodec/avpacket.c
> @@ -271,6 +271,7 @@ uint8_t *av_packet_new_side_data(AVPacket *pkt, enum 
> AVPacketSideDataType type,
>  data = av_malloc(size + AV_INPUT_BUFFER_PADDING_SIZE);
>  if (!data)
>  return NULL;
> +memset(data + size, 0, AV_INPUT_BUFFER_PADDING_SIZE);
>  
>  ret = av_packet_add_side_data(pkt, type, data, size);
>  if (ret < 0) {

+1, though I also have trouble believing that just clearing the entire
thing (including side data contents) would cause any kind of performance
trouble. (I.e. using av_mallocz.)

(Disregard my other empty reply I possibly sent, not sure.)
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

[libav-devel] [PATCH] avpacket: Initialize the allocated padding area in side data

2018-02-01 Thread Martin Storsjö 
This makes sure that consumers of the side data actually can
rely on the padding as intended, without having the callers of
av_packet_new_side_data to explicitly zero initialize it.
---
 libavcodec/avpacket.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
index 93e9eb6ae7..c705df3d59 100644
--- a/libavcodec/avpacket.c
+++ b/libavcodec/avpacket.c
@@ -271,6 +271,7 @@ uint8_t *av_packet_new_side_data(AVPacket *pkt, enum 
AVPacketSideDataType type,
 data = av_malloc(size + AV_INPUT_BUFFER_PADDING_SIZE);
 if (!data)
 return NULL;
+memset(data + size, 0, AV_INPUT_BUFFER_PADDING_SIZE);
 
 ret = av_packet_add_side_data(pkt, type, data, size);
 if (ret < 0) {
-- 
2.14.3 (Apple Git-98)

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

Re: [libav-devel] [PATCH] flvdec: Initialize the padding in new extradata side data

2018-02-01 Thread Luca Barbato 

On 01/02/2018 12:42, Martin Storsjö wrote:
The other alternative would be to make av_packet_new_side_data 
zero-initialize the side data that it allocates (or at least the 
padding?), to avoid having to keep assumptions like these over here. 
(That would also match what ffmpeg does.) Would that perhaps be an even 
nicer solution, or does someone prefer keeping the allocation 
uninitialized like now?


Probably shouldn't hurt and avoid some issues in the future.

lu
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] flvdec: Initialize the padding in new extradata side data

2018-02-01 Thread Martin Storsjö 

On Thu, 1 Feb 2018, Luca Barbato wrote:


On 01/02/2018 11:44, Martin Storsjö wrote:

We already allocate the internal extradata buffer with initialized
padding in flv_queue_extradata, but when copied into side data
(which is allocated by av_packet_new_side_data, which does allocate
the padding but doesn't initialize it), we forgot to initialize
the padding there.
---
  libavformat/flvdec.c | 4 
  1 file changed, 4 insertions(+)

diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c
index 81a71d39f4..23d320900b 100644
--- a/libavformat/flvdec.c
+++ b/libavformat/flvdec.c
@@ -989,6 +989,10 @@ skip:
  if (side) {
  memcpy(side, flv->new_extradata[is_audio],
 flv->new_extradata_size[is_audio]);
+// av_packet_new_side_data allocates space for padding but 

doesn't

+// initialize it.
+memset(side + flv->new_extradata_size[is_audio], 0,
+   AV_INPUT_BUFFER_PADDING_SIZE);
  av_freep(>new_extradata[is_audio]);
  flv->new_extradata_size[is_audio] = 0;
  }



Ok.


The other alternative would be to make av_packet_new_side_data 
zero-initialize the side data that it allocates (or at least the 
padding?), to avoid having to keep assumptions like these over here. (That 
would also match what ffmpeg does.) Would that perhaps be an even nicer 
solution, or does someone prefer keeping the allocation uninitialized like 
now?


// Martin
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] qsvenc: AVBR is not supported on non-windows OS

2018-02-01 Thread Luca Barbato 

On 01/02/2018 10:47, Maxym Dmytrychenko wrote:

it actually should fail-down to VBR but we have VBR already exposed.

LGTM , no other objection(s) ?



Seems the easiest way to go.

lu

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

Re: [libav-devel] [PATCH] flvdec: Initialize the padding in new extradata side data

2018-02-01 Thread Luca Barbato 

On 01/02/2018 11:44, Martin Storsjö wrote:

We already allocate the internal extradata buffer with initialized
padding in flv_queue_extradata, but when copied into side data
(which is allocated by av_packet_new_side_data, which does allocate
the padding but doesn't initialize it), we forgot to initialize
the padding there.
---
  libavformat/flvdec.c | 4 
  1 file changed, 4 insertions(+)

diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c
index 81a71d39f4..23d320900b 100644
--- a/libavformat/flvdec.c
+++ b/libavformat/flvdec.c
@@ -989,6 +989,10 @@ skip:
  if (side) {
  memcpy(side, flv->new_extradata[is_audio],
 flv->new_extradata_size[is_audio]);
+// av_packet_new_side_data allocates space for padding but doesn't
+// initialize it.
+memset(side + flv->new_extradata_size[is_audio], 0,
+   AV_INPUT_BUFFER_PADDING_SIZE);
  av_freep(>new_extradata[is_audio]);
  flv->new_extradata_size[is_audio] = 0;
  }



Ok.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

[libav-devel] [PATCH] flvdec: Initialize the padding in new extradata side data

2018-02-01 Thread Martin Storsjö 
We already allocate the internal extradata buffer with initialized
padding in flv_queue_extradata, but when copied into side data
(which is allocated by av_packet_new_side_data, which does allocate
the padding but doesn't initialize it), we forgot to initialize
the padding there.
---
 libavformat/flvdec.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c
index 81a71d39f4..23d320900b 100644
--- a/libavformat/flvdec.c
+++ b/libavformat/flvdec.c
@@ -989,6 +989,10 @@ skip:
 if (side) {
 memcpy(side, flv->new_extradata[is_audio],
flv->new_extradata_size[is_audio]);
+// av_packet_new_side_data allocates space for padding but doesn't
+// initialize it.
+memset(side + flv->new_extradata_size[is_audio], 0,
+   AV_INPUT_BUFFER_PADDING_SIZE);
 av_freep(>new_extradata[is_audio]);
 flv->new_extradata_size[is_audio] = 0;
 }
-- 
2.14.3 (Apple Git-98)

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

Re: [libav-devel] [PATCH] qsvenc: AVBR is not supported on non-windows OS

2018-02-01 Thread Maxym Dmytrychenko 
it actually should fail-down to VBR but we have VBR already exposed.

LGTM , no other objection(s) ?

On Tue, Jan 30, 2018 at 11:07 AM, Zhong Li  wrote:

> AVBR is supported from API 1.3 but only available for Windows
>
> Signed-off-by: Zhong Li 
> ---
>  libavcodec/qsvenc.c | 17 ++---
>  libavcodec/qsvenc.h |  2 ++
>  2 files changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c
> index 24d9ec4..16d942f 100644
> --- a/libavcodec/qsvenc.c
> +++ b/libavcodec/qsvenc.c
> @@ -84,7 +84,9 @@ static const struct {
>  { MFX_RATECONTROL_CBR, "CBR" },
>  { MFX_RATECONTROL_VBR, "VBR" },
>  { MFX_RATECONTROL_CQP, "CQP" },
> +#if QSV_HAVE_AVBR
>  { MFX_RATECONTROL_AVBR,"AVBR" },
> +#endif
>  #if QSV_HAVE_LA
>  { MFX_RATECONTROL_LA,  "LA" },
>  #endif
> @@ -163,11 +165,14 @@ static void dump_video_param(AVCodecContext *avctx,
> QSVEncContext *q,
>  } else if (info->RateControlMethod == MFX_RATECONTROL_CQP) {
>  av_log(avctx, AV_LOG_VERBOSE, "QPI: %"PRIu16"; QPP: %"PRIu16";
> QPB: %"PRIu16"\n",
> info->QPI, info->QPP, info->QPB);
> -} else if (info->RateControlMethod == MFX_RATECONTROL_AVBR) {
> +}
> +#if QSV_HAVE_AVBR
> +else if (info->RateControlMethod == MFX_RATECONTROL_AVBR) {
>  av_log(avctx, AV_LOG_VERBOSE,
> "TargetKbps: %"PRIu16"; Accuracy: %"PRIu16"; Convergence:
> %"PRIu16"\n",
> info->TargetKbps, info->Accuracy, info->Convergence);
>  }
> +#endif
>  #if QSV_HAVE_LA
>  else if (info->RateControlMethod == MFX_RATECONTROL_LA
>  #if QSV_HAVE_LA_HRD
> @@ -333,10 +338,14 @@ static int select_rc_mode(AVCodecContext *avctx,
> QSVEncContext *q)
>  else if (avctx->rc_max_rate == avctx->bit_rate) {
>  rc_mode = MFX_RATECONTROL_CBR;
>  rc_desc = "constant bitrate (CBR)";
> -} else if (!avctx->rc_max_rate) {
> +}
> +#if QSV_HAVE_AVBR
> +else if (!avctx->rc_max_rate) {
>  rc_mode = MFX_RATECONTROL_AVBR;
>  rc_desc = "average variable bitrate (AVBR)";
> -} else {
> +}
> +#endif
> +else {
>  rc_mode = MFX_RATECONTROL_VBR;
>  rc_desc = "variable bitrate (VBR)";
>  }
> @@ -522,11 +531,13 @@ static int init_video_param(AVCodecContext *avctx,
> QSVEncContext *q)
>  q->param.mfx.QPB = av_clip(quant * fabs(avctx->b_quant_factor) +
> avctx->b_quant_offset, 0, 51);
>
>  break;
> +#if QSV_HAVE_AVBR
>  case MFX_RATECONTROL_AVBR:
>  q->param.mfx.TargetKbps  = avctx->bit_rate / 1000;
>  q->param.mfx.Convergence = q->avbr_convergence;
>  q->param.mfx.Accuracy= q->avbr_accuracy;
>  break;
> +#endif
>  #if QSV_HAVE_LA
>  case MFX_RATECONTROL_LA:
>  q->param.mfx.TargetKbps  = avctx->bit_rate / 1000;
> diff --git a/libavcodec/qsvenc.h b/libavcodec/qsvenc.h
> index 088a61d..725651e 100644
> --- a/libavcodec/qsvenc.h
> +++ b/libavcodec/qsvenc.h
> @@ -46,10 +46,12 @@
>  #define QSV_HAVE_LA_HRD QSV_VERSION_ATLEAST(1, 11)
>
>  #if defined(_WIN32)
> +#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)
>  #define QSV_HAVE_QVBR   QSV_VERSION_ATLEAST(1, 11)
>  #else
> +#define QSV_HAVE_AVBR   0
>  #define QSV_HAVE_ICQ0
>  #define QSV_HAVE_VCM0
>  #define QSV_HAVE_QVBR   0
> --
> 1.8.3.1
>
> ___
> libav-devel mailing list
> libav-devel@libav.org
> https://lists.libav.org/mailman/listinfo/libav-devel
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel