Re: [libav-devel] [PATCH 4/7] x264: Use the framerate information instead of the timebase

[Luca Barbato]

On 17/02/2018 04:53, Vittorio Giovara wrote:

On Fri, Feb 16, 2018 at 12:02 PM, Luca Barbato  wrote:


Unbreaks the rate-control behaviour.



Same comment as for the x265 patch.
This seems to properly use x264 API.



Locally amended with:

The API expects an average framerate and a timebase, the
timebase is often 1/1000 while the reported average framerate
is 30. Use the correct information to avoid surprising results.

lu



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

Re: [libav-devel] [PATCH 3/7] x265: Use the framerate information instead of the timebase

[Luca Barbato]

On 16/02/2018 18:28, Diego Biurrun wrote:

On Fri, Feb 16, 2018 at 06:02:05PM +0100, Luca Barbato wrote:

Unbreaks the rate-control behaviour.


How does this unbreak what?


Locally amended adding:

The API expects an average framerate, the timebase is often
1/1000 while the reported average framerate is 30.

Before this patch the resulting bitrate would be surprising if
the timebase provided isn't the inverse of the average framerate.

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

Re: [libav-devel] [PATCH 10/16] cbs: Refcount all the things!

[Luca Barbato]

On 12/02/2018 02:08, Mark Thompson wrote:

On 12/02/18 00:31, James Almer wrote:

On 2/11/2018 3:14 PM, Mark Thompson wrote:

+int ff_cbs_alloc_unit_content(CodedBitstreamContext *ctx,
+  CodedBitstreamUnit *unit,
+  size_t size,
+  void (*free)(void *opaque, uint8_t *data))
+{
+av_assert0(!unit->content && !unit->content_ref);
+
+unit->content = av_mallocz(size);
+if (!unit->content)
+return AVERROR(ENOMEM);
+
+unit->content_ref = av_buffer_create(unit->content, size,
+ free ? free
+  : _buffer_default_free,


av_buffer_create() defaults to av_buffer_default_free() on its own if
free is NULL, so no need to do it here.


Aha, so it does!  Fixed in all three places I used it explicitly.



Feel free to push it as well.

lu

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

Re: [libav-devel] [PATCH 04/16] vaapi_encode: Allocate slice structures and parameter buffers dynamically

[Luca Barbato]

On 14/02/2018 23:45, Mark Thompson wrote:

On 11/02/18 18:14, Mark Thompson wrote:

From: Jun Zhao 

This removes the arbitrary limit on the allowed number of slices and
parameter buffers.

 From ffmpeg commit e4a6eb70f471eda36592078e8fa1bad87fc9df73.

Signed-off-by: Mark Thompson 
---
  libavcodec/vaapi_encode.c | 42 ++
  libavcodec/vaapi_encode.h |  6 ++
  2 files changed, 32 insertions(+), 16 deletions(-)

diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
index 47795ba73..8cba847f7 100644
--- a/libavcodec/vaapi_encode.c
+++ b/libavcodec/vaapi_encode.c
...
@@ -313,15 +321,14 @@ static int vaapi_encode_issue(AVCodecContext *avctx,
  }
  }
  
-av_assert0(pic->nb_slices <= MAX_PICTURE_SLICES);

+pic->slices = av_mallocz_array(pic->nb_slices, sizeof(*pic->slices));
+if (!pic->slices) {
+err = AVERROR(ENOMEM);
+goto fail;
+}


This fails for non-slice codecs (VP8, VP9) because av_mallocz_array() returns NULL when 
asked for zero array elements.  (ffmpeg has a hack marked "OS X on SDK 10.6 has a 
broken posix_memalign implementation" which makes this case call malloc(1) and 
return a real pointer instead.)

Changed locally to only do the allocation if nb_slices > 0.


Ok.

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

Re: [libav-devel] [PATCH 01/16] cbs: Allocate the context inside the init function

[Luca Barbato]

On 11/02/2018 19:14, Mark Thompson wrote:

... instead of making the caller allocate it themselves.  This is
more consistent with other APIs in libav.
---
This one and the following two have been hanging around for a while (they came 
from the merge into the other tine).


  libavcodec/cbs.c| 20 +---
  libavcodec/cbs.h|  6 +++---
  libavcodec/h264_metadata_bsf.c  | 20 ++--
  libavcodec/h264_redundant_pps_bsf.c | 18 +-
  libavcodec/h265_metadata_bsf.c  | 18 +-
  libavcodec/mpeg2_metadata_bsf.c | 16 
  libavcodec/trace_headers_bsf.c  | 14 +++---
  libavcodec/vaapi_encode_h264.c  | 14 +++---
  libavcodec/vaapi_encode_h265.c  | 10 +-
  libavcodec/vaapi_encode_mpeg2.c | 10 +-
  10 files changed, 80 insertions(+), 66 deletions(-)



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

Re: [libav-devel] [PATCH 16/16] h264_metadata: Add option to delete filler data

[Luca Barbato]

On 11/02/2018 19:14, Mark Thompson wrote:

Deletes both filler NAL units and filler SEI messages.  (Annex B zero_bytes
between NAL units are already discarded by the read/write process.)
---
  libavcodec/h264_metadata_bsf.c | 43 ++
  1 file changed, 43 insertions(+)



Ok.

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

Re: [libav-devel] [PATCH 14/16] cbs_h264: Move slice_group_id array out of PPS structure

[Luca Barbato]

On 11/02/2018 19:14, Mark Thompson wrote:

It's very large, and is only used in some FMO streams.
---
  libavcodec/cbs_h264.h |  4 +++-
  libavcodec/cbs_h2645.c| 10 +-
  libavcodec/cbs_h264_syntax_template.c |  3 +++
  3 files changed, 15 insertions(+), 2 deletions(-)



Probably Ok.

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

Re: [libav-devel] [PATCH 15/16] cbs_h264: Add support for filler NAL units

[Luca Barbato]

On 11/02/2018 19:14, Mark Thompson wrote:

---
  libavcodec/cbs_h264.h |  6 ++
  libavcodec/cbs_h2645.c| 21 +
  libavcodec/cbs_h264_syntax_template.c | 29 +
  3 files changed, 56 insertions(+)



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

Re: [libav-devel] [PATCH 13/16] h264_metadata: Always add the SEI user data to the first access unit

[Luca Barbato]

On 11/02/2018 19:14, Mark Thompson wrote:

This should be added even if the first access unit does not contain
parameter sets.
---
E.g. this is helpful for the "-bsf:v 
'h264_metadata=sei_user_data=dc45e9bde6d948b7962cd820d923eeef+x264 - core 150'" hack 
workaround for old files with stripped metadata.


  libavcodec/h264_metadata_bsf.c | 8 ++--
  1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/libavcodec/h264_metadata_bsf.c b/libavcodec/h264_metadata_bsf.c
index ce85781c6..88e1a7750 100644
--- a/libavcodec/h264_metadata_bsf.c
+++ b/libavcodec/h264_metadata_bsf.c
@@ -62,6 +62,7 @@ typedef struct H264MetadataContext {
  int crop_bottom;
  
  const char *sei_user_data;

+int sei_first_au;
  } H264MetadataContext;
  
  
@@ -288,14 +289,17 @@ static int h264_metadata_filter(AVBSFContext *bsf, AVPacket *out)

  }
  }
  
-// Only insert the SEI in access units containing SPSs.

-if (has_sps && ctx->sei_user_data) {
+// Only insert the SEI in access units containing SPSs, and also
+// unconditionally in the first access unit we ever see.
+if (ctx->sei_user_data && (has_sps || !ctx->sei_first_au)) {
  H264RawSEIPayload payload = {
  .payload_type = H264_SEI_TYPE_USER_DATA_UNREGISTERED,
  };
  H264RawSEIUserDataUnregistered *udu =
  _data_unregistered;
  
+ctx->sei_first_au = 1;

+
  for (i = j = 0; j < 32 && ctx->sei_user_data[i]; i++) {
  int c, v;
  c = ctx->sei_user_data[i];



Fine for me, even if the shed is upside down. (sei_non_first_au is ugly 
I know).

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

Re: [libav-devel] [PATCH 12/16] h264_metadata: Use common SEI addition function

[Luca Barbato]

On 11/02/2018 19:14, Mark Thompson wrote:

---
  libavcodec/h264_metadata_bsf.c | 70 --
  1 file changed, 19 insertions(+), 51 deletions(-)



Ok.

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

Re: [libav-devel] [PATCH 11/16] cbs_h264: Add utility functions to insert/delete SEI messages

[Luca Barbato]

On 11/02/2018 19:14, Mark Thompson wrote:

---
  libavcodec/cbs_h264.h  | 19 +++
  libavcodec/cbs_h2645.c | 89 ++
  2 files changed, 108 insertions(+)



Seems fine.

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

Re: [libav-devel] [PATCH 09/16] cbs_h264: Add hack for pic_timing with no active SPS

[Luca Barbato]

On 11/02/2018 19:14, Mark Thompson wrote:

If there is exactly one possible SPS but it is not yet active then just
assume that it should be the active one.
---
  libavcodec/cbs_h264_syntax_template.c | 16 
  1 file changed, 16 insertions(+)

diff --git a/libavcodec/cbs_h264_syntax_template.c 
b/libavcodec/cbs_h264_syntax_template.c
index 0fe18441c..c2fd54682 100644
--- a/libavcodec/cbs_h264_syntax_template.c
+++ b/libavcodec/cbs_h264_syntax_template.c
@@ -560,6 +560,22 @@ static int FUNC(sei_pic_timing)(CodedBitstreamContext 
*ctx, RWContext *rw,
  int err;
  
  sps = h264->active_sps;

+if (!sps) {
+// If there is exactly one possible SPS but it is not yet active
+// then just assume that it should be the active one.
+int i, k = -1;
+for (i = 0; i < H264_MAX_SPS_COUNT; i++) {
+if (h264->sps[i]) {
+if (k >= 0) {
+k = -1;
+break;
+}
+k = i;
+}
+}
+if (k >= 0)
+sps = h264->sps[k];
+}
  if (!sps) {
  av_log(ctx->log_ctx, AV_LOG_ERROR,
 "No active SPS for pic_timing.\n");



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

Re: [libav-devel] [PATCH 08/16] cbs_h2645: Remove active ps references when it is replaced

[Luca Barbato]

On 11/02/2018 19:14, Mark Thompson wrote:

---
  libavcodec/cbs_h2645.c | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c
index e3b5bf618..9d05d5915 100644
--- a/libavcodec/cbs_h2645.c
+++ b/libavcodec/cbs_h2645.c
@@ -674,6 +674,8 @@ static int cbs_h26 ## h26n ## _replace_ ## 
ps_var(CodedBitstreamContext *ctx, \
 " id : %d.\n", id); \
  return AVERROR_INVALIDDATA; \
  } \
+if (priv->ps_var[id] == priv->active_ ## ps_var) \
+priv->active_ ## ps_var = NULL ; \
  av_freep(>ps_var[id]); \
  priv->ps_var[id] = av_malloc(sizeof(*ps_var)); \
  if (!priv->ps_var[id]) \



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

Re: [libav-devel] [PATCH 05/16] vaapi_encode: Destroy output buffer pool before VA context

[Luca Barbato]

On 11/02/2018 19:14, Mark Thompson wrote:

The buffers are created associated with the context, so they should be
destroyed before the context is.  This is enforced by the iHD driver.
---
This patch and the following one were posted late last year, but there was no 
conclusion.  There is some suggestion that the driver might be fixed for either 
or both of them, but I think it would be best to apply them anyway.



better safe than sorry IMHO.

+1 on this patch.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH 07/16] cbs: Demote the "decomposition unimplemented" warning

[Luca Barbato]

On 11/02/2018 19:14, Mark Thompson wrote:

This is harmless and should not be a warning - unknown units are passed
through to the write functions unchanged, and no other code will interact
with them.
---
  libavcodec/cbs.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/cbs.c b/libavcodec/cbs.c
index e5819afce..a8d252f6c 100644
--- a/libavcodec/cbs.c
+++ b/libavcodec/cbs.c
@@ -135,7 +135,7 @@ static int cbs_read_fragment_content(CodedBitstreamContext 
*ctx,
  
  err = ctx->codec->read_unit(ctx, >units[i]);

  if (err == AVERROR(ENOSYS)) {
-av_log(ctx->log_ctx, AV_LOG_WARNING,
+av_log(ctx->log_ctx, AV_LOG_VERBOSE,
 "Decomposition unimplemented for unit %d "
 "(type %"PRIu32").\n", i, frag->units[i].type);
  } else if (err < 0) {



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

Re: [libav-devel] [PATCH 06/16] vaapi_h265: Mark unused entries in RefPicList[01] as explicitly invalid

[Luca Barbato]

On 11/02/2018 19:14, Mark Thompson wrote:

The iHD driver looks at entries beyond num_ref_idx_l[01]_active_minus1
for unknown reasons.
---
  libavcodec/vaapi_encode_h265.c | 21 +++--
  1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/libavcodec/vaapi_encode_h265.c b/libavcodec/vaapi_encode_h265.c
index 38c9e2521..52ac4a687 100644
--- a/libavcodec/vaapi_encode_h265.c
+++ b/libavcodec/vaapi_encode_h265.c
@@ -767,8 +767,6 @@ static int 
vaapi_encode_h265_init_slice_params(AVCodecContext *avctx,
  
  .num_ref_idx_l0_active_minus1 = sh->num_ref_idx_l0_active_minus1,

  .num_ref_idx_l1_active_minus1 = sh->num_ref_idx_l1_active_minus1,
-.ref_pic_list0[0] = vpic->reference_frames[0],
-.ref_pic_list1[0] = vpic->reference_frames[1],
  
  .luma_log2_weight_denom = sh->luma_log2_weight_denom,

  .delta_chroma_log2_weight_denom = sh->delta_chroma_log2_weight_denom,
@@ -802,6 +800,25 @@ static int 
vaapi_encode_h265_init_slice_params(AVCodecContext *avctx,
  },
  };
  
+for (i = 0; i < FF_ARRAY_ELEMS(vslice->ref_pic_list0); i++) {

+vslice->ref_pic_list0[i].picture_id = VA_INVALID_ID;
+vslice->ref_pic_list0[i].flags  = VA_PICTURE_HEVC_INVALID;
+vslice->ref_pic_list1[i].picture_id = VA_INVALID_ID;
+vslice->ref_pic_list1[i].flags  = VA_PICTURE_HEVC_INVALID;
+}
+
+av_assert0(pic->nb_refs <= 2);
+if (pic->nb_refs >= 1) {
+// Backward reference for P- or B-frame.
+av_assert0(pic->type == PICTURE_TYPE_P ||
+   pic->type == PICTURE_TYPE_B);
+vslice->ref_pic_list0[0] = vpic->reference_frames[0];
+}
+if (pic->nb_refs >= 2) {
+// Forward reference for B-frame.
+av_assert0(pic->type == PICTURE_TYPE_B);
+vslice->ref_pic_list1[0] = vpic->reference_frames[1];
+}
  
  return 0;

  }



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

Re: [libav-devel] [PATCH 03/16] cbs: Minor comment fixes / cosmetics

[Luca Barbato]

On 11/02/2018 19:14, Mark Thompson wrote:

---
  libavcodec/cbs.h  | 35 +++
  libavcodec/cbs_internal.h |  3 +++
  2 files changed, 34 insertions(+), 4 deletions(-)



Fine for me.

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

Re: [libav-devel] [PATCH 02/16] cbs: Add an explicit type for coded bitstream unit types

[Luca Barbato]

On 11/02/2018 19:14, Mark Thompson wrote:

Also fix conversion specifiers used for the unit type.
---
  libavcodec/cbs.c   | 12 +++-
  libavcodec/cbs.h   | 19 +++
  libavcodec/cbs_h2645.c |  2 +-
  libavcodec/cbs_mpeg2.c |  4 ++--
  4 files changed, 25 insertions(+), 12 deletions(-)



Seems fine, I was waiting for Diego to dig on it since he loves this :)

lu

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