Re: [libav-devel] [PATCH] avutil: make AV_NOPTS_VALUE signed negative

[Vittorio Giovara]

On Sun, Nov 5, 2017 at 6:21 AM, Rémi Denis-Courmont  wrote:

> libav generally uses int64_t to represent timestamps, and thus
> AV_NOPTS_VALUE has to fit witin the range of int64_t.
>
> The current definition of AV_NOPTS_VALUE results in AV_NOPTS_VALUE
> having the same type as uint64_t, since its value is positive and
> cannot be represented by int64_t.
>
> See also ISO C11 §6.4.4.1 clause 5.
>
> This patch ensures that AV_NOPTS_VALUE is an int64_t, avoiding
> undefined overflowing conversion from uint64_t to int64_t.
> ---
>  libavutil/avutil.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavutil/avutil.h b/libavutil/avutil.h
> index 2339fe3c9c..6f90043952 100644
> --- a/libavutil/avutil.h
> +++ b/libavutil/avutil.h
> @@ -238,7 +238,7 @@ enum AVMediaType {
>   * either pts or dts.
>   */
>
> -#define AV_NOPTS_VALUE  INT64_C(0x8000)
> +#define AV_NOPTS_VALUE  INT64_MIN
>
>  /**
>   * Internal time base represented as integer
> --
>

is a version bump necessary for this change?
-- 
Vittorio
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

[libav-devel] [PATCH 2&3/10] lavc: Add hardware config metadata for decoders supporting hardware output

[Mark Thompson]

This includes a pointer to the associated hwaccel for decoders using
hwaccels - these will be used later to implement the hwaccel setup
without needing a global list.

Also added is a new file listing all hwaccels as external declarations -
this will be used later to generate the hwaccel list at configure time.
---
On 05/11/17 19:27, Diego Biurrun wrote:
> On Sun, Nov 05, 2017 at 12:00:01AM +, Mark Thompson wrote:
>> These will be used later to generate the hwaccel list at configure time,
>> but are needed now for the config structures in the following patch.
>> ---
>>  libavcodec/hwaccels.h | 59 
>> +++
>>  1 file changed, 59 insertions(+)
>>  create mode 100644 libavcodec/hwaccels.h
> 
> I really don't understand why this is not part of the next patch. IMO
> this extra commit just creates churn in the Git history and should be
> avoided.

Ok, squashed.


 libavcodec/h263dec.c   | 10 
 libavcodec/h264dec.c   | 28 ++
 libavcodec/hevcdec.c   | 22 +
 libavcodec/hwaccel.h   | 48 +
 libavcodec/hwaccels.h  | 59 ++
 libavcodec/mmaldec.c   |  7 ++
 libavcodec/mpeg12dec.c | 27 -
 libavcodec/mpeg4videodec.c | 10 
 libavcodec/qsvdec.c| 13 ++
 libavcodec/qsvdec.h|  3 +++
 libavcodec/qsvdec_h2645.c  |  2 ++
 libavcodec/qsvdec_other.c  |  3 +++
 libavcodec/vc1dec.c| 37 +
 libavcodec/vp8.c   |  7 ++
 14 files changed, 275 insertions(+), 1 deletion(-)
 create mode 100644 libavcodec/hwaccels.h

diff --git a/libavcodec/h263dec.c b/libavcodec/h263dec.c
index 921ff5fb9..b883c 100644
--- a/libavcodec/h263dec.c
+++ b/libavcodec/h263dec.c
@@ -31,6 +31,7 @@
 #include "flv.h"
 #include "h263.h"
 #include "h263_parser.h"
+#include "hwaccel.h"
 #include "internal.h"
 #include "mpeg_er.h"
 #include "mpeg4video.h"
@@ -677,4 +678,13 @@ AVCodec ff_h263_decoder = {
   AV_CODEC_CAP_TRUNCATED | AV_CODEC_CAP_DELAY,
 .flush  = ff_mpeg_flush,
 .pix_fmts   = ff_h263_hwaccel_pixfmt_list_420,
+.hw_configs = (const AVCodecHWConfigInternal*[]) {
+#if CONFIG_H263_VAAPI_HWACCEL
+HWACCEL_VAAPI(h263),
+#endif
+#if CONFIG_MPEG4_VDPAU_HWACCEL
+HWACCEL_VDPAU(mpeg4),
+#endif
+NULL
+},
 };
diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c
index 7a8293efa..440644f61 100644
--- a/libavcodec/h264dec.c
+++ b/libavcodec/h264dec.c
@@ -44,6 +44,7 @@
 #include "h264chroma.h"
 #include "h264_mvpred.h"
 #include "h264_ps.h"
+#include "hwaccel.h"
 #include "mathops.h"
 #include "me_cmp.h"
 #include "mpegutils.h"
@@ -786,6 +787,33 @@ AVCodec ff_h264_decoder = {
 .capabilities  = /*AV_CODEC_CAP_DRAW_HORIZ_BAND |*/ 
AV_CODEC_CAP_DR1 |
  AV_CODEC_CAP_DELAY | AV_CODEC_CAP_SLICE_THREADS |
  AV_CODEC_CAP_FRAME_THREADS,
+.hw_configs= (const AVCodecHWConfigInternal*[]) {
+#if CONFIG_H264_CUVID_HWACCEL
+   HWACCEL_CUVID(h264),
+#endif
+#if CONFIG_H264_DXVA2_HWACCEL
+   HWACCEL_DXVA2(h264),
+#endif
+#if CONFIG_H264_D3D11VA_HWACCEL
+   HWACCEL_D3D11VA(h264),
+#endif
+#if CONFIG_H264_D3D11VA2_HWACCEL
+   HWACCEL_D3D11VA2(h264),
+#endif
+#if CONFIG_H264_VAAPI_HWACCEL
+   HWACCEL_VAAPI(h264),
+#endif
+#if CONFIG_H264_VDPAU_HWACCEL
+   HWACCEL_VDPAU(h264),
+#endif
+#if CONFIG_H264_VDA_HWACCEL
+   HW_CONFIG_AD_HOC_HWACCEL(VDA, 
ff_h264_vda_hwaccel)
+#endif
+#if CONFIG_H264_VDA_OLD_HWACCEL
+   HW_CONFIG_AD_HOC_HWACCEL(VDA_VLD, 
ff_h264_vda_old_hwaccel)
+#endif
+   NULL
+   },
 .caps_internal = FF_CODEC_CAP_INIT_THREADSAFE | 
FF_CODEC_CAP_EXPORTS_CROPPING,
 .flush = flush_dpb,
 .init_thread_copy  = ONLY_IF_THREADS_ENABLED(decode_init_thread_copy),
diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c
index a1619cf4b..145c14289 100644
--- a/libavcodec/hevcdec.c
+++ b/libavcodec/hevcdec.c
@@ -39,6 +39,7 @@
 #include "hevc.h"
 #include "hevc_data.h"
 #include "hevcdec.h"
+#include "hwaccel.h"
 #include "profiles.h"
 
 const uint8_t ff_hevc_qpel_extra_before[4] = { 0, 3, 3, 3 };
@@ -3113,4 +3114,25 @@ AVCodec ff_hevc_decoder = {
  AV_CODEC_CAP_FRAME_THREADS,
 .profiles  = NULL_IF_CONFIG_SMALL(ff_hevc_profiles),
 .caps_internal = FF_CODEC_CAP_EXPORTS_CROPPING | 
FF_CODEC_CAP_INIT_THREADSAFE,
+.hw_configs= (const AVCodecHWConfigInternal*[]) {
+#if CONFIG_HEVC_CUVID_HWACCEL

Re: [libav-devel] [PATCH 01/10] lavc: Add codec metadata to indicate hardware support

[Mark Thompson]

On 05/11/17 19:36, Diego Biurrun wrote:
> This is a general remark rather than a review comment, I just happened to 
> notice
> something in this patch that I have been wondering about for a long time.
> 
> On Sun, Nov 05, 2017 at 12:00:00AM +, Mark Thompson wrote:
>> --- a/libavcodec/avcodec.h
>> +++ b/libavcodec/avcodec.h
>> @@ -2735,6 +2736,61 @@ typedef struct AVProfile {
>>  
>> +enum {
>> +AV_CODEC_HW_CONFIG_METHOD_HW_DEVICE_CTX = 0x01,
>> +AV_CODEC_HW_CONFIG_METHOD_HW_FRAMES_CTX = 0x02,
>> +AV_CODEC_HW_CONFIG_METHOD_INTERNAL  = 0x04,
>> +AV_CODEC_HW_CONFIG_METHOD_AD_HOC= 0x08,
>> +};
> 
> Why create an anonymous, instead of a named, enum here (or really anywhere)?
> If it is named, it could be passed as a function parameter and similar
> things...

It contains flags - combinations of values are not members of the enum type.

(While the rules do imply that they are representable because it must be 
compatible with some implementation-chosen integer type, I would prefer that 
variables of enum type really do only contain members of the enumeration.)

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

Re: [libav-devel] [PATCH 01/10] lavc: Add codec metadata to indicate hardware support

[Diego Biurrun]

This is a general remark rather than a review comment, I just happened to notice
something in this patch that I have been wondering about for a long time.

On Sun, Nov 05, 2017 at 12:00:00AM +, Mark Thompson wrote:
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -2735,6 +2736,61 @@ typedef struct AVProfile {
>  
> +enum {
> +AV_CODEC_HW_CONFIG_METHOD_HW_DEVICE_CTX = 0x01,
> +AV_CODEC_HW_CONFIG_METHOD_HW_FRAMES_CTX = 0x02,
> +AV_CODEC_HW_CONFIG_METHOD_INTERNAL  = 0x04,
> +AV_CODEC_HW_CONFIG_METHOD_AD_HOC= 0x08,
> +};

Why create an anonymous, instead of a named, enum here (or really anywhere)?
If it is named, it could be passed as a function parameter and similar
things...

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

Re: [libav-devel] [PATCH 02/10] lavc: Add a new file listing all hwaccels as external declarations

[Diego Biurrun]

On Sun, Nov 05, 2017 at 12:00:01AM +, Mark Thompson wrote:
> These will be used later to generate the hwaccel list at configure time,
> but are needed now for the config structures in the following patch.
> ---
>  libavcodec/hwaccels.h | 59 
> +++
>  1 file changed, 59 insertions(+)
>  create mode 100644 libavcodec/hwaccels.h

I really don't understand why this is not part of the next patch. IMO
this extra commit just creates churn in the Git history and should be
avoided.

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

Re: [libav-devel] [PATCH] vp9_superframe_bsf: cache packets by creating new references instead of moving pointers

[Luca Barbato]

On 05/11/2017 17:44, James Almer wrote:

Fixes invalid reads after free.

Signed-off-by: James Almer 
---
  libavcodec/vp9_superframe_bsf.c | 25 +
  1 file changed, 21 insertions(+), 4 deletions(-)



Sounds good

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

[libav-devel] [PATCH] vp9_superframe_bsf: cache packets by creating new references instead of moving pointers

[James Almer]

Fixes invalid reads after free.

Signed-off-by: James Almer 
---
 libavcodec/vp9_superframe_bsf.c | 25 +
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/libavcodec/vp9_superframe_bsf.c b/libavcodec/vp9_superframe_bsf.c
index 3669216009..ad66cb599b 100644
--- a/libavcodec/vp9_superframe_bsf.c
+++ b/libavcodec/vp9_superframe_bsf.c
@@ -148,8 +148,9 @@ static int vp9_superframe_filter(AVBSFContext *ctx, 
AVPacket *out)
 goto done;
 }
 
-s->cache[s->n_cache++] = in;
-in = NULL;
+res = av_packet_ref(s->cache[s->n_cache++], in);
+if (res < 0)
+goto done;
 if (invisible) {
 res = AVERROR(EAGAIN);
 goto done;
@@ -165,7 +166,7 @@ static int vp9_superframe_filter(AVBSFContext *ctx, 
AVPacket *out)
 goto done;
 
 for (n = 0; n < s->n_cache; n++)
-av_packet_free(>cache[n]);
+av_packet_unref(s->cache[n]);
 s->n_cache = 0;
 
 done:
@@ -175,13 +176,28 @@ done:
 return res;
 }
 
+static int vp9_superframe_init(AVBSFContext *ctx)
+{
+VP9BSFContext *s = ctx->priv_data;
+int n;
+
+// alloc cache packets
+for (n = 0; n < MAX_CACHE; n++) {
+s->cache[n] = av_packet_alloc();
+if (!s->cache[n])
+return AVERROR(ENOMEM);
+}
+
+return 0;
+}
+
 static void vp9_superframe_close(AVBSFContext *ctx)
 {
 VP9BSFContext *s = ctx->priv_data;
 int n;
 
 // free cached data
-for (n = 0; n < s->n_cache; n++)
+for (n = 0; n < MAX_CACHE; n++)
 av_packet_free(>cache[n]);
 }
 
@@ -193,6 +209,7 @@ const AVBitStreamFilter ff_vp9_superframe_bsf = {
 .name   = "vp9_superframe",
 .priv_data_size = sizeof(VP9BSFContext),
 .filter = vp9_superframe_filter,
+.init   = vp9_superframe_init,
 .close  = vp9_superframe_close,
 .codec_ids  = codec_ids,
 };
-- 
2.14.2

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

Re: [libav-devel] [PATCH] avutil: make AV_NOPTS_VALUE signed negative

[Mark Thompson]

On 05/11/17 11:21, Rémi Denis-Courmont wrote:
> libav generally uses int64_t to represent timestamps, and thus
> AV_NOPTS_VALUE has to fit witin the range of int64_t.
> 
> The current definition of AV_NOPTS_VALUE results in AV_NOPTS_VALUE
> having the same type as uint64_t, since its value is positive and
> cannot be represented by int64_t.
> 
> See also ISO C11 §6.4.4.1 clause 5.
> 
> This patch ensures that AV_NOPTS_VALUE is an int64_t, avoiding
> undefined overflowing conversion from uint64_t to int64_t.


The overflowing conversion is implementation-defined, not undefined - see 
§6.3.1.3 clause 3.


> ---
>  libavutil/avutil.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavutil/avutil.h b/libavutil/avutil.h
> index 2339fe3c9c..6f90043952 100644
> --- a/libavutil/avutil.h
> +++ b/libavutil/avutil.h
> @@ -238,7 +238,7 @@ enum AVMediaType {
>   * either pts or dts.
>   */
>  
> -#define AV_NOPTS_VALUE  INT64_C(0x8000)
> +#define AV_NOPTS_VALUE  INT64_MIN
>  
>  /**
>   * Internal time base represented as integer
> 

LGTM.

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

Re: [libav-devel] [PATCH] configure: Use right variable and right value for AIX ar flags

[Luca Barbato]

On 04/11/2017 20:13, Diego Biurrun wrote:

---

.. grmbl ..

  configure | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure b/configure
index 50e7591b7c..ee13b4db81 100755
--- a/configure
+++ b/configure
@@ -3895,7 +3895,7 @@ case $target_os in
  SHFLAGS=-shared
  add_cppflags '-I\$(SRC_PATH)/compat/aix'
  enabled shared && add_ldflags -Wl,-brtl
-ar_default='ar -Xany'
+arflags='-Xany -r -c'
  ;;
  android)
  disable symver



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